Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Align code in this library to Adafruit code for ESP32, ESP8266, etc. #23

Open
dachshund-digital opened this issue Apr 28, 2019 · 5 comments

Comments

@dachshund-digital
Copy link

dachshund-digital commented Apr 28, 2019

The current Adafruit code for ESP32, ESP8266 BME280 libraries/APIs support correct calculation for sea level pressure by accepting an elevation value and pressure value. And that calculated pressure at sea level is then used for altitude result.

This library only supports a crude static value for pressure at sea level. This is functionally incorrect, and should be address. For example ESP32 and ESP8266 library by Adafruit correctly reports sea level pressure at an elevation of 41 meters for my location. But this library reports the altitude for my location as 68 meters.

@siddacious
Copy link

@dachshund-digital I understand that you may be frustrated but none the less please phrase your request in a manner that is less accusatory and more constructive.

@dachshund-digital
Copy link
Author

dachshund-digital commented Apr 28, 2019

To your point about being constructive, valid point. Thus, below is the formula for proper calculation of sea level pressure, for a given elevation, temperature, and pressure at said elevation location. This should save some time and effort when the code is revised appropriately to address the issue at hand.

[https://keisan.casio.com/exec/system/1224575267]

@ladyada
Copy link
Member

ladyada commented Apr 28, 2019

@dachshund-digital thanks, please submit a PR and we'll check it out!

@adafruit adafruit locked and limited conversation to collaborators Apr 28, 2019
@barbudor
Copy link
Contributor

barbudor commented Jun 3, 2019

While I was in the others BME280 issues, I started to have a look at this one.

What I understand so far is that there are mainly 2 main formulas to link pressure and altitude: The "full" version is the one given in the above link and the "Barometric" which works fine for altitude below 11,000 meters. The formula currently used in the library is the "Barometric" formula with an additional approximation using a static temperature at 15°C (seems to be a common approximation)
I trust 11,000m is enough for most of the applications of Adafruit products, with the exception of stratospheric balloon. This limitation could probably be written somewhere.
2 points seems to remains from my view:

  1. Should the formula be changed to offer the ability to take temperature into account ? It seems temperature has a measurable impact on the 44330 constant used in the formula (44300 = (15°C + 273.15)/0.0065 where 15°C is converted in Kelvin and 0.0065 is a constant). What I understand is the temperature should be the outdoor temperature, which is not necessarily the temperature measured by the BME280 if set indoor. So probably not a good idea to directly use BME's temperature.
    https://en.wikipedia.org/wiki/Barometric_formula

  2. As of today the library is using a fixed value for sea-level pressure (101,325 Pa). As we know, this may vary a lot between days. I feel the major improvement to the lib should be to offer a way for the user to provide current-day calibration by proving a reference altitude and pressure. Most altimeters, either in avionics or even used in sports like mountaineering offer a way to set a reference point.
    The Arduino library have such a function :
    float Adafruit_BME280::seaLevelForAltitude(float altitude, float atmospheric)
    I believe this would be worth a dedicated issue.

Happy to help is you want.
If anyone with knowledge in thermodynamics and physics could jump in, this would be appreciated...
let me know.

@ladyada
Copy link
Member

ladyada commented Jun 17, 2019

hiya, for Q2, you have to set sea_level_pressure that's a property - the default is just the default. so if you want to make an example for that, we'd love it.
for Q1, we use the calculations from the BME datasheet - which takes temp into account, the self-heating isn't too intense, maybe a few degrees

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants