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

All pulled data is zero #2

Open
nemirog1 opened this issue Aug 27, 2019 · 8 comments
Open

All pulled data is zero #2

nemirog1 opened this issue Aug 27, 2019 · 8 comments

Comments

@nemirog1
Copy link

First, thank you for putting the effort into this library! Have an issue where I can see the Part ID, Manufacturer ID, and it seems to set the configuration commands. After that, all data pulled is set to zero. I tried modifying the code to directly see the results of errors, but it is always zero. If you use the Arduino example, you get something like:
10:20:43.950 -> LTR303-ALS example sketch
10:20:43.950 -> Got Sensor Part ID: 0XA0
10:20:43.950 -> Got Sensor Manufacturer ID: 0X5

10:20:43.950 -> data0: 0
10:20:43.950 -> data1: 0
10:20:43.950 -> lux: 0.00
10:20:43.950 -> (good)

So it seems that it thinks the I2C communications are good, but all the data is zero. I thought I might have a bad device, so I tried on a separate sensor and Arduino, same results. I saw that someone else was posting up with this issue on various forums. Do you have any ideas on what to check?

@ericbeaudry
Copy link

@nemirog1 Any luck on making this sketch work?

@nemirog1
Copy link
Author

nemirog1 commented Sep 27, 2019

Yes! The sketch is not written very well, and as a result, not only does the LTR-303 not work, but the sketch spits out inaccurate lux numbers (actually, just zeroes). I received all of the data sheets, and made some adjustments. I built this to operate around a Teensy 3.6, but it should be pretty straightforward to modify for any other Arduino based setup.

Attached is a zip file that includes my updated version of the driver, a sketch that should work to show how the sensor is supposed to work, the original datasheet, and the rare (for whatever reason) Appendix A for the sensor.

LTR-303_test.zip

@ericbeaudry
Copy link

Many thanks. Looking into that right away

@ericbeaudry
Copy link

For anyone trying this code, required changes are (thanks to @nemirog1 )
First, in the setPowerUp you have to send 0x01 instead of 0x03 like that: return(writeByte(LTR303_CONTR, 0x01));
Second, the getLux() function really needs a major tuneup
`
bool getLux(byte gain, unsigned char IntTime, unsigned int CH0, unsigned int CH1, double& lux) {
// Convert raw data to lux
// gain: 0 (1X) or 7 (96X), see getControl()
// integrationTime: integration time in ms, from getMeasurementRate()
// CH0, CH1: results from getData()
// lux will be set to resulting lux calculation
// returns true (1) if calculation was successful
// returns false (0) AND lux = 0.0 IF EITHER SENSOR WAS SATURATED (0XFFFF)

double ratio, ALS_INT;
int ALS_GAIN;

// Determine if either sensor saturated (0xFFFF)
// If so, abandon ship (calculation will not be accurate)
if ((CH0 == 0xFFFF) || (CH1 == 0xFFFF)) {
	lux = 0.0;
	return(false);
}

// We will need the ratio for subsequent calculations
ratio = CH1 / (CH0 + CH1);

// Gain can take any value from 0-7, except 4 & 5
// If gain = 4, invalid
// If gain = 5, invalid
switch (gain) {
case 0:			   // If gain = 0, device is set to 1X gain (default)
	ALS_GAIN = 1;
	break;
case 1:			   // If gain = 1, device is set to 2X gain
	ALS_GAIN = 2;
	break;
case 2:			  // If gain = 2, device is set to 4X gain	 
	ALS_GAIN = 4;
	break;
case 3:			  // If gain = 3, device is set to 8X gain	  
	ALS_GAIN = 8;
	break;
case 6:		     // If gain = 6, device is set to 48X gain
	ALS_GAIN = 48;
	break;
case 7:			  // If gain = 7, device is set to 96X gain  
	ALS_GAIN = 96;
	break;
default:		  // If gain = 0, device is set to 1X gain (default)	 	 
	ALS_GAIN = 1;
	break;
}

switch (IntTime) {
case 0:				 // If integrationTime = 0, integrationTime will be 100ms (default)
	ALS_INT = 1;
	break;
case 1:			 	 // If integrationTime = 1, integrationTime will be 50ms
	ALS_INT = 0.5;
	break;
case 2:				 // If integrationTime = 2, integrationTime will be 200ms
	ALS_INT = 2;
	break;
case 3:				  // If integrationTime = 3, integrationTime will be 400ms
	ALS_INT = 4;
	break;
case 4:				  // If integrationTime = 4, integrationTime will be 150ms
	ALS_INT = 1.5;
	break;
case 5:				  // If integrationTime = 5, integrationTime will be 250ms
	ALS_INT = 2.5;
	break;
case 6:				  // If integrationTime = 6, integrationTime will be 300ms
	ALS_INT = 3;
	break;
case 7:				  // If integrationTime = 7, integrationTime will be 350ms
	ALS_INT = 3.5;
	break;
default:		 	 // If integrationTime = 0, integrationTime will be 100ms (default)
	ALS_INT = 1;
	break;
}

// Determine lux per datasheet equations:
if (ratio < 0.45) {
	lux = ((1.7743 * CH0) + (1.1059 * CH1)) / ALS_GAIN / ALS_INT;
	return(true);
}

if ((ratio < 0.64) && (ratio >= 0.45)) {
	lux = ((4.2785 * CH0) + (1.9548 * CH1)) / ALS_GAIN / ALS_INT;
	return(true);
}

if ((ratio < 0.85) && (ratio >= 0.64)) {
	lux = ((0.5926 * CH0) + (0.1185 * CH1)) / ALS_GAIN / ALS_INT;
	return(true);
}

// if (ratio >= 0.85)
else {
	lux = 0.0;
	return(true);
}

}
`
Once these two changes are in place you start getting proper data out of the sensor.
Again many thanks to @nemirog1 who saved me quite a bit of bug hunting time!

@Steven1408
Copy link

Steven1408 commented Nov 28, 2020

Thanks for all the anseres, this helped me to get mine going.

However it only works if this part is unchanged: (For the easiest way to get this going "out-of-the-box".

boolean LTR303::getLux([...]

not

bool getLux([...]

in the LTR303.cpp.

It would be highly appreaciated if the lib gets fixed.

@Frank-Buss
Copy link

The example also didn't work. Calling "setPowerUp()" sets it to standby mode. I had to comment this line, and then use "setControl(gain, false, true)".

Looks like the library is abandoned. Anyone who wants to fork it and integrate all the fixes in this issue? The library should also save the values of gain and integration time as member variables of the class, and then "getLux" can use this, doesn't make sense to pass it as parameters. The same parameters could be used for setPowerUp. Or just remove setPowerUp, it is kind of redundant because setControl sets already all bits in the control register.

And while we are at it: passing CH0 and CH1 doesn't make sense either in getLux. The function should read the sensor. I don't care about the low-level details as a user of the library, I just want the Lux value. If I care, I can use the low-level getData function.

@Frank-Buss
Copy link

Second, the getLux() function really needs a major tuneup

double ratio, ALS_INT;
int ALS_GAIN;

// Determine if either sensor saturated (0xFFFF)
// If so, abandon ship (calculation will not be accurate)
if ((CH0 == 0xFFFF) || (CH1 == 0xFFFF)) {
	lux = 0.0;
	return(false);
}

// We will need the ratio for subsequent calculations
ratio = CH1 / (CH0 + CH1);

This is not right, because it will do an integer division, so ratio is often 0. One solution would be this:

  ratio = double(CH1) / (double(CH0) + double(CH1));

if ((ratio < 0.64) && (ratio >= 0.45)) {
lux = ((4.2785 * CH0) + (1.9548 * CH1)) / ALS_GAIN / ALS_INT;
return(true);
}

This is wrong. The right formula is this for the range from 0.64 to 0.85:

  if ((ratio < 0.64) && (ratio >= 0.45)) {
    lux = (4.2785 * CH0 - 1.9548 * CH1) / ALS_GAIN / ALS_INT;
    return true;
  }

You probably never noticed it, because your ratio was always wrong as well. Also note that you don't need as many parentheses as you used.

// if (ratio >= 0.85)
else {
lux = 0.0;
return(true);
}

I would probably write it without else, because it is the only remaining case. But might be good to test it anyway, in case there was a division by zero error for the ratio, but which should be tested anyway earlier.

@ericbeaudry
Copy link

Second, the getLux() function really needs a major tuneup

double ratio, ALS_INT;
int ALS_GAIN;

// Determine if either sensor saturated (0xFFFF)
// If so, abandon ship (calculation will not be accurate)
if ((CH0 == 0xFFFF) || (CH1 == 0xFFFF)) {
	lux = 0.0;
	return(false);
}

// We will need the ratio for subsequent calculations
ratio = CH1 / (CH0 + CH1);

This is not right, because it will do an integer division, so ratio is often 0. One solution would be this:

  ratio = double(CH1) / (double(CH0) + double(CH1));

if ((ratio < 0.64) && (ratio >= 0.45)) {
lux = ((4.2785 * CH0) + (1.9548 * CH1)) / ALS_GAIN / ALS_INT;
return(true);
}

This is wrong. The right formula is this for the range from 0.64 to 0.85:

  if ((ratio < 0.64) && (ratio >= 0.45)) {
    lux = (4.2785 * CH0 - 1.9548 * CH1) / ALS_GAIN / ALS_INT;
    return true;
  }

You probably never noticed it, because your ratio was always wrong as well. Also note that you don't need as many parentheses as you used.

// if (ratio >= 0.85)
else {
lux = 0.0;
return(true);
}

I would probably write it without else, because it is the only remaining case. But might be good to test it anyway, in case there was a division by zero error for the ratio, but which should be tested anyway earlier.

I've rolled out my own version of the function for a while now and dealt with the ratio not being a double but the one thing that I completely missed was that minus (-) in the formula. Thanks to your eagle eye, I'll fix that!

As for the parenthesis, you're also right. You can reduce. It's an old habit that I have to make sure I don't rely on someone's else understanding of operator priority when they read or refactor the code especially when mixing operators.

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

No branches or pull requests

4 participants