feat: Refactor serial and I2C code. #73
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
haydenroche5
commented
Aug 31, 2023
haydenroche5
commented
- Get rid of conditionals on transaction manager existence and if serial locking should be used by adding no-op implementations of each.
- Handle serial and I2C locking with decorators, serial_lock and i2c_lock, respectively.
- Add timeout functionality to I2C locking. Prior to this commit, it would spin forever if the lock couldn't be acquired.
- For both serial and I2C, combine common code from Transaction and Command into an internal function, _transact. Locking happens over this function.
- Migrate all freestanding serial functions to methods of OpenSerial.
- Create a _read_byte abstraction for the three platforms we support (CPython, MicroPython, CircuitPython).
- 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.
- Rename I2C's _send_payload to _transmit.
This was referenced Aug 31, 2023
Closed
haydenroche5
force-pushed
the
refactor
branch
2 times, most recently
from
45b5199
to
e7305e6
Compare
m-mcgowan
reviewed
Sep 6, 2023
notecard/notecard.py
Outdated
| 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: |
There was a problem hiding this comment.
There's an opportunity here to also use a strategy for i2c read/write to avoid the conditional.
- Get rid of conditionals on transaction manager existence and if serial locking should be used by adding no-op implementations of each. - Handle serial and I2C locking with decorators, serial_lock and i2c_lock, respectively. - Add timeout functionality to I2C locking. Prior to this commit, it would spin forever if the lock couldn't be acquired. - For both serial and I2C, combine common code from Transaction and Command into an internal function, _transact. Locking happens over this function. - Migrate all freestanding serial functions to methods of OpenSerial. - Create a _read_byte abstraction for the three platforms we support (CPython, MicroPython, CircuitPython). - 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. - Rename I2C's _send_payload to _transmit. - Move Transaction and Command into the base Notecard class. They are now fully abstracted from the underlying comms layer.
haydenroche5
force-pushed
the
refactor
branch
from
e7305e6
to
6afd1f7
Compare
haydenroche5
changed the title
|
Addressed review feedback with latest commit (minus the context manager for TransactionManager, which I'm shifting to the backlog, for now). Tested locally against Swan + CircuitPython, ESP32 + MicroPython, and Raspberry Pi. |
m-mcgowan
approved these changes
Sep 12, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.