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

Increased care with serial reads, retries and new passive mode for requesting data #8

Merged
merged 12 commits into from
Apr 23, 2021

Conversation

kevinjwalters
Copy link

@kevinjwalters kevinjwalters commented Jan 6, 2021

Some care is required with processing data from the serial connection on the Plantower PMS5003 particulate matter sensor. In its default mode it streams data at a variable rate which is not exactly 1Hz. Buffered data can cause a wide variety of error conditions. This change handles these internally and also adds new functionality based on @fivesixzero's work in adafruit/Adafruit_CircuitPython_PM25#13 for a passive mode (Plantower's terminology). In passive mode data is requested via a transmitted command rather than streamed continuously. The active mode is the default and is retained as the default in the constructor to avoid impact to existing application code.

unit tests

$ date ; PYTHONPATH=. py.test-3.6 -v -r wsx
Thu  7 Jan 20:26:25 GMT 2021
=================================================== test session starts ====================================================
platform linux -- Python 3.6.8, pytest-2.9.2, py-1.4.32, pluggy-0.3.1 -- /usr/bin/python3.6
cachedir: .cache
rootdir: /pms5003-circuitpython/library, inifile:
collected 22 items

tests/test_setup.py::test_setup PASSED
tests/test_setup.py::test_double_setup PASSED
tests/test_setup.py::test_read PASSED
tests/test_setup.py::test_read_fail PASSED
tests/test_setup.py::test_checksum_pass PASSED
tests/test_setup.py::test_checksum_fail PASSED
tests/test_setup.py::test_checksum_fail_withretries PASSED
tests/test_setup.py::test_checksum_retries_ok PASSED
tests/test_setup.py::test_data_checksum_pass PASSED
tests/test_setup.py::test_data_checksum_pass_alt PASSED
tests/test_setup.py::test_data_checksum_fail PASSED
tests/test_setup.py::test_data_checksum_fail_alt PASSED
tests/test_setup.py::test_buffer_full_truncation PASSED
tests/test_setup.py::test_buffer_full_lucky PASSED
tests/test_setup.py::test_buffer_full_badframelen_long1 PASSED
tests/test_setup.py::test_buffer_badframelen_long2 PASSED
tests/test_setup.py::test_buffer_full_badframelen_short PASSED
tests/test_setup.py::test_active_mode_read PASSED
tests/test_setup.py::test_passive_mode_read PASSED
tests/test_setup.py::test_active_mode_to_passive PASSED
tests/test_setup.py::test_active_mode_to_passive_to_active PASSED
tests/test_setup.py::test_active_mode_to_passive_unlucky PASSED

================================================ 22 passed in 60.60 seconds ================================================

modetest

Output from modetest with Feather nRF52840 Express running 6.0.0.

Adafruit CircuitPython 6.0.0 on 2020-11-16; Adafruit Feather nRF52840 Express with nRF52840
>>>
soft reboot

Auto-reload is on. Simply save files over USB to run them or enter REPL to disable.
code.py output:
MODE TESTS
Instatiating in the non-default passive mode...
passive   1 at 3.923187 took 0.041626 PM ug/m3 2.5=2 10=2
passive   2 at 5.315796 took 0.041626 PM ug/m3 2.5=2 10=2
passive   3 at 7.057953 took 0.041626 PM ug/m3 2.5=3 10=4
passive   4 at 7.950470 took 0.041565 PM ug/m3 2.5=2 10=4
passive   5 at 8.493500 took 0.041626 PM ug/m3 2.5=2 10=3
passive   6 at 8.836304 took 0.041656 PM ug/m3 2.5=2 10=3
passive   7 at 8.978790 took 0.041534 PM ug/m3 2.5=2 10=3
passive   8 at 9.111633 took 0.041595 PM ug/m3 2.5=2 10=3
passive   9 at 9.235992 took 0.041504 PM ug/m3 2.5=2 10=3
active   10 at 10.649201 took 0.767456 PM ug/m3 2.5=3 10=3
active   11 at 11.557496 took 0.906525 PM ug/m3 2.5=2 10=3
active   12 at 12.466096 took 0.906830 PM ug/m3 2.5=3 10=3
active   13 at 13.374786 took 0.906891 PM ug/m3 2.5=2 10=3
active   14 at 14.283447 took 0.906921 PM ug/m3 2.5=2 10=3
passive  15 at 16.220979 took 0.041565 PM ug/m3 2.5=2 10=2
passive  16 at 17.063812 took 0.041595 PM ug/m3 2.5=2 10=2
passive  17 at 17.586243 took 0.041565 PM ug/m3 2.5=2 10=3
passive  18 at 17.908478 took 0.041565 PM ug/m3 2.5=2 10=3
passive  19 at 18.150665 took 0.041534 PM ug/m3 2.5=2 10=3
passive  20 at 18.392915 took 0.041626 PM ug/m3 2.5=2 10=3

