search

RobTillaart / SHT2x

Arduino library for the SHT2x series temperature and humidity sensors including SHT20, 21, 25.
MIT License
19 stars 5 forks source link

Fixed getRawTemperature and getRawHumidity #11

Closed theandy94 closed 2 years ago

theandy94 commented 2 years ago

The internal variables that the raw values are read from were swapped.

RobTillaart commented 2 years ago

Thanks for catching this bug I will investigate it later today.

RobTillaart commented 2 years ago

Looks like I was somewhere completely else with my mind when writing this code.

theandy94 commented 2 years ago

Happens to the best of us 😉 Nice to see you are this fast to react to pull requests 👍

Have a nice sunday :)

RobTillaart commented 2 years ago

Nice to see you are this fast to react to pull requests 👍

If they are this obvious one has no choice 😁

RobTillaart commented 2 years ago

@theandy94

Triggered by your PR a new 0.2.0 release is prepared. See the develop branch

If you have time to test with real hardware, would be appreciated. (especially resolution) I have no hardware available at the moment, and still need to write an example sketch.

update: example added (loop over resolutions, should give all same temp and hum +- noise)

theandy94 commented 2 years ago

The only hardware I've got is a HTU21D connected to an ESP32 devboard using its 3.3v to supply the sensor.

I've downloaded the current dev branch with 0.2.0 and ran the SHT2x_resolution.ino sketch. Upon starting I get: STAT: 0 BATT: OK

I can confirm, that the temperature and humidity readings are the same for each resolution setting +-noise.

Always glad to help :)

UPDATE: I also tested getEIDA() and getEIDB() which do return values, although I am not sure how to validate correctness of the values. EIDA: 10275022 EIDB: 839993428 The getFirmwareVersion() call returns 0 for my HTU21D, not sure what to make of this... maybe a difference between SHT and HTU chips?

RobTillaart commented 2 years ago

Thanks for testing, I will dive into the datasheets tomorrow to check for diffs. The EIDA is less important than the setresolution. To be continued

RobTillaart commented 2 years ago

updated the develop with crc8 test in the read() it will set the error flag, but will not fail on invalid crc. (don't know if it adds value as the error rate seems to be pretty low)

RobTillaart commented 2 years ago

I read several datasheets and they look pretty the same. However I noticed I made a mistake in the masking of the resolution bits, so expect a fix asap.

update: fixed, but needs another round of testing.

If you have time for testing, can you post the output of SHT2x_resolution.ino? Thanks,

theandy94 commented 2 years ago

Output from SHT2x_resolution.ino from latest develop:

SHT2x_LIB_VERSION:      0.2.0
STAT:   0
BATT:   OK
RES:    0
        102448  23.9    58.2
RES:    1
        101452  23.8    58.2
RES:    2
        101452  23.8    58.3
RES:    3
        101452  23.8    58.1
RES:    0
        101452  23.8    58.0
RES:    1
        101452  23.8    58.0
RES:    2
        101452  23.8    58.1
RES:    3
        101452  23.8    58.1
RES:    0
        101452  23.8    58.2
RES:    1
        101452  23.8    58.1
RES:    2
        101452  23.8    58.1
RES:    3
        101452  23.8    58.1
RES:    0
        101452  23.8    58.1
RES:    1
        101452  23.8    58.1
RES:    2
        101452  23.8    58.1
RES:    3
        101452  23.8    58.1
RobTillaart commented 2 years ago

Thanks, i notice the timing is still hard-coded. Will look into that tomorrow. Without shorter delays there is no reason to set a lower resolution. Tbc

RobTillaart commented 2 years ago

Just pushed a change with adjusted timing for the different resolutions. (based upon table 7 SHT21) also changed the decimals to 3 in the test sketch to see the difference in resolution.

Please rerun the test sketch.

If it is OK, I am going to merge it into master and release version 0.2.0

theandy94 commented 2 years ago

Output from latest develop:

SHT2x_LIB_VERSION:      0.2.0
STAT:   0
BATT:   OK
RES:    0       116452  24.300  60.727
RES:    1       1113875 24.300  60.727
RES:    2       1084875 24.279  60.727
RES:    3       1102875 24.279  60.727
RES:    0       116452  24.268  61.291
RES:    1       1113874 24.268  61.291
RES:    2       1084875 24.268  61.291
RES:    3       1102875 24.268  61.291
RES:    0       116452  24.290  60.795
RES:    1       1113874 24.290  60.795
RES:    2       1084875 24.290  60.795
RES:    3       1102875 24.290  60.795
RES:    0       116452  24.279  60.689
RES:    1       1113874 24.279  60.689
RES:    2       1084875 24.268  60.689
RES:    3       1102875 24.268  60.689
RES:    0       116452  24.290  60.620
RES:    1       1113874 24.290  60.620
RES:    2       1084875 24.279  60.620
RES:    3       1102875 24.279  60.620
RES:    0       116452  24.279  60.559
RES:    1       1113874 24.279  60.559
RES:    2       1084875 24.279  60.559
RES:    3       1102875 24.279  60.559

I noticed, that the humidity value only ever changed after resetting the resolution setting to 0 while staying the same throughout the for loop.

During investigating your code if there might be some hidden reason, I noticed the different delays based on the resolution setting (if else if...) and was wondering why for temperature it was 3 then 1 then 2 but for humidity 1 then 2 then 3. I guess it wouldn't hurt to add a comment from the datasheet for your own sanity and anyone reading the code: "Default measurement resolution is 14bit (temperature) / 12bit (humidity). It can be reduced to 12/8bit, 11/11bit or 13/10bit by command to user register."

Otherwise I foresee someone in the future trying to "fix" this code but actually breaking correctness.

Anyways, thanks for your work :)

RobTillaart commented 2 years ago

Thanks for testing, Good tip to improve comments here. Still the time used by the various resolutions make no sense except for zero. Tbc

RobTillaart commented 2 years ago

The expected times versus the measured times

RES HUM TEMP TOTAL REAL
0 29 85 114 116
1 4 22 26 1113
2 9 43 52 1084
3 15 11 26 1102

So only the resolution == 0 meets the expectation. I do not see a pattern in the deviations and

Which processor do you use?

What I do notice is that the sensor gives 4x the same temperature in a row. And humidity values seem to repeat too. That could indicate that time between measurements is too short.

I am going to rewrite the example. Please be so kind to run another test.

Example updated

theandy94 commented 2 years ago

I've used an ESP32 which is known to not be very precise with its delays...

I now used an Arduino Nano with the latest develop version. Output is the following:

SHT2x_LIB_VERSION:      0.2.0
STAT:   0
BATT:   OK
RES:    0       115368  23.946  59.887
RES:    0       115376  23.882  60.017
RES:    0       115364  23.871  60.193
RES:    0       115368  23.871  60.139
RES:    0       115364  23.861  60.101
RES:    1       112292  23.861  60.101
RES:    1       113008  23.861  60.101
RES:    1       113016  23.861  60.101
RES:    1       113012  23.861  60.101
RES:    1       113000  23.861  60.101
RES:    2       83300   23.839  60.101
RES:    2       84396   23.839  60.101
RES:    2       83360   23.828  60.101
RES:    2       83364   23.828  60.101
RES:    2       84388   23.818  60.101
RES:    3       101796  23.818  60.101
RES:    3       100716  23.818  60.101
RES:    3       101752  23.818  60.101
RES:    3       101748  23.818  60.101
RES:    3       101736  23.818  60.101

done...
RobTillaart commented 2 years ago

Looks better than last test but still most are longer than expected. Either the datasheet is incorrect or ? I will make a remark in the readme and start merging process later today. Enough time spent on it for now. Thanks

theandy94 commented 2 years ago

Always glad to help.

RobTillaart commented 2 years ago

Note: that the timing in the last test is very similar per resolution MINUS 1 second ... (Will dive into it later again as said before, but I want to keep this observation)

RobTillaart commented 2 years ago

Released 0.2.0