Split library into package and adapt examples+doc

dglaude commented 4 years ago

This is a response to #27 .

I have split the best I could the various driver, updated all the example and tested the one I could (mostly Scroll pHAT HD and LedShim).

The sub-driver for those two being done by me, I adapted the copyright now that it is in a separate file, for the other it is a copy of the original comment and copyright.

The move to init.py and cleaning done there is visible in the git history, but copy of the file to make sub-driver mean loosing the git history on those. Maybe there is a way to inherit this history???

I tested the other MCU example on the Scoll pHAT HD, but not on Blinka.

Exemples and README should be up to date.

There is one thing I would like to discuss, it's the fact that one piece of hardware is priviledged, and the other inherit from that one and modify the x, y and formula to write pixels. So Matrix is imported differently than all the other. I would prefer to have a super class that has no implementation and make each sub-driver specify things.

dglaude commented 4 years ago

Please advice if I should use a syntax like the following to simplify the example so that only the import vary:

# uncomment next line if you are using Feather CharlieWing LED 15 x 7
from adafruit_is31fl3731.CharlieWing import CharlieWing as MatrixDisplay

# uncomment next line if you are using Adafruit 16x9 Charlieplexed PWM LED Matrix
# from adafruit_is31fl3731 import Matrix as MatrixDisplay
# uncomment next line if you are using Adafruit 16x8 Charlieplexed Bonnet
# from adafruit_is31fl3731.CharlieBonnet import CharlieBonnet as MatrixDisplay
# uncomment next line if you are using Pimoroni Scroll Phat HD LED 17 x 7
# from adafruit_is31fl3731.ScrollPhatHD import ScrollPhatHD as MatrixDisplay

i2c = busio.I2C(board.SCL, board.SDA)

display = MatrixDisplay(i2c)

(Assuming this does works... it is a bit untested)

tannewt commented 4 years ago

Thank you for this change! Definitely an improvement.

The move to init.py and cleaning done there is visible in the git history, but copy of the file to make sub-driver mean loosing the git history on those. Maybe there is a way to inherit this history???

I don't think there is a good way to have history of multiple files track back to one. It'll be clear enough when tracked back to the commit that moved everything.

There is one thing I would like to discuss, it's the fact that one piece of hardware is priviledged, and the other inherit from that one and modify the x, y and formula to write pixels. So Matrix is imported differently than all the other. I would prefer to have a super class that has no implementation and make each sub-driver specify things.

I totally agree. This shouldn't be too hard. You can rename Matrix to IS31FL3731 and then create a new Matrix class with the specifics of the matrix.

Please advice if I should use a syntax like the following to simplify the example so that only the import vary:

# uncomment next line if you are using Feather CharlieWing LED 15 x 7
from adafruit_is31fl3731.CharlieWing import CharlieWing as MatrixDisplay

# uncomment next line if you are using Adafruit 16x9 Charlieplexed PWM LED Matrix
# from adafruit_is31fl3731 import Matrix as MatrixDisplay
# uncomment next line if you are using Adafruit 16x8 Charlieplexed Bonnet
# from adafruit_is31fl3731.CharlieBonnet import CharlieBonnet as MatrixDisplay
# uncomment next line if you are using Pimoroni Scroll Phat HD LED 17 x 7
# from adafruit_is31fl3731.ScrollPhatHD import ScrollPhatHD as MatrixDisplay

i2c = busio.I2C(board.SCL, board.SDA)

display = MatrixDisplay(i2c)

(Assuming this does works... it is a bit untested)

I would do as Display so that it isn't confused with the actual Matrix class. Also, the filenames should be lower case so it'd be: from adafruit_is31fl3731.charlie_wing import CharlieWing as Display.

dglaude commented 4 years ago

I think I have implemented what was discussed by @tannewt:

lower case file
rename Matrix in IS31FL3731
make a new Matrix library
import with from adafruit_is31fl3731.charlie_wing import CharlieWing as Display

All the example should be adapted (and that last point actually limit the change to only the import statement.

Finally, I have not put protection against somebody from importing the abstract parent class. I do not really master OO in Python.

This is becoming rather complex a PR and I can only test with Scroll pHAT HD and LedShim... so I would welcome someone to test that independantly.

dglaude commented 4 years ago

Ok, I have cleaned the documentation part.

With Sphynx locally installed I could see the effect of my change and update the example and the presentation of those.

FoamyGuy commented 4 years ago

I can test this out over the upcoming weekend with: https://www.adafruit.com/product/3137. I don't think have any of the other pieces of supported hardware.

dglaude commented 4 years ago

So I have rebase(?) so that #31 is included in this PR.

FoamyGuy commented 4 years ago

I tested all of the example scripts except for the pillow and led_shim ones with a CharlieWing LED 15 x 7 and Feather Sense. Aside from the minor import typo mentioned above in the text example everything is working well.

Thanks for splitting these up!

tannewt commented 4 years ago

Do we have a plan to update any dependent drivers and guides?

FoamyGuy commented 4 years ago

I found these Guides that I think would need updated:

makermelissa commented 3 years ago

Closing as I have included all these changes as part of #38. Thanks for all this work.

adafruit / Adafruit_CircuitPython_IS31FL3731

Split library into package and adapt examples+doc #32