adafruit / Adafruit_CircuitPython_PM25

CircuitPython library for PM2.5 sensors
MIT License
28 stars 16 forks source link

UART fails to operate with PMS5003 #9

Open fivesixzero opened 3 years ago

fivesixzero commented 3 years ago

Thanks for this library! Its works great with the I2C PMSA003i device I bought from Adafruit a few weeks ago. :)

Unfortunately, I was having trouble getting this library to work with a PMS5003 connected over UART using the Pimoroni Envirio+ hat on a Pi Zero. It looks like this is being caused by two different but somewhat related problems.

In one case, the PM2.5 sensor was set to "passive" mode when using another library or its default on power-on may actually be to run in "passive" mode. In this case _read_into_buffer() always times out.

In the other case, it looks like _read_into_buffer() is sometimes misreading the UART data stream coming in, leading to "Invalid checksum" errors when read() tries to parse the buffer's data after an otherwise successful _read_into_buffer().

fivesixzero commented 3 years ago

While troubleshooting, I looked at the way a few others have solved this problem.

https://github.com/dobra-dobra/Python_PMS5003/blob/master/pms5003.py

https://github.com/pimoroni/pms5003-python/blob/master/library/pms5003/__init__.py

After digging into those other implementations and reading through the data sheet it looked like it might be possible to address both issues with a "passive mode" constructor. This would init the sensor in passive mode then trigger reads on each _read_into_buffer().

I also found a related datasheet that included some details on what the "mode change response" bytes mean, which was helpful in determining whether a mode change worked as intended.

https://www.evelta.com/content/datasheets/203-PMS9003M.pdf

I've got some code I put together in a fork that looks like it fixes the issues I described in the original post. I'll set up a PR if that helps.

fivesixzero commented 3 years ago

PR #10 has the code changes.

dglaude commented 3 years ago

Hi, this seems very interesting.

There is another pending PR that split I2C and UART version for "memory saving", and I believe it could solve your "invalid checksum". But this is likely conflicting with your PR.

Also @Gadgedtoid from Pimoroni is also planning to bring the improvement they have in their library to avoid maintaining two CircuitPython Library for Enviro+ FeatherWing.

But I want to try your code as I don't like the way UART works right now. If I understand the passive mode, this is command and response, so it is much more easier to handle than a constent flow of data pushed over UART.

If not provided in your PR, maybe you should provide code to switch between active and passive to avoid lock user into one mode or the other. I only have one sensor that I connect to a Feather or a Pi (I have the serial break-out), but I want to make sure I can test both working mode.

fivesixzero commented 3 years ago

@dglaude - Thanks for taking a look. I haven't looked very closely at the other PR involving I2C vs UART yet, but it sounds like a good idea.

Looking at my PR again after a month or two I'm not sure if its fit to merge as-is. If anything it may be more useful as a proof-of-concept to illustrate how the passive and active modes differ when it comes to setup and reading incoming bytes.

I'm sure it would be possible to offer a single constructor with the option to switch modes, but it'd probably require more structural changes than I was comfortable with in my initial pass at it. If I get some free time this weekend I'll take another crack at it and see what I can do.

That said, if the excellent Pimeroni crew are going to be bringing their own improvements into this project then I'm guessing they'll have a solid fix for this issue there. I hope they get around to it! I'd be happy to help with testing, at least. :)

dglaude commented 3 years ago

Sorry, my PR to split into multiple file has been integrated. So your PR is conflicting with current code. Maybe it make it easier now to make a uart_passive.py and offer the option to pick one module or another depending on what works best.

fivesixzero commented 3 years ago

I've closed PR #10 and opened a new one that fits the new structure - PR #13.

This one has been tested heavily using a QT Py running the CircuitPython 6.0 so I'm a lot more confident in with this one. :)

edit: Looks like pylint had some issues with this PR so I'll be reworking it, but its available for testing if anyone wants to take a look.

edit 2: Made pylint happy, ready for a proper review

kevinjwalters commented 3 years ago

@fivesixzero From what I've seen of the protocol this is a simple case of the device powers up spitting data at a certain rate and if you don't read all that data then eventually the rx buffer will fill and the reader is likely to get partial frames due to missed or truncated data and it goes downhill from there. Does this match your experience?

Do you have a link for documentation on the Plantower serial protocol? Oh, I think this is in the Appendix of the link you gave https://www.evelta.com/content/datasheets/203-PMS9003M.pdf - do you know if that's the complete protocol?

Do you know if the PMS5003 always responds instantly to a "passive" read?

fivesixzero commented 3 years ago

@kevinjwalters The PDF I linked above has the protocol's basics, but all of the docs I've seen leave out one or more important bits of info. For development I've basically scoured the web for PDFs for any of the Plantower and related UART PM2.5 devices I can find and stitched together a more holistic picture by testing the devices on the bench. With a logic analyzer to keep a close eye on the serial bus development is a lot easier.

