Bump midea-local to 11.0.1 - #181405
Conversation
home-assistant
Bot
added
bugfix
cla-signed
dependency
|
Hey there @rokam, @caibinqing, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
🟡 Changes recommended
Two moderate config-flow regressions must be fixed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates the Midea integration to midea-local 11.0.1 and surfaces actionable cloud authentication errors.
Changes:
- Bumps
midea-localfrom 10.1.0 to 11.0.1. - Maps cloud exceptions to translated config-flow errors.
- Adds regression coverage for cloud authentication failures.
File summaries
| File | Description |
|---|---|
tests/components/midea/test_config_flow.py |
Tests cloud-error handling; test docstring should be condensed. |
requirements_all.txt |
Updates the generated dependency requirement; PR type metadata needs correction. |
homeassistant/components/midea/strings.json |
Adds translated cloud-error messages. |
homeassistant/components/midea/manifest.json |
Updates the integration dependency; PR version metadata needs correction. |
homeassistant/components/midea/config_flow.py |
Handles typed cloud errors, but must preserve default-key fallback and the phase-one cloud error; one docstring also needs shortening. |
Review details
Suppressed comments (3)
homeassistant/components/midea/config_flow.py:138
- Shorten this docstring so it accurately describes all call sites.
_clear_login_state()is also called before successful entry creation, so the claim that it is called right before re-showing a form is incorrect; the detailed lifecycle narration can be replaced with the actual invariant.
The pending cloud error/code are intentionally left in place: this is
called right before re-showing a form that still needs to render that
error. They are refreshed on the next cloud call (``_check_cloud_login``
resets them) or when ``async_step_auto`` re-runs with input.
requirements_all.txt:1623
- Select only the dependency-upgrade checkbox in the PR description. The template explicitly requires exactly one type, but both Dependency upgrade and Bugfix are currently selected.
midea-local==11.0.1
tests/components/midea/test_config_flow.py:1405
- Condense this docstring to the non-obvious behavior under test. The following parameter cases and assertions already show the two branches, so narrating each control-flow outcome adds maintenance burden without clarifying a constraint.
"""Test cloud API errors on the preset auth path surface a specific message.
Either the preset login itself is rejected (stays on auth_method), or the
login succeeds but the cloud refuses to issue a token/key for the device
(falls back to the auto step).
- Files reviewed: 5/5 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
silamon
left a comment
silamon
left a comment
There was a problem hiding this comment.
This also takes the v11 release in, please add a release link.
It seems that the breaking changes cause mypy to be unhappy as well.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Done
Alligned and cleaned-up |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved climate capability and config-flow fallback regressions can cause exposed actions to fail and valid devices to be rejected.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (4)
homeassistant/components/midea/config_flow.py:550
- Preserve the phase-one cloud error when the fallback login fails generically.
_check_cloud_login()clears the saved error before returningFalse, so a specific phase-one error such asdevice_not_registeredis replaced withpreset_login_failedon this branch even though the fallback failure carries no better diagnosis.
if not await self._check_cloud_login(force_login=True):
error = self._cloud_error or "preset_login_failed"
self._clear_login_state()
return await self.async_step_auto(error=error)
homeassistant/components/midea/config_flow.py:457
- Keep attempting the built-in default keys after a cloud exception. Before v11,
get_cloud_keys()returned an empty result for these failures, so this method still mergedget_default_keys()and attempted the well-known key (the behavior asserted bytest_auto_flow_v3_preset_phase1_default_key_success); returning here means every typed error now skips that fallback and rejects devices that can authenticate with it. Preserve the cloud error, try defaults whendefault_keyis true, and surface the specific error only if those attempts also fail.
return {"cloud_error": err.translation_key}
homeassistant/components/midea/manifest.json:16
- Select only
Dependency upgradein the PR description. The required template says to check exactly one type, but this PR currently checks both Dependency upgrade and Bugfix.
"requirements": ["midea-local==11.0.1"]
tests/components/midea/test_config_flow.py:1405
- Condense this implementation narration to a one-line behavioral docstring. The parameter cases already show both branches, so restating their control flow adds maintenance overhead without documenting a non-obvious constraint.
"""Test cloud API errors on the preset auth path surface a specific message.
Either the preset login itself is rejected (stays on auth_method), or the
login succeeds but the cloud refuses to issue a token/key for the device
(falls back to the auto step).
- Files reviewed: 10/10 changed files
- Comments generated: 1
- Review effort level: Balanced
|
I think the use of MideaClimate should be placed in another PR. This PR was supposed to just bump the lib and fix the breaking changes. Also, the HVACMode conversion is wrong. It isn't the same. |
f311bd4 to
2d64c15
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Swing support can be incorrectly advertised and fail at runtime; the PR metadata also needs correction.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
homeassistant/components/midea/manifest.json:16
- Correct the PR comparison URL to start at
v10.1.0. The changed manifest shows the actual installed baseline is 10.1.0, while the description compares from 10.0.1 and therefore includes already-integrated upstream changes.
"requirements": ["midea-local==11.0.1"]
homeassistant/components/midea/manifest.json:16
- Select only the
Dependency upgradetype in the PR description. The repository template requires exactly one type checkbox, but bothDependency upgradeandBugfixare selected.
"requirements": ["midea-local==11.0.1"]
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Balanced
rokam
left a comment
rokam
left a comment
There was a problem hiding this comment.
I do have some observations.
|
I'm going to hold off reviewing while there's discussion, feel free to give a ping when there's an agreement. |
|
@silamon, ready ;-) |
|
Thx ! |
Proposed change
main release: https://github.com/midea-lan/midea-local/releases/tag/v11.0.0
patch release: https://github.com/midea-lan/midea-local/releases/tag/v11.0.1
diff: midea-lan/midea-local@v10.0.1...v11.0.1
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: