zigpy / zha

Zigbee Home Automation
Apache License 2.0
10 stars 4 forks source link

Add `is_on` property to siren #63

Closed TheJulianJES closed 2 weeks ago

TheJulianJES commented 2 weeks ago

This adds a is_on property to the siren platform to align it with the rest of the on/off platforms we have. The change will allow us to address this comment: https://github.com/home-assistant/core/pull/120190#discussion_r1667428899

The added property is used in the state property, like done by other platforms (and thus covered by tests already).

codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.37%. Comparing base (378e536) to head (2289a26).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #63 +/- ## ======================================= Coverage 95.37% 95.37% ======================================= Files 61 61 Lines 9335 9338 +3 ======================================= + Hits 8903 8906 +3 Misses 432 432 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

TheJulianJES commented 2 weeks ago

Currently, we need the "whole state" for maybe_emit_state_changed_event (and maybe elsewhere?): https://github.com/zigpy/zha/blob/378e53641d02a450a99a67dccf79e97324196d77/zha/application/platforms/__init__.py#L249-L250

The naming of the individual entries seems to be a bit inconsistent though. Sometimes, the "on/off" state is called "on". But most of the times, like here, it's just "state". I guess because that's the only real state of the entity, while light has more "states".