-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
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
Add rainbird request debouncer and immediately update entity switch state #112152
Add rainbird request debouncer and immediately update entity switch state #112152
Conversation
Hey there @konikvranik, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
@allenporter thanks for your work |
It's not merged yet. Once merged it will be in a release. We haven't tagged this for a specific patch release so it will probably be in the 2024.4 release. |
Thanks for explaining. I assume there is no way to test it before as a "beta" ? |
There are nightly dev releases, then betas are cut a week before the prod release for early community testing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @allenporter 👍
As this is a bugfix, is it fine to ship it with the next release, or are there concerns about not doing it? @allenporter |
I'm just being conservative... This integration has had these problems for a couple years. There are 2 changes that probably need to go in given a merge conflict, but they are both stability fixes. I assume the current beta cycle is slowing down, but I guess we could put both changes in and make this round. |
Glad that we have maybe a fix for this after years ;-) Otherwise it's not
stable for a real automation for me :-)
I already signed up to beta and waiting to test it ;-)
Am Mo., 4. März 2024 um 15:07 Uhr schrieb Allen Porter <
***@***.***>:
… As this is a bugfix, is it fine to ship it with the next release, or are
there concerns about not doing it? @allenporter
<https://github.com/allenporter>
I'm just being conservative... This integration has had these problems for
a couple years.
There are 2 changes that probably need to go in given a merge conflict,
but they are both stability fixes. I assume the current beta cycle is
slowing down, but I guess we could put both changes in and make this round.
—
Reply to this email directly, view it on GitHub
<#112152 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AQOAZJ7HHGBB5VHKJCWFOULYWR5ZLAVCNFSM6AAAAABEEPDTMWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZWGY3DCMBQHA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
The other change makes a connection pool change, and while I tested manually maybe there another side effect I missed... I broke this integration a couple times before with what I thought was a small change so maybe I just prefer the full test cycle... |
Proposed change
Add rainbird request debouncer and immediately update entity switch state. This avoids a problem where when setting a switch it goes back to the old state after a few seconds because the rainbird device state takes a moment to actually reflect the new state.
This adds a request debouncer with a cooldown of 5 seconds so that it will request the new state with a few seconds for it to be updated.
The existing tests still pass without modification, and this was tested manually against a live device.
Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: