-
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
Added support for the Bosch BMP280 temperature and pressure sensor. #158
Conversation
Hi @nerdmeister thanks for the contribution. Can you please add this sensor to the smoke tests like this: https://github.com/tinygo-org/drivers/blob/dev/Makefile#L136-L137 Thank you! |
It should be added to the README.md 's list too : https://github.com/nerdmeister/drivers/blob/bmp280/README.md |
As requested, BMP280 has been added to the readme.md and the smoke-test. |
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.
I added a few comments, let me know what you think about them
examples/bmp280/main.go
Outdated
println(t) | ||
|
||
// Temperature in degrees Celsius | ||
//fmt.Printf("Temperature: %.2f\n", float32(t)/100) |
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.
I'd remove the commented lines, also, ReadTemperature returns milli Celsiuis, so it should be /1000 instead of /100, or am I missing someting?
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.
Thank for catching this one, ReadTemperature() and ReadPressure() now return in milli degrees celsius and milli pascal.
And the example has been adjusted to now properly show the temp in degrees celsius and pressure in hecto pascal.
t2 int16 | ||
t3 int16 | ||
|
||
// Pressure compensation |
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.
what do you think about using an array instead?
p [8]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.
I would prefer to not change this, since the compensation formula from the datasheet is already hard to read and I don't want to add another layer of confusion by changing everything from say t1 to t[0] and p3 to p[2].
|
||
rawPres := convert3Bytes(data[0], data[1], data[2]) | ||
|
||
// Pressure compensation |
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 would be nice if you could point to the part of the datasheet where this calculations come from
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.
Has been added.
…t32. ReadPressure() now returns value in milli pascal as an int32. Added references to datasheet. Renamed t_fine to tFine. Removed some unnecessary parentheses in Pressure compensation formula. Adjusted example to show temperature in degrees celsius and pressure in hecto pascal.
Thank you very much for this contribution @nerdmeister now squash/merging. Thanks again! |
…inygo-org#158) * bmp280: Added support for the Bosch BMP280 temperature and pressure sensor.
No description provided.