adafruit / Adafruit_CircuitPython_BME280

CircuitPython driver for the BME280
MIT License
63 stars 42 forks source link

Add class attributes to allow more control over the sensor #22

Closed jraber closed 5 years ago

jraber commented 5 years ago

Add attributes for IIR filter, standby period, operation mode, and overscan for temperature, pressure and humidity.

Add Enum classes for the overscan, standby, and iir filter values

I started working on this after reading issue #21 . Admittedly, I haven't tested this (I don't have a BME280), so there may be some silly errors here. If someone is willing to test, I'd bet happy to fix any bugs.

Almost all of these changes could be applied to the BMP280 library with very little effort.

jraber commented 5 years ago

Closing this pull request, as I found a mistake in the overscan_temperature property. I'll submit another PR later (after I figure out why Travis failed)

makermelissa commented 5 years ago

Hi, you can just leave the PR open and commit new code to your repo instead of needing to create a new one each time.

jraber commented 5 years ago

Oh! Thanks @makermelissa I'm still getting a fail from Travis CI, I guess because of the pylint message. Should I throw a "disable=too-many-instance-attributes" at it?

Any suggestions are welcome.

makermelissa commented 5 years ago

Yeah, that should be fine to add.

makermelissa commented 5 years ago

Hi, I connected a BME280 to a Feather M4 Express running the latest version of CircuitPython. However, it is not finding the enum class that you are using and thus doesn't run. I'm curious what your test setup was.

jraber commented 5 years ago

My (limited) testing was with python 3.7.2, but enums has been part of the python standard library since 3.4. I don't have a BME280, so I haven't actually tested with the device. I am very interested to know your results.

I added an examples/bme280_normal_mode.py to show how I expected for it to work. Could you give that a look?

jraber commented 5 years ago

So, I'm just realizing the CircuitPython != Python.

If CircuitPython doesn't have enum, I can re-implement those enums using the const type from micropython like is used for the other constants in this library.

makermelissa commented 5 years ago

Hi @jraber. Yeah, it doesn't appear CircuitPython currently supports Enums. Also, thanks for updating the example to show off your changes. I'm excited to test your changes.

jraber commented 5 years ago

Hi @makermelissa . I removed the references to enums. Please let me know if you see any other issues.

makermelissa commented 5 years ago

Hi, I went ahead and tested it and the normal_mode example works, but the numbers appear to be off.

Current Code Results

Temperature: 24.5 C
Humidity: 28.6 %
Pressure: 1008.9 hPa
Altitude = 36.16 meters

Changed Code Results

Temperature: 188.8 C
Humidity: 100.0 %
Pressure: 300.0 hPa
Altitude = 9165.37 meters

Also, simpletest doesn't appear to be working anymore. I took a closer look at the code and it appears to now default to sleep mode. Can we change it so that it basically starts the same as before by default, but maybe have the option to put it in sleep mode? I'm sure this library is in use by someone and I don't want them to update and have their code be suddenly broken. Thanks for all your work on this.

jraber commented 5 years ago

Hi @makermelissa . Thanks for testing! I'm not sure why you're getting odd values from the normal_mode example. I pushed a couple of changes today. If you have a moment, could you test with those changes?

I adapted these changes to the BMP280 library and was able to successfully run both of the examples on a Raspberry PI with the BMP280 connected via I2C.

About the simple example, what did it do when you ran it? Did it not return any values? Crash?

I was trying to keep the default functionality the same as the current version.

