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

Add BME280 sensor with example #96

Merged
merged 3 commits into from
Apr 6, 2020
Merged

Conversation

ba58smith
Copy link
Collaborator

@ba58smith ba58smith commented Apr 4, 2020

Per a request from someone on Slack. Issue #96 .

@ba58smith ba58smith requested review from joelkoz and mairas April 4, 2020 16:35
@ba58smith ba58smith self-assigned this Apr 4, 2020
@ba58smith ba58smith added the enhancement New feature or request label Apr 4, 2020
examples/bme280_example.cpp Outdated Show resolved Hide resolved
@mairas
Copy link
Collaborator

mairas commented Apr 6, 2020

Looks really good except for the two comments!

@ba58smith ba58smith merged commit 9182ed6 into SignalK:master Apr 6, 2020
@ba58smith ba58smith deleted the BME280 branch April 6, 2020 18:36
@KEGustafsson
Copy link
Contributor

KEGustafsson commented Aug 25, 2020

If BME280 device is not present or broken then SensESP startup fails and device is halted.
␛[0m(I) (check_status) Could not find a valid BME280 sensor. Check wiring, address, and sensor ID. ␛[0m(I) (check_status) SensorID is: 0x255 ␛[0m(I) (check_status) 0xFF: is a BMP180 or BMP085, or a bad address ␛[0m(I) (check_status) 0x56-0x58 is a BMP280 ␛[0m(I) (check_status) 0x60 is a BME280 ␛[0m(I) (check_status) 0x61 is a BME680 ␛[0m
Log reporting stops here.

Is this needed?
https://github.com/SignalK/SensESP/blob/master/src/sensors/bme280.cpp#L27

@mairas
Copy link
Collaborator

mairas commented Aug 26, 2020

Is this needed?
https://github.com/SignalK/SensESP/blob/master/src/sensors/bme280.cpp#L27

I'd say that's harmful. Would be much better to catch the issue internally and just disable reading the values for the sensor and allow the rest of the program to continue.

Recommend opening an issue, though. Discussions in already merged PRs are quite difficult to find for most people. :-)

@KEGustafsson
Copy link
Contributor

KEGustafsson commented Aug 26, 2020

I'll do that and I'll copy your comment as an option to overcome the issue.
#151

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants