4upstream

[bablokb]

add getter/setter for proximity-gain (PGAIN) and gesture-gain (GGAIN).

[ladyada]

hiya! thanks so much for submitting a PR! we can review & merge PRs once they have passed continuous integration (CI). that means that code is 'linted' - that means it is formatted and passes basic structure tests, so that we maintain the same text formatting for all new code if your code isnt passing, check the CI output (click on the red X next to the PR to scroll through the log and find where the error is

here is a tutorial on pylint and black: https://learn.adafruit.com/improve-your-code-with-pylint

and overall how to contribute PRs to circuitpython: https://learn.adafruit.com/contribute-to-circuitpython-with-git-and-github

once you get that green checkmark that indicates CI has passed, please comment reply to this post so we know its time for another review (we may not get notified on CI pass and miss that its time to look!)

[bablokb]

Hm, the log does not tell me why the check failed.

[ladyada]

you need to run black, that part failed

black....................................................................Failed

[bablokb]

Hi, checks from CI are now successful

[ladyada]

@caternuson wanna check it?

[caternuson]

Looks good and generally works:

Adafruit CircuitPython 7.1.1 on 2022-01-14; Adafruit ItsyBitsy M4 Express with samd51g19
>>> import board
>>> from adafruit_apds9960.apds9960 import APDS9960
>>> apds = APDS9960(board.I2C())
>>> apds.proximity_gain
0
>>> apds.proximity_gain = 3
>>> apds.proximity_gain
3
>>> apds.gesture_gain
2
>>> apds.gesture_gain = 0
>>> apds.gesture_gain
0
>>> 

@bablokb What do you think about adding a simple dictionary (or similar) to map the gain values, like 8x to the register values, like 3 and use that behind the scenes. That way users could think in terms of the actual gain values, for example:

# set gain to 8x 
apds.proximity_gain = 8

It looks like both prox gain and gesture gain have the same general mapping. So could use one for both.

And throw a ValueError exception for any invalid value.

# this should throw an exception
apds.proximity_gain = 100
# and this, since the is no 3x gain
apds.proximity_gain = 3

[bablokb]

With a dictionary PGAIN and GGAIN would have a different logic than AGAIN (color_gain), unless you want to break existing code. What about adding constants PGAIN_1X and so on? We could add these constants also for color-gain without breaking things. But new code would be more readable.

Checking values and throwing ValueErrors is currently not consistent within the library. I will be happy to add that, but there is a footprint tradeoff. So this is more of a policy decision.

[caternuson]

Thanks. Good point about color_gain (AGAIN). I think being consistent is better at this point. My suggestion can be deferred to some later PR. You're also right that it'd be breaking. So would require some consideration.

@ladyada lgtm, including the _APDS9960_CONTROL change you flagged.

[ladyada]

thanks :)