Press any key to enter the REPL. Use CTRL-D to reload.

pms5003-modetest-pr8-1-all

resettest

Output from resettest with Feather nRF52840 Express running 6.0.0.

Adafruit CircuitPython 6.0.0 on 2020-11-16; Adafruit Feather nRF52840 Express with nRF52840
>>>
soft reboot

Auto-reload is on. Simply save files over USB to run them or enter REPL to disable.
code.py output:
RESET TESTS
Sleeping for a bit
Instatiating in active mode
Initialisation took 2.66656 seconds
active   1 at 2.669312 took 0.001709 PM ug/m3 2.5=3 10=3
active   2 at 3.672211 took 0.001678 PM ug/m3 2.5=3 10=3
active   3 at 4.755219 took 0.001678 PM ug/m3 2.5=5 10=5
Reset took 2.66345 seconds
active   4 at 7.503693 took 0.002167 PM ug/m3 2.5=2 10=2
active   5 at 8.507202 took 0.001678 PM ug/m3 2.5=2 10=2
active   6 at 9.510101 took 0.001678 PM ug/m3 2.5=4 10=4
Instatiating in passive mode
Initialisation took 2.88132 seconds
passive   1 at 2.976074 took 0.093719 PM ug/m3 2.5=0 10=0
passive   2 at 4.018585 took 0.041565 PM ug/m3 2.5=4 10=4
passive   3 at 5.061523 took 0.041534 PM ug/m3 2.5=3 10=3
Reset took 2.88052 seconds
passive   4 at 8.066254 took 0.041656 PM ug/m3 2.5=0 10=0
passive   5 at 9.109375 took 0.041534 PM ug/m3 2.5=3 10=3
passive   6 at 10.152375 took 0.041565 PM ug/m3 2.5=3 10=3
Instatiating in active mode
Initialisation took 2.66632 seconds
active   1 at 2.669067 took 0.001709 PM ug/m3 2.5=2 10=2
active   2 at 3.575317 took 0.404938 PM ug/m3 2.5=2 10=2
active   3 at 4.483643 took 0.407013 PM ug/m3 2.5=2 10=2
Reset took 2.66339 seconds
active   4 at 7.151917 took 0.002167 PM ug/m3 2.5=2 10=2
active   5 at 8.059906 took 0.407104 PM ug/m3 2.5=1 10=1
active   6 at 8.968262 took 0.407257 PM ug/m3 2.5=1 10=1

Press any key to enter the REPL. Use CTRL-D to reload.

pms5003-resettest-pr8-1-all

