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

Allow binary sensors and sensors to be added to driving favorites #3732

Merged
merged 3 commits into from
Jul 31, 2023

Conversation

dshokouhi
Copy link
Member

@dshokouhi dshokouhi commented Jul 28, 2023

Summary

Fixes #3248 by allow binary_sensor and sensor to be added to the favorites list. Since there is a potential for majority of users to have more entities than their vehicle limit will allow I opted not to add a new domain category to the main supported list.

When a non actionable entity is added to the grid we are going to skip the click listener so they cannot be selected

Screenshots

Link to pull request in Documentation repository

Documentation: home-assistant/companion.home-assistant#967

Any other notes

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.

While I understand that users want to have sensors visible, I'm still not sure this is the right approach. If the question "is [sensor] [sensor state]?" is answered with a non-desirable answer, how would you act on it? IMO a helper/template entity based on the sensor that allows you to do said action is better. Google seems to agree that having something you can interact with is better:

SHOULD | Have an action associated with each grid item (information-only items are not recommended).

@dshokouhi
Copy link
Member Author

While I understand that users want to have sensors visible, I'm still not sure this is the right approach. If the question "is [sensor] [sensor state]?" is answered with a non-desirable answer, how would you act on it? IMO a helper/template entity based on the sensor that allows you to do said action is better. Google seems to agree that having something you can interact with is better:

SHOULD | Have an action associated with each grid item (information-only items are not recommended).

So there is one issue regarding helper/template entity. None of the supported domains today will allow for states seen by sensor, now binary_sensor can be put into a template/helper easily but you can't do that with a temperature sensor or any other type of different data. A sensor can for example let someone show the state of a number or select entity. So I do see some value there. The items are also in a grid so users can be a little unique in their designs and make rows with visual data.

@jpelgrom
Copy link
Member

don't think its needed as these are non-actionable domains and we are currently not listing which domains favorites supports

It might be worth a comment in the documentation because sensors are only supported using favorites.

@dshokouhi
Copy link
Member Author

don't think its needed as these are non-actionable domains and we are currently not listing which domains favorites supports

It might be worth a comment in the documentation because sensors are only supported using favorites.

docs added :)

@jpelgrom
Copy link
Member

This seems to be working mostly fine except that state changes are not reflected until restarting Auto (not limited to sensors, any favorite entity) - do you want to address that here or in another PR?

@dshokouhi
Copy link
Member Author

This seems to be working mostly fine except that state changes are not reflected until restarting Auto (not limited to sensors, any favorite entity) - do you want to address that here or in another PR?

im actually trying to fix that in a different branch since that issue was introduced previously, admittedly I am stuck on a proper working solution 🤔

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.

Let's add this and see what the user feedback is (and Google feedback...), hard to deny it is a popular request and we should do something to make it easier.

@JBassett JBassett merged commit a111776 into home-assistant:master Jul 31, 2023
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.

Android Auto sensors display
3 participants