adafruit / Adafruit_AS7341

Other
29 stars 18 forks source link

Error checking, setSMUXLowChannels() rearranging, read all channels while looping, fix of setBank() #3

Closed stephenf7072 closed 3 years ago

stephenf7072 commented 4 years ago

• Moved the initiating of a reading and waiting for the results out of setSMUXLowChannels(), so that function can be used for a non-waiting read (next pull request) • Add a potential escape from while loops if there is a sensor error and data never becomes available (a bit of a cop-out using a default value of 0/forever, but might be useful in other stuff to be added). A better solution would be to use the expected integration time, but this would need storage of ASTEP and ATIME values, or perhaps doing a read. • Query as to whether the 500ms delay in flicker detection is needed to be that long

stephenf7072 commented 4 years ago

Mmm I'm not sure why the check fails - it compiles and works ok for me.

stephenf7072 commented 4 years ago

I've make some additional and more interesting changes. I'd sort of intended to do them as a separate pull request, but perhaps this is ok? I'm still a bit of a Github rookie, open to critique on better technique, etc. Extra features include: • Added functions to do other things in loop() while waiting for a reading (useful in higher integrations times – max is 47sec, so wait time will be twice this) • The setBank(true) statements in enableLED() and setLEDCurrent() left the wrong bank selected, which led to strange results in future readings as nothing corrects this. These functions are currently the only ones using setBank, so I made them revert to setBank(false). However using the correct bank is something which needs care, and the setGPIOValue function probably lacks some calls to setBank(). I have started issue #5 about this as it may need a discussion on how (or if) to implement.

siddacious commented 4 years ago

Thank you @stephenf7072 ! Your contributions are always welcome. Let's keep this PR scoped to the current changes and handle any others in another PR. That way one set of changes won't have to wait on the other to be finished before it can be released.

I will review and test this ASAP, hopefully later today however an initial skim looks good. You can fix the failing build by formatting the code with clang-format and pushing the updates.

I've added some info on using clang-format here: https://github.com/adafruit/Adafruit_AS7341/blob/master/README.md#formatting-and-clang-format

stephenf7072 commented 4 years ago

Damn, still failing after I ran clang.

stephenf7072 commented 4 years ago

Hey guys, checking in on this one.

siddacious commented 4 years ago

@stephenf7072 I made a PR to your fork with a fix for the errors that are blocking this. If you merge it, this one should pass

stephenf7072 commented 4 years ago

Nope, I merged your pull request into my fork, and the master is still failing checks :(

siddacious commented 3 years ago

@stephenf7072 ok, thanks for nothing clang-format :|

I'll go ahead and merge and then push a fix

Thanks for your patience and your contribution!

stephenf7072 commented 3 years ago

Cool, no worries.