(The capture doesn't quite run long enough to cover whole test.)

manual tests

PMS5003 disconnected test:

Adafruit CircuitPython 6.0.0 on 2020-11-16; Adafruit Feather nRF52840 Express with nRF52840
>>>
>>> from pimoroni_pms5003 import PMS5003
>>> pms5003 = PMS5003()  # exception does not occur here
>>> pms5003.read()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/lib/pimoroni_pms5003/__init__.py", line 332, in read
  File "/lib/pimoroni_pms5003/__init__.py", line 328, in read
  File "/lib/pimoroni_pms5003/__init__.py", line 343, in _read_data
ReadTimeoutError: PMS5003 Read Timeout: Could not find start of frame
>>>
Adafruit CircuitPython 6.0.0 on 2020-11-16; Adafruit Feather nRF52840 Express with nRF52840
>>> from pimoroni_pms5003 import PMS5003
>>> pms5003 = PMS5003(mode="passive")  # exception does not occur here
>>> pms5003.read()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/lib/pimoroni_pms5003/__init__.py", line 332, in read
  File "/lib/pimoroni_pms5003/__init__.py", line 328, in read
  File "/lib/pimoroni_pms5003/__init__.py", line 343, in _read_data
ReadTimeoutError: PMS5003 Read Timeout: Could not find start of frame
>>> pms5003 = PMS5003(mode="junk")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/lib/pimoroni_pms5003/__init__.py", line 206, in __init__
ValueError: Invalid mode

Kevin J Walters added 8 commits December 23, 2020 16:59
…moroni#4.

Adding new FrameLengthError exception.
Moving checksum calculation code to PMS5003Data constructor.
Checking both the field length field and actual data length using a new classmethod in PMS5003Data.
Tests awaiting FrameLengthError now look for it.
4 new tests for PMS5003Data construction.
…zero approach in https://github.com/adafruit/Adafruit_CircuitPython_PM25 pimoroni#5

Adding mode argument to PMS5003 class CHANGING the order of arguments too.
Object instantiation and reset() now wait for a data frame to be sent from the PMS5003 by checking serial in_waiting property.
PMS5003 setup() now works if reset and enable pins are not defined.
New PMS5003CmdResponse to deal with PMS5003 repsonding to active/passive mode change commands.
New PMS5003Response parent class for PMS5003CmdResponse and original PMS5003Data.
Re-arranging buffer data addition in the tests to cater for the object needing the data frame response over serial to instantiate.
Two new CircuitPython examples to test this new functionality on Feather board.
…are but observed case of an inopportune data frame sneaking in front of the response to command.

Added three tests around that. pimoroni#5
…ctive mode.

Adding test for data_available() to existing test_active_mode_read pimoroni#5
The PMS5003 now defaults to 5 retries for read() operations and only throws (the first) exception if all fail.
Retaining existing behavior for some tests by setting retries to 0.
Adding test_checksum_fail_withretries and test_checksum_retries_ok.
@kevinjwalters kevinjwalters changed the title NOT YET FINISHED - increase care with serial reads, new passive mode for requesting data NOT YET FINISHED - increase care with serial reads, retries and new passive mode for requesting data Jan 6, 2021
@kevinjwalters
Copy link
Author

kevinjwalters commented Jan 7, 2021

@dglaude This looks good if you wish to test on your FeatherS2 setup with Enviro+ FeatherWing and PMS5003. This goes with the standard Pimoroni setup described in https://www.instructables.com/Using-the-Pimoroni-Enviro-FeatherWing-With-the-Ada/ and pimoroni/EnviroPlus-FeatherWing#24

@kevinjwalters
Copy link
Author

@dglaude FYI, the release notes for recent 6.1.0-rc.0 mention:

UnexpectedMaker FeatherS2: Correct pins and add default board devices. adafruit/circuitpython#3925, adafruit/circuitpython#3914. Thanks UnexpectedMaker , askpatrickw.

@kevinjwalters kevinjwalters changed the title NOT YET FINISHED - increase care with serial reads, retries and new passive mode for requesting data Increase care with serial reads, retries and new passive mode for requesting data Jan 7, 2021
@kevinjwalters
Copy link
Author

kevinjwalters commented Jan 7, 2021

@Gadgetoid This together with pimoroni/EnviroPlus-FeatherWing#24 makes the Enviro+ FeatherWing more robust and more memory efficient, let me know what you think.

This could also be patched to make an update for https://github.com/pimoroni/pms5003-python which is very similar to this library. Note: new use of in_waiting.

@kevinjwalters kevinjwalters changed the title Increase care with serial reads, retries and new passive mode for requesting data Increased care with serial reads, retries and new passive mode for requesting data Jan 7, 2021
@kevinjwalters
Copy link
Author

kevinjwalters commented Jan 7, 2021

Just fixed #5 (comment) which I had noted but missed.

Kevin J Walters added 3 commits January 7, 2021 20:32
…oroni#5

Catching exceptions in the constructor where ReadTimeoutError can occur if PMS5003 not connected.
Some minor reformatting.
@Gadgetoid
Copy link
Member

This is really amazing stuff and I'll give it a proper look as soon as I can. I'm currently in house-move-limbo so I have no office and most of my kit in boxes. Bear with me, and thanks for putting in the time to make what appears to be a herculean changeset.

@kevinjwalters
Copy link
Author

kevinjwalters commented Jan 24, 2021

Glad you like the look of it so far. I was trying to make all of this more robust as I gifted two of them this Christmas. I've kept the interface mostly the same but there's a fair amout of general change in there so it warrants a good review and testing by others.

Am I right in thinking there are Enviro/Enviro+ users on Raspberry Pi than FeatherWing? It looks that way from Pimoroni Forum chatter.

BTW, ESP32-S2-based FeatherS2 gives confusing gas sensor output due to its 2.6V capped ADC, all rather disappointing. You have two explicit recommendations in the product page for Feather boards but perhaps you also want to add a note on this otherwise nice little board being unsuitable or limited due to that issue? More detail in Feather ADC comparison including 2.6V limited ESP32-S2.

@kevinjwalters
Copy link
Author

I wrote a more detailed explanation, see Software Bugs in a Particulate Matter Sensor Library.

… read() to improve exceptions.

Removing superfluous conversion of serial data from bytes to bytearray pimoroni#4
@kevinjwalters
Copy link
Author

I noticed if I didn't have a PMS5003 plugged in the exception was printed as

object of type 'NoneType' has no len()

That TypeError is now the intended SerialTimeoutError:

PMS5003 Read Timeout: Failed to read start of frame byte

@Gadgetoid
Copy link
Member

This- plus the linked article- are truly great stuff!

My situation has changed to:

  • House
  • Office
  • Time

Let's see if I can make some of the latter.

@dglaude are you still armed with a feather and PMS5003? I'd appreciate your input if you have the time/inclination to provide it.

@dglaude
Copy link
Contributor

dglaude commented Apr 21, 2021

I am still armed with a many featherS (M4 and NRF52 and other) and a PMS5003, and also a Adafruit SCD-30.
So I should be able to review many of the PR by @kevinjwalters (here and in Enviro+ Featherwing and maybe elsewhere).

Maybe it is best for me to synchronise with him to check each PR and report so that you have some confidence that it is working.

@dglaude
Copy link
Contributor

dglaude commented Apr 21, 2021

Wait...

What is the best place to maintain such a driver for CircuitPython?
Here or https://github.com/adafruit/Adafruit_CircuitPython_PM25...
A lot of the improvement from here have been ported there.

@kevinjwalters
Copy link
Author

The timing didn't seem right to move to the Adafruit version when I started this. Given that adafruit/Adafruit_CircuitPython_PM25#13 still hasn't been merged I don't think it is now either. I've duplicated the features from that PR.

@kevinjwalters
Copy link
Author

These fixes and features might also be applied to https://github.com/pimoroni/pms5003-python - from memory that was very similar and could almost be generated with a diff from this library.

@Gadgetoid
Copy link
Member

The timing didn't seem right to move to the Adafruit version when I started this. Given that adafruit/Adafruit_CircuitPython_PM25#13 still hasn't been merged I don't think it is now either. I've duplicated the features from that PR.

Well, I mean this hasn't been merged either 😆

If I'm honest, I had completely forgotten we'd produced a CircuitPython library for this (it's been a heck of a year and then some). Maybe we can get all this merged, updated and released and then compare notes later but I do tend to agree that leaving CircuitPython stuff in Adafruit's ballpark as much as we can helps because we don't necessarily have the people-power to maintain it.

Though Pico CircuitPython might be changing the game a little.

@Gadgetoid Gadgetoid merged commit 359622f into pimoroni:master Apr 23, 2021
@Gadgetoid
Copy link
Member

Okay so I've switched this repository from Travis to GitHub actions and merged this PR so we can move forward from a slightly less broken/old codebase.

Cobbling together my Feather bits to see if I can't get this running!

@Gadgetoid
Copy link
Member

Now up and running on an Enviro+ Feather M4 Express combo on my desk. Will let it sit overnight and hope my USB doesn't explode again.

@Gadgetoid
Copy link
Member

3 days later... it's still running. Absolutely bulletproof. Exceptional work! 🎉

@fivesixzero
Copy link

@kevinjwalters - I'm glad that PR I set up late last year has proven to be useful after all! Between moving home and taking care of my partner while she battles cancer I haven't had time to push that one over the finish line, let alone do the testing I've wanted to do with the stack of PMx00x UART sensors I've accumulated. Hoping to get back to it too, but this is solid progress in the right direction!

@Gadgetoid
Copy link
Member

@kevinjwalters - I'm glad that PR I set up late last year has proven to be useful after all! Between moving home and taking care of my partner while she battles cancer I haven't had time to push that one over the finish line, let alone do the testing I've wanted to do with the stack of PMx00x UART sensors I've accumulated. Hoping to get back to it too, but this is solid progress in the right direction!

Sheesh it's been a hard year without any extra complications- I hope you're both doing okay! 🤗

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.

4 participants