arduino-sps
arduino-sps copied to clipboard
Sensirion
Improve error reporting and handling
Currently, this library returns very limited error status, only STATUS_OK or STATUS_FAIL. It would be convenient if some additional codes would be returned (i.e. STATUS_NO_ACK, or STATUS_CRC_FAIL or STATUS_SHORT_READ etc.).
I considered having a go at implementing this, but since this touches on code that comes from the non-arduino-specific embedded-sps repo, which again has code from the embedded-common repo, I'm not going to figure out how all this plays together and where to contribute each part for now.
A related suggestion, is that the I2c implementation should probably check the return value of Wire.requestFrom, which returns the number of bytes actually read. I was using this library on an STM32 platform, which also has a 32-byte buffer. Currently, this makes sensirion_i2c_read silently only return 32 meaningful bytes, with the rest filled with garbage, which will then (usually) be detected by the CRC-checks, but it would be nicer if the short read was detected by itself (and with the above suggestion of different status codes, actually returned as such).
Agreed, that would make debugging easier. As you saw a lot of that will have to implemented in a project this one integrated, but having the issue here should help keep track.
Totally agree on checking Wire.request from, which is again in a file integrated from embedded-common.
Finally, all recent SPS versions support readout modes that fit in the 32 byte buffer, so I created issue #24 to add that in the (hopefully near) future.
Hi @matthijskooijman, you're speaking from my heart.. a better error handling would improve the developer experience. We have an issue open in our internal issue tracker. As you correctly noted, this is mostly flowing ouf of embedded-common, so if you want a placeholder to post some ideas, it'd be best to open another issue there https://github.com/Sensirion/embedded-common/issues
@rnestler FYI
There was actually some work in this direction in https://github.com/Sensirion/embedded-common/pull/45/.
