Adding SEN0322 O2 Sensor Driver Support #1075
Conversation
| hz: 100_000, | ||
| ...options.sensor | ||
| }); | ||
| this.#onError = options.onError |
| { | ||
| #io; | ||
| #key; | ||
| #onError; |
| const mv = options.mv ?? 0; | ||
| if (vol < 0 || vol > 25) | ||
| throw new RangeError("invalid O2 concentration (must be 0-25)"); | ||
| else |
There was a problem hiding this comment.
The throw above will always, so the content of this else clause can be directly inline below.
| } | ||
| close() | ||
| { | ||
| this.#io.close(); |
There was a problem hiding this comment.
close is defined to be safe to call more than once. This implementation will throw after the first time it is called. We typically use this.#io?.close() here instead.
| sample() | ||
| { | ||
| const io = this.#io; | ||
| let ret = {}; |
There was a problem hiding this comment.
This is allocated very far from when it is used. Maybe simplify to:
return {O: oxygenPercent * 10_000}; // O2 concentration in PPM| @@ -0,0 +1,80 @@ | |||
| import Timer from "timer"; | |||
There was a problem hiding this comment.
Our ECMA-419 sensor drivers usually include a header with:
- Copyright notice
- Full name of part
- Link to relevant manufacturer data sheets
- Link to any implementations used as a reference
| io.write(Uint8Array.of(Register.OXYGEN_DATA)); | ||
| Timer.delay(100); | ||
| const rxbuf = new Uint8Array(io.read(3)); | ||
| const oxygenPercent = ((this.#key) * ((rxbuf[0]) + (rxbuf[1] / 10.0) + (rxbuf[2] / 100.0))); |
There was a problem hiding this comment.
There's a diversity of approaches to parenthesis in this source file.
For example, this line is very careful about parenthesizing just about everything (including this.#key and the entire expression).
Compare to line 42 which only parenthesizes a number literal (mv < 0.000001 && mv > (-0.000001)). More consistent style with this repository would be ((mv < 0.000001( && (mv > -0.000001))) or a better way to express between, (-0.000001 < mv) && (mv < 0.000001)). That could apply to line 34 as well.
| const io = this.#io; | ||
| this.#setKey(); | ||
| io.write(Uint8Array.of(Register.OXYGEN_DATA)); | ||
| Timer.delay(100); |
There was a problem hiding this comment.
Hmm, this pauses the VM. I would not use this driver for this alone. Would be nice to find a different way to code this. At the very least document it at the top or readme.
There was a problem hiding this comment.
Agreed. A simple solution would be to have a timer in the driver to poll the value every 100ms. The driver would store the most recent value and return it from sample().
Because sample()1 is allowed to return undefined`, the driver could clear the cached sample after it is read once. The benefit is that a caller will know when they have a truly new reading.
Created sensor driver as well as example project