Better library and class names ?

[caternuson]

The library name is too technical. The HT16K33 is the driver chip. If this were a driver for nothing but a HT16K33 breakout board, that would make sense However, it gets used on the various LED Backpack products. The Arduino and Python versions reflect this in their names. Something similar for this library would be good as well and be easier to find and remember for beginners.

Also, some of the class names are a little non-obvious. It might be better if these reflect the same kind of wording used on the product pages. Ex: AlphaNumeric instead of Seg14x4.

How about something like this:

\adafruit_led_backpack
    ht16k33.py
        HT16K33
    matrix.py
        Matrix8x8
        Matrix16x8
        Bicolor8x8
    segmented.py
        SevenSegment
        AlphaNumeric
    bargraph.py
        Bicolor24

[deshipu]

I actually think we should have separate libraries for all those boards. The amount of common code is very small, and the whole elaborate inheritance structure I created only wastes memory. There is very little chance that you will be using any two of those devices together, too, so nothing is saved by having common code.

[deshipu]

The same applies to the RGB Display library, by the way.

[deshipu]

Note that I originally wrote this library for my own projects, and not for Adafruit's products — those were added later.

[caternuson]

@deshipu Thanks for the input.

What're you thinking for the separate libs? One lib per board? Or like one lib for the matrices, one lib for the segmented, etc?

I think having the HT16K33 base class makes sense. I don't see an issue with the current inheritance structure.

[deshipu]

I'm thinking one module per board, possibly all in a common repository. It doesn't make sense to group them by function or looks, because at any one time you are going to be using only one of them anyways — they can't stack. This way you only import (and waste memory on) what you actually use.

The inheritance here is wasteful and doesn't really abstract anything — especially since the move to framebuf, which removed most of the logic from the HT16K33 class. All you have there is the brightness and blink rate, which should be converted to I2C registers anyways, as #2 suggests.

[deshipu]

Or even better, have one repo for the LED backpacks, with separate files for each of the backpack kinds, one repo for the FeatherWings, and one for the Trellis (already separate). Name them after the devices, but please no CamelCase in Python module names.

[tannewt]

Something like this works for me:

\adafruit_led_backpack
    ht16k33.py
        HT16K33
    matrix8x8.py
        Matrix8x8
    matrix16x8.py
        Matrix16x8
    bicolor8x8.py
        Bicolor8x8
    seven_segment.py
        SevenSegment
    alpha_numeric.py
        AlphaNumeric
    bargraph.py
        Bicolor24

[caternuson]

It terms of memory savings, is there any difference between the one class per module approach vs. selectively importing what's needed? For example, assume this layout (focusing on just matrix):

\adafruit_led_backpack
    matrix.py
        Matrix8x8
        Matrix16x8
        Bicolor8x8

and then use it like this:

from adafruit_led_backpack.matrix import Matrix8x8
disp = Matrix8x8(...)

VS. this layout

\adafruit_led_backpack
    matrix8x8.py
        Matrix8x8
    matrix16x8.py
        Matrix16x8
    bicolor8x8.py
        Bicolor8x8

and its use:

from adafruit_led_backpack.matrix8x8 import Matrix8x8
disp = Matrix8x8(...)

but, could also do this:

import adafruit_led_backpack.matrix8x8
disp = adafruit_led_backpack.matrix8x8.Matrix8x8(...)

which is an approach to avoid for the first layout as it would waste memory.

[tannewt]

The three file approach is better. Classes refer to their outer namespace which includes the other classes anyway.

[caternuson]

Crazy idea? How about breaking things up in to 2 repos / libraries. One that's just for the HT16K33 and one for the backpacks (which would require the ht16k33 lib)? The HT16K33 is reasonably general purpose, for example Trellis uses it. So those other devices could all (in theory) use the same HT16K33 base.

[deshipu]

The HT16K33 is general purpose (I use it in some of my projects myself), but there isn't really much code to share. Basically just the brightness and blinking registers. The frame memory is going to be differently arranged for each project, and the drawing functions are handled by framebuf.

[caternuson]

There's also the keyscan stuff.

[deshipu]

That is only used in trellis, no?

[caternuson]

Yep, just pointing it out as another feature of the HT16K33. The hypothetical HT16K33 library would support it. The Trellis library would use it, the LED backpack library would not.

Maybe this is overkill and not memory efficient? Since the HT16K33 is relatively simple, may be better to just have each library implement only what is needed.

[tannewt]

Splitting it up makes sense to me.

…

On Thu, May 24, 2018 at 7:20 AM Carter Nelson ***@***.***> wrote: Yep, just pointing it out as another feature of the HT16K33. The hypothetical HT16K33 library would support it. The Trellis library would use it, the LED backpack library would not. Maybe this is overkill and not memory efficient? Since the HT16K33 is relatively simple, may be better to just have each library implement only what is needed. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#8 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADNqa9GwudUEFu9fHyGCEO2IFi8OQ68ks5t1sGdgaJpZM4UCImL> .

[kattni]

@caternuson Is this something you're still interested in working on?

[caternuson]

We can close this issue.

The discussion here has good info for any future work that's related. As is, I think it's too broad of an issue to become a nice tidy PR.