-
Notifications
You must be signed in to change notification settings - Fork 18
integration_time property uses raw sensor values and differs to C/Arduino library #17
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
Comments
ya could add |
Is this a duplicate of #24? |
Not sure, they might need retitling, this one could be for a |
Taking a look at this I propose the following,
|
I've been looking closely at this driver this week and in my dev environment I've been experimenting with several things along the lines of @jposada202020 's proposals above. This is a pretty interesting (and occasionally very finicky) sensor with a ton of config options, many of which aren't entirely intuitive. Exposing all of these to users could be useful but it'd cause the library to grow quite a bit in size which isn't so useful. But I've been looking into options for implementing (or improving implementation of) a few critical ones, like the integration time mentioned here for the color/light sensor, that could have a pretty big impact on "normal use case" reliability. Finding sensible defaults (overridable by well-documented kwargs) is also part of that too. It'll probably be a bit before I emerge from the rabbit hole with a PR or two. Its a bit time consuming since I want to make sure to test all three core features (prox, color/als, gesture) independently as well as together. I'm also looking at fully implementing the four internal interrupts, the configurable external interrupt, and proper interrupt clear for all of those. So far I've tested out the proximity interrupts and they're really useful, with the caveat that setting things up is a bit counter-intuitive at first. Should be a fun, educational journey. :D |
I think this has been resolved by #39 |
The C/Arduino library has a method called
setADCIntegrationTime
which takes auint16_t iTimeMS
and converts it into values that the APDS9660 uses, full code can be seen here https://github.com/adafruit/Adafruit_APDS9960/blob/master/Adafruit_APDS9960.cpp#L130-L149 (NB that code does not do a particularly explicit conversion from FP to (8 bit) integer).The CircuitPython library currently just sends the raw value but the property is a bit misleading and called
integration_time
. The library defaults this to 0x01 which translates to(256 - 1) * 2.78ms = 708.9ms
.I'm not sure if this should be corrected to match the Arduino library with its more intuitive and user-friendly values? Changing this value now would disrupt existing users as there's no (clean!) way to code this with the existing property name. Is there a way to code this to happen in CP 5+ libraries only? Is that a good approach?
The text was updated successfully, but these errors were encountered: