Add iteration support for touchpads #113
Conversation
…nd deinitialize as needed
Can be added in a different PR
|
Sounds good, I can look at removing anything not immediately needed as well. If any of the above functionalities aren't needed, let me know an I can try and factor them out. Also, if there's a good way to test this library as frozen. |
|
@tekktrik There absolutely is a way to test it! That's the next step before trying to slim down the code. Do you know how to build CircuitPython? Testing a frozen library is pretty simple once you have a CircuitPython build environment set up. |
|
No, I don't actually, I think I've seen guides though, is there a Learn guide for it? |
|
Yep! It's here. Please message me on Discord if you need assistance. Once you have the build environment working, I'll show you how to test the frozen library. |
|
So it builds, and it currently doesn't look like I have too much in the way of "unused" code. So I think it really depends on what functionalities are worth introducing and just cutting the rest:
Let me know what to cutout! |
|
Also, it may make sense to hide this class in the documentation, let me know if I should exclude it in |
|
I started to do a review, but will start at the higher level. The main motivation for this was two things:
I cannot think of a practical use case for this . If a pin is being used as a touchpad, you would not in general want to connect anything else up to it. Electrically, that would interfere with its use as as touchpad. In a single program, you there is not a common need to deinit and reinit a touchpad. So I'd say just leave this out. We still need create
I don't think it is a good idea to have multiple ways to name a pin. We already have a good way, which is the So I suggest:
I think that will cover the existing issue. You can use the existing @property
def touch_pins(self):
return [touch for touch in self._touches if isinstance(touch, touchio.TouchIn)]
@property
def touched(self):
return [touche for touch in self.touch_pins if touch.value] |
| if value in (7, "A7", "TX"): | ||
| return "TX" | ||
| if isinstance(value, int): | ||
| if value not in range(1, 7): |
There was a problem hiding this comment.
This code is no longer needed given my general comment, but note that range(1, 7) will allocate storage. Also, the upper limit is exclusive and is not included; it will only go from 1 to 6.
| if value not in range(1, 7): | |
| if not (1 <= value <= 7): |
There was a problem hiding this comment.
Oh neat, I didn't think about that, thanks!
|
Thanks for reviewing! Let me take a stab at this, but I may end up just creating a new branch for this if it makes more sense to start from scratch :) |
|
@tekktrik Starting a new PR is perfectly fine. Simply make sure you reference this PR in the new PR notes so there's some continuity there. You can leave this open until you create the new one, if you like. |
|
Closing in favor of #114 |
Resolves #103 as follows:
touchedproperty for showing list of of pin names currently registering a touchtouchpadsproperty got showing list of pin names that are set up as touchpad inputstouchio.TouchInas neededThis is all powered by the new
IterableInputclass, which should remain hidden from the user but provides the above functionalities.