davidgnx commented on issue #12665:
URL: https://github.com/apache/nuttx/issues/12665#issuecomment-2222512712
Hi @linguini1!
First of all: the code looks beautiful, understandable and nicely commented,
thanks for this.
Also thank you for your motivation and welcome to the NuttX community!
I know that your fork is work-in-progress, but I tried to figure out what is
wrong to help you.
During this "review" I have noticed a few things that you are probably not
aware of...
These are just suggestions, but they may improve the usability of this
driver. :)
1.) **Temperature range:**
The sensor is capable of measuring temperatures between -40 C and +125 C
with a resolution of 0.01 C and an accuracy of 0.1 C (SHT45).
Your choice to store the results in an int16_t represented as mC or mF
limits the theoretical measurement range to: -32.768 C ... +32.767 C in Celsius
mode and to -35.98 C ... 0.43 C in Fahrenheit mode. This means that in
Fahrenheit mode this driver will barely be able to measure any temperature
above the freezing point of water (which is fairly suboptimal for many users).
2.) **sht4x_calc_temp():**
There are massive integer overflows/underflows for huge ranges of valid
input values (input values: raw measurement data points). This is related to
1.), see above.
3.) **Humidity range:**
The sensor is capable of measuring relative humidity between 0 and 100
with a resolution of 0.01 %RH and an accuracy of 1 %RH (SHT45).
You store the result of the measurement with a resolution of 1% RH,
therefore you lose two digits precision.
Hint: The accuracy of the sensor is 1%, so losing two digits from the
resolution is not necessarily a problem, it depends on the use case.
4.) **sht4x_calc_hum():**
hum is an unsigned integer (uint16_t), therefore it can never be < 0.
Those values, that are supposed to be < 0 will be mapped to 100 instead of 0
because of an integer overflow/underflow. So your clamped output will look like
[ ..., 100, 100, 100, 0, 0, 1, 1, ..., 99, 99, 100, 100, 100, 100 ...] instead
of [ ..., 0, 0, 0, 0, 1, 1, ..., 99, 99, 100, 100, 100, 100 ...] (note the
wrongly mapped values below zero).
5.) **sht4x_register():**
This may (or may not) answer your original question, I didn't verify, just
looked at your code briefly. You declare the function in
include/nuttx/sensors/sht4x.h, but you do not define it anywhere, or at least I
haven't found it in the fork you've linked. There is a function named
sht**21**_register() though (defined in drivers/sensors/sht4x.c), I guess it's
a typo or copy-paste error.
If this was the problem, feel free to close this issue.
Greetings,
David
p.s.: **The second question (dynamically loading drivers) is not answered by
this post, so feel free to comment, thank you!**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
