Add support for BME680 SPI Interface #38854

leonardp · 2021-09-27T06:35:46Z

This Pull request adds support for the BME680 SPI Interface.
It is mostly analogous to the BME280.

Signed-off-by: Leonard Pollak [email protected]

MaureenHelm · 2021-10-28T15:00:12Z

drivers/sensor/bme680/bme680.c

Should this be a condition of the device instance? i.e., would this work if you had a SPI instance and an I2C instance in your application?

Good question. It probably won't work..
I could add a new member to the bme680_config (e.g. bus_type) and then check for that.
Do you think that would make sense?

Why not fold this into the individual bus_io->read routines as needed instead?

This is roughly what I ended up doing.
I made a separate function that does the mem page switching and I am also calling it from the bus_io->write routine.

teburd

As mentioned the compile ifdef for setting the mem page when using SPI seems an issue to me as well, everything else looks great

github-actions · 2022-01-10T00:29:32Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

leonardp · 2022-01-10T08:36:58Z

Sorry for shelving this for so long without notice.
Thanks for the reviews so far.
I will (most probably) be able to get this done by the end of next week.

leonardp · 2022-01-21T18:39:28Z

I've updated the PR and tested it with I2C and SPI instance enabled simultaneously.

leonardp · 2022-02-20T16:14:22Z

I split the pull request into 4 commits as suggested by @mbolivar-nordic.
I hope everything is in order now.
Thanks for all the reviews so far!

mbolivar-nordic

Thank you, that was indeed easier to review

mbolivar-nordic · 2022-02-21T18:46:17Z

drivers/sensor/bme680/bme680.c

Thanks for explaining. In that case I think it would be a little better to write it something like this:

static inline bool bme680_is_on_spi(struct device *dev) { const struct bme680_config *config = dev->config; return config->bus_io == &bme680_bus_io_spi; } /* ... */ #if BME680_BUS_SPI if (bme680_is_on_spi(dev)) { err = ...; if (err < 0) { return err; } } #endif

IMO it is a bit better to have the I2C devices initialize the same way always, regardless of whether a SPI device happens to be in the same system. It may ease debugging in some cases and it definitely enhances consistency.

But it's not a blocking request; just please consider it.

Thanks for taking another look at it and giving some more insight on that matter.

To have it consistent across all possible configurations wouldn't it make sense to
remove the #if BME680_BUS_SPI directive aswell?

