Skip to content

Add SPI driven NeoPixel. #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 9, 2019
Merged

Conversation

caternuson
Copy link
Contributor

Add a class, NeoPixel_SPI, that takes in a SPI bus object and uses its MOSI pin to synthesize a NeoPixel data stream.

Basic usage example:

        import board
        import neopixel

        pixels = neopixel.NeoPixel_SPI(board.SPI(), 10)
        pixels.fill(0xff0000)

Things I'm not super jazzed about:

  • The try/except needed on the imports up top.
  • Needing the leading 80us low time (self.RESET) on the write.

neopixel.py Outdated

def _transmogrify(self):
"""Turn every BIT of buf into a special BYTE pattern."""
out_buf = bytearray()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we can pre-alloc the size of the bytearray instead of using append, that will be much faster :)

@caternuson
Copy link
Contributor Author

Wonder if I could get that down to just the one buffer...

@caternuson
Copy link
Contributor Author

...yes, but kind of pulls a thread, so I think best saved for a follow on PR.

@ladyada
Copy link
Member

ladyada commented Oct 7, 2019

agreed! i was looking for speed increase, not memory reduction

@ladyada
Copy link
Member

ladyada commented Oct 9, 2019

works once brightness adjust is done - see line you need to add :)

@ladyada ladyada merged commit b94f06d into adafruit:master Oct 9, 2019
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Oct 10, 2019
@tannewt
Copy link
Member

tannewt commented Dec 2, 2019

Sorry that I missed this when it was added but this is not the right spot for this change. neopixel.py is commonly frozen into CircuitPython and should not include any uncommon uses such as this.

Instead, I think this should be done as a neopixel_write implementation for the FT232 that takes in a pin and raw buffer every show. Alternatively, it could be its own library and inherit from PixelBuf for the standard RGB API going forwards.

Either way, Neopixel_SPI should be removed from this library in order to reduce the memory and code size footprint of neopixel.

Thanks!

@tannewt tannewt mentioned this pull request Dec 2, 2019
@ladyada
Copy link
Member

ladyada commented Dec 2, 2019

we could also make a new library that subclasses this one ?

@tannewt
Copy link
Member

tannewt commented Dec 2, 2019

@ladyada You could but I don't think that is what you want. What do you want to support besides FT232?

@ladyada
Copy link
Member

ladyada commented Dec 2, 2019

handy if people are bringing up new boards that dont have neopixel_write, for any other linux boards

@tannewt
Copy link
Member

tannewt commented Dec 3, 2019

@ladyada Wouldn't having it in Blinka work for that?

@ladyada
Copy link
Member

ladyada commented Dec 3, 2019

yes! there's no reason it wouldnt work for plain circuitpython boards ... wouldn't make a lot of sense to use since we normally add neopixel_write when we port...but perhaps others (like spresense?) would not get to it.... Neopixel_SPI its a generic library that only depends on busio

@makermelissa
Copy link
Collaborator

@tannewt, it works on some boards, but not others. Like I think it worked on the Raspberry Pi, but not the Orange Pi.

@caternuson
Copy link
Contributor Author

The generic use case for Neopixel_SPI is for boards that have a hardware SPI peripheral, but can not otherwise output the data signal on a low level pin. This hack doesn't work on any pin, just the MOSI pin of a hardware SPI port. It essentially off loads the neopixel timing requirement to the SPI peripheral. That's the reasoning for it taking in the SPI instance:

pixels = neopixel.NeoPixel_SPI(board.SPI(), 10)

which then allows NeoPixel_SPI to be used generically, and not just with FT232H.

Do we need a special place for libraries that are only needed by hardware using Blinka? The Blinka repo itself is just the shim to make it all work, so maybe not the best place?

@tannewt
Copy link
Member

tannewt commented Dec 4, 2019

If we want to use this on CircuitPython as well as Blinka, let's split it out into its own library.

If we want it just for Blinka then we can add it as a neopixel_write implementation there. It can play tricks to use the SPI internally then.

@ladyada
Copy link
Member

ladyada commented Dec 4, 2019

yeah, id rather have it in a proper library - its pure python and github repos are freeee :)

@caternuson
Copy link
Contributor Author

OK, I can set up the separate lib repo. Repo name = Adafruit_CircuitPython_NeoPixel_SPI or Adafruit_CircuitPython_NeoPixelSPI ? Does it get added to the bundle?

@ladyada
Copy link
Member

ladyada commented Dec 4, 2019

yeah add to the bundle, name should match the object which is Adafruit_CircuitPython_NeoPixel_SPI right?

@caternuson
Copy link
Contributor Author

Works for me. @tannewt work for you?

@tannewt
Copy link
Member

tannewt commented Dec 5, 2019

@caternuson Either is fine with me as long as the module name matches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants