search

CarletonURocketry / fetcher

A QNX process for fetching data from sensors over I2C.
https://carletonurocketry.github.io/fetcher/
GNU General Public License v3.0
1 stars 0 forks source link

Create SHT41 Sensor Driver #17

Closed AngusJull closed 4 months ago

AngusJull commented 4 months ago

Closes #10

linguini1 commented 4 months ago

Some minor review comments before I merge, but this looks really clean. I don't know much about CRC so thank you for your implementation. Good idea to use conditional compilation.

What's the difference between the two CRC checks (lookup table vs polynomial)? And the hard-coded lookup table would apply to more than just the SHT41?

linguini1 commented 4 months ago

I'm approving this, but if you could at some point change the humidity to be .2f of precision when printed and just round the values internally in sht41_read() that would be ideal.

Great work and nice job doing it so quickly! I'll let you hit the merge button.

AngusJull commented 4 months ago

Some minor review comments before I merge, but this looks really clean. I don't know much about CRC so thank you for your implementation. Good idea to use conditional compilation.

What's the difference between the two CRC checks (lookup table vs polynomial)? And the hard-coded lookup table would apply to more than just the SHT41?

A quick way of describing CRCs is that you take the input data in bytes, then divide it using a polynomial. The remainder is a CRC and can be used for detecting errors in the transmission of the data - basically, we could tell if I2C was having problems. Doing a CRC like you would by hand, you do a bunch of bit shifts and XORs for every byte - but since the division of a byte by a certain polynomial is always the same, you may as well just store it in a lookup table and save yourself the hassle.

Most people choose to give up some memory for much faster CRC calculation using a table. The lookup table is applicable to any CRC-8 calculation, so long as it uses the same polynomial, which isn't a certainty.

AngusJull commented 4 months ago

Sorry for the push notifications, I should probably not be doing so much revision of the commits themselves.

Last time I force push on this one - promise :)