Skip to content
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

Use none as sensor state when no alarm or timer is set #839

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

EuleMitKeule
Copy link

This PR introduces a small change to the alarm and timer sensor entities. When no alarm or timer is set, the entity returns its state as None instead of "unavailable". This will display a localized string "Unknown" in the UI instead of marking the entity as unavailable. This still does not reflect the actual state, since it is indeed known that no alarms or timers are set, but should be a temporary improvement over having the entity marked as unavailable which usually indicates to the user that there is some kind of problem, which is not the case here.

Closes #278

@EuleMitKeule
Copy link
Author

Something went wrong in the CI while installing dependencies I think.

@KapJI
Copy link
Collaborator

KapJI commented Feb 18, 2024

  1. This will break existing automations.
  2. Most likely this will break UI as well.

Please follow discussion in home-assistant/architecture#635

Copy link
Collaborator

@KapJI KapJI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should change it right now without changes in HA Core.

@EuleMitKeule
Copy link
Author

This will break existing automations.

True, but I would argue that the current behaviour is a bug, since an entity being unavailable indicates to the user that there is some kind of problem, which there is not. So in my opinion the change is necessary even though it will be a breaking change.

Most likely this will break UI as well.

It does not. As I stated the state will be displayed as "Unknown" without any errors. This is the intended way to handle such a case with a timestamp sensor.

I don't think we should change it right now without changes in HA Core.

The HASS dev docs state that timestamp sensors should either return a datetime object or None. I think as long as this is the intended behaviour for timestamp sensors this integration should adhere to this convention and provide a better user experience by not indicating configuration or connection problems when there aren't any.

@EuleMitKeule
Copy link
Author

EuleMitKeule commented Jul 20, 2024

@KapJI @leikoilja I guess your take on this hasn't changed and this can be closed? I still think the current behaviour is wrong and not working the way home assistant intended.

I would appreciate a reply on my comments though.

@leikoilja
Copy link
Owner

@KapJI @leikoilja I guess your take on this hasn't changed and this can be closed? I still think the current behaviour is wrong and not working the way home assistant intended.

I would appreciate a reply on my comments though.

thanks for the ping @EuleMitKeule. I have nothing against this change and agree that sensor returning None as a default value is more user-intuitive then unavailable.
@KapJI, do you have a strong opinion on this one?

@KapJI
Copy link
Collaborator

KapJI commented Aug 12, 2024

I don't like "Unknown" state because it's misleading as we know the state of alarms. I agree that "Unavailable" isn't much better but I don't think we should introduce a breaking change when alternative isn't significantly better.

Proper solution could be pushing discussing in home-assistant/architecture#635 to introduce new "unset" state or something with similar meaning. If you are willing to contribute there, that will be a welcome improvement.

@EuleMitKeule
Copy link
Author

Once again I am getting annoyed by this behaviour of the alarm entities of this integration. When using the Watchman integration to show entities that have gone down/unavailable to indicate problems the alarm entities of course are included, even though nothing is wrong with them.

I don't think the architecture proposal will ever lead anywhere, since the alarm entities are sensors and all sensor device classes behave the same. They either have a numeric value or a string value and always show "Unknown" when the this value is not present.

Of course it would be a breaking change - users would have to change occurences where they check if the state equals unavailable to unknown - but I don't think this is that much to ask for. On the other hand the integration is currently unusable for users that don't want entities that report configuration issues when there aren't any.

@KapJI
Copy link
Collaborator

KapJI commented Nov 19, 2024

Watchman integration allows to specify list of ignored entities.

Before better solution is available, I don't think it's worth it to introduce a breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Timer / Alarm entities set to Unavailable if no timers/alarms, should be none
3 participants