Make I2C improvements #72
Conversation
- 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.
- For functions that need to acquire the I2C lock, use a new decorator, i2c_lock. This decorator also adds new functionality. Specifically, it has a timeout if it fails to get the lock. - Consolidate common code between I2C Transaction and Command into a new internal function, _transmit. This is the same pattern used for serial.
haydenroche5
force-pushed
the
cleanup_i2c
branch
from
9d752a3
to
70efdf1
Compare
| req_json = _prepare_request(req, self._debug) | ||
|
|
||
| try: | ||
| transaction_timeout_secs = 30 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
|
||
| try: | ||
| transaction_timeout_secs = 30 | ||
| if self._transaction_manager: |
There was a problem hiding this comment.
How about a NoopTransactionManager that is always immediately available?
There was a problem hiding this comment.
Good idea. I will do this as a subsequent PR.
| 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'") |
There was a problem hiding this comment.
Ideally this check would be done outside of the lock.
| self.unlock() | ||
| if self._transaction_manager: | ||
| self._transaction_manager.stop() | ||
| read_data = self._receive(30, 0.05, True) |
There was a problem hiding this comment.
As for Serial, it would be nice to have the lock on _transmitAndReceive so that it's used for the minimum possible time.
|
Consolidating into this PR: #73 |
NOTE: Builds on the other open PR here: #70. I've created this PR prior to that other one being merged so that I can run the HIL tests. :)
i2c_lock. This decorator also adds new functionality. Specifically, it has a
timeout if it fails to get the lock.
internal function, _transmit. This is the same pattern used for serial.