fix: fix enum names to include none states#561
fix: fix enum names to include none states#561allenporter merged 1 commit intoPython-roborock:mainfrom
none states#561Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes enum name properties to return 'none' for enums with value 0, reverting a previous change that omitted these states. The change ensures backward compatibility with HomeAssistant, which relies on 'none' values for matching error codes.
Key Changes:
- Modified enum name property checks to use explicit
is not Nonecomparisons instead of truthy checks - Updated test snapshot to reflect that
error_code_name='none'is now returned instead ofNone
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| roborock/data/v1/v1_containers.py | Changed truthiness checks to explicit is not None checks for five enum name properties to properly handle enums with value 0 |
| tests/devices/snapshots/test_v1_device.ambr | Updated test snapshot to expect error_code_name='none' instead of None |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
This was a change in Python-roborock#503 that changed behavior to omit `0` states, however this is reverting it to preserve compatibility with old behavior. In practice, HomeAssistant uses `none` for matching error codes for example.
Lash-L
left a comment
There was a problem hiding this comment.
Sorry I didn't catch this during the review!
4a32c31 to
b6b18b3
Compare
|
Good news is the snapshot test now would catch this behavior change. I had to force push to fix commit lint if you don't mind approving one more time. |
Actually looks like it didn't dismiss my review surprisingly |
This was a change in #503 that changed behavior to omit
0states, however this is reverting it to preserve compatibility with old behavior. In practice, HomeAssistant usesnonefor matching error codes for example.