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.

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

Doesn't timeout and retry when rebooting the device #31

Closed
spacelama opened this issue Sep 24, 2024 · 2 comments · Fixed by #32
Closed

Doesn't timeout and retry when rebooting the device #31

spacelama opened this issue Sep 24, 2024 · 2 comments · Fixed by #32

Comments

@spacelama
Copy link
Contributor

My tasmota devices insist on changing certain parameters every time, and rebooting (eg, SetOption68="ON"). This causes the play to lock up and never timeout, retry or fail (gave up waiting before the hour was up I think, but that's hardly practical when trying to set dozens of parameters across dozens of devices).

I was hoping to implement TimeoutHTTPAdapter per https://stackoverflow.com/questions/61974206/timeout-within-session-while-sending-requests but can't convince send() to actually run and don't know enough python to fully understand (verified that the inherited init is being called with my timeout=5, but not send()).

Since the session doesn't seem to retry automatically upon timeout, next best thing was to just unconditionally retry requests.get() once only (per parameter, allowing for reboots between each, and assuming 10 seconds for a reboot), and this survived my tasmota devices all rebooting at random multiple points throughout the play:

--- a/action_plugins/tasmota.py
+++ b/action_plugins/tasmota.py
@@ -106,7 +106,10 @@ class ActionModule(ActionBase):
         status_params.update( {'cmnd' : command } )
 
         # execute command
-        status_response = requests.get(url = endpoint_uri, params = status_params)
+        try:
+            status_response = requests.get(url = endpoint_uri, params = status_params, timeout=10)
+        except:
+            status_response = requests.get(url = endpoint_uri, params = status_params, timeout=10)
         # get response data
         data = status_response.json()
         display.v("data: %s, response code: %s" % (data, status_response.status_code))
@@ -241,7 +244,10 @@ class ActionModule(ActionBase):
                 else:
                     change_params.update( { 'cmnd' : ("%s %s" % (command, incoming_value)) } )
 
-                change_response = requests.get(url = endpoint_uri, params = change_params)
+                try:
+                    change_response = requests.get(url = endpoint_uri, params = change_params, timeout=10)
+                except:
+                    change_response = requests.get(url = endpoint_uri, params = change_params, timeout=10)
                 if status_response.status_code != 200:
                     raise AnsibleRuntimeError("Unexpected response code: %s" % (status_response.status_code))
 

Mind implementing some sort of timeout more competently than I can?

@tobias-richter
Copy link
Owner

Hi @spacelama sadly I have no use-case for SetOption68 and due to limit time and this being an hobby project it is quite likely that I will not take implement a fix/change. But you never know, maybe there will be a use-case in the future.

But here are some ideas/tipps:

  • Wouldn't it be an option to move SetOption68 to the end of the commands?
  • Have you tried to run it in verbose mode (-vvv). Maybe there are some hints why it is always detected as changed. maybe instead of "on" the value 1 is returned, or something like that.

@spacelama
Copy link
Contributor Author

I care less about the command rerunning every time (I acknowledge there's a whole bunch of commands that need mapping between input and output, and the job seems too big). My own tasmota plays want to change a whole bunch of params every time, and a number of them cause tasmota to reboot.

What this issue is more about retrying those and not failing. My timeout/retry hack works quite well for the devices I have, but the code smells. But it's better than not having that workaround! It looks like there's already an attempt to transparently retry via the use of requests.Session, but it doesn't quite seem to work by itself, hence my suggested change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants