-
Notifications
You must be signed in to change notification settings - Fork 196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sensor interface PR proof of concept #335
base: dev
Are you sure you want to change the base?
Conversation
|
||
func (d *Device) Update(which drivers.Measurement) (err error) { | ||
// Temperature is needed for presssure calculation. | ||
if which|drivers.Temperature|drivers.Pressure == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall it not return some sort of error when measurement requested is not temperature or pressure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason behind it not being an error is to allow for grouping of Sensors.
allSensors := []Sensor{imu, bmp, tempSens}
updateAll := func (w Measurement) {
for _, sens := range allSensors {
sens.Update(w) // illustrative, please handle your errors
}
}
This way you can have precise requests of what sensor measurements you want to update. This is especially useful if the characteristic time of a sensor is very slow compared to other sensors in the group, i.e Temperature sensor vs. voltage sensor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be worth thinking about this a bit more. Maybe it's worth having a io.EOF
sentinel like error for the case you mention Yurii, for example: drivers.ErrNoMeasurement
.
} | ||
|
||
// Channel returns an ADC measurement for a specific channel. Must call Update Beforehand | ||
func (d *BufferedDev) Channel(ch int) uint16 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this name? Shall be "Voltage"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So maybe this name should be Get, the reason it is not Voltage is because it does not represent the voltage, merely the bits read by the ADC. I have added the Voltage method separate as a conversion of Channel.
|
||
// UNTESTED | ||
func (d *BufferedDev) Update(which drivers.Measurement) error { | ||
if which|drivers.Voltage == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now it only reads voltage, can read more things?
If only voltage ever, why have which drivers.Measurement
at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above
3bb7ba6
to
de642c6
Compare
de642c6
to
a5b5fbf
Compare
Proof of concept for #321. WIP