home-assistant / android

:iphone: Home Assistant Companion for Android
https://companion.home-assistant.io/
Apache License 2.0
2.34k stars 656 forks source link

Add car speed sensor #4722

Closed dshokouhi closed 1 month ago

dshokouhi commented 1 month ago

Summary

Adds a car speed sensor whose state will contain display speed in meters per second, Attributes for raw speed and also the display units are supported (untested due to head unit emulator limitations). The values can be either positive, negative or 0 depending on vehicle movement (driving, reverse, stopped)

Screenshots

image

Link to pull request in Documentation repository

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

Any other notes

jpelgrom commented 1 month ago

I'm not sure I agree with this sensor having attributes. They seem unnecessary and don't all describe the sensor state.

With the default sensor update speed, this isn't very relevant or up-to-date. Are there any intents to listen for (like completely stopped/started moving, thresholds passed) or can you add a recommendation for a combination that does this?

dshokouhi commented 1 month ago

I'm not sure I agree with this sensor having attributes. They seem unnecessary and don't all describe the sensor state.

With the default sensor update speed, this isn't very relevant or up-to-date. Are there any intents to listen for (like completely stopped/started moving, thresholds passed) or can you add a recommendation for a combination that does this?

I'll remove the attributes but the unit is interesting admittedly I wonder if US cars and EU have a difference in what reports? With that said the display unit doesnt matter because were always getting the state in m/s and HA core can do the conversion.

I don't believe there is an intent however this sensor won't update anymore than the other more important sensors like battery and fuel level. This more closes the loop on the sensors that can be added on the list.

Also I'm curious to see if my car supports this sensor 🙈

jpelgrom commented 1 month ago

There is a merge conflict but other than that it looks good to me!