adafruit / Adafruit_BME280_Library

Arduino Library for BME280 sensors
Other
328 stars 301 forks source link

Bmp280 support #61

Closed dirkmueller closed 4 years ago

dirkmueller commented 4 years ago

BMP280 is register downwards-compatible with a BME280, so we could allow using it against BMP280 as well and avoid duplicate code for projects wanting to support either BMP or BME280.

siddacious commented 4 years ago

ooh, great work @dirkmueller. We'll take a look at this as it may allow us to retire the BMP280 lib.

siddacious commented 4 years ago

Unfortunately while I think many people would get use out of adding this functionality, it will likely create confusion for people that have a BMP280 and expect to be able to call readHumidity and will think something is broken when they don't get a humidity measurement back.

If you can come up with a solution to address that issue in a user friendly way, we can take another look but for now we'll have to go with the "if it's not broken, don't fix it" approach.

Your work is a great idea, just not a great fit for an adafruit supported library. You can always keep your fork and make it available to people who might find it useful.

dirkmueller commented 4 years ago

@siddacious what is a user friendly way? I can certainly point this out in the method documentation that this only works in the case of a BME class sensor.

There is also the possibility to log a message. I do handle this case by returning NAN.

A third way would be that I change the class to inherit from a BMP280 base class. This way we can share the code but the user would have two classes to instanciate.

Let me know what your preferred option is.

ladyada commented 4 years ago

we would prefer having one super/subclass the other so there are distinct classes, than return a NaN which will be confusing to people :)