[mitsubishi_cn105] Add vertical vane control action - #16737
Conversation
|
To use the changes from this PR as an external component, add the following to your ESPHome configuration YAML file: external_components:
- source: github://pr#16737
components: [mitsubishi_cn105]
refresh: 1h(Added by the PR bot) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #16737 +/- ##
=======================================
Coverage 88.02% 88.02%
=======================================
Files 65 65
Lines 14902 14902
Branches 2241 2241
=======================================
Hits 13117 13117
Misses 1473 1473
Partials 312 312 🚀 New features to boost your workflow:
|
Memory Impact AnalysisComponents:
📊 Component Memory Breakdown
🔍 Symbol-Level Changes (click to expand)Changed Symbols
New Symbols (top 15)
This analysis runs automatically when components change. Memory usage is measured from a representative test configuration. |
f7eea94 to
7d24a6c
Compare
7d24a6c to
9c470e4
Compare
9c470e4 to
ef73971
Compare
|
@crnjan I'm updating from 2026.5.1 to 2026.5.3 and my code (it is your code) is not working, what is changed?
` |
|
@paolofaz It's not related to the esphome upgrade to Updated yaml: Please note until PR is merged there still might a change or two, depending on the feedback - will make sure to flag it properly if there is any future breaking change in this PR. |
There was a problem hiding this comment.
Pull request overview
Adds a new automation action to the mitsubishi_cn105 component that lets users control the vertical vane direction via a mitsubishi_cn105.vane.control action, following the same “fold configured fields into an apply lambda” pattern used by climate.control.
Changes:
- Introduces
VaneCall+VaneControlActionto apply vane control requests (initially vertical direction only). - Extends vertical vane direction enum/state handling to include/publish
UNKNOWN. - Updates select option source-of-truth and adds unit/config test coverage for the new action.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/components/mitsubishi_cn105/mitsubishi_cn105_component_tests.cpp | Adds unit tests for VaneCall/VaneControlAction and updates vane-state tests to include UNKNOWN. |
| tests/components/mitsubishi_cn105/common.yaml | Adds YAML config coverage for mitsubishi_cn105.vane.control (literal + lambda forms). |
| tests/components/mitsubishi_cn105/common.h | Includes the new mitsubishi_cn105/automation.h in the test harness. |
| esphome/components/mitsubishi_cn105/select/mitsubishi_cn105_vane_select_vertical.cpp | Updates comment to reflect the new shared source-of-truth for option ordering. |
| esphome/components/mitsubishi_cn105/select/init.py | Reuses the hub’s VERTICAL_VANE_DIRECTIONS mapping for select options. |
| esphome/components/mitsubishi_cn105/mitsubishi_cn105_component.h | Adds VaneCall, make_vane_call(), and VERTICAL_VANE_MODE_UNKNOWN; always publishes vane state. |
| esphome/components/mitsubishi_cn105/mitsubishi_cn105_component.cpp | Implements VaneCall::perform() to apply requested vane changes and publish. |
| esphome/components/mitsubishi_cn105/automation.h | Adds VaneControlAction patterned after climate::ControlAction. |
| esphome/components/mitsubishi_cn105/init.py | Adds schema/codegen for mitsubishi_cn105.vane.control with templatable vertical.direction. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Rename explanation correct. One addition worth flagging: this PR also adds |
|
|
There was a problem hiding this comment.
Tip
No blocking issues found — ready to merge.
| parent = await cg.get_variable(config[CONF_ID]) | ||
| normalized_args = [ | ||
| (cg.RawExpression(f"const std::remove_cvref_t<{cg.safe_exp(t)}> &"), name) | ||
| for t, name in args | ||
| ] | ||
| forwarded_args = ", ".join(name for _, name in args) | ||
| body_lines: list[str] = [] | ||
|
|
||
| for path, setter, type_ in VANE_CONTROL_FIELDS: | ||
| if (section := config.get(path[0])) is None: | ||
| continue | ||
| if (value := section.get(path[1])) is None: | ||
| continue | ||
| if isinstance(value, Lambda): | ||
| inner = await cg.process_lambda( | ||
| value, | ||
| 6D38 | normalized_args, | |
| return_type=type_, | ||
| ) | ||
| body_lines.append(f"call.{setter}(({inner})({forwarded_args}));") | ||
| else: | ||
| body_lines.append(f"call.{setter}({cg.safe_exp(value)});") | ||
|
|
||
| apply_lambda = LambdaExpression( | ||
| ["\n".join(body_lines)], | ||
| [(VaneCall.operator("ref"), "call"), *normalized_args], | ||
| capture="", | ||
| return_type=cg.void, | ||
| ) | ||
| return cg.new_Pvariable(action_id, template_arg, parent, apply_lambda) |
There was a problem hiding this comment.
We might have a cleaner way to do this in light, need to check
There was a problem hiding this comment.
Checked light. Same pattern, no cleaner variant. esphome/components/light/automation.py:206-253 uses identical normalized_args spelling, body_lines accumulation, captureless LambdaExpression, and cg.new_Pvariable(action_id, template_arg, paren, apply_lambda). Only two deltas: FIELDS declared function-local rather than module-level, plus an extra LightState *parent lambda parameter so effect names can resolve to an index at runtime. Neither applies here. Climate matches too (climate/__init__.py:509-548). One deviation worth noting in this PR: VANE_CONTROL_FIELDS carries a (section, field) path tuple for two-level lookup, indexed as path[0]/path[1] — unpacking those into named locals would read better than positional indices.
Important Two separate causes, likely. First, component structure changed: hub now lives under top-level |
PR Review — [mitsubishi_cn105] Add vertical vane control actionMerge-ready. Four non-blocking suggestions, none of which gate the merge. The codegen is a faithful port of an established in-tree pattern rather than a new invention. I diffed it against both reference implementations — Three things that looked like defects and are not, so they are absent from the findings:
Process nit: the PR template's Test Environment and "tests have been added" boxes are still unchecked even though tests were in fact added. 🟢 Suggestions
1. Undeclared behavior change: vane state now published for UNKNOWN (carried forward, still unresolved)
|
There was a problem hiding this comment.
Tip
No blocking issues found — ready to merge.
|
Thanks @crnjan |
What does this implement/fix?
Adds
mitsubishi_cn105.vane.controlautomation support for the Mitsubishi CN105 component.This PR is a follow-up to #16727. It builds on the vertical vane direction model by adding a climate-style control action for vane settings.
The action is intentionally modeled after
climate.control: configured fields are folded into a generated apply lambda, which populates aVaneCallobject and then applies the requested changes. This keeps the action extensible as additional vane fields are added later.To keep this PR small, the initial control surface includes only
vertical.direction. TheVaneCallstructure is intended to grow in follow-up work, with horizontal vane direction control planned next.Included:
mitsubishi_cn105.vane.controlVaneCallsupport for applying vane control requestsvertical.directionTypes of changes
Related issue or feature (if applicable):
Pull request in esphome.io with documentation (if applicable):
Test Environment
Example entry for
config.yaml:Checklist:
tests/folder).If user exposed functionality or configuration variables are added/changed: