Skip to content

Conversation

@haydenroche5
Copy link
Collaborator

@haydenroche5 haydenroche5 commented Oct 1, 2023

Add support for binary transfers to/from the Notecard.

  • The binary transfer methods mirror their note-c counterparts and live in
    binary_helpers.py. In note-c, our philosophy was that these new functions aren't
    core: they're helpers. Hence the new module rather than putting this
    functionality in notecard.py.
  • There's an example of how to use the new functionality in
    examples/binary_loopback_example.py.
  • There's a new HIL test under test/hitl/test_binary.py.

- Add CRC support.
- Align serial and I2C code with note-c. This encompasses mimicking the
algorithms used in note-c and also things like delay lengths, number of retries,
transaction timeout lengths, etc.
- Align transaction logic with note-c. The majority of this work is in the
reworked `Transaction` function, which is analogous to
`noteTransactionShouldLock` in note-c.
- Rework serial/I2C locking so that the lock can be held for the entirety of a
binary transfer. For example, for a binary receive operation, the host needs to
send a `card.binary.get` and then immediately receive the binary data from the
Notecard. The serial/I2C lock should be held for the entirety of this operation,
rather than be released after the `card.binary.get` and reacquired for the
receipt of the binary data.
- Add two methods for both serial and I2C: `transmit` and `receive`. `transmit`
is used to transmit arbitrary bytes to the Notecard (e.g. after
`card.binary.put`). `receive` is used to read a stream of bytes from the
Notecard, stopping after reading a newline (e.g. after a `card.binary.get`).
These are analogous to the `chunked(Receive|Transmit)` functions in note-c.
- Overhaul unit testing. This commit adds many new unit tests, in addition
to reorganizing things. Tests for the `Notecard` base class are in
test_notecard.py. `OpenSerial` tests are in test_serial.py, and `OpenI2C` tests
are in test_i2c.py.

There is a some code repetition here, especially between the `_transact` and
`Reset` methods of `OpenSerial` and `OpenI2C`. Again, this follows the pattern
in note-c and was done consciously. We may want to factor out that repetition
in the future, but for now, I'm prioritizing parity with the "source of truth"
(note-c).
- Fail example scripts if an exception is raised at any point.
- Rework `_crc_add` to produce a reliable CRC. This requires us to rely more on
string manipulation and less on python dicts. Prior to this commit, the CRC was
computed over the string representation of the request (via `json.dumps`) and
then it was added to the request dict. Then, when it was time to serialize the
request for transmission and we converted the dict to a string again, because
the CRC field had been added, the order of the fields changed and the CRC was
invalidated. This is because Python dicts are fundamentally unordered
collections.
- Add missing timeout reset to `OpenSerial`'s `receive` method.
- Increase cpy_example.py UART RX buffer size. I was seeing the end of responses
get cut off with the default size (64). Increased to 128.
See https://forum.micropython.org/viewtopic.php?f=2&t=11114. I also went ahead
and changed to the new format for multiline strings that don't contain
f-strings.
- The binary transfer methods mirror their note-c counterparts and live in
binary_helpers.py. In note-c, our philosophy was that these new functions aren't
core: they're helpers. Hence the new module rather than putting this
functionality in notecard.py.
- There's an example of how to use the new functionality in
examples/binary_loopback_example.py.
- There's a new HIL test under test/hitl/test_binary.py.
@haydenroche5 haydenroche5 marked this pull request as ready for review October 20, 2023 18:28
@haydenroche5 haydenroche5 changed the title WIP: COBS. Add support for binary transfers to/from the Notecard. Oct 20, 2023
@bsatrom bsatrom self-requested a review November 2, 2023 20:28
@haydenroche5 haydenroche5 merged commit 25c19c3 into blues:main Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants