Skip to content
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.

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

UART Fixes and Featrues #13

Closed
wants to merge 11 commits into from
216 changes: 195 additions & 21 deletions adafruit_pm25/uart.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,12 @@

**Hardware:**

Works with most (any?) Plantower UART or I2C interfaced PM2.5 sensor.
Works with most (any?) Plantower UART interfaced PM2.5 sensor.

Tested with:

* PMS5003 on QT Py M0
* On power on, this device defaults to 'active' mode unless a mode change command is received

**Software and Dependencies:**

Expand All @@ -46,38 +51,207 @@
from digitalio import Direction
from . import PM25

PLANTOWER_HEADER = b"\x42\x4D"

PLANTOWER_CMD_MODE_PASSIVE = b"\xE1\x00\x00"
PLANTOWER_CMD_MODE_ACTIVE = b"\xE1\x00\x01"
PLANTOWER_CMD_READ = b"\xE2\x00\x00"
PLANTOWER_CMD_SLEEP = b"\xE4\x00\x00"
PLANTOWER_CMD_WAKEUP = b"\xE4\x00\x01"

UART_RETRY_COUNT = 5
FRAME_RETRY_COUNT = 3
MAX_FRAME_SIZE = 32


class PM25_UART(PM25):
"""
A driver for the PM2.5 Air quality sensor over UART
"""

def __init__(self, uart, reset_pin=None):
def __init__(self, uart, reset_pin=None, set_pin=None, mode="passive"):

Choose a reason for hiding this comment

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

The keyword argument of mode="passive" is going to change the behaviour for existing users of this library. It seems undesirable for them to get this change without requesting it explicitly by setting mode from the program?

Copy link
Author

@fivesixzero fivesixzero Dec 27, 2020

Choose a reason for hiding this comment

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

That's a good point. I spent a bit of time mulling over it and looking for guidance from docs before settling on this. Here's my general train of thought.

At a high level, behabior should be unchanged. The end result should be identical operation, based on the driver's design. When a request is made for a read() the user will get a reading as expected.

The only difference for a user would be at a lower level beyond the driver's abstraction. Rather than having a stream of frames coming in every 900-1000ms over the UART, the UART would only see incoming data frames in response to requests (triggered by read().

With this constructor arg set up the user could request an "active" mode configuration if their use case required it. For example, if their implmentation was reading the UART frames outside of the CircuitPython program/MCU, or using lower-level UART reads outside of our driver.

Additionally, I noticed that active-mode reads are generally unreliable, leading to errors from if frames are picked up in the middle and/or truncated before the checksum comes in. That was the quirk that led me down this rabbit hole in the first place. 😄

That was my original intent and internal argument though. I can see the case for making the default "active", if retaining that convention for UART devices in this driver is paramount.

Copy link
Author

Choose a reason for hiding this comment

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

I didn't address this in my commits this morning, but I'd be happy to make any changes suggested by maintainers here.

Its my first time working on a proper CircuitPython driver so I'm very open to guidance and discussion. :)

self._uart = uart
self._reset_pin = reset_pin
self._set_pin = set_pin
self._mode = mode

if reset_pin:
# Reset device
reset_pin.direction = Direction.OUTPUT
reset_pin.value = False
time.sleep(0.01)
reset_pin.value = True
# it takes at least a second to start up
time.sleep(1)
# Reset device on init, for good measure
self._reset_pin.direction = Direction.OUTPUT
self._pin_reset()

if set_pin:
# Pull set pin high to 'working' status (pulling low puts device to sleep)
self._set_pin.direction = Direction.OUTPUT
self._set_pin.value = True

if self._mode == "passive":
self._cmd_mode_passive()
elif self._mode == "active":
self._cmd_mode_active()
else:
raise RuntimeError("Invalid mode")

self._uart = uart
super().__init__()

def _read_into_buffer(self):
if self._mode == "passive":
self._cmd_passive_read()
self._buffer = self._read_uart()

def _cmd_mode_passive(self):
"""
Sends command to device to enable 'passive' mode

In passive mode, data frames are only sent after a read command
"""
self._uart.reset_input_buffer()
self._uart.write(self._build_cmd_frame(PLANTOWER_CMD_MODE_PASSIVE))
cmd_response = self._read_uart()
self._mode = "passive"
time.sleep(1)
return cmd_response

def _cmd_mode_active(self):
"""
Sends command to device to enable 'active' mode

In active mode, data frames are sent repeatedly every second
fivesixzero marked this conversation as resolved.
Show resolved Hide resolved
"""
self._uart.reset_input_buffer()
self._uart.write(self._build_cmd_frame(PLANTOWER_CMD_MODE_ACTIVE))
cmd_response = self._read_uart()
self._mode = "active"
time.sleep(1)
return cmd_response

def _cmd_sleep(self):
"""
Sends command to put device into low-power sleep mode via UART
"""
self._uart.reset_input_buffer()
self._uart.write(self._build_cmd_frame(PLANTOWER_CMD_SLEEP))
cmd_response = self._read_uart()
time.sleep(1)
return cmd_response

def _cmd_wakeup(self):
"""
Sends command to wake device from low-power sleep mode via UART

Device isn't fully available until 3 seconds after wake is received

This command does not trigger a response, so we don't need to pull one off the buffer
"""
self._uart.reset_input_buffer()
self._uart.write(self._build_cmd_frame(PLANTOWER_CMD_WAKEUP))
time.sleep(3)

def _cmd_passive_read(self):
"""
Sends command to request a data frame whlie in 'passive' mode and immediately reads in frame
"""
self._uart.reset_input_buffer()
self._uart.write(self._build_cmd_frame(PLANTOWER_CMD_READ))

def _pin_reset(self):
"""
Resets device via RESET pin, but only if pin has been assigned

Reset via pin requires about 3 seconds before the device becomes available
"""
if self._reset_pin is not None:
self._reset_pin.value = False
time.sleep(0.01)
self._reset_pin.value = True
time.sleep(3)

def _pin_sleep(self):
"""
Sleeps device via SET pin, but only if pin has been assigned
"""
if self._set_pin is not None and self._set_pin.value:
self._set_pin.value = False

def _pin_wakeup(self):
"""
Wakes device via SET pin, but only if pin has been assigned

Wakeup from sleep via pin takes about 3 seconds before device is available
"""
if self._set_pin is not None and not self._set_pin.value:
self._set_pin.value = True
time.sleep(3)

@staticmethod
def _build_cmd_frame(cmd_bytes):
"""
Builds a valid command frame byte array with checksum for given command bytes
"""
if len(cmd_bytes) != 3:
raise RuntimeError("Malformed command frame")
cmd_frame = bytearray()
cmd_frame.extend(PLANTOWER_HEADER)
cmd_frame.extend(cmd_bytes)
cmd_frame.extend(sum(cmd_frame).to_bytes(2, "big"))
return cmd_frame

def _read_uart(self):
# Disable too-many violations for this method, since it actually needs these branches
# pylint: disable=too-many-nested-blocks,too-many-branches
"""
Reads a single frame via UART

Ignores bytes that are not frame headers to avoid reading in frames mid-stream
"""
uart_timeouts = 0
error_count = 0
first_bytes_tried = 0
while True:
b = self._uart.read(1)
if not b:
raise RuntimeError("Unable to read from PM2.5 (no start of frame)")
if b[0] == 0x42:
break
self._buffer[0] = b[0] # first byte and start of frame
serial_data = bytearray()
first_byte = self._uart.read(1)
if first_byte is not None:
Copy link

@kevinjwalters kevinjwalters Jan 5, 2021

Choose a reason for hiding this comment

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

I just tested a timeout at REPL. I've unplugged PMS5003, set timeout to 4 on UART and read(1) (and read()) give me b'' whereas this code is expecteding None. RX line would be floating here, I think for my particular test.

Copy link
Author

Choose a reason for hiding this comment

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

Another good catch. I still don't have my bench set up again unfortunately.

Do you have a suggestion for a better way to handle the initial "did we get any actaul data" check?

I wasn't aware that the uart driver could send us an empty byte array in these scenarios. Maybe we could just check for both is not None (to cover normal, non-floating RX scenarios) as well as the content (or lack thereof) of the byte array we get back from uart.read(1) (for device-disconnect/floating RX scenarios)?

I feel like this should be a problem for many Circuitpython drivers like this that need to do incremental per-byte reads to find frame starts. Or is this the only one? 😄

if ord(first_byte) == PLANTOWER_HEADER[0]:
serial_data.append(ord(first_byte))
second_byte = self._uart.read(1)
if ord(second_byte) == PLANTOWER_HEADER[1]:
serial_data.append(ord(second_byte))
frame_length_bytes = self._uart.read(2)
frame_length = int.from_bytes(frame_length_bytes, "big")
if 0 <= frame_length <= (MAX_FRAME_SIZE - 4):
serial_data.extend(frame_length_bytes)
data_frame = self._uart.read(frame_length)
if len(data_frame) > 0:
serial_data.extend(data_frame)
frame_checksum = serial_data[-2:]
checksum = sum(serial_data[:-2]).to_bytes(2, "big")
if frame_checksum != checksum:
# Invalid checksum, ignore the frame, try again
error_count += 1
else:
return serial_data
else:
# Data frame empty, ignore the frame, try again
error_count += 1
else:
# Invalid frame length, ignore the frame, and try again
error_count += 1
else:
# Invalid header low bit, ignore the frame, and try again
error_count += 1
else:
# First bit isn't a header high bit, ignore and retry until we get a header bit
first_bytes_tried += 1
else:
# If we didn't get a byte during our read, increment timeouts and move on
uart_timeouts += 1

remain = self._uart.read(31)
if not remain or len(remain) != 31:
raise RuntimeError("Unable to read from PM2.5 (incomplete frame)")
self._buffer[1:] = remain
if error_count >= FRAME_RETRY_COUNT:
raise RuntimeError("Frame error count exceded retry threshold")

if uart_timeouts >= UART_RETRY_COUNT:
raise RuntimeError("Unable to read from PM2.5")

# print([hex(i) for i in self._buffer])
if first_bytes_tried > MAX_FRAME_SIZE:
# If we haven't found a frame header then increment count and try again
error_count += 1