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 #10

Closed
wants to merge 5 commits into from
Closed

UART fixes #10

wants to merge 5 commits into from

Conversation

fivesixzero
Copy link

@fivesixzero fivesixzero commented Sep 8, 2020

  • Added code to set the PM2.5 device to "active" via UART commands in the PM25_UART init
  • Added new PM25_UART_PASSIVE class with an init and _read_into_buffer()
  • Added safety check and retry in PM25_UART_PASSIVE's _read_into_buffer()` to retry in cases where the device wasn't ready after a mode change or the uart still had an 8-byte mode change response waiting
  • Added uart.read() after mode change in each init to make sure that the change worked as expected and that the mode change bytes are removed from the buffer before any future reads
  • Added comment block in example to illustrate usage of this new sub-class

Tested with a PM5003 on a RPi with an Enviro+ hat

Fixes issue #9

@tannewt tannewt requested a review from a team September 8, 2020 20:07
@dglaude
Copy link
Contributor

dglaude commented Nov 15, 2020

I tryied to implement your change on the current state of the repo, where files for uart and i2c are split.
My first attempt was to put the code to set to passive in uart.py.

So that is everything you have between self._uart = uart and super().__init__().

That first attempt fail on self._uart.write([0x42, 0x4D, 0xE1, 0x00, 0x01, 0x01, 0x71]) and I get this error:

TypeError: object with buffer protocol required

I tried changing to

self._uart.write(bytes("\x42\x4D\xE1\x00\x01\x01\x71", 'utf-8'))

But that fail with this error message a few steps later:

"Error configuring PM2.5 sensor for active reading, checksum failure on mode change response"

Right now I have the following feeling:

  1. You tested on a Pi with the full CPython, and maybe the uart write is different in CircuitPython
  2. Maybe my attempt at fixing that is wrong, or my hardware does not understand those mode change
  3. Your passive mode code should be in uart_active.py
  4. Adding the mode change between active and passive add code to uart.py that most user will not need, and also code to uart_passive.py ... so maybe this could be in a separate file such as uart_mode_change.py with one fonction to go passive and one function to go active.

I am not sure about everything, and obviously my attempt at using your code failed.
At least I tried to try your code.

@dglaude
Copy link
Contributor

dglaude commented Nov 15, 2020

Also notice that I have a PR pending #12 with the improvement that @Gadgetoid from Pimoroni did on a previous version/branch/fork. It is only in init and uart, so it should not stop you from doing an uart_active.py

Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

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

I did not do any testing, but David's comments seem to indicate that this does not work on CircuitPython as written.

@fivesixzero
Copy link
Author

My brain's been stuck in Java/SQL mode for weeks thanks to dayjob stuff but during the long weekend I'm planning to shift gears back to CircuitPython to take a fresh look at this.

At the very least I'll need to apply the changes to the newer structure with UART and I2C in separate files, based on David's commentary above.

Also, I've got some MagTags, QT Pys, and a few other random MCUs on hand that should provide a solid platform for testing this outside of the RPi world where I did the initial work.

@fivesixzero
Copy link
Author

Given that the structure has changed since this PR was made, I'm going to close it and open another once my development is complete. Thanks for the comments, @dglaude and @jepler! I'll make sure that the next PR is fully tested on MCUs running CircuitPython instead of just testing with a full Python environment on a RPi. 😂

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.

3 participants