Repository navigation
feat(mower): add typed status models and protocol enums - #1002
Merged
Merged
Conversation
Add protobuf-named status enums with integer wire codes, explicit unknown fallbacks, and a typed GPS coordinate model. Keep compact and detailed task state domains separate, using the captured detailed domain for scalar DPS 123. Retain RoborockMowerStateCode with all existing names and values and preserve MowerStatus.from_dps. Cover typed parsing, serialization, unknown values, and the retained entry points in the mirrored mower data tests.
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 12 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Lash-L
commented
Oct 31, 2026
Lash-L
left a comment
Lash-L
left a comment
Collaborator
Author
There was a problem hiding this comment.
Automated code review by Gemini 4 Argon in Antigravity using the /code-review skill (.agents/skills/code-review/SKILL.md / AGENTS.md).
Summary
Well-scoped first mower support PR replacing raw int / Any fields on MowerStatus with strongly typed MowerEnum (RoborockModeEnum) classes and MowerGpsCoordinate (RoborockBase), while retaining the legacy RoborockMowerStateCode export and MowerStatus.from_dps() factory.
- Layer 1 — Strongly Typed Models & Conformance: Eliminates
AnyonMowerStatus.offline_statusandMowerStatus.gps_coordinate, replacing them withMowerOfflineStatusandMowerGpsCoordinate(RoborockBase). - Layer 3 — Wire Protocol & Enum Resilience: Separates compact (
MowerMowState) and detailed (MowerDetailState) task state numeric domains, preserves wire misspellings (MAP_ERASEING), and ensures every status enum resolves unrecognized firmware codes toUNKNOWNwithout crashing. - Layer 5 — Test Mirroring & Colocation: Moves
tests/data/test_mower_containers.pyinto the mirroredtests/data/mower/test_mower_containers.pyandtests/data/mower/test_mower_code_mappings.pysuites with parameterized round-trip and boundary tests.
One minor item noted inline:
MowerEnum.from_codeUnreachable Fallback Branches (roborock/data/mower/mower_code_mappings.py#L129-L137): EveryMowerEnumsubclass definesUNKNOWNas its first member, leaving the loop continuation andraise ValueErrorbranches unreachable (flagged by Codecov).
Lash-L
commented
Nov 1, 2026
Lash-L
commented
Nov 1, 2026
Lash-L
commented
Nov 1, 2026
allenporter
approved these changes
Nov 1, 2026
allenporter
reviewed
Nov 1, 2026
allenporter
left a comment
allenporter
left a comment
Contributor
There was a problem hiding this comment.
Adding some post-merge comments to consider for followup
| class MowerErrorCode(MowerEnum): | ||
| """User-facing error code reported by DPS 120.""" | ||
|
|
||
| UNKNOWN = ("unknown", 0) |
Contributor
There was a problem hiding this comment.
double check that you're happy with the no error case (e.g. MowerErrorCode.NONE isn't a thing, but None is. MowerErrorCode.UNKNOWN is an error, not none, etc.
| class MowerGpsCoordinate(RoborockBase): | ||
| """Mower GPS position.""" | ||
|
|
||
| latitude: float | None = None |
Contributor
There was a problem hiding this comment.
Add latitude and longitude and gpsCoordinate to redact keys in diagnostics.py?
| """Resolve a wire value by name, string value, or integer code.""" | ||
| if (member := cls.from_any_optional(code)) is not None: | ||
| return member | ||
| return cast(Self, cls.UNKNOWN) # type: ignore[attr-defined] |
Contributor
There was a problem hiding this comment.
Do you care about logging on unknown values? even at debug?
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Mower status fields currently expose mostly raw integers. Add typed status enums with protobuf names and integer wire codes, plus a typed GPS coordinate model. Parsing accepts integer codes, decimal strings and protobuf names, with explicit fallback members for unknown firmware values.
Keep compact mowing states separate from detailed robot task states. The scalar DPS 123 field uses the detailed domain observed in the captured RockNeo Q105 status. Enum values and GPS fields were checked against RR_API's mower schema sources.
Preserve the existing
RoborockMowerStateCodeexport, including all 60 original member names and values, and retainMowerStatus.from_dps()for mixed integer/string DPS keys. Tests move into the matchingtests/data/mower/directory and cover those entry points alongside typed parsing and round trips.API behavior:
MowerStatusfields now contain the new typed enums and GPS model;as_dict()serializes those enums using protobuf names. Retaining the old enum export does not make new status values instances of that legacy enum.Validation:
pytest -q -p no:cacheprovider --benchmark-disable: 1,721 passed, 53 expected failures, 11 deselected; 99 snapshots passed. Benchmark tests run once without timing measurements.pre-commit run --all-files: all hooks passed, including Ruff, Mypy and Codespell.This is the first mower support PR and contains only data models, enum mappings and their tests. No live mower commands were sent.