adafruit / Adafruit_CircuitPython_BMP280

CircuitPython driver for the BMP280
MIT License
36 stars 25 forks source link

Add Calculated Sea Level Pressure based on Altitude #38

Closed somenice closed 1 year ago

somenice commented 1 year ago

Hello there, this PR adds a function to calculate sea level pressure from altitude. The formula is copied from the BMP085.py library and allows you to select an altitude and calculates sea level pressure.

Example: bmp280.sea_level_pressure = bmp280.p0

The benefit is the BMP280 can be set with a known altitude rather than sea level pressure and altitude can be held while pressure changes.

jposada202020 commented 1 year ago

Hello :) @somenice. Thank you for the PR when you mentioned the BMP085 I guess you are referring to the arduino library right?

https://github.com/adafruit/Adafruit-BMP085-Library/blob/28e55478d55ece9c692fde5c9ac2d5d10a4b0364/Adafruit_BMP085.cpp#LL245C29

Regarding the CI failure:

  1. you are using the property with an argument. Normally, the property decorator returns a value and the setter will set the value (in this particular case: p0). See for example

https://github.com/adafruit/Adafruit_CircuitPython_BMP280/blob/9c9721c21f6bff5d8e97d7ea24c9a0605f47263c/adafruit_bmp280.py#L275-288

  1. You could use pre-commit to reformat the files, to make black and pylint checking happy, more on that here https://learn.adafruit.com/creating-and-sharing-a-circuitpython-library/check-your-code

  2. It would be nice that the property could use a descriptive name instead of p0. Thanks Again :)

caternuson commented 1 year ago

I'd suggest adding this via an altitude setter, instead of returning p0, ex:

@altitude.setter
def altitude(self, value: float) -> None:
    #
    # back compute SLP and set it as `self.sea_level_pressure`
    #

Then it'd be used like this:

bmp280.altitude = 1234 # current known altitude in meters

adding this for general ref: https://learn.adafruit.com/clue-altimeter/dealing-with-changes#calibrating-using-current-altitude-3067203

somenice commented 1 year ago

Hello :) @somenice. Thank you for the PR when you mentioned the BMP085 I guess you are referring to the arduino library right?

https://github.com/adafruit/Adafruit-BMP085-Library/blob/28e55478d55ece9c692fde5c9ac2d5d10a4b0364/Adafruit_BMP085.cpp#LL245C29

Regarding the CI failure:

  1. you are using the property with an argument. Normally, the property decorator returns a value and the setter will set the value (in this particular case: p0). See for example

https://github.com/adafruit/Adafruit_CircuitPython_BMP280/blob/9c9721c21f6bff5d8e97d7ea24c9a0605f47263c/adafruit_bmp280.py#L275-288

  1. You could use pre-commit to reformat the files, to make black and pylint checking happy, more on that here https://learn.adafruit.com/creating-and-sharing-a-circuitpython-library/check-your-code
  2. It would be nice that the property could use a descriptive name instead of p0. Thanks Again :)

Thanks for the help @jposada202020 The BMP_085.py library was from way back by Tony D https://github.com/adafruit/Adafruit_Python_BMP/blob/master/Adafruit_BMP/BMP085.py

I've implemented the suggestions from @caternuson, tested on Adafruit CircuitPython 8.0.5 on 2023-03-31; Adafruit Feather ESP32S2 with ESP32S2 and re-saved the pr. (Thanks @caternuson for the tips and the learning guide, exactly the reason I was trying to do this.) Please let me know if there is anything else I should do to help contribute. Cheers, Andrew

somenice commented 1 year ago

Looks like I'm running into issues again, will look at pre-commit

caternuson commented 1 year ago

Cool, yah, that's what I was thinking.

The CI fail just looks like black formatting, which is something you generally just run and let it do what it wants: https://learn.adafruit.com/creating-and-sharing-a-circuitpython-library/black