I have prepared the change as suggested by you (without the #if). If that makes sense to you I can push them.

That's a fair point -- if it's safe to do regardless of bus type, might as well be consistent on all of them. Given the ifdef, I was assuming there was a reason to prefer not to do it on I2C, but if there isn't, then I'm fine either way.

Well, it was probably me just over-complicating things -.-;
I pushed the changes, this way it is definitely easier on the eyes.

...disregard that.. the directives are needed or else it won't build for I2C, obviously..
Sorry for the noise.

leonardp · 2022-02-23T11:09:01Z

@teburd would you take another look?

drivers/sensor/bme680/bme680.h

github-actions · 2022-04-25T00:29:48Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

MaureenHelm · 2022-04-25T15:54:22Z

I think this PR got lost in the shuffle while the merge window was closed for the 3.0 release.

@leonardp will you please rebase and resolve the merge conflict?

Out of date

This fixes the constant for the mem page and replaces a magic number with the already defined `BME680_LEN_COEFF2` constant. Signed-off-by: Leonard Pollak <[email protected]>

Consolidate the initialization routines and change the include guard to conform with the coding guidelines as a preparation for the following commits which add support for the SPI interface. Signed-off-by: Leonard Pollak <[email protected]>

This enables the SPI interface for the BME680 sensor driver. Signed-off-by: Leonard Pollak <[email protected]>

Add BME680 (SPI) sensor driver to build_all tests. Signed-off-by: Leonard Pollak <[email protected]>

carlescufi · 2022-04-28T11:25:36Z

@MaureenHelm @mbolivar-nordic I rebased this, please re-ack.

Senso4sLab · 2024-03-20T13:26:54Z

I tested bme680 sample with SPI and I got the following error bme680: Bad BME680 chip id: 0x0. I used the following prj.conf settings

&spi0 {
	compatible = "nordic,nrf-spi";	
	status = "okay";
	pinctrl-0 = <&spi0_default>;
	pinctrl-1 = <&spi0_sleep>;
	pinctrl-names = "default", "sleep";
    cs-gpios = <&gpio0 13 GPIO_ACTIVE_LOW>,
               <&gpio1 6 GPIO_ACTIVE_LOW>;              
    
    bme6800:bme680_0@0 {
		compatible = "bosch,bme680";
        status = "okay";
		reg = <0>;
        label="bme680_1";
		spi-max-frequency = <1000000>;        
	};
    bme6801:bme680_1@1 {
		compatible = "bosch,bme680";
        status = "okay";
		reg = <1>;
        label="bme680_2";
		spi-max-frequency = <1000000>; 
	};
     
};

I did some additional tests and I found out that if both bme 680 chips were connected to the board none of them was detected. If only one bme 680 chip is connected to the board, the address is figured out successfully. Based on this observation I assumed that something was wrong with the implementation details of the bme 680 library.

I will appreciate for help!

leonardp requested review from MaureenHelm and nashif as code owners September 27, 2021 06:35

github-actions bot added area: Devicetree area: Devicetree Binding PR modifies or adds a Device Tree binding area: Tests Issues related to a particular existing or missing test labels Sep 27, 2021

leonardp force-pushed the bme680_spi branch from 2968cf7 to 14b344d Compare September 27, 2021 06:44

leonardp changed the title ~~Add support for BME680s SPI Interface~~ Add support for BME680 SPI Interface Sep 27, 2021

zephyrbot added area: Sensors Sensors area: Build System labels Sep 27, 2021

zephyrbot requested review from avisconti, galak, mbolivar-nordic and tejlmand September 27, 2021 15:44

zephyrbot assigned MaureenHelm Sep 27, 2021

leonardp force-pushed the bme680_spi branch from 14b344d to 4d5750d Compare September 29, 2021 14:24

MaureenHelm reviewed Oct 28, 2021

View reviewed changes

MaureenHelm requested a review from teburd October 28, 2021 15:07

teburd previously requested changes Oct 28, 2021

View reviewed changes

github-actions bot added the Stale label Jan 10, 2022

github-actions bot removed the Stale label Jan 11, 2022

leonardp force-pushed the bme680_spi branch 3 times, most recently from 17783e0 to 8de7551 Compare January 21, 2022 17:52

leonardp requested review from MaureenHelm and teburd January 21, 2022 18:39

leonardp force-pushed the bme680_spi branch 2 times, most recently from 5eedfb4 to a12865e Compare January 22, 2022 07:47

leonardp requested a review from mbolivar-nordic February 20, 2022 16:14

leonardp force-pushed the bme680_spi branch from 158d8cb to 1ff2cab Compare February 20, 2022 16:18

mbolivar-nordic previously approved these changes Feb 21, 2022

View reviewed changes

leonardp dismissed mbolivar-nordic’s stale review via 7779014 February 23, 2022 07:12

leonardp force-pushed the bme680_spi branch 2 times, most recently from 7779014 to a4606a4 Compare February 23, 2022 08:31

mbolivar-nordic previously approved these changes Feb 23, 2022

View reviewed changes

mbolivar-nordic added this to the v3.1.0 milestone Feb 23, 2022

MaureenHelm previously approved these changes Feb 23, 2022

View reviewed changes

drivers/sensor/bme680/bme680.h Outdated Show resolved Hide resolved

github-actions bot added the Stale label Apr 25, 2022

github-actions bot removed the Stale label Apr 26, 2022

Leonard Pollak added 4 commits April 28, 2022 13:02

drivers: sensor: bme680: fix constants

e9d7d5c

This fixes the constant for the mem page and replaces a magic number with the already defined `BME680_LEN_COEFF2` constant. Signed-off-by: Leonard Pollak <[email protected]>

drivers: sensor: bme680: prep work

90b3eeb

Consolidate the initialization routines and change the include guard to conform with the coding guidelines as a preparation for the following commits which add support for the SPI interface. Signed-off-by: Leonard Pollak <[email protected]>

drivers: sensor: bme680: Add SPI interface

568dd96

This enables the SPI interface for the BME680 sensor driver. Signed-off-by: Leonard Pollak <[email protected]>

tests: drivers: Add bme680-spi to build_all

9548c78

Add BME680 (SPI) sensor driver to build_all tests. Signed-off-by: Leonard Pollak <[email protected]>

carlescufi dismissed stale reviews from MaureenHelm and mbolivar-nordic via 9548c78 April 28, 2022 11:25

carlescufi force-pushed the bme680_spi branch from a4606a4 to 9548c78 Compare April 28, 2022 11:25

carlescufi requested review from MaureenHelm and mbolivar-nordic April 28, 2022 11:25

MaureenHelm approved these changes Apr 28, 2022

View reviewed changes

carlescufi approved these changes Apr 28, 2022

View reviewed changes

carlescufi merged commit f7171e2 into zephyrproject-rtos:main Apr 28, 2022

Add support for BME680 SPI Interface #38854

Add support for BME680 SPI Interface #38854

Uh oh!

Conversation

leonardp commented Sep 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teburd left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 10, 2022

Uh oh!

leonardp commented Jan 10, 2022

Uh oh!

leonardp commented Jan 21, 2022

Uh oh!

leonardp commented Feb 20, 2022

Uh oh!

mbolivar-nordic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leonardp commented Feb 23, 2022

Uh oh!

Uh oh!

github-actions bot commented Apr 25, 2022

Uh oh!

MaureenHelm commented Apr 25, 2022

Uh oh!

carlescufi commented Apr 28, 2022

Uh oh!

Senso4sLab commented Mar 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

leonardp commented Sep 27, 2021 •

edited

Loading

Senso4sLab commented Mar 20, 2024 •

edited

Loading