HandyHat / ha-hildebrandglow-dcc

Home Assistant integration for UK SMETS (Smart) meters pulling data from the DCC via the Hildebrand Glow API
MIT License
230 stars 33 forks source link

Intermittent results #187

Closed ColinRobbins closed 2 years ago

ColinRobbins commented 2 years ago

Describe the bug I am seeing intermittent results, when looking at the output via the Energy integration. There are no errors showing in the log. 86E4FA53-5706-4A29-A47F-17FF88C4262B Version This seems to have occurred since the 0.6 rollout.

Debugging By removing the if statement to only check for data in 5 minute windows, the error goes away. (Line 210 sensor.py)

I cannot yet see what the issue is, this check was the advice from the Glow forum.

Is anyone else seeing the issue?

HandyHat commented 2 years ago

I believe this is to do with Hildebrand's rate limiting - I'm working with them on a solution šŸ¤žšŸ¾

viking2010 commented 2 years ago

Yeah I have noticed the same issue. Can you advise where (or even if I should!) I remove the IF statement? I mean, I know it's in the sensor.py file, but not exactly sure what I need to comment out/remove/change. Perhaps I should wait for an update :-D

ColinRobbins commented 2 years ago

@viking2010 , I changed

    async def _glow_update(self, func: Callable) -> None:
        """Get updated data from Glow."""
        if self.initialised is True:
            minutes = datetime.now().minute
            if not ((0 <= minutes <= 5) or (30 <= minutes <= 35)):
                # only need to update one per every 30 minutes
                # anything else Glow will ignore
                return

        self.initialised = True

to

    async def _glow_update(self, func: Callable) -> None:
        """Get updated data from Glow."""

        self.initialised = True

I.e., delete the lines.

This seems to mainly fix the issue, although some results are dropped.

@HandyHat , not quite sure how this relates to rate limiting, as this should make the problem worse, rather than fix it.

I suspect some sort of timing issue, as to exactly when the 30 minutes updates are done.

viking2010 commented 2 years ago

@ColinRobbins Thanks for sharing the edits. I've made the same change now and will see how things look. Like you say though, you'd think this would make the issue of rate limiting worse rather than better. Hopefully @HandyHat will be able to investigate and resolve with Hildebrand's support.

HandyHat commented 2 years ago

@HandyHat , not quite sure how this relates to rate limiting, as this should make the problem worse, rather than fix it.

From the explanation that Hildebrand provided, what I understand is that the rate limiting entails caching the yearly value for an hour after the last call, preventing it from updating if it has been less than an hour when it is next polled in home assistant. By reducing the polling interval, it checks more often and is going to poll soon after the cache expires, fixing the problem.

ColinRobbins commented 2 years ago

Thanks for the explanation, I can see now. This confirms my worry about timing accuracy in HA, as the current 0.6 code should request an update every 30 minutes, so should work. Every other request should be ignored. But in some case 2 requests are ignored. My suspicion is sometimes the second call comes after 59:59, and not 60:00 so is rejected.

HandyHat commented 2 years ago

It's definitely an interesting edge case, because there's only a second or so in it and sometimes it does work fine

HandyHat commented 2 years ago

I've been in contact with Hildebrand and they've kindly agreed to reduce the cache to 59 minutes and 50 seconds after the first call, so hopefully this issue should now be resolved?

ColinRobbins commented 2 years ago

Sounds good. Letā€™s seeā€¦

viking2010 commented 2 years ago

@HandyHat I've recently rebuilt my HA implementation (made too many mistakes with my first attempt!!) and I'm no longer getting any intermittent results. Previously I had removed some of the code from the IF statement in line 210 of sensor.py but this is no longer necessary.

For me this issue is resolved.

HandyHat commented 2 years ago

Great - thanks @viking2010