Add property style access to thresholds

[caternuson]

Fix for #12.

Kept it pretty simple. You either pass in a tuple or get a tuple of (touch, release) threshold values. Can be done for all channels:

>>> mpr.thresholds = (24, 8)
>>> mpr.thresholds
((24, 8), (24, 8), (24, 8), (24, 8), (24, 8), (24, 8), (24, 8), (24, 8), (24, 8), (24, 8), (24, 8), (24, 8))
>>> 

or per channel:

>>> mpr[3].thresholds = (12, 6)
>>> mpr[3].thresholds
(12, 6)
>>> 

[brennen]

I'm not sure if there's a broader convention that this is in line with, so I might be off base here, but something about this leaves me pretty uncomfortable:

>>> mpr.thresholds = (24, 8)
>>> mpr.thresholds
((24, 8), (24, 8), (24, 8), (24, 8), (24, 8), (24, 8), (24, 8), (24, 8), (24, 8), (24, 8), (24, 8), (24, 8))
>>> 

I guess my naive expectation as a user of the library would be that mpr.thresholds would return the value - or at least the same type of value - that I assign to it. I think I'd rather just see set_thresholds() used for this operation?

[caternuson]

I agree. That's why we code review. I struggled with this a bit myself and decided to just throw the current approach out and see what people thought. Ideally it would be symmetric. I thought it would be convenient to have a quick way to set all channels, since that's probably the most frequent use case. So that's where mpr.thresholds = (24, 8) comes from. But since there's no guarantee that all channels are the same, the query part can't really return a single tuple.

But I don't think we want a set_thresholds() method, not pythonic, etc.

Here are some ideas, if anyone has others, please respond.

Option 1 Require a 12 tuple and use it to set each channel. You could no longer do this:

>>> mpr.thresholds = (24, 8)

but could create a 12 tuple on the fly:

>>> mpr.thresholds = ((24, 8), ) * 12

Option 2 Check the length of the supplied tuple. If 1, then use that value for all channels. So you can still do this:

>>> mpr.thresholds = (24, 8)

But if it is 12, then use each one per channel.

>>> mpr.thresholds = ((24, 8), (52, 6), (45, 7), (22, 3), (41, 9), (33, 1), (71, 21), (24, 8), (27, 8), (21, 5), (44, 6), (51, 9))

I doubt that syntax would ever get used, but it would at least support symmetry.

[brennen]

But I don't think we want a set_thresholds() method, not pythonic, etc.

I mean, it's already defined, and it seems to communicate intent to the reader pretty clearly, but my sense of the Python aesthetic is extremely not developed at this point.

Option 1 above seems to me in some sense the most "correct" thing, albeit verbose and a little unfriendly. Option 2 continues to seem sort of... fuzzily DWIM in a way that subverts my expectations of the syntax.

With that, I have probably exhausted my useful input on this one. :)

[tannewt]

Why not move it to the channel object? Then you can:

for channel in mpr:
  mpr.threshold = 24

That way you match the TouchIn API.

You can add a separate release_threshold property for the second bit.

[caternuson]

I like it. So basically do everything through the already existing per channel access and just use iterator syntax to set them all. And get rid of the tuple in favor of two separate properties.

[caternuson]

For now, I've left in the pluralized thresholds that allows tuple access.

Adafruit CircuitPython 3.1.1 on 2018-11-02; Adafruit ItsyBitsy M4 Express with samd51g19
>>> import mpr_test as test
>>> mpr = test.mpr121
>>> for channel in mpr:
...     channel.threshold = 20
...     channel.release_threshold = 10
... 
>>> mpr[0].thresholds
(20, 10)
>>> for channel in mpr:
...     channel.thresholds = (12, 4)
... 
>>> mpr[0].thresholds
(12, 4)
>>> mpr[0].threshold
12
>>> mpr[0].release_threshold
4
>>> 

[tannewt]

I'd drop thresholds myself because returning tuples requires a memory allocation, leaves the values unnamed and increases the memory footprint of the library. If you insist then you can leave it.

[caternuson]

Nope. Just needed a reason.