Implement pydantic for lights #436
Conversation
|
Figured we'd sort out the typing stuff before I move on to fixing the tests. |
|
|
||
| @property | ||
| def dimmer(self): | ||
| def dimmer(self) -> int | None: | ||
| """Return dimmer if present.""" | ||
| if self.supported_features & SUPPORT_BRIGHTNESS: |
There was a problem hiding this comment.
dimmer is not optional in the response model. Can it still be not supported?
There was a problem hiding this comment.
I can't think of any light device that is not dimmable. Perhaps we should drop this supported_features-check and just return the int?
There was a problem hiding this comment.
Yeah, maybe. Ok to leave for the future though.
There was a problem hiding this comment.
I'll leave it for now. I'm expecting to have to update a few tests in this PR so I con't want to mistakenly cause regression errors here.
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
| return None | ||
|
|
||
| @property | ||
| def hsb_xy_color(self): | ||
| def hsb_xy_color( |
There was a problem hiding this comment.
Maybe add checks for None here as well?
There was a problem hiding this comment.
How is this used? Returning both hsb and xy together is unfamiliar to me.
There was a problem hiding this comment.
There was a problem hiding this comment.
Side note for the future: Since we return xy color from another property I would expect this one to only return hsb color. Maybe add a property that does that and deprecate this one?
I agree that we should only return a tuple if all items are not None.
There was a problem hiding this comment.
Want me to deprecate here in this PR?
There was a problem hiding this comment.
Nah, just some thoughts for the future.
tests/test_device.py
Outdated
| """Test light on.""" | ||
| device_data = deepcopy(LIGHT_WS) | ||
| device_data[ATTR_LIGHT_CONTROL][0][ATTR_DEVICE_STATE] = 1 | ||
| device = Device(deepcopy(device_data)) |
There was a problem hiding this comment.
Why do we copy twice?
There was a problem hiding this comment.
A mistake, will remove
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
|
When this is merged, we should be good to bump in |
|
I would complete the typing so we can add py.typed before bumping in core if it's only for typing purposes. |
No description provided.