From 585ce8288dbd77b405d96a424266e4ccaa24c2db Mon Sep 17 00:00:00 2001 From: Hayden Roche Date: Thu, 27 Jul 2023 15:31:01 -0700 Subject: [PATCH 1/3] Clean up serial comms code. - Add a serial_lock decorator that can be used on functions that attempt to acquire a lock on the serial port. - Make serialReadByte, serialReset, serialCommand, and serialTransaction member functions of OpenSerial instead of being free functions. - Refactor serialCommand and serialTransaction (now renamed Command and Transaction, respectively) to unify common code in a new internal function, _transmit. --- notecard/notecard.py | 201 ++++++++++++++++--------------------------- 1 file changed, 76 insertions(+), 125 deletions(-) diff --git a/notecard/notecard.py b/notecard/notecard.py index 3a84df7..cd4eda7 100644 --- a/notecard/notecard.py +++ b/notecard/notecard.py @@ -75,99 +75,20 @@ def _prepare_request(req, debug=False): return req_json -def serialReadByte(port): - """Read a single byte from a Notecard.""" - if sys.implementation.name == 'micropython': - if not port.any(): - return None - elif sys.implementation.name == 'cpython': - if port.in_waiting == 0: - return None - return port.read(1) - - -def serialReset(port): - """Send a reset command to a Notecard.""" - for i in range(10): - try: - port.write(b'\n') - except: - continue - time.sleep(0.5) - somethingFound = False - nonControlCharFound = False - while True: - data = serialReadByte(port) - if (data is None) or (data == b''): - break - somethingFound = True - if data[0] >= 0x20: - nonControlCharFound = True - if somethingFound and not nonControlCharFound: - break - else: - raise Exception("Notecard not responding") - - -def serialTransaction(port, req, debug, txn_manager=None): - """Perform a single write to and read from a Notecard.""" - req_json = _prepare_request(req, debug) - - transaction_timeout_secs = 30 - if txn_manager: - txn_manager.start(transaction_timeout_secs) - - try: - seg_off = 0 - seg_left = len(req_json) - while True: - seg_len = seg_left - if seg_len > CARD_REQUEST_SEGMENT_MAX_LEN: - seg_len = CARD_REQUEST_SEGMENT_MAX_LEN - - port.write(req_json[seg_off:seg_off + seg_len].encode('utf-8')) - seg_off += seg_len - seg_left -= seg_len - if seg_left == 0: - break - time.sleep(CARD_REQUEST_SEGMENT_DELAY_MS / 1000) - finally: - if txn_manager: - txn_manager.stop() +def serial_lock(fn): + """Attempt to get a lock on the serial channel used for Notecard comms.""" - rsp_json = port.readline() - if debug: - print(rsp_json.rstrip()) - - rsp = json.loads(rsp_json) - return rsp - - -def serialCommand(port, req, debug, txn_manager=None): - """Perform a single write to and read from a Notecard.""" - req_json = _prepare_request(req, debug) + def decorator(self, *args, **kwargs): + if use_serial_lock: + try: + with self.lock.acquire(timeout=5): + return fn(self, *args, **kwargs) + except Timeout: + raise Exception('Notecard in use') + else: + return fn(self, *args, **kwargs) - transaction_timeout_secs = 30 - if txn_manager: - txn_manager.start(transaction_timeout_secs) - - try: - seg_off = 0 - seg_left = len(req_json) - while True: - seg_len = seg_left - if seg_len > CARD_REQUEST_SEGMENT_MAX_LEN: - seg_len = CARD_REQUEST_SEGMENT_MAX_LEN - - port.write(req_json[seg_off:seg_off + seg_len].encode('utf-8')) - seg_off += seg_len - seg_left -= seg_len - if seg_left == 0: - break - time.sleep(CARD_REQUEST_SEGMENT_DELAY_MS / 1000) - finally: - if txn_manager: - txn_manager.stop() + return decorator class Notecard: @@ -225,51 +146,81 @@ def SetTransactionPins(self, rtx_pin, ctx_pin): class OpenSerial(Notecard): """Notecard class for Serial communication.""" - def Command(self, req): - """Perform a Notecard command and exit with no response.""" + def _transmit(self, req): req = self._preprocess_req(req) + req_json = _prepare_request(req, self._debug) + + transaction_timeout_secs = 30 + if self._transaction_manager: + self._transaction_manager.start(transaction_timeout_secs) + + seg_off = 0 + seg_left = len(req_json) + while seg_left > 0: + seg_len = seg_left + if seg_len > CARD_REQUEST_SEGMENT_MAX_LEN: + seg_len = CARD_REQUEST_SEGMENT_MAX_LEN + + self.uart.write(req_json[seg_off:seg_off + seg_len].encode('utf-8')) + seg_off += seg_len + seg_left -= seg_len + time.sleep(CARD_REQUEST_SEGMENT_DELAY_MS / 1000) + + if self._transaction_manager: + self._transaction_manager.stop() + + def _read_byte(self): + """Read a single byte from the Notecard.""" + if sys.implementation.name == 'micropython': + if not self.uart.any(): + return None + elif sys.implementation.name == 'cpython': + if self.uart.in_waiting == 0: + return None + return self.uart.read(1) + + @serial_lock + def Command(self, req): + """Send a command to the Notecard. The Notecard response is ignored.""" if 'cmd' not in req: raise Exception("Please use 'cmd' instead of 'req'") - if use_serial_lock: - try: - self.lock.acquire(timeout=5) - serialCommand(self.uart, req, self._debug, self._transaction_manager) - except Timeout: - raise Exception("Notecard in use") - finally: - self.lock.release() - else: - serialCommand(self.uart, req, self._debug, self._transaction_manager) + self._transmit(req) + @serial_lock def Transaction(self, req): """Perform a Notecard transaction and return the result.""" - req = self._preprocess_req(req) - if use_serial_lock: - try: - self.lock.acquire(timeout=5) - return serialTransaction(self.uart, req, self._debug, - self._transaction_manager) - except Timeout: - raise Exception("Notecard in use") - finally: - self.lock.release() - else: - return serialTransaction(self.uart, req, self._debug, - self._transaction_manager) + self._transmit(req) + + rsp_json = self.uart.readline() + if self._debug: + print(rsp_json.rstrip()) + + rsp = json.loads(rsp_json) + return rsp + @serial_lock def Reset(self): """Reset the Notecard.""" - if use_serial_lock: + for i in range(10): try: - self.lock.acquire(timeout=5) - serialReset(self.uart) - except Timeout: - raise Exception("Notecard in use") - finally: - self.lock.release() - else: - serialReset(self.uart) + self.uart.write(b'\n') + except: + continue + time.sleep(0.5) + somethingFound = False + nonControlCharFound = False + while True: + data = self._read_byte() + if (data is None) or (data == b''): + break + somethingFound = True + if data[0] >= 0x20: + nonControlCharFound = True + if somethingFound and not nonControlCharFound: + break + else: + raise Exception('Notecard not responding') def __init__(self, uart_id, debug=False): """Initialize the Notecard before a reset.""" From 8a42fc5b3d1e3a40da289adc7b4aa83d2d3cd84f Mon Sep 17 00:00:00 2001 From: Hayden Roche Date: Wed, 30 Aug 2023 16:31:23 -0700 Subject: [PATCH 2/3] Address review feedback. --- notecard/notecard.py | 139 ++++++++++++++++++++++++++++--------------- 1 file changed, 91 insertions(+), 48 deletions(-) diff --git a/notecard/notecard.py b/notecard/notecard.py index cd4eda7..9228b77 100644 --- a/notecard/notecard.py +++ b/notecard/notecard.py @@ -40,13 +40,19 @@ use_periphery = False use_serial_lock = False -if sys.implementation.name == 'cpython': - if sys.platform == 'linux' or sys.platform == 'linux2': - use_periphery = True - from periphery import I2C +if sys.implementation.name == 'cpython' and (sys.platform == 'linux' or sys.platform == 'linux2'): - use_serial_lock = True - from filelock import Timeout, FileLock + use_periphery = True + from periphery import I2C + + use_serial_lock = True + from filelock import FileLock + from filelock import Timeout as SerialLockTimeout +else: + class SerialLockTimeout(Exception): + """A null SerialLockTimeout for when use_serial_lock is False.""" + + pass use_i2c_lock = not use_periphery and sys.implementation.name != 'micropython' @@ -75,18 +81,35 @@ def _prepare_request(req, debug=False): return req_json +class NullContextManager: + """A null context manager for use with NoOpSerialLock.""" + + def __enter__(self): + """Null enter function. Required for context managers.""" + pass + + def __exit__(self, exc_type, exc_value, traceback): + """Null exit function. Required for context managers.""" + pass + + +class NoOpSerialLock(): + """A no-op serial lock class for when use_serial_lock is False.""" + + def acquire(*args, **kwargs): + """Acquire the no-op lock.""" + return NullContextManager() + + def serial_lock(fn): """Attempt to get a lock on the serial channel used for Notecard comms.""" def decorator(self, *args, **kwargs): - if use_serial_lock: - try: - with self.lock.acquire(timeout=5): - return fn(self, *args, **kwargs) - except Timeout: - raise Exception('Notecard in use') - else: - return fn(self, *args, **kwargs) + try: + with self.lock.acquire(timeout=5): + return fn(self, *args, **kwargs) + except SerialLockTimeout: + raise Exception('Notecard in use') return decorator @@ -146,40 +169,57 @@ def SetTransactionPins(self, rtx_pin, ctx_pin): class OpenSerial(Notecard): """Notecard class for Serial communication.""" + @serial_lock def _transmit(self, req): req = self._preprocess_req(req) req_json = _prepare_request(req, self._debug) - transaction_timeout_secs = 30 - if self._transaction_manager: - self._transaction_manager.start(transaction_timeout_secs) + try: + transaction_timeout_secs = 30 + if self._transaction_manager: + self._transaction_manager.start(transaction_timeout_secs) - seg_off = 0 - seg_left = len(req_json) - while seg_left > 0: - seg_len = seg_left - if seg_len > CARD_REQUEST_SEGMENT_MAX_LEN: - seg_len = CARD_REQUEST_SEGMENT_MAX_LEN + seg_off = 0 + seg_left = len(req_json) + while seg_left > 0: + seg_len = seg_left + if seg_len > CARD_REQUEST_SEGMENT_MAX_LEN: + seg_len = CARD_REQUEST_SEGMENT_MAX_LEN - self.uart.write(req_json[seg_off:seg_off + seg_len].encode('utf-8')) - seg_off += seg_len - seg_left -= seg_len - time.sleep(CARD_REQUEST_SEGMENT_DELAY_MS / 1000) + self.uart.write(req_json[seg_off:seg_off + seg_len].encode('utf-8')) + seg_off += seg_len + seg_left -= seg_len + time.sleep(CARD_REQUEST_SEGMENT_DELAY_MS / 1000) + finally: + if self._transaction_manager: + self._transaction_manager.stop() - if self._transaction_manager: - self._transaction_manager.stop() + @serial_lock + def _transmit_and_receive(self, req): + self._transmit(req) - def _read_byte(self): - """Read a single byte from the Notecard.""" - if sys.implementation.name == 'micropython': - if not self.uart.any(): - return None - elif sys.implementation.name == 'cpython': - if self.uart.in_waiting == 0: - return None + rsp_json = self.uart.readline() + if self._debug: + print(rsp_json.rstrip()) + + return json.loads(rsp_json) + + def _read_byte_micropython(self): + """Read a single byte from the Notecard (MicroPython).""" + if not self.uart.any(): + return None + return self.uart.read(1) + + def _read_byte_cpython(self): + """Read a single byte from the Notecard (CPython).""" + if self.uart.in_waiting == 0: + return None + return self.uart.read(1) + + def _read_byte_circuitpython(self): + """Read a single byte from the Notecard (CircuitPython).""" return self.uart.read(1) - @serial_lock def Command(self, req): """Send a command to the Notecard. The Notecard response is ignored.""" if 'cmd' not in req: @@ -187,17 +227,9 @@ def Command(self, req): self._transmit(req) - @serial_lock def Transaction(self, req): """Perform a Notecard transaction and return the result.""" - self._transmit(req) - - rsp_json = self.uart.readline() - if self._debug: - print(rsp_json.rstrip()) - - rsp = json.loads(rsp_json) - return rsp + return self._transmit_and_receive(req) @serial_lock def Reset(self): @@ -232,7 +264,18 @@ def __init__(self, uart_id, debug=False): self._debug = debug if use_serial_lock: - self.lock = FileLock('serial.lock', timeout=1) + self.lock = FileLock('serial.lock') + else: + self.lock = NoOpSerialLock() + + if sys.implementation.name == 'micropython': + self._read_byte = self._read_byte_micropython + elif sys.implementation.name == 'cpython': + self._read_byte = self._read_byte_cpython + elif sys.implementation.name == 'circuitpython': + self._read_byte = self._read_byte_circuitpython + else: + raise NotImplementedError(f'Unsupported platform: {sys.implementation.name}') self.Reset() From 806a44f92155fc8e7da16752da311bc89878c3cf Mon Sep 17 00:00:00 2001 From: Hayden Roche Date: Thu, 31 Aug 2023 11:54:35 -0700 Subject: [PATCH 3/3] Make more improvements. - Merge the functionality of _preprocess_req into _prepare_request. These are always used together, so it doesn't really make sense to have them as separate functions. Additionally, have _prepare_request do everything needed to transform the request data for transmission. Namely, encode the request string as UTF-8 bytes. Finally, make _prepare_request a method of Notecard. There's no reason for it to be a free function. - Refactor low-level serial comms by adding a new function, _transact. This replaces _transmit and _transmit_and_receive. It acquires the serial lock and uses the transaction manager. It will read a response from the Notecard if the rsp_expected parameter is True. --- notecard/notecard.py | 84 ++++++++++++++++++++++---------------------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/notecard/notecard.py b/notecard/notecard.py index 9228b77..f43e088 100644 --- a/notecard/notecard.py +++ b/notecard/notecard.py @@ -71,16 +71,6 @@ class SerialLockTimeout(Exception): I2C_CHUNK_DELAY_MS = 20 -def _prepare_request(req, debug=False): - """Format the request string as a JSON object and add a newline.""" - req_json = json.dumps(req) - if debug: - print(req_json) - - req_json += "\n" - return req_json - - class NullContextManager: """A null context manager for use with NoOpSerialLock.""" @@ -137,15 +127,25 @@ def __init__(self): self._user_agent['os_family'] = os.uname().machine self._transaction_manager = None - def _preprocess_req(self, req): - """Inspect the request for hub.set and add the User Agent.""" + def _prepare_request(self, req): + """Prepare a request for transmission to the Notecard.""" + # Inspect the request for hub.set and add the User Agent. if 'hub.set' in req.values(): # Merge the User Agent to send along with the hub.set request. req = req.copy() req.update({'body': self.GetUserAgent()}) self._user_agent_sent = True - return req + + # Serialize the JSON request to a string. + req_string = json.dumps(req) + if self._debug: + print(req_string) + + req_string += "\n" + + # Encode the request string as UTF-8 bytes. + return req_string.encode('utf-8') def GetUserAgent(self): """Return the User Agent String for the host for debug purposes.""" @@ -170,9 +170,9 @@ class OpenSerial(Notecard): """Notecard class for Serial communication.""" @serial_lock - def _transmit(self, req): - req = self._preprocess_req(req) - req_json = _prepare_request(req, self._debug) + def _transact(self, req, rsp_expected): + """Perform a low-level transaction with the Notecard.""" + rsp = None try: transaction_timeout_secs = 30 @@ -180,29 +180,24 @@ def _transmit(self, req): self._transaction_manager.start(transaction_timeout_secs) seg_off = 0 - seg_left = len(req_json) + seg_left = len(req) while seg_left > 0: seg_len = seg_left if seg_len > CARD_REQUEST_SEGMENT_MAX_LEN: seg_len = CARD_REQUEST_SEGMENT_MAX_LEN - self.uart.write(req_json[seg_off:seg_off + seg_len].encode('utf-8')) + self.uart.write(req[seg_off:seg_off + seg_len]) seg_off += seg_len seg_left -= seg_len time.sleep(CARD_REQUEST_SEGMENT_DELAY_MS / 1000) + + if rsp_expected: + rsp = self.uart.readline() finally: if self._transaction_manager: self._transaction_manager.stop() - @serial_lock - def _transmit_and_receive(self, req): - self._transmit(req) - - rsp_json = self.uart.readline() - if self._debug: - print(rsp_json.rstrip()) - - return json.loads(rsp_json) + return rsp def _read_byte_micropython(self): """Read a single byte from the Notecard (MicroPython).""" @@ -225,11 +220,18 @@ def Command(self, req): if 'cmd' not in req: raise Exception("Please use 'cmd' instead of 'req'") - self._transmit(req) + req_bytes = self._prepare_request(req) + self._transact(req_bytes, False) def Transaction(self, req): """Perform a Notecard transaction and return the result.""" - return self._transmit_and_receive(req) + req_bytes = self._prepare_request(req) + rsp_bytes = self._transact(req_bytes, True) + rsp_json = json.loads(rsp_bytes) + if self._debug: + print(rsp_json) + + return rsp_json @serial_lock def Reset(self): @@ -283,17 +285,17 @@ def __init__(self, uart_id, debug=False): class OpenI2C(Notecard): """Notecard class for I2C communication.""" - def _send_payload(self, json): + def _send_payload(self, data): chunk_offset = 0 - json_left = len(json) + data_left = len(data) sent_in_seg = 0 write_length = bytearray(1) - while json_left > 0: - chunk_len = min(json_left, self.max) + while data_left > 0: + chunk_len = min(data_left, self.max) write_length[0] = chunk_len - write_data = bytes(json[chunk_offset:chunk_offset + chunk_len], - 'utf-8') + write_data = data[chunk_offset:chunk_offset + chunk_len] + # Send a message with the length of the incoming bytes followed # by the bytes themselves. if use_periphery: @@ -303,7 +305,7 @@ def _send_payload(self, json): self.i2c.writeto(self.addr, write_length + write_data) chunk_offset += chunk_len - json_left -= chunk_len + data_left -= chunk_len sent_in_seg += chunk_len if sent_in_seg > CARD_REQUEST_SEGMENT_MAX_LEN: @@ -378,8 +380,7 @@ def Command(self, req): if 'cmd' not in req: raise Exception("Please use 'cmd' instead of 'req'") - req = self._preprocess_req(req) - req_json = _prepare_request(req, self._debug) + req_bytes = self._prepare_request(req) while not self.lock(): pass @@ -389,7 +390,7 @@ def Command(self, req): if self._transaction_manager: self._transaction_manager.start(transaction_timeout_secs) - self._send_payload(req_json) + self._send_payload(req_bytes) finally: self.unlock() if self._transaction_manager: @@ -397,8 +398,7 @@ def Command(self, req): def Transaction(self, req): """Perform a Notecard transaction and return the result.""" - req = self._preprocess_req(req) - req_json = _prepare_request(req, self._debug) + req_bytes = self._prepare_request(req) rsp_json = "" while not self.lock(): @@ -409,7 +409,7 @@ def Transaction(self, req): if self._transaction_manager: self._transaction_manager.start(transaction_timeout_secs) - self._send_payload(req_json) + self._send_payload(req_bytes) read_data = self._receive(transaction_timeout_secs, 0.05, True) rsp_json = "".join(map(chr, read_data))