Mini Color TFT with Joystick FeatherWing

[makermelissa]

Here's my take on a Mini TFT with Joystick FeatherWing Library. This one is complete with simpletest and documentation changes.

[deshipu]

this const() does nothing

[deshipu]

this could be a class attribute, not an object attribute

[deshipu]

this could be a class attribute, not an object attribute

[deshipu]

I wonder if there could be some caching here, so that if we want to check 5 different buttons, we don't make 5 separate calls to the seesaw.

[deshipu]

What if there was just a buttons property, that would return a named tuple of True/False values for all the buttons at once? Then you could do:

buttons = wing.buttons
if buttons.right:
    ...
elif buttons.left:
    ...

[deshipu]

Is there any reason for this to be a property and not a straightforward attribute?

[deshipu]

What if there was just a buttons property, that would return a named tuple of True/False values for all the buttons at once? Then you could do:

buttons = wing.buttons
if buttons.right:
    ...
elif buttons.left:
    ...

[hexthat]

with this how do you adjust the brightness?

[makermelissa]

Good point @hexthat. I’ll add that.

[makermelissa]

Thanks @deshipu for reviewing this. I’ll make some changes tonight.

[deshipu]

I'm playing with PyBadge now, and I came up with this code for the buttons for it:

import board
import digitalio
import bitbangio
import time
import collections


Buttons = collections.namedtuple('Buttons',
                                 'left up down right select start a b')


class PyBadgeButtons:
    def __init__(self):
        self._bus = bitbangio.SPI(clock=board.BUTTON_CLOCK,
                                  MISO=board.BUTTON_OUT)
        while not self._bus.try_lock():
            pass
        self._bus.configure(polarity=1, phase=1)
        self._latch = digitalio.DigitalInOut(board.BUTTON_LATCH)
        self._latch.switch_to_output(value=0)
        self._buffer = bytearray(1)

    @property
    def pressed(self):
        self._latch.value = 1
        self._bus.readinto(self._buffer)
        self._latch.value = 0
        pressed = self._buffer[0]
        return Buttons(*[bool(pressed & (1 << button)) for button in range(8)])


buttons = PyBadgeButtons()
while True:
    print(buttons.pressed)
    time.sleep(1)

I think we could use something very similar here.

[makermelissa]

Ok, backlight added, unnecessary const() removed, display is an attribute, and buttons returned as named tuple so they are only read once per loop instead of 7 times.

[deshipu]

Looks good, some nit-picks.

[deshipu]

You can you put this on the class, like this:

class MiniTFTFeatherWing:
    _button_mask = ((1 << BUTTON_RIGHT) |
                 (1 << BUTTON_DOWN) |
                 (1 << BUTTON_LEFT) |
                 (1 << BUTTON_UP) |
                 (1 << BUTTON_SEL) |
                 (1 << BUTTON_A) |
                 (1 << BUTTON_B))

    def __init_(self, ...

[deshipu]

Could you do this as well? I think making it a property is wasteful.

[deshipu]

Can you move this outside the class, where the constants are, so that users have access to it? Also, since it's a type, it should start with upper case letter.

[deshipu]

You can write this as:

return Buttons(*[not button_values & (1 << button) for button in
                 (BUTTON_UP, BUTTON_DOWN, BUTTON_LEFT, BUTTON_RIGHT, BUTTON_A, BUTTON_B,BUTTON_SEL)])

[makermelissa]

Ok, requested changes implemented.

[deshipu]

Thank you, it looks great!