Is31pixelbuf

[gamblor21]

Added new functions to the is31fl3741 module similar to neopixel_write but for is31fl3741 LEDs. Included an initialize function as well. These two functions are used with a yet to be submitted python library to use with with the adafruit_pixelbuf class.

Also includes a small bug fix to the existing library and changes the map from list to tuple upon advice I was given. There are no libraries released that were lists so best to do so now.

[tannewt]

Why does this need to be done in C? Can't write be done from Python alone?

[gamblor21]

Why does this need to be done in C? Can't write be done from Python alone?

I did it in C as I thought it would need it for speed (especially the mapping) and for coordination with the displayio work I did. I will do a test of C vs Python to see if it is required. Now that I understand how all the parts work doing it in Python may make sense.

[tannewt]

I didn't see any coordination here. Did I miss it?

If it does need to be coordinated, then I'd expect it to be part of the existing object.

It'd also be helpful if you'd link to the prototype library.

Thanks!

[gamblor21]

The python library (not yet merged upstream) https://github.com/gamblor21/Adafruit_CircuitPython_IS31FL3741/tree/native_changes

I added a is31fl3741_pixelbuf.py that is a subclass of pixelbuf the same way Neopixel is. Also led_glasses_map.py that provides the pre-defined mapping coordinates that a user will need if they're using the glasses.

I did some testing to see if the is31fl3741_write makes sense in C vs python, all this was modeled after the neopixel_write submodule. Doing the mapping of the LEDs from pixels on the glasses is faster in C (just over twice as fast). For the test I just changed every pixel color repeatedly. Just like the neopixel_write function this is meant for the entire display being written, same as the existing buffered functionality.

It may be easiest to discuss this at the next CP meeting.

[tannewt]

@gamblor21 Sorry I wasn't at this weeks meeting. I took the day off. What did you conclude there? 2x speed-up seems worth it.

[gamblor21]

@gamblor21 Sorry I wasn't at this weeks meeting. I took the day off. What did you conclude there? 2x speed-up seems worth it.

I was checking with Kattni on the library end of things which looks good (it won't break the existing library on smaller boards). I'll clean up the code I have with the comments above and also submit the PR to the library so they can be looked at together.

[tannewt]

I see you pushed the library PR. Let me know when you'd like me to look at this and I will. Thanks!

[gamblor21]

I see you pushed the library PR. Let me know when you'd like me to look at this and I will. Thanks!

Both PRs are passing checks and I did a final check on my hardware here so should be good for reviews.

[tannewt]

Thank you for the reorg. I like the separation you have between the IS31FL3741 and FrameBuffer classes! I just requested some doc updates to match. Thanks!

[tannewt]

Update this please.

[tannewt]

Suggested change

	//| def is31fl3741_write(mapping: Tuple[int, ...], buf: ReadableBuffer) -> None:
	//| def write(mapping: Tuple[int, ...], buf: ReadableBuffer) -> None:

[tannewt]

Very close! Just a typo and a question about kw_only required arguments.

[gamblor21]

Not sure why there was one cancelled check, but with the rest passing I don't think it is worth re-running the whole thing.

[tannewt]

Sorry, two more minor things. I suggested conflicting changes to you before wrt the * and kw_only. More folks are using the stubs so think it's worth correcting now.

[tannewt]

Thank you! This looks good. It just needs conflicts resolved and then will be good to go.

[gamblor21]

All the merge fixes are good. One build failed with a 403 error that doesn't seem related to the changes.

[tannewt]

One spot where I think you want a write_read.

[tannewt]

Looks good to me! Thank you!