Remove force from async_schedule_update_ha_state for HMIPC #31796
Conversation
probot-home-assistant
bot
added
has-tests
integration: homematicip_cloud
small-pr
|
It's not only the decorator. Good that @MartinHjelmare identified the other one. |
This comment has been minimized.
This comment has been minimized.
| @@ -95,10 +96,19 @@ def supported_features(self) -> int: | |||
| """Register callbacks.""" | |||
| self._home.on_update(self._async_device_changed) | |||
|
|
|||
| @callback | |||
There was a problem hiding this comment.
Why does it matter if we decorate this method? Isn't this callback called from the device library in the home instance?
There was a problem hiding this comment.
it makes no difference, but i thought it has to be there. should it be removed?
There was a problem hiding this comment.
It doesn't hurt, but it has no effect.
There was a problem hiding this comment.
It's good to have decorators on all methods that we expect to be only called inside the event loop. It helps understand the code.
There was a problem hiding this comment.
We will never be able to do that everywhere. I don't think we should try.
There was a problem hiding this comment.
We actually already do everywhere. See:
git grep -B1 " def async_" | grep -v 'callback' | grep -v '\-\-' | grep -v ' def '
git grep -B1 " def _async_" | grep -v 'callback' | grep -v '\-\-' | grep -v ' def '
I think that it makes it very clear for people reading the code about what kind of context the method can/should be called.
There was a problem hiding this comment.
We'll never decorate other library functions 100%. We won't be able to trust that this decorator is the truth about this. You have to read other things in the code to find out.
It's a war we cannot win. I think we should pick our battles.
MartinHjelmare
changed the title
Proposed change
remove force from async_schedule_update_ha_state for HMIPC.
Type of change
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: