claws / BH1750

An Arduino library for the digital light sensor breakout boards containing the BH1750FVI IC
MIT License
249 stars 108 forks source link

URGENT: Bug fixing #27

Closed coelner closed 6 years ago

coelner commented 6 years ago

fix: relocate switch/case to ensure return value and predefine ack with 5 instead of false fix: relocate I2C mode send to ensure continuous mode

claws commented 6 years ago

Can you provide a some more description of the problems you encountered? I can't quite tell what the problems are from the brief one-liners in the original description.

claws commented 6 years ago

As you listed this urgent I have pushed the changes that I think are good to go from a separate pull request. This approach allows means we can have those changes without waiting for you to address my review comments.

Can you rebase this to the latest?

coelner commented 6 years ago

the problem with the wrong switch/case block: You get a true as return value because the false was translated to 0. Therefore we need a value out of range. And it was capsulated by the first switch/case block, so we had the problem never to reach a return statement. I did not use the continuous mode, therefore I never detect that issue. But currently I tested your example and get always the value 0. When I selected the one time mode, I get updated results. This leads also to #28, because the first returned value in the example is 0, after a delay of 120ms, the values are updated.

claws commented 6 years ago

Thanks for pointing out the errors in the library causing it not to function in some cases.

I spent a bit of time testing it. Apart from initialising level to NAN I believe that #31 captures the suggested changes in this pull request. The examples all appear to work properly now including the first measurement. Sensor readings appear to be realistic.

If you get around to testing the latest master please let me know if you uncover any further issues.