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

Discard junk input bytes on creation #19

Merged
merged 1 commit into from
Feb 8, 2024

Conversation

dhalbert
Copy link
Contributor

@dhalbert dhalbert commented Feb 8, 2024

adafruit/circuitpython#8887 and https://forums.adafruit.com/viewtopic.php?t=208042, by @vladak, describe a problem caused by the ESP32-S3 reading a single junk byte when a busio.UART() is first created.

Prevent that problem by waiting long enough for the byte to appear, and then clearing out all the input bytes, so that .distance and .temperature will only read wanted bytes.

Also fixes #18 by adding some documentation.

@vladak Could you test? This is like the workaround I gave you, so test without that.

@vladak
Copy link
Contributor

vladak commented Feb 8, 2024

Works for my use case, however according to the __init__() comment it seems to me that all the ESP32-S3 use cases that read data over UART would have to receive similar fix.

Comment on lines +42 to +43
time.sleep(0.1)
uart.reset_input_buffer()
Copy link
Contributor

Choose a reason for hiding this comment

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

I have 2 concerns about this:

  • if this is ESP32-S3 specific, this will unnecessarily delay other HW
  • if the issue is specific to UART (on ESP32-S3) rather than US-100, the fix should probably reside in CP itself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The delay only happens once, in the constructor. I could probably make this 2 msecs instead of 100 msecs. I can test that.

Yes, ESP32-S3 should be fixed. I am looking at that. But this makes the library more robust for any case like this where it might get out of sync. And it makes the library usable without having to upgrade CircuitPython.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a fair argument, esp. the second part.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Looks good to me! Feel free to merge when ready.

@dhalbert dhalbert merged commit c183479 into adafruit:main Feb 8, 2024
1 check passed
@dhalbert dhalbert deleted the discard-junk-input branch February 8, 2024 22:53
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Feb 9, 2024
Updating https://github.com/adafruit/Adafruit_CircuitPython_US100 to 1.1.17 from 1.1.16:
  > Merge pull request adafruit/Adafruit_CircuitPython_US100#19 from dhalbert/discard-junk-input
  > Merge pull request adafruit/Adafruit_CircuitPython_US100#20 from vladak/units

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA:
  > Updated download stats for the libraries
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.

document that getting values results in the code sleeping for non-trivial amounts of time
3 participants