search

yryz / ds18b20

Golang get temperature from ds18b20 sensor connected to a Raspberry PI (GPIO w1 pin).
Apache License 2.0
64 stars 12 forks source link

Validate temperature data on read #2

Closed MaxRenaud closed 4 years ago

MaxRenaud commented 5 years ago

Summary: This library does not validate that a correct temperature was obtained. Note this is only to validate that the 1Wire communication was successful; the DS18b20 sensor does not have self-test.

Observed behavior: The first line of the w1 "file" is a CRC validation of the temperature value. At this moment, the yryz/ds18b20 library does not validate that the CRC is valid before returning a temperature reading.

Desired behavior: Expect the CRC tag to be present and "YES" immediately after. Should this condition fail, return an error.

yryz commented 4 years ago

@MaxRenaud Is there a default value for t when CRC != YES? This is easy to change. I want to test it. Can you tell me how to test it？ image

yryz commented 4 years ago

https://github.com/yryz/ds18b20/commit/448a8e7af431e8fc064944bd85a0839febf7983e

MaxRenaud commented 4 years ago

Thanks for closing. I am not sure how you'd test it using your current testing methodology. If you want to refactor a bit, you can have a function that takes an io.Reader interface in your .go file. That way, you can decouple the file reading and the temperature parsing.

In your test, you can use strings.NewReader("Something") to create an an object that satisfies io.Reader and you can pass around.

As for the default value, I'm not sure if the w1 drivers override the read value when the CRC fails or if it puts puts the read value as is. I think your commit is doing the right thing by checking for a CRC match, then returning an error if it doesn't match.