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

BME680 module initial commit #2151

Merged
merged 4 commits into from
Nov 19, 2017
Merged

BME680 module initial commit #2151

merged 4 commits into from
Nov 19, 2017

Conversation

vsky279
Copy link
Contributor

@vsky279 vsky279 commented Oct 28, 2017

Fixes #2114.

Make sure all boxes are checked (add x inside the brackets) when you submit your contribution, remove this sentence before doing so.

  • This PR is for the dev branch rather than for master.
  • This PR is compliant with the other contributing guidelines as well (if not, please describe why).
  • I have thoroughly tested my contribution.
  • The code changes are reflected in the documentation at docs/en/*.

This is a simple driver of the Bosch's BME680 sensor. In addition to the BME280 it is able to measure the indoor air quality. Unfortunately the IAQ (Indoor Air Quality) Index calculation algorithm is not publicly available. So we need to live with the gas resistance value which should still be some proxy to the IAQ.

Definitely it would be good if someone can further test the module before it is merged.

@vsky279 vsky279 mentioned this pull request Oct 28, 2017
| :----- | :-------------------- | :---------- | :------ |
| 2017-10-28 | [vsky279](https://github.com/vsky279) | [vsky279](https://github.com/vsky279) | [bme680.c](../../../app/modules/bme680.c)|

Copyright (C) 2017 - 2018 Bosch Sensortec GmbH
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this copyright for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using parts of the bme680.c driver from Bosch - it says we should retain the copyright. I think maybe just one line should be enough.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw that, yes. However, I thought leaving the copyright in the source code would be enough. It feels somewhat out of place here and makes it look like Bosch contributed the NodeMCU module (and wrote this documentation).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems reasonable. I will make a block in the code with these functions and will put the (c) note there.

@marcelstoer
Copy link
Member

it would be good if someone can further test the module

I'm afraid that due to lack of hardware this will probably not happen. However, a code review can still be conducted.

@pjsg
Copy link
Member

pjsg commented Nov 2, 2017

I see that this uses doubles to perform calculations... Does this work in an integer build?

@vsky279
Copy link
Contributor Author

vsky279 commented Nov 2, 2017

Yes, it does. This is even tested in the BME280 module.

@marcelstoer marcelstoer added this to the 2.1 follow-up II milestone Nov 19, 2017
@marcelstoer marcelstoer merged commit 87a6a9b into nodemcu:dev Nov 19, 2017
crasu pushed a commit to crasu/nodemcu-firmware that referenced this pull request Jan 11, 2018
dnc40085 pushed a commit to dnc40085/nodemcu-firmware that referenced this pull request Mar 3, 2018
@vsky279 vsky279 deleted the BME680 branch August 10, 2018 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants