Check for data ready not present in example

[clayton13]

Hi Sensirion team, thank you for investing resources into this. I was able to quickly get started with the base demo. I wanted to point out an issue that unaltered demo currently has which others may come across.

In the example file scd30/scd30_example_usage.c, there is no data ready check or usage of scd30_get_data_ready as seen in tests/scd-test.cpp and indicated in the datasheet.

Without this check, the device occasionally returns correctly checksummed 0xFF bytes which do not indicate any immediate errors. On my bench there is a seemingly erroneous measurement every ~8 samples. The simple fix is to just add a check before sampling as done in the test file.

[abrauchli]

Hi @clayton13 many thanks for the report. That's understandably unfortunate.. we'll investigate on how to handle this. I'm not too happy with always calling data_ready in the synchronous case. I'm thinking of just returning a measurement failure in that case along with a recommendation (in the form of a header comment) to skip or retry this measurement.

As that what causes this, my feeling says that the clocks are drifting, causing the one second measurement interval to slowly drift way until both measurements fall into the same second - that could indicate that the sensor's clock is too slow (undervoltage?) or the host software to pull too fast (wrong timings in the code or implementation of sensirion_sleep_usec that's too fast) - those are all hypotheses.

What do you think, @rnestler ?

[rnestler]

Well we used to have the polling once and removed it in 54ebd28 with the reasoning:

The data ready flag should either be polled constantly or the value just
read every measurement interval.
Reason is, that when a new measurement is available, the flag is only
set for 100ms. Now if our clock is imprecise this may lead to a slow
shift which at some point may exceed 100ms, leading to a missed
measurement and a one second delay during polling.

Maybe it would be best to poll with a 50ms delay between the polls.

[clayton13]

@abrauchli is correct. There is a slight drift of about 11ms, which I did not expect to cause an issue with a 2s window after a few samples. But as described above, the flag is only set for 100ms which would line up with the drift I have after 8-9samples.

I was initially thinking I had a power supply issue with the sensor rail and started investigating before I realized it was a timing issue. As this code is meant for quickly testing, it is the best place to come across this potential timing case. However, as the issue wasn't immediately clear (to me at least) there were concerns before moving on.

Overall this is a good thing to come across early on and make note of for end users. I feel a little documentation for this particular case or heads up could be added. Maybe this could be as simple as a comment or even self-documented through this issue?

[rnestler]

@clayton13 I created #44. Does this fix the issue for you?

[rnestler]

Without this check, the device occasionally returns correctly checksummed 0xFF bytes which do not indicate any immediate errors. On my bench there is a seemingly erroneous measurement every ~8 samples. The simple fix is to just add a check before sampling as done in the test file.

I can reproduce the error, also receiving 0xFF's, but it should get interpreted as -NaN when using them as floats:

0xffffffff
0xffffffff
0xffffffff
measured co2 concentration: -nan ppm, measured temperature: -nan degreeCelsius, measured humidity: -nan %RH

[clayton13]

Sorry for the delay @rnestler. As standing #44 does not fix the issue.

Digging deeper in the datasheet, the 1.4.4 Get data ready status section:

the read header should be send with a delay of > 3ms following the write sequence.

This is not currently present in the library. Changing the scd.c scd30_get_data_ready to use the sensirion_i2c_delayed_read_cmd(..., 30000, ...) function internally along with #44 fixes the issue.

After applying the above changes, (and a printf dataready after scd30_get_data_ready in the main method) I have yet to miss a sample. With over 250 so far. Looking at the log, after the first few, it appears the rest are 8 or 9 loops in. Which I believe correspond to be 30 - 20ms early from 2s on my setup.

See log: scd30_log.txt

[clayton13]

I can reproduce the error, also receiving 0xFF's, but it should get interpreted as -NaN when using them as floats

That is an issue in my printf implementation as it is not following stdlib. Something I did not recognize until now, thank you for pointing that out.

[rnestler]

This is not currently present in the library. Changing the scd.c scd30_get_data_ready to use the sensirion_i2c_delayed_read_cmd(..., 30000, ...) function internally along with #44 fixes the issue.

Thanks for that, good catch 🙂 I applied this change in #45. But did you mean to use 3000 instead of 30000?

[rnestler]

@clayton13 Could you please verify that the fix which we finally merged in #44 does work for you as well?

[clayton13]

@rnestler All is working well. Thank you.

But did you mean to use 3000 instead of 30000?

Oops, yes I did. Good catch for a good catch 🙂

[kevinjwalters]

The datasheet also says the 3ms delay applies to Read measurement in section 1.4.5. Is there a reason this was only applied to Get data ready status?

We are discussing this over in adafruit/Adafruit_CircuitPython_SCD30#13

[rnestler]

This was changed in the datasheet but it was forgotten to update the driver here. It looks like we should add a read delay in read measurement as well.