Skip to content

feat: Fix AppInitStatus to handle missing fields and add RoborockParsingException#819

Merged
allenporter merged 4 commits intoPython-roborock:mainfrom
allenporter:missing-required-field
Apr 26, 2026
Merged

feat: Fix AppInitStatus to handle missing fields and add RoborockParsingException#819
allenporter merged 4 commits intoPython-roborock:mainfrom
allenporter:missing-required-field

Conversation

@allenporter
Copy link
Copy Markdown
Contributor

Fix AppInitStatus to handle missing fields and improves debug messages for handling missing fields overall.

home-assistant/core#167674

Copilot AI review requested due to automatic review settings April 26, 2026 16:56
Copy link
Copy Markdown
Contributor

Copilot AI left a 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 PR improves robustness and debuggability of v1 trait parsing by introducing a dedicated parsing exception and ensuring AppInitStatus can be parsed even when some fields are missing, aligning behavior with the Home Assistant core issue linked in the description.

Changes:

  • Add RoborockParsingException and wrap common v1 trait conversion failures with richer context (trait, command, payload, inner error).
  • Make AppInitStatus.new_feature_info optional-by-default to handle missing new_feature_info in responses.
  • Update and add tests to validate the new exception behavior and missing-field parsing.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
roborock/exceptions.py Introduces RoborockParsingException to standardize parsing failures with context.
roborock/devices/traits/v1/common.py Wraps converter parsing errors during V1TraitMixin.refresh() into RoborockParsingException.
roborock/devices/traits/v1/rooms.py Adds parsing-exception wrapping around room conversion.
roborock/devices/traits/v1/clean_summary.py Wraps clean-record parsing failures with RoborockParsingException.
roborock/data/v1/v1_containers.py Defaults AppInitStatus.new_feature_info to 0 to tolerate missing field.
tests/devices/traits/v1/test_status.py Updates expectation from ValueError to RoborockParsingException for invalid status payloads.
tests/devices/traits/v1/test_common.py Adds coverage for parsing-exception message/context on bad payloads.
tests/data/v1/test_v1_containers.py Adds test ensuring missing new_feature_info is handled correctly.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/devices/traits/v1/test_common.py Outdated
Comment thread roborock/devices/traits/v1/rooms.py
allenporter and others added 3 commits April 26, 2026 10:11
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Collaborator

@Lash-L Lash-L left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great - good exception for us to have and I think the default value will work logically wise

But I don't think this is a true fix for home-assistant/core#167674

I think the device manager gate is being too liberal. We are just checking if protocol is v1, then we call our normal commands. I think we instead need to check to see if protocol is v1, then also check if the category mower. In my head we were already gating it, but I guess not.

@allenporter allenporter merged commit aeb320a into Python-roborock:main Apr 26, 2026
7 checks passed
@allenporter
Copy link
Copy Markdown
Contributor Author

OK so perhaps this just makes it not crash or at least makes the error message better. But you're saying it shouldn't call this method?

@Lash-L
Copy link
Copy Markdown
Collaborator

Lash-L commented Apr 26, 2026

OK so perhaps this just makes it not crash or at least makes the error message better. But you're saying it shouldn't call this method?

It might work - but my thought is an error will pop up down the line. I.e it will try to do other vacuum specific commands on the mower. But maybe the feature checks will stop it?

I don't know enough about the mowers logic yet. I haven't looked at rr api for it yet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants