GPS Altitude Wrong Unit of measure

sfstar / hass-victron

Integration for Home Assistant to fetch data from the victron gx device via modbusTCP

Apache License 2.0

174 stars 25 forks source link

GPS Altitude Wrong Unit of measure #28

Closed Next9999 closed 6 months ago

Next9999 commented 1 year ago

GPS Altitude Wrong Unit of measure

m/s should be meter only

flyrmyr commented 1 year ago

I've noticed the same with my manual setup in the past. The Cerbo is actually passing that as the unit value... Seems the error might be with VenusOS. Also, my value is just wildly off. Head scratcher.

sfstar commented 1 year ago

Thank you for reporting. It indeed is an incorrect defintion in the modbus specification sheet

Please see my comment in #36 as to why fixing this in the integration is currently on hold

askpatrickw commented 1 year ago

Looks like Victron isn't engaging on the open issue, How about we test a fix?

Is the fix to change "gps_altitude": RegisterInfo(2808, INT32, UnitOfSpeed.METERS_PER_SECOND, 10) to "gps_altitude": RegisterInfo(2808, INT32, UnitOfSpeed.METERS, 10)

https://github.com/sfstar/hass-victron/blob/3b7fd201de6d7bc2daac1fd51b51ec46990660e3/custom_components/victron/const.py#L618C81-L618C81

Yeah, I think the change should just move forward. The commit which added Altitude modified the .CSV and the code. I'd ignore the XLXS, IMHO since no one is replying.

askpatrickw commented 1 year ago

https://github.com/victronenergy/dbus_modbustcp/issues/31 is now closed and they mention they will update the docs accordingly.

sfstar commented 6 months ago

Hello @askpatrickw,

And thank you for tracking the remote issue. Unfortunately, your updates went under my radar the last couple of months. Will create an PR to fix this issue now that the documentation has been patched.

sfstar commented 6 months ago

Closing issue as fixed