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 device class enum where possible and supply possible states for those sensors #4394

Merged
merged 4 commits into from May 13, 2024

Conversation

dshokouhi
Copy link
Member

Summary

Fixes #4393

the device class is only set for when the new device class was added.

i decided to keep the options for all users as I can personally see value in it for user on older versions.

i reviewed all our sensors and these ones have a defined list in our code.

Screenshots

image

Link to pull request in Documentation repository

Documentation: home-assistant/companion.home-assistant#n/a

Any other notes

@jpelgrom
Copy link
Member

jpelgrom commented May 7, 2024

For CarSensorManager: these values seem like they would be localized by the framework? Do we also know if this is a limited list or if Google can add new ones?

I see those are strings we defined now. Can we maybe do a small refactor so the values aren't duplicated?

@dshokouhi
Copy link
Member Author

dshokouhi commented May 8, 2024

I see those are strings we defined now. Can we maybe do a small refactor so the values aren't duplicated?

hmm not really sure I see how we can share a list of strings with the when statement like that 🤔

edit: figured it out, created a map and used that instead

Copy link
Member

@jpelgrom jpelgrom left a comment

Choose a reason for hiding this comment

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

Nice QoL improvement!

The 'options' attribute also shows up in-app in the attributes list but I think that's fine as the frontend also shows 'Possible states' as an attribute.

@dshokouhi dshokouhi merged commit d640b9d into home-assistant:master May 13, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FR] Use device_class: enum for string sensors with known enumerable values.
2 participants