tobias-richter / ansible-tasmota

Ansible Role for managing tasmota devices with tasmota commands
Apache License 2.0
31 stars 13 forks source link

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

Closed spacelama closed 3 weeks ago

spacelama commented 1 month ago

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 commented 1 month ago

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:

spacelama commented 1 month ago

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.