-
Notifications
You must be signed in to change notification settings - Fork 10
Add start low power periodic measurement #6
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
Conversation
@enosh Thanks for your contribution! I am working with this sensor and will test it either today or Tuesday. Running CI now, which will verify syntax and formatting, etc. |
didnt test but lgtm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I double checked the datasheet just to verify the _SCD4X_STARTLOWPOWERPERIODICMEASUREMENT
hex of 0x21AC
is correct. The command is correct, but in the datasheet it says there's no delay needed after the command is sent.
Is there a reason you added a delay of 0.01s?
I used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If that's the reasoning, then I'd err on the side of following the datasheet, and not having a delay. My main motivation is because I usually use datasheets as my mental 'square one' so if I have a bug I can't easily solve, I go back to that to make sure I can follow along with the code.
While you're right, a delay doesn't hurt unless you're doing really time sensitive stuff, the sensor reads have a built in delay with if scd4x.data_ready
, so I think it is safe to remove this delay in favor of keeping with the sensor's datasheet.
Outside of that, awesome PR! Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again, @enosh. I agree with @CrakeNotSnowman on the delay. Please remove it, and if there are issues in the future, we have this discussion to fall back on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That matches the datasheet as far as I'm able to tell.
Thank you for making these additional changes! I really appreciate it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for adding this. @enosh!
Updating https://github.com/adafruit/Adafruit_CircuitPython_SCD4X to 1.2.0 from 1.1.0: > Merge pull request adafruit/Adafruit_CircuitPython_SCD4X#6 from enosh/low_power_start Updating https://github.com/adafruit/Adafruit_CircuitPython_MacroPad to 2.0.0 from 1.4.1: > Merge pull request adafruit/Adafruit_CircuitPython_MacroPad#27 from kattni/speaker-enable
Saw this command in the data sheet and wanted to use it.
The function name is
start_low_periodic_measurement
instead ofstart_low_power_periodic_measurement
to fit the length pylint wants.Tested with a Feather RP2040 and the new SCD40 STEMMA QT breakout. Around 30 seconds between measurements.

First time submitter! Hope I'm not missing anything.