GeorgetownMakerHubOrg / air-quality

An Open Source Platform For Georgetown's STIA Air Quality Courses
https://www.coursicle.com/georgetown/courses/STIA/436/
Other
4 stars 0 forks source link

Report redundant sensors individually #15

Closed theFestest closed 5 years ago

theFestest commented 5 years ago

From the readme:

We still need to either combine the results of redundant sensors or report both

This is currently in progress and being worked on (#6), I'm creating this to document the issue and be able to accumulate all of our issue tracking in one location. I'm removing these as notes in the readme.

My opinion is that it is better to report the data from each sensor even if there is redundancy.

fpgirard commented 5 years ago

Can one of you append the I2C address to the returned key value of the BME280 and BME680 sensors (tph.py and tphg.py)?    Also, we should include the BME280 library in the /lib since we are shipping the 905 GUAQ with the 280 and 680.  Colton, this is my oversight.

Pas

On July 12, 2019 at 10:48:31 AM, Michael Bartholic (notifications@github.com) wrote:

From the readme:

We still need to either combine the results of redundant sensors or report both

This is currently in progress and being worked on (#6), I'm creating this to document the issue and be able to accumulate all of our issue tracking in one location. I'm removing these as notes in the readme.

My opinion is that it is better to report the data from each sensor even if there is redundancy.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

cmpadden commented 5 years ago

See pull request #6: I've refactored the tph and tphg libraries to support multiple sensors, appending the I2C address to the result keys, and included the BME280 library.

All that's left is some final validation.

cmpadden commented 5 years ago

@pascal, @michaelbartholic, I've tested the refactored "redundant sensor" code -- here are some example readings for the bme680 and bme280. I'm going to go ahead and merge the pull-request into the development branch. I'm not really sure we truly need the development branch since the master branch is being tagged for releases, but I'll go with it for now.

{'humidity-bme280_0x76': 42.28809, 'temperature-bme280_0x76': 70.016, 'pressure-bme280_0x76': 65.1344}
{'pressure-bme680_0x77': 1008.55, 'voc': 30507.71, 'humidity-bme680_0x77': 55.218, 'temperature-bme680_0x77': 25.5}
.
.
.
{'humidity-bme280_0x76': 48.38379, 'temperature-bme280_0x76': 77.28799, 'pressure-bme280_0x76': 100.6496}
{'pressure-bme680_0x77': 1008.6, 'voc': 35888.26, 'humidity-bme680_0x77': 55.39, 'temperature-bme680_0x77': 25.56}
.
.
.
{'humidity-bme280_0x76': 48.75391, 'temperature-bme280_0x76': 77.414, 'pressure-bme280_0x76': 100.6607}
{'pressure-bme680_0x77': 1008.59, 'voc': 41095.51, 'humidity-bme680_0x77': 55.664, 'temperature-bme680_0x77': 25.65}
theFestest commented 5 years ago

Cool, those examples look great. And yeah, I suppose you're right about not really needing the development branch. I'd envisioned only merging development into master after sufficient physical testing with a full GUAQ setup, but with tagging, this same behavior can be accomplished by only tagging once the testing is done. Good call. My only thought then would be maybe requiring a subset of the core team to approve PRs before going into master (or development) to make sure the core team maintains a high level of awareness of what's actually going in there in the context of the whole project. Who all can approve/merge PRs right now and how do we think this subset of contributors will grow as we bring in more of a team?

cmpadden commented 5 years ago

@michaelbartholic , I do not have access to see which users have the permissions to merge pull requests, but I agree that there should be a core team of individuals who are required to review a pull request before it is merged into the master branch.