fix: fix Q7 code mapping issue for workmode - #917
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates the Roborock B01/Q7 status code mappings to handle an additional “working_sleep” status and to avoid breaking deserialization when encountering unknown future status codes.
Changes:
- Add
WORKING_SLEEP(code11) toWorkStatusMapping. - Add an
UNKNOWNfallback for unrecognized Q7 work status codes during deserialization. - Expand the B01/Q7 container tests to cover all known status codes, the new status code 11, and unknown-code behavior (including warning logging).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
roborock/data/b01_q7/b01_q7_code_mappings.py |
Extends Q7 work status mappings and adds a tolerant from_code fallback to UNKNOWN. |
tests/data/b01_q7/test_b01_q7_containers.py |
Adds test coverage for status code mapping, status=11 parsing, and unknown status handling/logging. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @classmethod | ||
| def from_code(cls, code: int) -> Self: | ||
| """Map unrecognized Q7 statuses to UNKNOWN without failing the response.""" | ||
| try: | ||
| return super().from_code(code) | ||
| except ValueError: | ||
| return cast(Self, cls.UNKNOWN) |
| @classmethod | ||
| def from_code(cls, code: int) -> Self: | ||
| """Map unrecognized Q7 statuses to UNKNOWN without failing the response.""" | ||
| try: | ||
| return super().from_code(code) | ||
| except ValueError: | ||
| return cast(Self, cls.UNKNOWN) |
allenporter
left a comment
allenporter
left a comment
There was a problem hiding this comment.
Co-pilot is not wrong with its comments about this overriding the default from code behavior and interaction with optional -- but i think this makes sense to support, allowing some enums to map to an unknown value. This case also seems to make sense (allowing the warning but still working). Approving.
As we look to other cases: Do you have a sense of what enums should support this kind of fallback vs cases that should stay strict?
Should handle: home-assistant/core#177387
adds Unknown similar to how we do elsewhere. will require a HA mapping update too