Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: DPS310 driver #463

Merged
merged 40 commits into from
Nov 3, 2022
Merged

feat: DPS310 driver #463

merged 40 commits into from
Nov 3, 2022

Conversation

trombik
Copy link
Collaborator

@trombik trombik commented Oct 25, 2022

Requested at #456

Sponserd by beriberikix.

Implemented I2C digital interface.

Requested at UncleRus#456

Sponserd by beriberikix

Implemented I2C digital interface.
@trombik trombik marked this pull request as draft October 25, 2022 06:34
@UncleRus UncleRus added area:ci enhancement New feature or request labels Oct 25, 2022
@UncleRus
Copy link
Owner

Thank you Sakurai-san. Very nice and neat code, as always with you.

while here, fix a bug in the example by initializing the device
descriptor with zeros.
* configure the device, and warn when something looks incorrect
* fix bugs in two_complement_of().
* rewrite the example for more realistic use cases`

while here, replace all the `DSP` and `dsp`.
esp_err_t dps310_is_ready_for(dps310_t *dev, uint8_t reg, uint8_t mask, bool *ready)
{
esp_err_t err = ESP_FAIL;
uint8_t reg_value;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initialize the variable (and do audit in other places).

static const char *TAG = "dps310/helper_i2c";

/* count the number of trailing zero in a value, usually bitmasks. */
static uint8_t count_trailing_zero_bits(uint8_t v)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefix it with _ (and document what it means).

@beriberikix
Copy link

I was able to test the sample on a ESP32-S3 & Adafruit sensor board and had no issues!

trombik added a commit to trombik/esp-idf-lib that referenced this pull request Oct 30, 2022
@trombik trombik force-pushed the feat-driver-dps310 branch from ae852c4 to 6ff7b98 Compare October 30, 2022 11:27
run measurements in the loop, which is what most users expect from an
example.
with `_oversampling_[tp]` and `_rate_[tp]`.

and fix a cirtical bug, in which sampling rate was incorrectly used
(instead of oversampling).
@trombik trombik force-pushed the feat-driver-dps310 branch from 30e3d33 to d7ad7d9 Compare October 30, 2022 12:43
@trombik trombik marked this pull request as ready for review October 30, 2022 12:44
@trombik
Copy link
Collaborator Author

trombik commented Oct 30, 2022

@beriberikix the public APIs are considered as stable now. please have a look at the public header, and the example. I would like to request for your comment, specifically; do the naming conventions in functions, enum, and typedef look clear? I kept original words where appropriate, but modified some (such as dps310_tmp_oversampling_t)

I will continue to work on background mode and interrupt, but the driver is now usable for simple use cases. SPI mode is a bit complicated because incompatible SPI APIs in ESP8266 RTOS SDK and esp-idf. the SPI interface is probably designed for use cases in which performance is critical (such as drones). but command mode is enough for your use case, i guess (and if you really need it, it is possible to implement your own by using public APIs).

@trombik
Copy link
Collaborator Author

trombik commented Nov 3, 2022

@UncleRus I believe that everything you mentioned is fixed. if you are okay with the PR. please merge.

I have already noticed that some functions do not have necessary CHECK_ARG(), but they have been fixed in another branch, which I will submit as PR later.

@UncleRus
Copy link
Owner

UncleRus commented Nov 3, 2022

Thank you!

@UncleRus UncleRus merged commit 5008419 into UncleRus:master Nov 3, 2022
@trombik trombik deleted the feat-driver-dps310 branch November 28, 2022 22:58
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants