-
Notifications
You must be signed in to change notification settings - Fork 60
Add an approach for determining if a dataclass field is supported #740
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
Add an approach for determining if a dataclass field is supported #740
Conversation
This consists of multiple concepts: - DeviceFeaturesTrait defines a method `is_field_supported` which will tell you if an optional field on a RoborockBase class type is supported or not - The RoborockBase field defins a metadata attribute for a product schea code which is used to check the field against some other source - We could substitue in other values in DeviceFeatures or status in the future as needed. This adds 6 fields that are straight forward to add.
4bd46cb to
718646f
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.
Pull request overview
This PR introduces a mechanism to determine if specific dataclass fields are supported on Roborock devices. It adds a is_field_supported method to the DeviceFeaturesTrait class that checks field support based on product schema codes. The implementation uses dataclass field metadata to map fields to their corresponding schema codes in the product information.
Key changes:
- Added
is_field_supportedmethod to DeviceFeaturesTrait for runtime field support checking - Introduced StatusField enum and FieldNameBase for type-safe field name references
- Added metadata annotations to 6 Status dataclass fields (state, battery, fan_power, water_box_mode, charge_status, dry_status) with their required schema codes
- Created
supported_schema_codesproperty on HomeDataProduct to expose available schema codes
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| roborock/devices/traits/v1/device_features.py | Added is_field_supported method and updated constructor to accept full product object instead of just nickname |
| roborock/data/v1/v1_containers.py | Added StatusField enum and FieldNameBase class; annotated Status fields with schema code metadata |
| roborock/data/containers.py | Added supported_schema_codes cached property to HomeDataProduct |
| roborock/devices/traits/v1/init.py | Updated DeviceFeaturesTrait initialization to pass full product object; added documentation about field metadata |
| tests/devices/traits/v1/test_device_features.py | Added new test for is_field_supported functionality with parametrized devices |
| tests/devices/traits/v1/fixtures.py | Updated fixtures to support parametrized device_info and products |
| tests/devices/traits/v1/snapshots/test_device_features.ambr | Added snapshot assertions for field support tests |
| tests/data/test_containers.py | Added assertion to verify supported_schema_codes property returns expected schema codes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| class StatusField(FieldNameBase): | ||
| """An enum that represents a field in the `Status` class. | ||
| This is used with `roborock.devices.traits.v1.status.DeviceFeaturesTrait` |
Copilot
AI
Jan 8, 2026
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.
The docstring incorrectly refers to "roborock.devices.traits.v1.status.DeviceFeaturesTrait" when it should be "roborock.devices.traits.v1.device_features.DeviceFeaturesTrait". The DeviceFeaturesTrait class is located in the device_features module, not the status module.
| This is used with `roborock.devices.traits.v1.status.DeviceFeaturesTrait` | |
| This is used with `roborock.devices.traits.v1.device_features.DeviceFeaturesTrait` |
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("device_info"), |
Copilot
AI
Jan 8, 2026
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.
The parameter name is incorrectly wrapped in a tuple. It should be "device_info" (a single string) instead of ("device_info") (a tuple with one element). This is a common pytest parametrize pattern error.
| ("device_info"), | |
| "device_info", |
roborock/data/v1/v1_containers.py
Outdated
| state: RoborockStateCode | None = field(metadata={"requires_schema_code": "state"}, default=None) | ||
| battery: int | None = field(metadata={"requires_schema_code": "battery"}, default=None) |
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 I know this was my suggestion and very similar to how I did it in device features. I wonder if a wrapper around field though that makes it easier to set the metadata would be worthwhile as this gets more complicated.
i.e. OurSubClassField(requires_schema_code=StatusField.BATTERY) or still just "battery" if that makes more sense. May help us avoid making any mistakes with typos in the metadata field name
The init of the field could create the metadata and everything else could stay the same but would likely lower the burden of contributing for other users.
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.
I did this though made a few alterations, PTAL.
Lash-L
left a comment
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.
Looks good! Thanks!
This consists of multiple concepts:
is_field_supportedwhich will tell you if an optional field on a RoborockBase class type is supported or notThis adds 6 fields that are straight forward to add.
This will be used to help us decouple discovering devices and supported entities from the initial refresh to get data values for a trait.
This is based on attempting to implement the specific proposal from @Lash-L in #722 from Lash-L