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

Fix possible division by zero #45

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Hoiss
Copy link

@Hoiss Hoiss commented Jul 27, 2021

Adafruit_TSL2591::calculateLux function:
It's dangerous in case ch0 is 0!
In case it is 0 use the next greater number 1 for the formula, which should least falsify the calculated result.

It's dangerous in case ch0 is 0. 
In case it is 0 use the next greater number 1, which should least falsify the result.
@DeqingSun
Copy link

I got a lot inf in late night, this fix is necessary.

@caternuson
Copy link
Contributor

caternuson commented Nov 7, 2023

Recreated issue using library example:
https://github.com/adafruit/Adafruit_TSL2591_Library/blob/master/examples/tsl2591/tsl2591.ino
and setting gain and integration time to lowest settings.

In dim light with finger over sensor:
Screenshot from 2023-11-07 08-18-59

@caternuson
Copy link
Contributor

Just to point out, the lux calculation for this sensor is a point of on going discussion:
#14
with the current assessment that it should be taken as a nominal lux value.

Would a better behavior here be just to return 0 for lux if the ch0 value is 0 (or below some low threshold)?

@caternuson
Copy link
Contributor

It also looks like #42 is an attempt to fix this same behavior.

@Hoiss
Copy link
Author

Hoiss commented Nov 7, 2023

Ha, some progress after +2years! Thx @DeqingSun

@caternuson It's....not.....the most frequent discussion in #14 :-D
I think the result is that the current implementation is not good but there is no better one. This is not a good thing.

@caternuson
Copy link
Contributor

OK, we can still improve the current low light behavior. I'd suggest checking the values and returning 0 instead of altering the value so it's not dividing by zero.

@tbattisti
Copy link

No progress here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants