New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update sensor to only query Glow API shortly after 0 and 30 mins past the hour #136
Conversation
Only query API between 0 and 5, and 30 and 35 Random delay up to 2 minutes when querying Sleep 5 minutes after to make sure we only do 1 update in the window
Narrow window to 2 minutes around each half hour Return interval to every 2 minutes Remove 5 minute wait at end
Fix typo
Remove unnecessary logging when no action required Move comment to correct line
We use asyncio.sleep not sleep
You are seeing the warning in the logs, because the approach of sleeping in the update code works against the HA design principles. |
Add required whitspace
Isn't the polling based on when HA starts the sensor, so if it starts at 12, the poll would be at 12 and 42, rather than around 0 and 30 when the data from Glow updates? I suppose it's not an issue, it just means you could have up to 29 minutes delay in the data (29 and 59 being the worst case) |
I am not sure I have seen an thing that says the update must be at 00 and 30 minutes past the hour? Rather called only every 30 minutes. If it has to at 00 and 30, the I’d suggest we need to modify the code to integrate with HA automation in some way. Adding a sleep in the code works against the HA design principles. Hence the log warning. |
https://forum.glowmarkt.com/index.php?p=/discussion/comment/544/#Comment_544
|
If that’s the case, I suggest we need to look at an alternate HA friendly
fix.
Rather than sleep, maybe return an error of some kind. Then modifying the
sensor to reuse old data.
On Tue, 8 Feb 2022 at 19:11, Jack ***@***.***> wrote:
https://forum.glowmarkt.com/index.php?p=/discussion/comment/544/#Comment_544
You need it to send once every 30 minutes, but soon after the actual :00
or :30
—
Reply to this email directly, view it on GitHub
<#136 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADGXZ6KN4G4HKTQC2C5C4TTU2FTENANCNFSM5N3JAUWA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you commented.Message ID:
***@***.***>
--
Colin Robbins
(from mobile)
|
Fine by me - I'm new to HA (<1 month since I started using it) and wanted to try and help out fix the addon/integration to keep Glow happy and not turn off the free DCC service :) |
Sorry if my comment seems negative - don’t be discouraged, there is a big
learning curve on getting started with programming HA, we were all new to
it once.
A halfway interim fix, might be to set the retry interval to 10 or 15
minutes. That way it’ll only do a few “wasted” retries, but also reduce
the time delay past the hour.
On Tue, 8 Feb 2022 at 20:02, Jack ***@***.***> wrote:
Fine by me - I'm new to HA (<1 month since I started using it) and wanted
to try and help out fix the addon/integration to keep Glow happy and not
turn off the free DCC service :)
—
Reply to this email directly, view it on GitHub
<#136 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADGXZ6ONTR4UA6AZWPTLAIDU2FZDZANCNFSM5N3JAUWA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you commented.Message ID:
***@***.***>
--
Colin Robbins
(from mobile)
|
Thinking about this more, we’ll have to be careful not to hardwire a specific time. |
Minimum 1min sleep
Indeed - which is what my PR does - in the 2 minute window around 0 and 30 it adds a random sleep between 1 minute and 3 minutes for each call - which hopefully will stagger the calls. Though that's with an ugly sleep approach! |
Not at all - my approach isn't ideal but I couldn't easily see a way to have a sensor update run on a schedule, only an interval. I'd need a better understanding of HA and Sensors overall |
HI, I am testing an alterative fix to this PR and #135 (@si458) I've moved your time test from
This way HA will still make a call to the code evey 2 minutes (backing off #135) The It may seem an overhead in calling the fuction every 2 minutes, but the overhead is tiny (and I'd argue it does if now - only worse as it makes the call out to Glow). Its like adding a condition in an automation. |
Makes sense to me! |
I wonder if we should increase the scan interval to say 3 or 4 minutes and widen the window so that API calls can be further spread out? |
I did think about making it 5 minutes, but given the current code is 2 minutes kept it as 2. |
@ColinRobbins sorry been driving to work haha Personally my PR to change the interval to 30mins has been perfect! No issues so fair and it's a simple easy fix no messing around with timers etc |
This is to address Glow's concerns referenced in #126
There's probably a better way to do this, but it seems to work. I'm new to HA development so finding my feet a little.
This does cause HA to complain that the sensor update is taking over 10s, but doesn't seem to be a problem