search

rpi-ws281x / rpi-ws281x-python

Python library wrapping for the rpi-ws281x library
BSD 2-Clause "Simplified" License
319 stars 102 forks source link

Make `PixelStrip` a subclass of `_LED_Data` #91

Closed Viicos closed 1 year ago

Viicos commented 1 year ago

Fixes #90.

Imo it might be better to have _LED_Data removed, and implement the __get/setitem__ methods in PixelStrip directly, as currently the private _channel attribute is duplicated and made public in _LED_Data.

Gadgetoid commented 1 year ago

Thank you for taking the time to do this!

I think you're probably right. This is some pretty legacy code that - to be frank - not much consideration went to at the time I grabbed it and packaged it up into a Python module. As long as the interface doesn't break, any change is probably a change for the better.

I'm aiming to release v5.0.0 of this library imminently to incorporate the unceremonious v1.0.0 release of rpi_ws281x (a few more supported boards). Would be nice to have these changes along for the ride!

Viicos commented 1 year ago

I think you're probably right. This is some pretty legacy code that - to be frank - not much consideration went to at the time I grabbed it and packaged it up into a Python module

Well as long as the library can be used pretty easily, this is perfectly fine. While we're at it, do you know the minimum Python version supported for this library? It is currently not specified in the setup.py file

(Will probably push the removal of _LED_Data today)

Gadgetoid commented 1 year ago

IIRC this was originally written for the Pi when Python 2.7 was still very much sticking around, so that's probably the baseline. But that's hilariously old and twice deprecated. I suspect we could safely cut off at 3.6 if we want to pick a number.

Certainly wouldn't kill me to add GitHub actions to this repo, too :grimacing:

Gadgetoid commented 1 year ago

Thank you for this, I've made some tweaks, rebased my v5.0.0 branch and released a test here: https://test.pypi.org/project/rpi-ws281x/