Conversation
pocs/sensors/arduino_io.py is based on peas/sensors.py, but ArduinoIO handles only a single device, and writes readings to a queue rather than to a database. Moved protocol_arduinosimulator.py to a new folder, pocs/serial_handlers. This will be the home of the other PySerial handlers in the future.
|
Partially address #420. |
Codecov Report
@@ Coverage Diff @@
## develop #422 +/- ##
==========================================
+ Coverage 69.24% 69.65% +0.4%
==========================================
Files 60 61 +1
Lines 4806 4943 +137
Branches 665 687 +22
==========================================
+ Hits 3328 3443 +115
- Misses 1301 1309 +8
- Partials 177 191 +14
Continue to review full report at Codecov.
|
wtgee
left a comment
wtgee
left a comment
There was a problem hiding this comment.
Big question is about the queue with ArduinoIO and what the expected usage is like. Docstrings and other miscellany.
| Returns: a list of PySerial's ListPortInfo objects. See: | ||
| https://github.com/pyserial/pyserial/blob/master/serial/tools/list_ports_common.py | ||
| """ | ||
| from serial.tools.list_ports import comports |
There was a problem hiding this comment.
Moved to top of file.
| https://github.com/pyserial/pyserial/blob/master/serial/tools/list_ports_common.py | ||
| """ | ||
| from serial.tools.list_ports import comports | ||
| return sorted(comports(), key=operator.attrgetter('device')) |
There was a problem hiding this comment.
Why not just key=lambda x: x.device?
There was a problem hiding this comment.
That works too. Not sure which is "better", though yours is shorter.
There was a problem hiding this comment.
I was thinking the docs recommended lambdas but looking at it now they go on to say it is such common usage that the operator module exists just for that. here :) So fine leaving as is.
| if port_infos: | ||
| fmt = '{:20s} {:30s} {}' | ||
| print(fmt.format('Device', 'Manufacturer', 'Description')) | ||
| for pi in rs232.get_serial_port_info(): |
There was a problem hiding this comment.
Why not just for pi in ports_infos?
|
|
||
| print() | ||
| boards_and_ports = arduino_io.auto_detect_arduino_devices() | ||
| for (board, port) in boards_and_ports: |
There was a problem hiding this comment.
Don't think the parens are necessary.
| print("Arduino devices: {}".format(", ".join(devices))) | ||
| else: | ||
| print("No Arduino devices found.") | ||
| sys.exit(1) |
There was a problem hiding this comment.
Is this used somewhere such that you want the 1 on exit?
There was a problem hiding this comment.
I figured this could be used by another script that is doing setup of a unit, so distinguishing between found some devices and found no devices is worthwhile. But, not used yet.
| self._thread.daemon = True | ||
| self._thread.start() | ||
|
|
||
| def stop_reading(self): |
| else: | ||
| self._thread = None | ||
|
|
||
| def write(self, text): |
There was a problem hiding this comment.
Docstrings (just mentioning for good measure 😄 )
| with self._serial_data_lock: | ||
| if not self._serial_data.is_connected: | ||
| self._serial_data.connect() | ||
| return self._serial_data.write(text) |
There was a problem hiding this comment.
I'm guessing the lock context manager does the right thing here with a return statement.
| response = bytes(response) | ||
| self.logger.info('FakeArduinoSerialHandler.read({}) -> {!r}', size, response) | ||
| # if size > 1: | ||
| # self.logger.debug('FakeArduinoSerialHandler.read({}) -> {!r}', size, response) |
There was a problem hiding this comment.
Removing commented out code.
It was too noisy because size=1 most of the time; this is because it gets called by readline in io.IOBase, the base of the serial impls. I experimented with only printing when size>1, but settled on adding a readline impl that prints the line that is read.
| class ArduinoIO(object): | ||
| """Reads the output from an arduino, and exposes the relays for change.""" | ||
|
|
||
| def __init__(self, board_name, port, output_queue, serial_config=None): |
There was a problem hiding this comment.
What is the overall picture of how this will be used? I mostly ask because I think it's a bit odd to pass in a queue rather than to have it create a queue.
There was a problem hiding this comment.
The aim is for it to deliver readings to a thread that will take care of writing to the database (or wherever). The advantage of providing the queue to the object is that we can use the same queue for multiple Arduinos, and then the db writer thread only needs to read from a single queue.
Rename list_arduino_ports to get_arduino_ports. Add missing docstrings.
|
PTAL |
2 participants
pocs/sensors/arduino_io.py is based on peas/sensors.py, but
ArduinoIO handles only a single device, and writes readings
to a queue rather than to a database.
Moved protocol_arduinosimulator.py to a new folder,
pocs/serial_handlers. This will be the home of the other PySerial
handlers in the future.