-
-
Notifications
You must be signed in to change notification settings - Fork 35.8k
Add ZHA migration retry steps for unplugged adapters #155537
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
Merged
TheJulianJES
merged 13 commits into
home-assistant:dev
from
TheJulianJES:tjj/zha_config_flow_guard_old_adapter_removal
Nov 4, 2025
Merged
Changes from 10 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
5e274f0
Show dialog if old adapter was unplugged before reset
TheJulianJES 8098732
Add option to skip reset of old adapter
TheJulianJES a17d5b5
Show dialog if new adapter was unplugged before restore
TheJulianJES 53b7816
Add test for `plug_in_new_radio` retry step
TheJulianJES 35d0104
Rename steps for old radio retry and skip reset
TheJulianJES ede212a
Add test for `plug_in_old_radio` retry step
TheJulianJES bdf3a2c
Add test for user removing ZHA config entry before trying old radio r…
TheJulianJES 5bcdf63
Move some test part out of radio manager patch
TheJulianJES f2308db
Remove self-explanatory comment also not used in other tests
TheJulianJES bcb573c
Remove config entry in side effect of attempted reset
TheJulianJES 30adebc
Change variable names in test
TheJulianJES c694652
Change comment explaining why guard exists
TheJulianJES f594696
Adjust string to add "warning"
TheJulianJES File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 the one line missing test coverage. Can write a test for this, but we should never be able to hit this.
We could assert that
config_entriesis not empty, or we could also save the "old adapter device path" to some instance variable inasync_step_maybe_reset_old_radio.Passing it as an additional argument also works, but it's not a pattern I've seen used for any config flows really.
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.
You conceivably could hit this by deleting ZHA halfway through the flow but I agree, this isn't something we should care about. There are many places in Core that break if you try hard enough.
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.
So the test added I added in
bdf3a2c(#155537) won't actually help to cover this, since theplug_in_old_radiowon't happen again. Theretry_old_radiostep goes tomaybe_reset_old_radioand that step itself will skip tomaybe_confirm_ezsp_restore, as there are no other config entries anymore.For this line to be hit, the config entry needs to be removed at precisely the exact moment we're trying to reset the adapter. I'll try something different..
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.
bcb573c(#155537) should cover this now and work completely reliable, even if this is almost impossible for a user to do: remove both the config entry and unplug the adapter whilst we try to reset it.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.
Alternative to this would be saving or passing the old adapter's path from
async_step_maybe_reset_old_radio. Then, we wouldn't need to get config entries in the new step displaying the old adapter's path, so we could avoid the potential step skip if the entry is removed in time, thus avoiding the whole tests for it as well.But now, we have the test and everything functions as expected if the user is able to hit this 😄