-
-
Notifications
You must be signed in to change notification settings - Fork 35.8k
Add progress to ZHA migration steps #155764
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Hey there @dmulcahey, @Adminiuga, @TheJulianJES, 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request refactors the ZHA config flow to use the new @progress_step() decorator for long-running operations, providing better user feedback during network formation and backup restoration. The changes convert manual progress tracking to the standardized decorator pattern introduced in Home Assistant's data entry flow system.
Key Changes
- Implements
@progress_step()decorator onasync_step_form_new_network()andasync_step_maybe_reset_old_radio() - Refactors
async_step_maybe_confirm_ezsp_restore()toasync_step_restore_backup()with proper progress tracking - Renames
maybe_confirm_ezsp_restorestep toconfirm_ezsp_ieee_overwritefor clarity - Adds
consume_progress_flow()helper function in tests to handle progress steps - Updates all affected tests to use the new progress flow helper
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| homeassistant/components/zha/config_flow.py | Implements progress tracking decorators and refactors restore backup flow with task-based progress handling |
| homeassistant/components/zha/strings.json | Adds progress messages, renames step from maybe_confirm_ezsp_restore to confirm_ezsp_ieee_overwrite, removes description from maybe_reset_old_radio |
| tests/components/zha/test_config_flow.py | Adds consume_progress_flow() helper and updates all tests to consume progress steps before checking results |
| if result["type"] != FlowResultType.SHOW_PROGRESS: | ||
| break | ||
|
|
||
| assert result["type"] is FlowResultType.SHOW_PROGRESS |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 194 contains a redundant assertion that will always pass because of the break condition at line 191-192. The assertion on line 194 (assert result["type"] is FlowResultType.SHOW_PROGRESS) is unreachable if the condition at line 191 is false. Consider removing the redundant assertion at line 194.
| assert result["type"] is FlowResultType.SHOW_PROGRESS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very similar to the tests in the Home Assistant hardware integrations:
core/tests/components/homeassistant_hardware/test_config_flow.py
Lines 353 to 371 in 485f7f4
| async def consume_progress_flow( | |
| hass: HomeAssistant, | |
| flow_id: str, | |
| valid_step_ids: tuple[str, ...], | |
| ) -> ConfigFlowResult: | |
| """Consume a progress flow until it is done.""" | |
| while True: | |
| result = await hass.config_entries.flow.async_configure(flow_id) | |
| flow_id = result["flow_id"] | |
| if result["type"] != FlowResultType.SHOW_PROGRESS: | |
| break | |
| assert result["type"] is FlowResultType.SHOW_PROGRESS | |
| assert result["step_id"] in valid_step_ids | |
| await asyncio.sleep(0.1) | |
| return result |
I do agree that it is redundant though, but maybe this was added for clarity.. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had an initial look at this and it generally looks good to me.
Since we want to merge #155537 first, I’ll do some testing after the rebase (and a more in-depth review), so we can hopefully get it ready for the release tomorrow.
6e7191f to
6157629
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good to go from my side. This is a major improvement when migrating (to ZBT) and something that should really go into the next beta/release.
I've tested this extensively and noticed no issues, other than the few missing strings for the options flow that I added.
Do keep the above comment in mind, regarding possibly removing redundant information from the progress dialog descriptions. Keeping it as is now should be good as well. Maybe some users could miss the title, soo..?
We can iterate on those details in the future (if there's another beta or maybe in patch releases as well), since those would just be minor updates to the strings. Either way, that's nothing critical and this PR is fine in the current state IMO.
Co-authored-by: TheJulianJES <TheJulianJES@users.noreply.github.com>



Proposed change
More than a few ZHA migration steps take 10-15s to complete. This PR reorganizes some of the steps for better translations and turns long-running tasks into progress steps, ensuring all stages have responsive feedback.
Demo:
zha.progress.mp4
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: