Thanks for the patch. It may take some time before we'll be able to look into this. In the meantime, there was another similar patch posted: #1950, it would be nice if you guys cooperated to produce one well tested module.

@misterdanb, Apparently there are two APA102 PRs floating about.
Personally I don't have a preference about which one is merged, but if you'd like, feel free to include any snippets of #1950 into this PR.

@misterdanb I was hoping to get a hold of you the see if we could combine our efforts.

@robertfoss Oh, sorry, didn't look into this pull request for quite a while (was a little bit busy with other stuff). I will have a look at this in the next week, i hope. Actually some parts of my code are not that nice, because I put everything in one function (as GPIO writes did not work within helper functions and I had no idea why, very weird behaviour or I do understand somthing very wrong), so I'm very fine with taking some of your code! I will test it on my devices as soon as possible :)…

@robertfoss I finally found some time to restructure code, and took some of yours for that. I hope things got somehow cleaner.

@misterdanb looks good to me!

@pfalcon This PR is ready to be merged.

@robertfoss: Can you please rebase onto the latest master?

Okay, so after fixing the mentioned issues, reviewing the whole pull request again and testing the code again with a 60 leds strip, it should be fine now. But please have a look at it again. too, of course!

@misterdanb : MicroPython uses rebase-based workflow, so all changes will need to be rebased onto the current master, and all merge commits removed. https://github.com/micropython/micropython/wiki/DevelWorkflow

Rebased everything, sorry for missing that workflow.

@misterdanb : Thanks, and getting there. In MicroPython, all (or almost all files) should have a copyright/licensing header, see e.g.
https://github.com/micropython/micropython/blob/master/esp8266/esponewire.c . Would you add it to espapa102.c/espapa102.h (with your and/or @robertfoss' copyrights)? The reason not to do that would be if you copied that code from somewhere, in which case it should have comments about its source and its licensing/copyright (and licensing should be compatible with MicroPython's of course). Thanks.

Added licensing information as seen in similar files. @robertfoss Are you okay with the MIT license?

Okay, since no one answers, I think MIT license is fine. ^^

@misterdanb: I assume @robertfoss is aware that MicroPython is MIT-licensed and was OK with that if he submitted his pull request. At least that's how we handle (as most of other community-backed projects here on github): we don't ask for contributor agreement or explicit sign-off, and assume that act of submitting pull request constitutes agreement with the license of the project.

And sorry for this still being backlogged, I'm actually looking into merging this today.

Ok, I tried to see how to squash this down to 2-3 commits, but ended up just squashing altogether. Thanks for contribution @misterdanb , @robertfoss and following thru the review process!

I 100% ok with MIT licensing my contributions. Thanks for verifying it
though.
On May 19, 2016 15:24, "Paul Sokolovsky" notifications@github.com wrote:

@misterdanb https://github.com/misterdanb: I assume @robertfoss
https://github.com/robertfoss is aware that MicroPython is MIT-licensed
and was OK with that if he submitted his pull request. At least that's how
we handle (as most of other community-backed projects here on github): we
don't ask for contributor agreement or explicit sign-off, and assume that
act of submitting pull request constitutes agreement with the license of
the project.

And sorry for this still being backlogged, I'm actually looking into
merging this today.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1941 (comment)

I'm looking into helping create an APA102 driver for the ESP32 port. I can see that the ESP8266 implementation is specific to that chipset but I guess I'm trying to understand why a bit-banged approach was chosen over using SPI?

I'm probably oversimplifying but it seems like a pure python APA102 driver that used machine.SPI would work across all ports and be optimised for ports that had (hardware-enabled) SPI...

I'm trying to understand why a bit-banged approach was chosen over using SPI?

I'm guessing that at the time it wasn't clear to us that SPI could be used... but @misterdanb would be the best person to answer that.

I'm probably oversimplifying but it seems like a pure python APA102 driver that used machine.SPI would work across all ports

Yes that sounds like it would work, and it would use less code because of the re-use of the SPI class. Probably the main drawback is that SPI currently requires SCK/MOSI/MISO so that would tie up an extra pin that's not needed (MISO). So one way thing to do would be extend the SPI class to allow it to work without MISO or MOSI (and possibly even SCK?). I would say that most MCUs could support this in their hardware SPI block, just by not configuring the relevant GPIO to be controlled by the SPI.

That is totally an option! I used SPI to drive the APA102 in another (Arduino based) project myself. The reason, why i chose to use the bit-bangend approach is that you are not limited to use the SPI, if you need it for something else (or even want to mux those pins in another way).
I am currently not so much aware of the ESP32's hardware, so I cannot speak for this one.

But even though it would be nice to have a more efficient way of driving the APAs. That this then comes for free for other MCUs wasn't on my mind at that time.

Got it, thanks @dpgeorge and @misterdanb!

I'll try to spend some time putting together a pure Python APA102 driver based on SPI and see what it looks like.

I understand the compromise about the extra pin currently needed for SPI but agree with @dpgeorge's suggestion that the SPI class should be more flexible in allowing pins to not be allocated (even though some hardware may not allow that). However, my opinion is that even if the SPI class isn't extended in this way having an efficient, portable implementation seems worth the potential loss of a pin.

I'll also take a closer look at existing solutions and see if there's value in porting something.

The thought did cross my mind that we could wrap FastLED but I'm not sure that'll be worth the effort...