Skip to content
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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Netgear switch state update #69597

Merged
merged 1 commit into from
Apr 10, 2022
Merged

Fix Netgear switch state update #69597

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions homeassistant/components/netgear/switch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,12 @@ def is_on(self):
async def async_turn_on(self, **kwargs):
"""Turn the switch on."""
await self._router.async_allow_block_device(self._mac, ALLOW)
await self.coordinator.async_request_refresh()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a 10 second debounce when using async_request_refresh. If we don't want a debounce we should use async_refresh.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MartinHjelmare I am a bit torn on this, I know that the netgear API can be overloaded by sending to many requests wich can actually cause the API to crash in some rare cases (a reboot of the router is then nessesary).
So if someone makes an automation to for instance block a list of 10 devices at a specific time (for instance at bedtime of there childeren) I don't want to have 10 update API requests at once. (these updates request a whole list of properties for all connected devices so it is a rather big XML file that is requested from the Router, a single update normaly takes around 2 seconds).

Therefore I think the debounce is actually a good thing.

Am I correct that it will run the update immediately if the last update was more then 10 seconds ago?

What happens if we were to call async_refresh 10 times within 1 second while the actual update takes about 2 seconds (so 2x10 = 20 seconds), Note there is a AsyncLock on the API?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the debouncer will run the refresh immediately if the last refresh was more than 10 seconds ago.

Since we have the lock, the calls will be done sequentially.

Should we stick with the debounced refresh then?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes lets stick to the debounced refresh, I think that will cause less problems.


async def async_turn_off(self, **kwargs):
"""Turn the switch off."""
await self._router.async_allow_block_device(self._mac, BLOCK)
await self.coordinator.async_request_refresh()

@callback
def async_update_device(self) -> None:
Expand Down