search

gfroerli / firmware

Firmware for the water temperature sensor project
GNU General Public License v3.0
6 stars 1 forks source link

Gracefully handle errors #87

Closed rnestler closed 3 years ago

rnestler commented 3 years ago

Fixes #68

rnestler commented 3 years ago 
When initialization of the DS18B20 fails, the code still panics slightly_smiling_face The sensor instances in the resources should probably be inside options.

Hmm. Do we want to support this use case? I mean we won't have any temperature data at all when this happens. The other changes are when there is a random glitch at runtime, no?

dbrgn commented 3 years ago

Hmm. Do we want to support this use case? I mean we won't have any temperature data at all when this happens.

You still have the voltage and the SHT temperature... If we still get measurement messages, but without water temperature, we know exactly where the problem is (sensor disconnected or broken). If we don't get any message at all, it's hard to diagnose...

rnestler commented 3 years ago

You still have the voltage and the SHT temperature... If we still get measurement messages, but without water temperature, we know exactly where the problem is (sensor disconnected or broken). If we don't get any message at all, it's hard to diagnose...

True. Also I already implemented it.

I don't think we should make the SHT optional as well, but I can replace the sht.wakeup(&mut delay).expect() call with just logging the error, which should give additional panic resistance.

dbrgn commented 3 years ago

Cool, works great now!

The only thing I'm missing is a line in the serial log indicating that the DS18B20 couldn't be initialized. Should be very simple to add, right?

rnestler commented 3 years ago

The only thing I'm missing is a line in the serial log indicating that the DS18B20 couldn't be initialized. Should be very simple to add, right?

Yes. Fixed. Maybe I should have actually tested with hardware, could have saved some iterations on this PR :see_no_evil: