search

leeyuentuen / polestar_api

Polestar Api - Home Assistant Component
MIT License
112 stars 18 forks source link

estimate_full_charge_range max value is set wrong for miles #109

Closed carrel-gr closed 4 months ago

carrel-gr commented 5 months ago

Currently the max value for estimate_full_charge_range is set to 660 km, but when the units are miles, the max is set to 410 km (not 410 mi). And my vehicle exceeds this. It is currently reporting 263 (mi) which is 424 (km). I have a lot of these in the log:

estimate_full_charge_range: Value 263.46138550862963 is higher than max value 410

It seems the max value is initially set to 660 (in POLESTAR_SENSOR_TYPES), but then is overridden based on the units. In the case of Miles, it is overridden to a max of 410. But it appears that the comparison is done in kilometers (because 263 is not greater than 410, but 424 is).

leeyuentuen commented 4 months ago

the reason why there is a max limit is to prevent the spikes on the values

see this https://github.com/leeyuentuen/polestar_api/issues/2

carrel-gr commented 4 months ago

There is still a max value. But it doesn't change based on the display units. With this change it always uses the max value in the native units. That numeric value is "660" which is 660 km. The problem was that when I set my display units to miles, the code was changing the max from 660 to 410. That would be OK if the 410 was miles, but it is really 410 km. So that value is too low.

leeyuentuen commented 4 months ago

i'll check later on the code

carrel-gr commented 4 months ago

My car is currently reporting a 282 mile full charge range. That corresponds to 454 km. Since 454 > 410, the integration is saying the value is invalid. But it is comparing km to miles, which is wrong. You did put a max value in the sensor definition for the two range sensors. They are both 660 and the native units are both km. That seems right to me. It's just this code that changes the max value that seems wrong. I don't think the max value wants to be changed when the display units are miles or meters.

Thanks for looking. (And, as always, thanks for this integration!!!)

leeyuentuen commented 4 months ago

can you check this beta if it solved your issue? https://github.com/leeyuentuen/polestar_api/releases/tag/1.6.6

carrel-gr commented 4 months ago

Thanks! I will definitely test this, but unfortunately I am out of town and can't do it until next week.

I looked at your code changes. It does seem like the change is correct, but it doesn't look to me like it will fix this issue. Your change does change how _attr_native_value is set (which is good), but the comparison on the second line doesn't become any different. Nonetheless, thanks! And I will test next week.

carrel-gr commented 4 months ago

Voila! I tested your fix this morning and it does solve my issue. Thanks!

carrel-gr commented 4 months ago

Fixed by https://github.com/leeyuentuen/polestar_api/commit/5cb899c75a20b9819c3df69e4b80c07f6b650bb8