From d00e948208fc146417a893bde54d560acc820131 Mon Sep 17 00:00:00 2001
From: Pim van den Berg <pim@nethuis.nl>
Date: Thu, 5 Nov 2020 13:35:57 +0100
Subject: [PATCH] feat(itho-wpu): rm slave thread + pass result to I2CMaster
 via a queue + add retry mechanism

Remove the slave thread, set_callback already spawns a thread.

I2CSlave.callback() now puts the result (if valid) in a queue. This item
can be picked up by I2CMaster for further processing.

I2CMaster will wait 0.21s for a result in the queue. 0.21s, because I
always see a response in < 0.2s. If there is no result, it will send the
request again (max 20 retries). This is because I don't get reliable
responses (via pigpiod?). In an earlier commit I mentioned to configure
pigpiod with -s 2 (sample rate 2 microseconds).  This doesn't seem to be
true. For example -s 10 gives the same reliability (and less CPU usage
by pigpiod).
---
 itho-wpu.py | 58 +++++++++++++++++++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 22 deletions(-)

diff --git a/itho-wpu.py b/itho-wpu.py
index 5cb119f..89970c1 100755
--- a/itho-wpu.py
+++ b/itho-wpu.py
@@ -6,7 +6,6 @@ import logging
 import pigpio
 import queue
 import sys
-import threading
 import time
 
 consolelogformatter = logging.Formatter("%(asctime)-15s %(levelname)s: %(message)s")
@@ -33,30 +32,26 @@ def parse_args():
                         help="Loglevel")
     parser.add_argument('--master-only', action='store_true', help="Only run I2C master")
     parser.add_argument('--slave-only', action='store_true', help="Only run I2C slave")
-    parser.add_argument('--slave-timeout', nargs='?', type=int, default=2,
-                        help="Slave timeout in seconds.")
+    parser.add_argument('--slave-timeout', nargs='?', type=int, default=60,
+                        help="Slave timeout in seconds when --slave-only")
 
     args = parser.parse_args()
     return args
 
 
 class I2CSlave():
-    def __init__(self, address):
+    def __init__(self, address, queue):
         self.address = address
+        self.queue = queue
         self.pi = pigpio.pi()
         if not self.pi.connected:
             logger.error("not pi.connected")
             return
 
-    def set_callback(self, q, timeout):
+    def set_callback(self):
         logger.debug("set_callback()")
-        e = self.pi.event_callback(pigpio.EVENT_BSC, self.callback)
+        self.event_callback = self.pi.event_callback(pigpio.EVENT_BSC, self.callback)
         self.pi.bsc_i2c(self.address)
-        logger.debug(f"Waiting {timeout}s for activity")
-        time.sleep(timeout)
-        e.cancel()
-        self.pi.bsc_i2c(0) # Disable BSC peripheral
-        self.pi.stop()
 
     def callback(self, id, tick):
         logger.debug(f"callback({id}, {tick})")
@@ -65,10 +60,11 @@ class I2CSlave():
         if b:
             logger.debug(f"Received {b} bytes! Status {s}")
             result = [hex(c) for c in d]
+            logger.debug(f"Callback Response: {result}")
             if self.is_checksum_valid(result):
-                logger.info(f"Response: {result}")
+                self.queue.put(result)
         else:
-            logger.error(f"Received number of bytes was {b}")
+            logger.debug(f"Received number of bytes was {b}")
 
     def is_checksum_valid(self, b):
         s = 0x80
@@ -78,18 +74,20 @@ class I2CSlave():
         if checksum == 256:
             checksum = 0
         if checksum != int(b[-1], 0):
-            logger.debug(f"Checksum invalid (0x{checksum:02x})")
+            logger.debug(f"Checksum invalid (0x{checksum:02x} != {b[-1]})")
             return False
         return True
 
     def close(self):
+        self.event_callback.cancel()
         self.pi.bsc_i2c(0)
         self.pi.stop()
 
 
 class I2CMaster:
-    def __init__(self, address, bus):
+    def __init__(self, address, bus, queue):
         self.i = i2c_raw.I2CRaw(address=address, bus=bus)
+        self.queue = queue
 
     def execute_action(self, action):
         actions = {
@@ -98,8 +96,19 @@ class I2CMaster:
             "getdatatype": [0x80, 0xA4, 0x00, 0x04, 0x00, 0x56],
             "getdatalog": [0x80, 0xA4, 0x01, 0x04, 0x00, 0x55],
         }
-        logger.info(f"Executing action: {action}")
-        self.i.write_i2c_block_data(actions[action])
+        result = None
+        for i in range(0, 20):
+            logger.debug(f"Executing action: {action}")
+            self.i.write_i2c_block_data(actions[action])
+            time.sleep(0.21)
+            logger.debug("Queue size: {}".format(self.queue.qsize()))
+            if self.queue.qsize() > 0:
+                result = self.queue.get()
+                break
+
+        if result is None:
+            logger.error(f"No valid result in 20 requests")
+        return result
 
     def close(self):
         self.i.close()
@@ -114,12 +123,17 @@ if __name__ == "__main__":
     q = queue.Queue()
 
     if not args.master_only:
-        slave = I2CSlave(address=0x40)
-        slave_thread = threading.Thread(target=slave.set_callback, args=[q, args.slave_timeout])
-        slave_thread.start()
+        slave = I2CSlave(address=0x40, queue=q)
+        slave.set_callback()
+        if args.slave_only:
+            time.sleep(args.slave_timeout)
 
     if not args.slave_only:
-        master = I2CMaster(address=0x41, bus=1)
+        master = I2CMaster(address=0x41, bus=1, queue=q)
         if args.action:
-            master.execute_action(args.action)
+            result = master.execute_action(args.action)
+            logger.info(f"Response: {result}")
         master.close()
+
+    if not args.master_only:
+        slave.close()