Skip to content

Make I2C improvements #72

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.

Sign up for GitHub

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

Closed
haydenroche5 wants to merge 2 commits into from
Closed

Make I2C improvements #72

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6ba782d
Clean up serial comms code.
haydenroche5 Jul 27, 2023
70efdf1
Make I2C improvements.
haydenroche5 Aug 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
277 changes: 112 additions & 165 deletions notecard/notecard.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,99 +75,47 @@ 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
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:
raise Exception("Notecard not responding")
return fn(self, *args, **kwargs)

return decorator

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)
def i2c_lock(fn):
"""Attempt to get a lock on the I2C bus used for Notecard comms."""

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:
def decorator(self, *args, **kwargs):
retries = 5
while use_i2c_lock and retries != 0:
if self.i2c.try_lock():
break
time.sleep(CARD_REQUEST_SEGMENT_DELAY_MS / 1000)
finally:
if txn_manager:
txn_manager.stop()

rsp_json = port.readline()
if debug:
print(rsp_json.rstrip())

rsp = json.loads(rsp_json)
return rsp
retries -= 1
# Try again after 100 ms.
time.sleep(.1)

if retries == 0:
raise Exception('Failed to acquire I2C lock.')

def serialCommand(port, req, debug, txn_manager=None):
"""Perform a single write to and read from a Notecard."""
req_json = _prepare_request(req, debug)
try:
ret = fn(self, *args, **kwargs)
finally:
if use_i2c_lock:
self.i2c.unlock()

transaction_timeout_secs = 30
if txn_manager:
txn_manager.start(transaction_timeout_secs)
return ret

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:
Expand Down Expand Up @@ -225,51 +173,82 @@ 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)

try:
transaction_timeout_secs = 30
Copy link
Contributor

@m-mcgowan m-mcgowan Aug 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This timeout may need to be computed based on the size of the request/response. I would prefer that we set quite a long timeout for the overall transaction, but also have a smaller timeout that checks whether data is still being sent/received. Same applies to Serial.

Copy link
Collaborator Author

@haydenroche5 haydenroche5 Aug 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that would be better, but in note-c Ray had us just set it to 30 and forget it. I would like to maintain parity until we've decided we want to go in a different direction.

if self._transaction_manager:
Copy link
Contributor

@m-mcgowan m-mcgowan Aug 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about a NoopTransactionManager that is always immediately available?

Copy link
Collaborator Author

@haydenroche5 haydenroche5 Aug 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. I will do this as a subsequent PR.

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)
finally:
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."""
Expand Down Expand Up @@ -374,83 +353,51 @@ def _receive(self, timeout_secs, chunk_delay_secs, wait_for_newline):
# behavior of other SDKs (e.g. note-c).
time.sleep(chunk_delay_secs)

if (timeout_secs != 0 and has_timed_out(start, timeout_secs)):
if timeout_secs != 0 and has_timed_out(start, timeout_secs):
raise Exception("Timed out while reading data from the Notecard.")

return read_data

def Command(self, req):
"""Perform a Notecard command and return with no response."""
if 'cmd' not in req:
raise Exception("Please use 'cmd' instead of 'req'")

def _transmit(self, req):
req = self._preprocess_req(req)
req_json = _prepare_request(req, self._debug)

while not self.lock():
pass

try:
transaction_timeout_secs = 30
if self._transaction_manager:
self._transaction_manager.start(transaction_timeout_secs)

self._send_payload(req_json)
finally:
self.unlock()
if self._transaction_manager:
self._transaction_manager.stop()

def Transaction(self, req):
"""Perform a Notecard transaction and return the result."""
req = self._preprocess_req(req)
req_json = _prepare_request(req, self._debug)
rsp_json = ""

while not self.lock():
pass
@i2c_lock
def Command(self, req):
"""Perform a Notecard command and exit with no response."""
if 'cmd' not in req:
raise Exception("Please use 'cmd' instead of 'req'")
Copy link
Contributor

@m-mcgowan m-mcgowan Aug 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally this check would be done outside of the lock.


try:
transaction_timeout_secs = 30
if self._transaction_manager:
self._transaction_manager.start(transaction_timeout_secs)
self._transmit(req)

self._send_payload(req_json)
@i2c_lock
def Transaction(self, req):
"""Perform a Notecard transaction and return the result."""
self._transmit(req)

read_data = self._receive(transaction_timeout_secs, 0.05, True)
rsp_json = "".join(map(chr, read_data))
finally:
self.unlock()
if self._transaction_manager:
self._transaction_manager.stop()
read_data = self._receive(30, 0.05, True)
Copy link
Contributor

@m-mcgowan m-mcgowan Aug 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for Serial, it would be nice to have the lock on _transmitAndReceive so that it's used for the minimum possible time.

rsp_json = "".join(map(chr, read_data))

if self._debug:
print(rsp_json.rstrip())

return json.loads(rsp_json)

@i2c_lock
def Reset(self):
"""Reset the Notecard."""
while not self.lock():
pass

try:
# Read from the Notecard until there's nothing left to read.
self._receive(0, .001, False)
finally:
self.unlock()

def lock(self):
"""Lock the I2C port so the host can interact with the Notecard."""
if use_i2c_lock:
return self.i2c.try_lock()
return True

def unlock(self):
"""Unlock the I2C port."""
if use_i2c_lock:
return self.i2c.unlock()
return True
# Read from the Notecard until there's nothing left to read.
self._receive(0, .001, False)

def __init__(self, i2c, address, max_transfer, debug=False):
"""Initialize the Notecard before a reset."""
Expand Down