-
-
Notifications
You must be signed in to change notification settings - Fork 30.2k
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
Write state if schedule update state from async context #31758
Conversation
Seems we should drop the force feature from the function and replace all references of force=false with just a write call? The force naming is a bit confusing anyway. |
That would be a too big of a breaking change for custom components I think. |
Suppose it would have to be done by marking it deprecated, and just having it call two new functions. But sure, probably not worth the hassle as a cleanup action. |
@SukramJ I am running into a failed test with Homematic IP cloud: This PR makes calls to |
The reason the other tests were failing is that we call 1 async function less. That 1 async function was waited for when we call |
if force_refresh: | ||
self.hass.async_create_task(self.async_update_ha_state(force_refresh)) | ||
else: | ||
self.async_write_ha_state() |
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.
The name of async_schedule_update_ha_state
doesn't match what happens here now.
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.
True. But the intention of the method is still true: write the state to HA without a guarantee that it's available after the function call is done. I'll update the doc string
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.
We're now removing the choice from the developer if the state should be updated in this iteration or a later iteration of the event loop. That might be ok. 🤷♂️
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.
I don't think that's not something for the developer to decide. The developer can only indicate that it has new data and wants to be updated. I can't think of a reason for a developer wanting to let us know, but not write it to the state machine as soon as possible.
|
||
@callback |
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.
update_list
is not async safe. We're loading games which loads a json file.
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.
Good catch, fixed.
Hi @balloob, BR |
I'm a bit confused why it needs to be forced now . So by writing a state faster to the state machine than before we can end up requiring to do a full update? Was there a race condition and the full update was always needed? |
Yes you are right |
10 Minutes too fast. I create a new PR to avoid the |
Breaking change
Proposed change
If we're in async land scheduling a state update and we know we're not going to trigger an update, write the state directly.
Type of change
Example entry for
configuration.yaml
:# Example configuration.yaml
Additional information
Checklist
black --fast 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
.The integration reached or maintains the following Integration Quality Scale: