Increase Tomato request timeout #104203
Increase Tomato request timeout #104203
Conversation
![home-assistant[bot]](https://avatars.githubusercontent.com/in/97978?s=60&v=4){kind=small}
There was a problem hiding this comment.
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
home-assistant
bot
added
Quality Scale: No score
cla-recheck
cla-signed
and removed
cla-recheck
cla-needed
labels
joostlek
changed the title
|
The change from 3 to 60 seconds is large. What is the minimal timeout time that would work? |
Agreed. In my circumstances I think I get response in some 6 seconds based on limitted testing, however, it might differ since Tomato is available for range of devices with different CPU/RAM and it also depends on the utilisation. I am 100% sure 3seconds is insufficient since Tomato v2022.8 as the integration doesn't work at all. In my case it's Netgear Nighthawk R7000. I suggest to give it a couple of hours of testing on 15 seconds and should it work well I push another PR with 15 second if that's acceptable? |
|
You need to update the timeout in the tests as well to make CI test pass so it seems. |
I'm still testing it. I can confirm with the increased limit of 60s it works without issue (recognises devices offline/online within minute when disconnected/connected). My router response is really fast (less than 1ms), but only when hitting the specific endpoint 'status-devices.asp' on my router takes forever. I'm attaching screenshot of the endpoint in the router loading, it takes as much as 45s: I tried to factory reset the router and the issue still persists. I'm not the only user reporting it but there was not too many people raising it. This was introduced about a year ago with the router running Tomato 2022.8 and still lasts until now, running Tomato 2023.4. Is there any chance we could make the Timeout value as an optional in config.yaml with default=3 and if someone has issues like I do they can change it to whatever they want, like 60? Alternatively, can someone else using Tomato confirm if their |
|
Let set the 60 sec time out. But update the CI tests to make them pass. |
|
@ertechdesign I assume the PR is ready for review. If that is the fact please make it ready for review again so that the PR can be merged, |
Proposed change
Tomato integration was not working for about a year for many users. It was enough to extend the timeout. As soon as the timeout period was extended the integration works like a charm.
#103892
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.To help with the load of incoming pull requests: