Re: IIO BME680 driver

Re: IIO BME680 driver

[Himanshu Jha]

+Cc linux-iio & David Frey

Hi Tuukka,

On Fri, Dec 14, 2018 at 09:49:06AM +0200, Tuukka Pasanen wrote:
> Hello,
> 
> I've been testing BME680 i2c IIO driver and it works as expected (actually
> I'm very happy with it). Before that I was using BME680 lib from Pimoroni
> and only thing I'm missing is that there is user defined prefix to fix
> incorrect values that are hard coded in ROM (Can be found here):
> 
> https://github.com/pimoroni/bme680-python/blob/e827e5d622fd70df2aee6a68ebac626cf539ee55/library/bme680/__init__.py#L89

Hmm. Why would anyone want to play with those calibration constants
programmed into the NVM of the sensor.

And I wonder how the following came up ?
https://github.com/pimoroni/bme680-python/blob/e827e5d622fd70df2aee6a68ebac626cf539ee55/library/bme680/__init__.py#L331

These algorithms are provided by Bosch and probably should be used as-is
unless you got some information from them. But I don't see any such
reference in the datasheet nor in their API.

Or it might be some heuristics. Not sure about it!

> This one adjust temperature every time one reads it. For example my room
> temperature is something like 21C (I live in north) and BME680 reports 28C
> so humidity and pressure are incorrect (not much but notable).

Now since you're using IIO driver, I see the problem here:
https://github.com/torvalds/linux/blob/master/drivers/iio/chemical/bme680.h#L52

	#define BME680_AMB_TEMP				25

Yes, it is hardcoded here and you should change it to according to your
room temperature(20-21 degC).


I really like the approach of *not* harcoding the ambient temperature
and instead using the temperature sensor readout as a substitute to the
ambient temp as evident in the python api above.

But the problem lies in the organisation of IIO driver of isolating
different channels. What I simply mean is, what if I only run
gas part without any temperature sensing part ? How would you get the
ambient temperature in the case ? In such a case, we need throw a dummy
temp read in the relevant readout functions and store them in the
private struct. But then following this approach is wastage of power
and increases latency in the readings.

David had earlier pointed out if we are already wasting resources by
doing the same thing each time for each of the readings which can be
accomplished in a single readout i.e., triggering forced mode does a
single TPHG cycle and after we can read all the data. But currently we
set it each time for each of temp, press, humid and gas reading.

And only advantage I see with the current implementation is you can read
a single channel value if you want without disturbing other channels
which you can't in that python implementation.

There are two things that I shall be soon working on as soon as my
current work with Zephyr ends:

1. add profile duration function
2. check for new available data before fetching
 ... and more if anything comes up.

Anyway, Bosch ported BME680 to The Zephyr Project and I reviewed+tested
the driver on nRF52840 development board. Works great!
Though the maintainers didn't merge the PR due to lack of
those compensation algorithms in the datasheet *chuckle*

At least Jonathan(IIO maintainer) merged with a note in the commit log ;)

Bosch claimed that new datasheet shall be soon be available with all the
missing info, and then anyone wouldn't need to reverse engineer their API
or BSEC to get the desired info :P


Thank you for using the IIO driver and I'm glad that there are users out
there testing it. Please let me know if there are any more issues :)

-- 
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology

Re: IIO BME680 driver

[Tuukka Pasanen]

Hello,

Hmm. Why would anyone want to play with those calibration constants
> programmed into the NVM of the sensor.
>
> And I wonder how the following came up ?
> https://github.com/pimoroni/bme680-python/blob/e827e5d622fd70df2aee6a68ebac626cf539ee55/library/bme680/__init__.py#L331
>
> These algorithms are provided by Bosch and probably should be used as-is
> unless you got some information from them. But I don't see any such
> reference in the datasheet nor in their API.
I think noone wants to tough those defaults. BME680 as said work as 
expected. Idea behind that line is when it's not getting correct value 
for temperature one adjust that little bit. It's ugly and it's not what 
datasheet or API tolds us todo but it's practical.
> Or it might be some heuristics. Not sure about it!
>
>> This one adjust temperature every time one reads it. For example my room
>> temperature is something like 21C (I live in north) and BME680 reports 28C
>> so humidity and pressure are incorrect (not much but notable).
> Now since you're using IIO driver, I see the problem here:
> https://github.com/torvalds/linux/blob/master/drivers/iio/chemical/bme680.h#L52
>
> 	#define BME680_AMB_TEMP				25
>
> Yes, it is hardcoded here and you should change it to according to your
> room temperature(20-21 degC).
Could this one turn into module parameter as it would be more 
convenient? As it's hard breaking in year 2019 to recompile kernel to 
make it fit to your room temperature. I can compile for testing but as I 
can imagine most of the earth population won't do it.
> There are two things that I shall be soon working on as soon as my
> current work with Zephyr ends:
>
> 1. add profile duration function
> 2. check for new available data before fetching
>   ... and more if anything comes up.

One thing that should be addressed is that if you only read one 
measurement and go to sleep then your VOC is incorrect as it needs 5 
mins or something to warm up. Of course I can read very rapidly to make 
that work and I assume that thread in mailing list with continous stream 
is all about that.

> Bosch claimed that new datasheet shall be soon be available with all the
> missing info, and then anyone wouldn't need to reverse engineer their API
> or BSEC to get the desired info :P
Hopefully this comes sooner than later as it would make things more easier
> Thank you for using the IIO driver and I'm glad that there are users out
> there testing it. Please let me know if there are any more issues :)

No problem.. I'll try to make it work as that Python library but I have 
to over come these few problems.

Sincerely,

Tuukka

Re: IIO BME680 driver

[Himanshu Jha]

On Sun, Dec 16, 2018 at 04:39:40PM +0200, Tuukka Pasanen wrote:
> Hello,
> 
> Hmm. Why would anyone want to play with those calibration constants
> > programmed into the NVM of the sensor.
> > 
> > And I wonder how the following came up ?
> > https://github.com/pimoroni/bme680-python/blob/e827e5d622fd70df2aee6a68ebac626cf539ee55/library/bme680/__init__.py#L331
> > 
> > These algorithms are provided by Bosch and probably should be used as-is
> > unless you got some information from them. But I don't see any such
> > reference in the datasheet nor in their API.
> I think noone wants to tough those defaults. BME680 as said work as
> expected. Idea behind that line is when it's not getting correct value for
> temperature one adjust that little bit. It's ugly and it's not what
> datasheet or API tolds us todo but it's practical.

OK.
I will give it a try sometime but I that would need a separate
temperature sensor to compare and identify any false readings.

> > Or it might be some heuristics. Not sure about it!
> > 
> > > This one adjust temperature every time one reads it. For example my room
> > > temperature is something like 21C (I live in north) and BME680 reports 28C
> > > so humidity and pressure are incorrect (not much but notable).
> > Now since you're using IIO driver, I see the problem here:
> > https://github.com/torvalds/linux/blob/master/drivers/iio/chemical/bme680.h#L52
> > 
> > 	#define BME680_AMB_TEMP				25
> > 
> > Yes, it is hardcoded here and you should change it to according to your
> > room temperature(20-21 degC).
> Could this one turn into module parameter as it would be more convenient? As
> it's hard breaking in year 2019 to recompile kernel to make it fit to your
> room temperature. I can compile for testing but as I can imagine most of the
> earth population won't do it.

Indeed!

I will add an attribute which user can write just like we set the
oversampling ratios.

Or I may try the taking the initial temperaure as the ambient temperature.

> > There are two things that I shall be soon working on as soon as my
> > current work with Zephyr ends:
> > 
> > 1. add profile duration function
> > 2. check for new available data before fetching
> >   ... and more if anything comes up.
> 
> One thing that should be addressed is that if you only read one measurement
> and go to sleep then your VOC is incorrect as it needs 5 mins or something
> to warm up. Of course I can read very rapidly to make that work and I assume
> that thread in mailing list with continous stream is all about that.

5 mins ? Are you sure ?

Datasheet:
---------

"The heating duration is specified by writing to the corresponding 
gas_wait_x<7:0> control register. Heating durations between 1 ms 
and 4032 ms can be configured. In practice, approximately 20–30 ms 
are necessary for the heater to reach the intended target temperature."