The UART (vs I2C) devices I've tested with so far (3 differently sourced PMS5003s) have all behaved as you mentioned - on power on they spit out a data frame every second or so with measurement data. I just got a few more - a PMS3003, a PMSA003, and a PMS7003. Next time I get proper bench time I'm gonna check those out and see if they work with this driver after my PR.

The docs don't make it particularly clear but these devices almost all support commands to put them in a "passive" mode, where data frames are only transmitted after a request is sent in. I think for most MCU-based use cases this is probably the preferred configuration, since it keeps power usage down and eliminates the need to continuously read until a frame header comes in. Many also support a "sleep" command that could also be really useful for power-conscious scenarios. My PR implements both of those commands.

As far as buffer-full situations leading to partial reads, that could explain the intermittent nature of of the behavior I saw. When the driver is reading it just reads bytes until it gets a start byte then pulls in the next 31 bytes frame from the buffer, hoping it's a data frame. But if you're only polling the UART intermittently, doing other things between driver read calls, then next time we try to read it looks like it's likely to be from the buffer (or deeper into the buffer). If we hit end of buffer and get a partial frame, weird things happen.

If that's what's going on then it looks like another good reason to use "passive" mode by default, which is basically how my PR works. And it could provide a starting point for making "active" mode more robust if possible.

fivesixzero commented 3 years ago

In my experience the commands trigger immediate response which should always be response frames, which are documented in most of the PDFs I've seen.

The exception is the "read" command for fetching a data frame in passive mode, which just respond with a data frame instead of a much smaller response frame.

benthorner commented 2 years ago

I've also been unable to get any data from my PMSA003 using this library. It's possible the factory default for these sensors is to actively spit out data. Ideally the library should still send commands to ensure this, since it relies on it.

I've found the PyPMS library / CLI works well - it switches the sensor to passive mode to sample readings.

fivesixzero commented 2 years ago

I've also been unable to get any data from my PMSA003 using this library. It's possible the factory default for these sensors is to actively spit out data. Ideally the library should still send commands to ensure this, since it relies on it.

Yep, spot on, @benthorner. That was the key thing I wanted to implement in the PR I put together last year, #13. At the time my knowledge of how CircuitPython handled UART reads/writes at a low level was pretty limited so I wasn't really confident in my code. It was also the first CircuitPython driver project I had worked on at all so I had a lot to learn. :)

Unfortunately, life got weird for awhile late last year (weirder than usual at least) and I wasn't able to test the changes in the PR within a reasonable timeframe. :(

Its funny though, since I inadvertently left the code from that PR running on two of my home environmental sensors since then and, well, they're still working a year later! So maybe the code's 'good enough', but those are using Pi Zero Ws with Blinka, not bare-metal CircuitPython. The testing I wanted to do involved making sure this works with bare-metal UART handling and on more constrained platforms like SAMD21. While moving house I didn't have a proper lab set up to do that. But I do now, so I should get back to it... :)

On power-up these indeed start in "active" mode, where they continuously stream data. That's fine if you've got a single-purpose controller (or one with a dedicated interrupt) that can hoover up or buffer the data in real time. But the way CircuitPython handles/buffers UART this can quickly turn into a bit of a mess. So the "passive" mode, where data frames are sent in response to a "read" command, tends to work a lot better with CircuitPython.

BTW I think the Pimoroni folks implemented UART control of "active" vs "passive" mode in their CircuitPython driver but I haven't checked that out in awhile.

kevinjwalters commented 2 years ago

I did the passive mode stuff (and fixed a few bugs) in https://github.com/pimoroni/pms5003-circuitpython/commits/master/library/pms5003/__init__.py based on the PR from @fivesixzero

I've used the Adafruit library for Instructables: Publishing Particulate Matter Sensor Data to Adafruit IO With Maker Pi Pico and ESP-01S and it worked well for me on a PMS5003 over serial. You need to catch exceptions as depending on timing + buffering it can end up reading bad data.

@benthorner The datasheet link for Adafruit's PMSA003I (is l suffix significant here?) says:

There are two options for digital output: passive and active. Default mode is active after power up. In this mode sensor would send serial data to the host automatically.

As it starts up in active mode it sounds like it should work. Have you tried just printing out what's coming back from the UART object to check data is appearing?

benthorner commented 2 years ago

@kevinjwalters you're right: if I cut the power and reboot the example program works 🎉. I still prefer the passive mode though for the reasons @fivesixzero has described, but at least the claim in the code is mostly correct: the library does work with this PMS sensor, provided it's not received a command to enter passive mode since it's been on.