From 0cc32e2a2d51fca57765cab5f8e8554f6fdc3abd Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Wed, 25 Nov 2020 15:48:14 -0800 Subject: [PATCH 1/4] Increase Retry Count on DBGet and DBModify Not sure why these are set to 0 or unset which defaults to 0. For i2 devices there is not a concern about getting confused as all messages have the address in them. --- insteon_mqtt/handler/DeviceDbGet.py | 2 +- insteon_mqtt/handler/DeviceDbModify.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/insteon_mqtt/handler/DeviceDbGet.py b/insteon_mqtt/handler/DeviceDbGet.py index 0d4cad2f..20066380 100644 --- a/insteon_mqtt/handler/DeviceDbGet.py +++ b/insteon_mqtt/handler/DeviceDbGet.py @@ -22,7 +22,7 @@ class DeviceDbGet(Base): Each reply is passed to the callback function set in the constructor which is usually a method on the device to update it's database. """ - def __init__(self, device_db, on_done, num_retry=0): + def __init__(self, device_db, on_done, num_retry=3): """Constructor The on_done callback has the signature on_done(success, msg, entry) diff --git a/insteon_mqtt/handler/DeviceDbModify.py b/insteon_mqtt/handler/DeviceDbModify.py index 73c632a1..40f95ba5 100644 --- a/insteon_mqtt/handler/DeviceDbModify.py +++ b/insteon_mqtt/handler/DeviceDbModify.py @@ -18,7 +18,7 @@ class DeviceDbModify(Base): modifications to the device's all link database class to reflect what happened on the physical device. """ - def __init__(self, device_db, entry, on_done=None): + def __init__(self, device_db, entry, on_done=None, num_retry=3): """Constructor Args: From 9f11198a6c3474929b7be9c68829b53505faa7ee Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Wed, 25 Nov 2020 15:52:37 -0800 Subject: [PATCH 2/4] Carry Through Retry to Super --- insteon_mqtt/handler/DeviceDbModify.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/insteon_mqtt/handler/DeviceDbModify.py b/insteon_mqtt/handler/DeviceDbModify.py index 40f95ba5..c98526a7 100644 --- a/insteon_mqtt/handler/DeviceDbModify.py +++ b/insteon_mqtt/handler/DeviceDbModify.py @@ -29,7 +29,7 @@ def __init__(self, device_db, entry, on_done=None, num_retry=3): added to the handler. Signature is: on_done(success, msg, entry) """ - super().__init__(on_done) + super().__init__(on_done, num_retry) self.db = device_db self.entry = entry From dc0f64a082c6247788a4d0f1ce1e5e03e1f3b17b Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Thu, 3 Dec 2020 12:09:29 -0800 Subject: [PATCH 3/4] No Retry on Device DB Get But Increase Timeout If the dumping of the database times out halfway through, there is nothing we can do but wait. Resending the request half way through doesn't cause the device to do anything and only risks talking over the device messages. Instead, since this routine is somewhat delicate, we give it double the time_out to allow any messages to arrive. --- insteon_mqtt/handler/DeviceDbGet.py | 14 ++++++++++++-- insteon_mqtt/handler/DeviceRefresh.py | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/insteon_mqtt/handler/DeviceDbGet.py b/insteon_mqtt/handler/DeviceDbGet.py index 20066380..f1a0912c 100644 --- a/insteon_mqtt/handler/DeviceDbGet.py +++ b/insteon_mqtt/handler/DeviceDbGet.py @@ -22,7 +22,7 @@ class DeviceDbGet(Base): Each reply is passed to the callback function set in the constructor which is usually a method on the device to update it's database. """ - def __init__(self, device_db, on_done, num_retry=3): + def __init__(self, device_db, on_done, num_retry=0, time_out=10): """Constructor The on_done callback has the signature on_done(success, msg, entry) @@ -38,8 +38,18 @@ def __init__(self, device_db, on_done, num_retry=3): handler times out without returning Msg.FINISHED. This count does include the initial sending so a retry of 3 will send once and then retry 2 more times. + Retries should be 0 for this handler. This is because the + only message sent out is the initial request for a dump of + the database. If the handler times out, there is no way + to recover, besides starting the request over again. + time_out (int): Timeout in seconds. The default for this handler + is double the default rate. This is because the + communication is almost entirely one-sided coming + from the device. There is nothing we can do from + this end if a message fails to arrive, so we keep + the network as quiet as possible. """ - super().__init__(on_done, num_retry) + super().__init__(on_done, num_retry, time_out) self.db = device_db #----------------------------------------------------------------------- diff --git a/insteon_mqtt/handler/DeviceRefresh.py b/insteon_mqtt/handler/DeviceRefresh.py index 869f7c76..8d90897e 100644 --- a/insteon_mqtt/handler/DeviceRefresh.py +++ b/insteon_mqtt/handler/DeviceRefresh.py @@ -127,7 +127,7 @@ def on_done(success, message, data): db_msg = Msg.OutExtended.direct(self.addr, 0x2f, 0x00, bytes(14)) msg_handler = DeviceDbGet(self.device.db, on_done, - num_retry=3) + num_retry=0) self.device.send(db_msg, msg_handler) # Either way - this transaction is complete. From 1dcb92152bebfc66f23dc36fb7083cff73fbdd12 Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Thu, 10 Dec 2020 09:32:46 -0800 Subject: [PATCH 4/4] Allow Retry and Regular Timeout to Initial Get DB Request Remove retries and increase the timeout once there is no longer anything to be sent from the modem end. --- insteon_mqtt/handler/DeviceDbGet.py | 29 +++++++++++++++++---------- insteon_mqtt/handler/DeviceRefresh.py | 2 +- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/insteon_mqtt/handler/DeviceDbGet.py b/insteon_mqtt/handler/DeviceDbGet.py index f1a0912c..76fb47f5 100644 --- a/insteon_mqtt/handler/DeviceDbGet.py +++ b/insteon_mqtt/handler/DeviceDbGet.py @@ -22,7 +22,7 @@ class DeviceDbGet(Base): Each reply is passed to the callback function set in the constructor which is usually a method on the device to update it's database. """ - def __init__(self, device_db, on_done, num_retry=0, time_out=10): + def __init__(self, device_db, on_done, num_retry=3, time_out=5): """Constructor The on_done callback has the signature on_done(success, msg, entry) @@ -38,16 +38,19 @@ def __init__(self, device_db, on_done, num_retry=0, time_out=10): handler times out without returning Msg.FINISHED. This count does include the initial sending so a retry of 3 will send once and then retry 2 more times. - Retries should be 0 for this handler. This is because the - only message sent out is the initial request for a dump of - the database. If the handler times out, there is no way - to recover, besides starting the request over again. - time_out (int): Timeout in seconds. The default for this handler - is double the default rate. This is because the - communication is almost entirely one-sided coming - from the device. There is nothing we can do from - this end if a message fails to arrive, so we keep - the network as quiet as possible. + Retries only apply to the initial get request and the ack + of that request. The subsequent messages are streamed from + the device without further requests. If the handler times + out after the initial request, there is no way to recover, + besides starting the request over again. + time_out (int): Timeout in seconds. The regular timeout applies to + the initial request. The subsequent messages are + streamed from the device without further action. + Because the communication from this point on is + entirely one-sided coming from the device. There is + nothing we can do from this end if a message fails to + arrive, so we keep the network as quiet as possible + by doubling the timeout. """ super().__init__(on_done, num_retry, time_out) self.db = device_db @@ -93,6 +96,10 @@ def msg_received(self, protocol, msg): if msg.flags.type == Msg.Flags.Type.DIRECT_ACK: LOG.info("%s device ACK response", msg.from_addr) + # From here on out, the device is the only one talking. So + # remove any remaining retries, and double the timeout. + self._num_retry = 0 + self._time_out = 2 * self._time_out return Msg.CONTINUE elif msg.flags.type == Msg.Flags.Type.DIRECT_NAK: diff --git a/insteon_mqtt/handler/DeviceRefresh.py b/insteon_mqtt/handler/DeviceRefresh.py index 8d90897e..869f7c76 100644 --- a/insteon_mqtt/handler/DeviceRefresh.py +++ b/insteon_mqtt/handler/DeviceRefresh.py @@ -127,7 +127,7 @@ def on_done(success, message, data): db_msg = Msg.OutExtended.direct(self.addr, 0x2f, 0x00, bytes(14)) msg_handler = DeviceDbGet(self.device.db, on_done, - num_retry=0) + num_retry=3) self.device.send(db_msg, msg_handler) # Either way - this transaction is complete.