-
Notifications
You must be signed in to change notification settings - Fork 229
Add Honeywell HSC TruStability SPI+I2C pressure sensor driver #799
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
base: dev
Are you sure you want to change the base?
Conversation
soypat
commented
Oct 14, 2025
- https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/zh-cn/localized/datasheets/sps-siot-trustability-hsc-series-high-accuracy-board-mount-pressure-sensors-50099148-a-ciid-151133-cn.pdf
examples/honeyhsc/example.go
Outdated
| func main() { | ||
| bus := machine.I2C0 | ||
| err := bus.Configure(machine.I2CConfig{ | ||
| Frequency: 10_000, |
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.
10kHz is an unusually low frequency, is this intentional? (Many chips don't even support such low frequencies).
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.
Very good catch. 100kHz is minimum for this IC.
| return h | ||
| } | ||
|
|
||
| func (d *DevI2C) Update(which drivers.Measurement) error { |
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.
Same here, please add documentation to this method (especially since it is exported).
honeyhsc/hsc.go
Outdated
| buf := &d.buf | ||
| const reg = 0 | ||
| value := (d.addr << 1) | 1 | ||
| err := d.bus.Tx(uint16(d.addr), []byte{reg, value}, buf[:]) |
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.
Note that the []byte{reg, value} will likely cause a heap allocation depending on the I2C implementation.
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.
thanks for the catch!
| buf [4]byte | ||
| } | ||
|
|
||
| func NewDevSPI(conn drivers.SPI, cs pinout, outMin, outMax uint16, pMin, pMax int32) (*DevSPI, error) { |
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.
Same here, this should get some explanation. At least clarifying with parameters like outMin mean.
honeyhsc/hsc.go
Outdated
| return h, nil | ||
| } | ||
|
|
||
| // Update implements the sensor interface. |
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.
That's technically correct, but not super helpful to users.
What about something like
Update reads the sensor values from the device. The values are stored in the driver object, and can be read with the
PressureandTemperaturemethods.
| } | ||
|
|
||
| // Temperature returns temperature in milliKelvin [mK]. | ||
| func (d *dev) Temperature() int32 { |
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.
By far most drivers use milli-degrees Celsius. See for example:
Lines 277 to 279 in 228e57c
| // Temperature returns the last read temperature in celsius milli degrees (1°C | |
| // is 1000). | |
| func (d *Device) Temperature() int32 { |
And:
Lines 180 to 181 in 228e57c
| // ReadTemperature returns the temperature in celsius milli degrees (°C/1000) | |
| func (d *Device) ReadTemperature() (int32, error) { |
I would like to keep that as a convention.
Also see #332 which I still think is a good idea. It's somewhat out of scope for this PR though.
|
@aykevl Thanks for the suggestions! good stuff :) |