bme280-python icon indicating copy to clipboard operation
bme280-python copied to clipboard

Double call of update_sensor() when getting altitude

Open JimMadge opened this issue 4 years ago • 2 comments

Hi,

I noticed that update_sensor() appears to be called twice in a row when calling get_altitude. First self.update_sensor() is called followed by self.get_pressure(),

https://github.com/pimoroni/bme280-python/blob/97c80ced20b02caa129e98c8801b31f1468ee4b0/library/bme280/init.py#L275-L279

However, the first line of get_pressure is also self.update_sensor(),

https://github.com/pimoroni/bme280-python/blob/97c80ced20b02caa129e98c8801b31f1468ee4b0/library/bme280/init.py#L267-L269

So, if we unrolled this we would get something like

self.update_sensor()
self.update_sensor()
pressure = self.pressure

I'm happy to open a PR changing this if it isn't intended.

JimMadge avatar Mar 13 '21 17:03 JimMadge

It's a side-effect of the fact it makes sense for all the readings to be processed at once, coupled with the fact I prefer methods over attributes for getting values... and you kind of expect a method to do something and fetch the data.

There are ways this could be mitigated, but it's one of those cases where running update_sensor() twice probably hurts less than contriving some method to prevent it.

With the benefit of hindsight, I'd probably rewrite the whole library API :grimacing: but since we can't really do that, I'm open to suggestions!

Gadgetoid avatar Mar 15 '21 16:03 Gadgetoid

It's a side-effect of the fact it makes sense for all the readings to be processed at once

On that point, it did occur to me that adding a get_readings method which returns a tuple (temperature, pressure, humidity) would be a nice addition for those who want to get all of the values, particularly in forced mode. I could do that if you'd like.

coupled with the fact I prefer methods over attributes for getting values...

This is precisely the kind of thing that makes me spend a day agonising over how I should do something rather than actually doing it 😆 . Python's flexibility can be a blessing and a curse.

I think it would be safe to remove self.update_sensor() from get_altitude as it will always be called anyway on the first line of get_pressure (and get_pressure requires that line to function correctly).

JimMadge avatar Mar 15 '21 18:03 JimMadge