For ensuring that the heater sink is heated to the target temperature, I
added the following in the driver, and if it fails to do that we just
abort:

<snip>

729         /*
730          * occurs if either the gas heating duration was insuffient
731          * to reach the target heater temperature or the target
732          * heater temperature was too high for the heater sink to
733          * reach.
734          */
735         if ((check & BME680_GAS_STAB_BIT) == 0) {
736                 dev_err(dev, "heater failed to reach the target temperature\n");
737                 return -EINVAL;
738         }

</snip>

And now I wonder that -EINVAL isn't the best way to represent such a
failure. -ERETRY or something more appropriate should have been used.

That thread was about adding power management support + triggered buffer
support.

Device is already power managed as it automatically goes to sleep mode
soon after a single TPHG cycle is performed.

OTOH, I'm not brave and smart enough to add triggered buffer support
even after the long string of email threads.

> > Bosch claimed that new datasheet shall be soon be available with all the
> > missing info, and then anyone wouldn't need to reverse engineer their API
> > or BSEC to get the desired info :P
> Hopefully this comes sooner than later as it would make things more easier
> > Thank you for using the IIO driver and I'm glad that there are users out
> > there testing it. Please let me know if there are any more issues :)
> 
> No problem.. I'll try to make it work as that Python library but I have to
> over come these few problems.

Sorry! I will try to recticy those and others that I have in mind, but
might not quick about it.


-- 
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology

Re: IIO BME680 driver

[Tuukka Pasanen]

Hello,


>> One thing that should be addressed is that if you only read one measurement
>> and go to sleep then your VOC is incorrect as it needs 5 mins or something
>> to warm up. Of course I can read very rapidly to make that work and I assume
>> that thread in mailing list with continous stream is all about that.
> 5 mins ? Are you sure ?
>
> Datasheet:
> ---------
>
> "The heating duration is specified by writing to the corresponding
> gas_wait_x<7:0> control register. Heating durations between 1 ms
> and 4032 ms can be configured. In practice, approximately 20–30 ms
> are necessary for the heater to reach the intended target temperature."
>
> For ensuring that the heater sink is heated to the target temperature, I
> added the following in the driver, and if it fails to do that we just
> abort:
>
> <snip>
>
> 729         /*
> 730          * occurs if either the gas heating duration was insuffient
> 731          * to reach the target heater temperature or the target
> 732          * heater temperature was too high for the heater sink to
> 733          * reach.
> 734          */
> 735         if ((check & BME680_GAS_STAB_BIT) == 0) {
> 736                 dev_err(dev, "heater failed to reach the target temperature\n");
> 737                 return -EINVAL;
> 738         }
>
> </snip>
>
> And now I wonder that -EINVAL isn't the best way to represent such a
> failure. -ERETRY or something more appropriate should have been used.
>
> That thread was about adding power management support + triggered buffer
> support.
>
> Device is already power managed as it automatically goes to sleep mode
> soon after a single TPHG cycle is performed.
>
> OTOH, I'm not brave and smart enough to add triggered buffer support
> even after the long string of email threads.
I don't know about data-sheet but when you rapidly read BME680 then 
resistance in VOC reader tends to rise something like 3-5 mins and then 
you can get 'correct' reading (rising stops in somepoint). To have 
something in kernel under public eye is brave enough :). But I have to 
investigate data-sheet better and see if I am doing something very very 
stupid and heater just heats up.
>>> Bosch claimed that new datasheet shall be soon be available with all the
>>> missing info, and then anyone wouldn't need to reverse engineer their API
>>> or BSEC to get the desired info :P
>> Hopefully this comes sooner than later as it would make things more easier
>>> Thank you for using the IIO driver and I'm glad that there are users out
>>> there testing it. Please let me know if there are any more issues :)
>> No problem.. I'll try to make it work as that Python library but I have to
>> over come these few problems.
> Sorry! I will try to recticy those and others that I have in mind, but
> might not quick about it.

No worries, take you time as I have 'working' solution and I can test 
new features as they appear as I have personal interest about this.

Sincerely,

Tuukka