Closed julien-lebot closed 8 years ago
Thanks for the pull request ... I'll need to properly test it out, but I'll merge if everything looks good.
The code seems to work for me... However, there are a few reasons why I think this pull request isn't ready for prime time yet. Some of them are just nitpicking (sorry :wink: ), some of them are improvement advices:
Sampling_None
-> normally, they should be spelled in capital letters as they are constant: SAMPLING_NONE
setMode()
, ... after calling begin()
". But we all know, the user will try this before reading the docs... My suggestion: Simply add the parameters to the begin()
function.begin()
for every single measurement again (in "normal" mode this is not the case). I really dislike this inconsistency and also it doesn't seem to be the "usual" way, at least not for the Arduino libraries I've seen.begin()
, because otherwise the sensor registers will all be initialised with zeroes and therefore the sensor will skip temp / pressure / humidity measurements and (I think) just return the last readings over and over again...NAN
for one measurement if it is deactivated, at least better / safer than the comment "You can try and read the pressure but it will be garbage !
"I've worked the last few days with your code, troubleshot the flaws mentioned above and I'm going to do a PR with the improved code. However, your work was really helpful, I could reuse lots of your code and even learned some new stuff (e.g. I didn't know it's possible to put a function into a struct (your get functions) - a really nice feature). Thanks! :+1:
--> This is my PR
Yeah no problems, I wasn't sure what the conventions were so I went with what I'm used to. I agree the API was sub-par, I even ran into the issues you mentioned later on myself - I should have spent a bit more time on QA. Just out of curiosity, is there a contribution guide for Adafruit's projects ?
I'll close this PR, thanks for the peer-review.
I've added setters for the missing registers along with some enumeration to set them easily. I've also modified the example project to demonstrate the scenarios found in the datasheet.