RIOT
RIOT copied to clipboard
drivers/abp2: add abp2 driver
Contribution description
- Add a driver for ABP2 Honeywell pressure and temperature series.
- Implement a SAUL interface.
- Support the SPI version, but not the I2C version (I don't have that one). However, prepare the code for easy I2C support.
- Document the code.
Testing procedure
Use the test application in RIOT/tests/drivers/abp2
.
Output of the test application:
=========================
Measuring
=========================
Pressure range = 0 .. 160000
Data: 0.0003 Bar
Data: 17.151 °C
Data: 0.0003 Bar
Data: 17.777 °C
Data: 0.0003 Bar
Data: 17.207 °C
------------------------------
Switch to blocking mode
------------------------------
Data: 0.0003 Bar
Data: 17.111 °C
Data: 0.0003 Bar
Data: 17.180 °C
Data: 0.0003 Bar
Data: 17.139 °C
------------------------------
Switch to SAUL mode
------------------------------
Dev: Pressure sensor (abp2) Type: SENSE_PRESS
Data: 0.0003 Bar
Dev: Temperature sensor (abp2) Type: SENSE_TEMP
Data: 17.180 °C
Dev: Pressure sensor (abp2) Type: SENSE_PRESS
Data: 0.0003 Bar
Dev: Temperature sensor (abp2) Type: SENSE_TEMP
Data: 17.220 °C
Dev: Pressure sensor (abp2) Type: SENSE_PRESS
Data: 0.0003 Bar
Dev: Temperature sensor (abp2) Type: SENSE_TEMP
Data: 17.166 °C
------------------------------
Switch to non-blocking mode
------------------------------
Data: 0.0003 Bar
Data: 17.247 °C
Data: 0.0003 Bar
Data: 17.201 °C
Data: 0.0003 Bar
Data: 17.175 °C
------------------------------
Switch to blocking mode
------------------------------
Issues/PRs references
Fixes #20233
your commits messages will need rewording
https://github.com/RIOT-OS/RIOT/blob/master/CONTRIBUTING.md#commit-conventions
your commits messages will need rewording
https://github.com/RIOT-OS/RIOT/blob/master/CONTRIBUTING.md#commit-conventions
Could you be more accurate ?
your commits messages will need rewording https://github.com/RIOT-OS/RIOT/blob/master/CONTRIBUTING.md#commit-conventions
Could you be more accurate ?
Each commit should target changes of specific parts/modules of RIOT. The commits use the following pattern:
area of code: description of changes
You can use multi-line commit messages if you want to detail more the changes. For example:
periph/timer: Document that set_absolute is expected to wrap
Most timers are implemented this way already, and keeping (documenting)
it that way allows the generic timer_set implementation to stay as
simple as it is.
so you commit messages should be for example
driver/abp2: driver for SPI sensors prepare for I2C
tests/driver/abp2: test implementation
or something similar
the scheme is <arena of work>: <description>
OK, done. Thanks for the details. It's my 1st contribution...
aside from the leftover in the test app this seems good to go
That's good news!
please provide current test results from the current version starting at shell prompt (I don't have the hardware to test)
What do you mean? What about the program output in the 1st message of this thread?
aside from the leftover in the test app this seems good to go
That's good news!
please provide current test results from the current version starting at shell prompt (I don't have the hardware to test)
What do you mean? What about the program output in the 1st message of this thread?
i see you updated it ( 1h ago there where non english words in it )
Murdock results
:heavy_check_mark: PASSED
b7198cc3fbb1756425e1601766ac8526d75e5a3b tests/drivers/abp2: test implementation
Success | Failures | Total | Runtime |
---|---|---|---|
10025 | 0 | 10026 | 10m:15s |
Artifacts
diff --git a/tests/drivers/saul_drivers/Makefile b/tests/drivers/saul_drivers/Makefile
index c729dc575e..da6d708ee5 100644
--- a/tests/drivers/saul_drivers/Makefile
+++ b/tests/drivers/saul_drivers/Makefile
@@ -53,6 +53,9 @@ ifneq (,$(filter vl6180x,$(DRIVERS)))
USEMODULE += vl6180x_als
USEMODULE += vl6180x_rng
endif
+ifneq (,$(filter abp2,$(DRIVERS)))
+ USEMODULE += abp2_spi
+endif
USEMODULE += saul_default
diff --git a/tests/drivers/saul_drivers/Makefile b/tests/drivers/saul_drivers/Makefile index c729dc575e..da6d708ee5 100644 --- a/tests/drivers/saul_drivers/Makefile +++ b/tests/drivers/saul_drivers/Makefile @@ -53,6 +53,9 @@ ifneq (,$(filter vl6180x,$(DRIVERS))) USEMODULE += vl6180x_als USEMODULE += vl6180x_rng endif +ifneq (,$(filter abp2,$(DRIVERS))) + USEMODULE += abp2_spi +endif USEMODULE += saul_default
Should I apply these changes and force push?
Also, I noticed that entries in this file are sorted. So these lines should be added further up in the file.
Should I apply these changes and force push?
yes (problem noticed by murdock)
Also, I noticed that entries in this file are sorted. So these lines should be added further up in the file.
please add the lines where they belong - good you noticed
btw are you also working towards i2c support?
btw are you also working towards i2c support?
I have no plan to do so because I don't have a I2C version at hand.
Hi, :up: Any chance this driver will ever make its way upstream ? :up:
I feel a little uncomfortable with the i2c integration hints (code) and pseudo_module, please remove. Talked to other maintainers and we seem to align on that. The comments "i2c communication should go here" -even though it is obvious ( everywhere where spi is called i2c would also be)- is ok for me.
- implementation wise it would be better to be able to use spi and i2c the at the same time -> the current hints are not the right direction (the module would provide either one or the other interface and work only for one i2c adress.
- at some places this will seem like this driver supports i2c (eg if you list available modules) but it isn't -- this leads to: "hey RIOT supports abp2-i2c" -- "just the implementation is missing".
- since you do not intend to write it, it will probably not exist.
If you want to hint: "hey the is something open TODO" write a TODO
implementation wise it would be better to be able to use spi and i2c the at the same time
Why?
Why?
Why should we Block that usecase (one might have a mix of both in one experiment or just have multiple sensors) the design hints just point into the direction of there will only be one sensor
simple example speed/flow (pito) and pressure measurement would need one differential and one absolute sensor (or two absolute) (ofcause these can be two spi (different cs), but with these hints i2c would not work (neither 2 i2c sensors nor can it be combined with spi)).