adafruit / Adafruit_CircuitPython_IS31FL3731

CircuitPython driver for the IS31FL3731 charlieplex IC
MIT License
22 stars 25 forks source link

Add Pimoroni 11x7 LED Matrix Breakout #37

Closed recantha closed 3 years ago

recantha commented 3 years ago

Hello. I've added support for the Pimoroni 11x7 LED Matrix Breakout. Copied the algorithm for mapping pixels from their repository and added a new class. Hopefully it's of some use to someone! :-)

recantha commented 3 years ago

Not too sure why it fails - I've just added a class and updated the README. Any ideas?

dglaude commented 3 years ago

It is black, that want to change the style of your python code.

I would suggest to save a copy of your code, run black. It will make your translation table ugly, so you want to take your copy and use a pragma to tell black not to touch it.

This is documented here: https://learn.adafruit.com/improve-your-code-with-pylint/black Then you will want to check your code with pylint too.

Maybe your table could be replaced by some math, that would save memory. Let's wait for your code to first pass, and then some review can be done on that topic.

dglaude commented 3 years ago

I believe the right math to replace your table is this:

def pixel_addr(x, y):
    if x <= 5:
        r = x*16 + y
    else:
        r = (x-5)*16 + y - 8
    return r

The multiply by 16 could be replaced by a binary shift for performance if needed.

I don't have the hardware, but I tested my formula by checking the output of a systematic check with this:

for y in range(7):
    for x in range(11):
        print(x, y, pixel_addr(x,y), my_pixel_addr(x,y))

If you could give this a try, then apply black for forcing the python style, and then try pylint for some other automated check. Pylint will tell you what line it does not like, it is possible to fix just with the CI message, but best if you use pylint locally.

recantha commented 3 years ago

Yay. All passed this time. Thanks for the steer and the extra maths (which, obviously, works), @dglaude :-)

makermelissa commented 3 years ago

Closing because I have included these changes as part of #38 which is a major refactor that would have broken this PR.