G-Two / homeassistant-subaru

Subaru STARLINK custom component for Home Assistant.
Apache License 2.0
54 stars 7 forks source link

EV time to fully charge displays 12/31/69 when fully charged #47

Closed bhaggs closed 2 years ago

bhaggs commented 2 years ago

Noticed after upgrading to 0.6.0 that the sensor for EV_TIME_TO_FULLY_CHARGED_UTC will display the epoch time once it has fully charged. This results in lovelace entity cards displaying the time to fully charge as 53 years ago. I don't recall this being the case in previous builds (I think it was just blank). Not sure if there is a better way to handle this given it's a timestamp.

stboch commented 2 years ago

@bhaggs can you get a debug of when this is reporting? I think I know the issue but since I don't have a ev can't test...

@G-Two I think this has to do with setting the timestamp to none when charged rather then last update timestamp. Home-assistant looks like it interprets Nones as 0 so it's set the the beginning of unixtime.

controller.py line 987

 ​                ​data​[​sc​.​EV_TIME_TO_FULLY_CHARGED_UTC​] ​=​ ​None
G-Two commented 2 years ago

I just released v0.6.1 which now reports this state as unavailable when the vehicle is fully charged or is otherwise not charging.

bhaggs commented 2 years ago

Loaded up v0.6.1 and now get the unavailable state once fully charged. Thanks for the fix.

bhaggs commented 2 years ago

Re-opening as now I'm seeing when it is charging I get invalid timestamp in an entity card. Attempting to load the history just results in an endless loading spinner. Though in the dev tools it does show a timestamp for current state.

Screen Shot 2022-02-15 at 10 26 30 AM
bhaggs commented 2 years ago

Looked at this a little more and noticed that the timestamp is missing T between the date and the time. When I manually edit the state the timestamp then renders properly in the entity card. Looking at the code it looks like there might need to be a tweak made to the subarulink library.

G-Two commented 2 years ago

I see the same problem on my end, but I'm having trouble finding a proper fix. The sensor value provided by subarulink is a datetime object with a UTC timezone (in accordance with HA requirements) , so it shouldn't be responsible for adding the T separator when converted to a string.

'EV_TIME_TO_FULLY_CHARGED_UTC': datetime.datetime(2022, 2, 19, 7, 8, 32, tzinfo=datetime.timezone.utc)

To make matters more confusing, I only show an invalid timestamp when viewing from my phone (using either the iOS app or iOS Safari/Chrome). Using Chrome on my laptop, the sensor shows a proper time remaining while charging. I agree the T should be there, and makes the error go away, but I think the problem is in the sensor.py implementation (possibly combined with a frontend bug?) rather than the value subarulink is providing. Of additional note, I get an error logged on the HA server whenever I view/refresh my dashboard that has the sensor on it:

2022-02-19 03:59:26 ERROR (MainThread) [frontend.js.latest.202202030] https://<hostname-from-client-point-of-view>/frontend_latest/729f9b9d.js:98:51104 RangeError: number argument must be finite

I tried tweaking a few things with subarulink 0.4.3 but it hasn't made a difference. I use other integrations with future timestamps showing the T separator properly, so I'll try to dig into those to see if they offer some clues.

stboch commented 2 years ago

@G-Two not sure if this is it but it looks like some people are saying that safari JS doesn't parse tz info and that might be the what is throwing as invalid. see if the other service is taking tz into account or just remove tz from your dt object and see if that improves the result.

G-Two commented 2 years ago

Thanks for the tip, but removing TZ info from the datetime object didn't seem to fix the invalid timestamp when viewing from my iOS device. It also induced a time delta error in the time remaining rendered on my laptop since HA assumed it was local time.

stboch commented 2 years ago

https://github.com/home-assistant/core/pull/52671

I think this is it. also cited another integration with the same safari issues homefully this helps figure it out.

bhaggs commented 2 years ago

This fallback/backward compatibility will be removed in Home Assistant 2022.2

Coincides with when I started noticing the issue. So it's possible it wasn't due to any change in this integration but backward compatibility being dropped in 2022.2.

G-Two commented 2 years ago

Thanks for the help! I've managed to fix it locally by inheriting the built-in SensorEntity class. So the timestamp is fixed now...but it broke unit conversions for all the other sensors. Once I get around to fixing the other sensors, I'll publish a new release.

G-Two commented 2 years ago

@bhaggs the issue is fixed in the v0.6.2-dev0 release. Thanks for reporting; it's good to have another EV user to help find and fix EV-specific issues. @stboch thanks for pointing me towards the relevant HA core issues.

bhaggs commented 2 years ago

Just loaded up the dev release and will keep an eye on it. So far, the timestamp is appearing correctly in entity cards while charging.

stboch commented 2 years ago

good thing you refactored because this update would have removed the fallback method you were using and would have broken anyway. https://github.com/home-assistant/core/pull/65734

bhaggs commented 2 years ago

48 Has been working well for me with no issues and also just upgrade Home Assistant to 2022.3 and remains working fine. Will close this issue out.