You're right that my code defaults to SLEEP mode, but that's actually the default mode for the sensor anyway. When _read_temperature is called, it will change the mode to FORCE (only if the mode isn't set to NORMAL). After each forced measurement is completed, the sensor sets itself back to sleep mode automatically.

makermelissa commented 5 years ago

Hi @jraber, I'm not sure what's going on with the code. I tried it with your latest update and I get the following when running bme280_normal_mode.py:

Traceback (most recent call last):
  File "code.py", line 14, in <module>
  File "/lib/adafruit_bme280.py", line 453, in __init__
  File "/lib/adafruit_bme280.py", line 120, in __init__
  File "/lib/adafruit_bme280.py", line 432, in _read_byte
  File "/lib/adafruit_bme280.py", line 461, in _read_register
  File "/lib/adafruit_bme280.py", line 457, in _read_register
  File "/lib/adafruit_bus_device/i2c_device.py", line 115, in write
OSError: [Errno 19] Unsupported operation

If it helps, I've been testing this on the Adafruit nRF52840 Express.

jraber commented 5 years ago

@makermelissa , I don't think any of my changes would cause that issue. Especially not the changes that I pushed recently. Could you check the wiring to the sensor? Could you try with an old version of the library, to be sure that the sensors is connected and working correctly?

jerryneedell commented 5 years ago

FYI - I just tried running the normal mode example on a Raspberry Pi and it executes normally

makermelissa commented 5 years ago

Thanks @jraber. I'll give it another try when I have a moment, unless you want to try it in CircuitPython @jerryneedell.

jerryneedell commented 5 years ago

Sorry, I can't test it easily since my bme280 is in a "deployed" raspberry pi. It was easy to test it there, butI don't have one I can test with CP right now.

makermelissa commented 5 years ago

I should be able to give this PR some more attention tonight.

makermelissa commented 5 years ago

Numbers are still way off for me:

Temperature: 188.8 C
Humidity: 100.0 %
Pressure: 300.0 hPa
Altitude = 9165.37 meters

vs

Temperature: 21.0 C
Humidity: 33.7 %
Pressure: 1006.2 hPa
Altitude = 59.02 meters

I'm going to dig in deeper to see if I can narrow down as to why they're off by so much.

makermelissa commented 5 years ago

This one is a head-scratcher. There's nothing obvious about the changes that should make it so the readings are off by so much. I'm thinking it may have to do with some register setting, as that is the major thing that changed. I'm going to continue messing around with this though.

makermelissa commented 5 years ago

Ok, I figured out where it was getting stuck on the simpletest at least. It seems to not be getting the status on line 143. I think this is part of some bigger issue and I'm continuing to trace it down. I suspect the numbers are off because in normal mode, it isn't waiting to let the conversion complete since lines 143 and 144 are now being skipped, however that may only apply to forced mode.

makermelissa commented 5 years ago

I'm getting a status of 255 no matter what. I also forgot to mention I've been using SPI. I'm going to try this with I2C to see if that makes a difference.

jraber commented 5 years ago

I suspect the numbers are off because in normal mode, it isn't waiting to let the conversion complete since lines 143 and 144 are now being skipped, however that may only apply to forced mode.

My understanding of normal mode is that you don't have to wait for the conversion to finish; there will always be data to read, either the new measurement (if the conversion is complete) or the previous measurement (if the conversion is still running).

I think the mode setter may have been causing an issue with the simpletest. Basically it would only allow the sensor to go into FORCE mode once. The latest commit may fix that.

makermelissa commented 5 years ago

Ok, trying it in I2C mode did make a difference. It works fine in I2C mode, but the numbers are way off with SPI.

makermelissa commented 5 years ago

FYI, this library is really finicky in SPI mode (even before this PR) and was much more pleasant to test with I2C.

jraber commented 5 years ago

I just (yesterday!) copied the SPI interface from this library to fill a gap in the BME680 library. Go figure, the BME680 gives crazy readings too.

makermelissa commented 5 years ago

Yeah, at least we've narrowed the problem down.

makermelissa commented 5 years ago

I wonder if it's related to the SPI mode. That would explain why I got 255 for status instead of 0 and why the numbers are so far off. The datasheet says it supports SPI mode 0 and 3.

makermelissa commented 5 years ago

Nope, didn't make a difference. It worked the same in either mode.

jraber commented 5 years ago

The issue may be that I am enabling SPI 3-wire mode. Bit 0 (spi3w_en) in register 0xF5 (config). I don't think that was set before. Let's turn that off...

makermelissa commented 5 years ago

Bingo! That was it.

makermelissa commented 5 years ago

I'm gonna try pulling your code again (free of my edits) and test. If it passes, I'll approve.

makermelissa commented 5 years ago

Ok, the normal mode example is fine (in fact, all finickyness is gone), but simpletest isn't running in SPI mode. I get: RuntimeError: Failed to find BME280! Chip ID 0xff Normally resetting the chip/board would take care of that. Let me see if there's just something that needs to be added/changed. It's so close.

makermelissa commented 5 years ago

My bad. I forgot I moved the CS wire. It's working great.

jraber commented 5 years ago

Sweet! Thank you @makermelissa for all of your debugging this!

makermelissa commented 5 years ago

You're welcome. Thank you for doing this and all of your patience.