Open sago35 opened 4 months ago
Any further feedback @soypat and @FilipVranesevic ?
@sago35 do you wish to squash commits in this PR or would you prefer it is done when it is merged?
Any further feedback @soypat and @FilipVranesevic ?
@deadprogram I'm not sure the stale data issue I noted was addressed properly. This is how I see it (@sago35 please correct me if I'm wrong since I just glanced over the sensor datasheet):
Consider this realistic scenario, I want to log temperature each hour. At the start of the program I initialise the sensor and call Update
. At 1PM I call Update
and Temperature
to get the reading. Since Update
method first reads values from the sensor and then jus triggers next measurement I would get the measurement from the time first Update
is called (at the start of the program). At 2PM I call Update
and Temperature
again and this time I get value from the measurement done at 1PM. I think this could be very confusing to the users of the driver even in this simple use case.
Hmm- I tend to agree with @FilipVranesevic. The sentinel error is problematic for reasons that have become apparent to me just now: Update()
returns nil error only when sensor values are correctly updated- in adding a sentinel error that signals a future call to Update()
will have the correctly updated values we are adding a lot of logic the sensor has to resolve:
I believe this should be resolved in a higher level of abstraction, not by the sensor, since this is an issue that would be present in many sensors, we don't want to solve staleness or async sensing in every single driver we implement in drivers. This is exactly why I pushed for the Sensor
interface- so we can abstract different Sensor behaviour! So... I believe:
Example of what an async API could look like. Do note I do not like this particular design, but it'd solve the aforementioned issue for all sensors with similar functioning to the DHT.
func main() {
d := dht.New(p, t)
as := NewAsyncSensor(d, 2, 100*time.Millisecond, func(dst []int32) {
dst[0] = d.Temperature()
dst[1] = d.Humidity()
})
for {
// Values are best attempt at getting values close to `period` staleness.
temp := as.Get(0)
humidity := as.Get(1)
print(time.Since(as.LastMeasurement()).String(), "ago ")
println(temp, humidity)
}
}
type AsyncSensor struct {
s Sensor
lastUpdate time.Time
period time.Duration
get func(dst []int32)
lastMeasurements []int32
}
func NewAsyncSensor(s Sensor, n int, period time.Duration, getter func([]int32)) *AsyncSensor {
as := &AsyncSensor{
s: s,
period: period,
get: getter,
lastMeasurements: make([]int32, n),
}
go as.startMeasuring()
return as
}
func (as *AsyncSensor) startMeasuring() {
for {
elapsed := time.Since(as.lastUpdate)
if elapsed < as.period {
time.Sleep(as.period - elapsed)
}
err := as.s.Update(drivers.AllMeasurements)
if err != nil {
time.Sleep(as.period)
continue
}
as.get(as.lastMeasurements[:])
as.lastUpdate = time.Now()
}
}
func (as *AsyncSensor) LastMeasurement() time.Time {
return as.lastUpdate
}
func (as *AsyncSensor) Get(i int) int32 {
return as.lastMeasurements[i]
}
Example of what an async API could look like. Do note I do not like this particular design, but it'd solve the aforementioned issue for all sensors with similar functioning to the DHT.
func main() { d := dht.New(p, t) as := NewAsyncSensor(d, 2, 100*time.Millisecond, func(dst []int32) { dst[0] = d.Temperature() dst[1] = d.Humidity() }) for { // Values are best attempt at getting values close to `period` staleness. temp := as.Get(0) humidity := as.Get(1) print(time.Since(as.LastMeasurement()).String(), "ago ") println(temp, humidity) } } type AsyncSensor struct { s Sensor lastUpdate time.Time period time.Duration get func(dst []int32) lastMeasurements []int32 } ...
@soypat I would probably call this functionality you propose as SensorPoller
or similar.AsyncSensor
would in my opinion be more like one shot non blocking update with callback or channel to signal the completion. I also agree that this sort of functionality should be higher level abstraction (if it is decided that is needed) and not part of the driver.
I am adding support for the DHT20 in this PR. I tested it using Seeed's
Grove - Temperature&Humidity Sensor (DHT20)
.https://wiki.seeedstudio.com/Grove-Temperature-Humidity-Sensor-DH20/ https://cdn-shop.adafruit.com/product-files/5183/5193_DHT20.pdf
The challenging part of the design was the relationship between having to wait 80ms after triggering and the Update() function. Currently, I have designed it so that even if Update() is called multiple times within 80ms, there will be no I2C communication. If this becomes an issue, we will need to consider an alternative solution.