Re: [PATCH] RFT: iio: gp2ap002: Replace LUT with math

[Jonathan Bakker <xc-racer2@live.ca>]

Just an FYI - the ADC_MIN should probably be 0 for full darkness,
but I understand the concept and like it :)

I believe the light sensor part to be a Sharp GA1A light detector or similar,
based on the fact that DigiKey had a page (1) that
mentions both together and that the specs for the GA1A1S202WP (2) line up quite well with
those of the GP2AP002.  Note that the datasheet for the GA1A1S202WP even mentions the
illuminance = 10 ^ (current / 10) formula, re-arranged as
current = 10 * log(illuminance), although it specifies uA as opposed to mA which is the same
as the Android libsensors (both the Nexus S (crespo) (3) and Galaxy Nexus (tuna) (4)) versions.
I suspect that this should be adjusted after the call to iio_read_channel_processed().

1. https://web.archive.org/web/20130303022051/http://www.digikey.com/catalog/en/partgroup/ga1a-and-gp2a/26402
2. https://web.archive.org/web/20121221163708/http://www.sharpsma.com/download/GA1A1S202WP-Specpdf
3. https://android.googlesource.com/device/samsung/crespo/+/refs/heads/jb-release/libsensors/LightSensor.cpp
4. https://android.googlesource.com/device/samsung/tuna/+/refs/tags/android-4.3_r3/libsensors/LightSensor.cpp

Thanks,
Jonathan

On 2020-02-10 11:33 a.m., Gregor Riepl wrote:
>>> Also: It looks like int_pow doesn't saturate, so even though it uses 64bit
>>> integer math, it might be better to move the range check before the calculation.
>>
>> How do you mean I should be doing that without actually
>> doing the power calculation? (Maybe a dumb question but
>> math was never my best subject.)
> 
> Well, if you clamp the input value to a valid range, there is no risk of
> under- or overflow:
> 
> #define GP2AP002_ADC_MIN 5
> #define GP2AP002_ADC_MAX 47
> /* ensure lux stays in a valid range
>    lux > 10^(5/10)
>    lux < 10^(47/10)
>  */
> clamp(res, GP2AP002_ADC_MIN, GP2AP002_ADC_MAX);
> lux = int_pow(10, (res/10));
> 
> However, there is another problem with this solution:
> If you divide the input value by 10 before raising it to the power of 10, you
> lose a lot of precision. Keep in mind that you're doing integer math here.
> The input range is very limited, so reducing it further will also reduce the
> number of lux steps: int((47-5)/10) = 4, so you will end up with only 4
> luminance steps.
> 
> Instead of messing with the precision, I propose simplifying the original code
> to a simple table lookup.
> This will reduce constant memory usage to 42 values * 16 bit = 84 bytes and
> computational complexity to one single memory reference.
> While I'm sure there is a more optimal solution, I think it's the easiest to
> understand with the least impact on accuracy and performance:
> 
> #define GP2AP002_ADC_MIN 5
> #define GP2AP002_ADC_MAX 47
> 
> /*
>  * This array maps current and lux.
>  *
>  * Ambient light sensing range is 3 to 55000 lux.
>  *
>  * This mapping is based on the following formula.
>  * illuminance = 10 ^ (current[mA] / 10)
>  */
> static const u16 gp2ap002_illuminance_table[] = {
> 	3, 4, 5, 6, 8, 10, 12, 16, 20, 25, 32, 40, 50, 63, 79, 100, 126, 158,
> 	200, 251, 316, 398, 501, 631, 794, 1000, 1259, 1585, 1995, 2512, 3162,
> 	3981, 5012, 6310, 7943, 10000, 12589, 15849, 19953, 25119, 31623,
> 	39811, 50119,
> };
> 
> static int gp2ap002_get_lux(struct gp2ap002 *gp2ap002)
> {
> 	const struct gp2ap002_illuminance *ill1;
> 	const struct gp2ap002_illuminance *ill2;
> 	int ret, res;
> 	u16 lux;
> 
> 	ret = iio_read_channel_processed(gp2ap002->alsout, &res);
> 	if (ret < 0)
> 		return ret;
> 
> 	dev_dbg(gp2ap002->dev, "read %d mA from ADC\n", res);
> 
> 	/* ensure we're staying inside the boundaries of the lookup table */
> 	clamp(res, GP2AP002_ADC_MIN, GP2AP002_ADC_MAX);
> 	lux = gp2ap002_illuminance_table[res - GP2AP002_ADC_MIN];
> 
> 	return (int)lux;
> }
>