search

tvoverbeek / rpi-pcm-ws281x

Library for controlling neopixels using the Raspberry Pi PCM hardware instead of PWM
MIT License
5 stars 2 forks source link

Would you consider a PR upstream? #1

Open Gadgetoid opened 7 years ago

Gadgetoid commented 7 years ago

I've just found the time to set this up and get it running. Absolutely brilliant! I'm genuinely surprised nobody has come up with this solution before. Stellar effort ;)

Would it be possible, and indeed would you consider, merging this method of operation with the upstream library which can be found here: https://github.com/jgarff/rpi_ws281x

I think a lot of users (this library has 351 stars and 134 forks) would be very happy to have PCM as an option in the canonical library.

I would envision the "NeoPixel" __init__ either accepting another argument to determine PCM or PWM, or automatically deciding which to use based on the given GPIO pin (great if there are no conflicts).

The upstream library also supports a whole variety of pixel options: https://github.com/jgarff/rpi_ws281x/blob/master/ws2811.h#L45-L64

I hate to say "this is great, more please!" since I feel it undermines just what you've accomplished here, but this is just too awesome to be consigned to Unicorn HAT alone!

Thank you!

tvoverbeek commented 7 years ago

Yes I have considered the same. Will see how easy/difficult it is to make a combined library and then submit a pull-request. As a start, I'll raise an issue on the jgarff rpi_ws281x library.

tvoverbeek commented 7 years ago

Phil,

Yes I have considered the same. Will see how easy/difficult it is to make a combined library and then submit a pull-request. As a start, I'll raise an issue on the jgarff rpi_ws281x library.

Ton

On Fri, Oct 21, 2016 at 12:13 PM, Philip Howard notifications@github.com wrote:

I've just found the time to set this up and get it running. Absolutely brilliant! I'm genuinely surprised nobody has come up with this solution before. Stellar effort ;)

Would it be possible, and indeed would you consider, merging this method of operation with the upstream library which can be found here: https://github.com/jgarff/rpi_ws281x

I think a lot of users (this library has 351 stars and 134 forks) would be very happy to have PCM as an option in the canonical library.

I would envision the "NeoPixel" init either accepting another argument to determine PCM or PWM, or automatically deciding which to use based on the given GPIO pin (great if there are no conflicts).

The upstream library also supports a whole variety of pixel options: https://github.com/jgarff/rpi_ws281x/blob/master/ws2811.h#L45-L64

I hate to say "this is great, more please!" since I feel it undermines just what you've accomplished here, but this is just too awesome to be consigned to Unicorn HAT alone!

Thank you!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/tvoverbeek/rpi-pcm-ws281x/issues/1, or mute the thread https://github.com/notifications/unsubscribe-auth/ABJE7pgNEw5VjSmO3nYUW2eRtQ1gmvj-ks5q2JBNgaJpZM4KdEx0 .

Gadgetoid commented 7 years ago

Brilliant!

Sorry for the lack of reply, it's been a busy few days.

Godzil commented 7 years ago

Hello Ton, what is the status on that point? :)

tvoverbeek commented 7 years ago

PR #135 for combined library was submitted upstream on 25 NOV 2016. Accepted, but moved into development branch for testing Phil @Gadgetoid has just submitted a PR #170 to move dev branch into master.

Gadgetoid commented 7 years ago

On my own fork I've already promoted dev to master, and will be distributing the library based upon it- replacing my hard-forked mess that's on pip at the moment with something more closely representative of upstream. This should bring support for @tvoverbeek's new features to Python at least.

Godzil commented 7 years ago

Great!