zephyr icon indicating copy to clipboard operation
zephyr copied to clipboard

boards: shields: Adding Adafruit 5x5 Neopixel Grid BFF

Open raveious opened this issue 1 year ago • 1 comments

The Adafruit 5x5 NeoPixel Grid BFF is a little shield for any of the Xiao compatible boards with a 5x5 grid of WS2812B RGB LEDs on the back.

raveious avatar Jan 15 '24 12:01 raveious

@soburi @simonguinot Would you two mind reviewing this?

raveious avatar Jan 24 '24 23:01 raveious

@soburi @simonguinot Any thoughts on this one?

raveious avatar Mar 01 '24 11:03 raveious

Hi @raveious,

First I apologize for the delay.

This PR looks good to me and except a typo (see my comment below) I don't have much to say.

But don't you want to wait for #68614 to be merged ? So you could add matrix support as well ?

I think the answer is "Yes", but both of our PRs seemed to be stuck.

raveious avatar Mar 01 '24 21:03 raveious

@soburi Do you have any guidance as to how I can add support for your matrix driver from #68614 ? Is it really just adding the matrix display option to the chosen section?

raveious avatar Mar 01 '24 22:03 raveious

@raveious

@soburi Do you have any guidance as to how I can add support for your matrix driver from #68614 ? Is it really just adding the matrix display option to the chosen section?

This PR is the first user!

This module may work with this patch and #68614 changes. Could you try it, please?

0001-bff_display.patch

And build and flush the west build -p -b adafruit_qt_py_rp2040 -d ../qt_py samples/drivers/display/ -- -DSHIELD=adafruit_neopixel_grid_bff_display

soburi avatar Mar 03 '24 00:03 soburi

@raveious

@soburi Do you have any guidance as to how I can add support for your matrix driver from #68614 ? Is it really just adding the matrix display option to the chosen section?

This PR is the first user!

This module may work with this patch and #68614 changes. Could you try it, please?

0001-bff_display.patch

And build and flush the west build -p -b adafruit_qt_py_rp2040 -d ../qt_py samples/drivers/display/ -- -DSHIELD=adafruit_neopixel_grid_bff_display

So I tried out your changes this morning and everything seems to be working as intended. I did have to add start-from-right and circulative to the device tree based on how the LEDs are oriented on the board.

I do have one request though. Can you add some kind of "power limit" to the driver? These LEDs can take up a lot of power, and I was concerned about how hot the PCB was getting. So if you're planning on using even larger displays then some kind of power limit may be a good idea to implement. One valid implementation would be to rescale the desired values with the new limit, but its ultimately up to you.

Do you think it would be a good idea to just have everything into a single device tree overlay?

If you are curious, I have everything on a new branch that I'll merge/rebase into this one when #68614 gets merged.

raveious avatar Mar 04 '24 11:03 raveious

@raveious

@soburi Do you have any guidance as to how I can add support for your matrix driver from #68614 ? Is it really just adding the matrix display option to the chosen section?

This PR is the first user! This module may work with this patch and #68614 changes. Could you try it, please? 0001-bff_display.patch And build and flush the west build -p -b adafruit_qt_py_rp2040 -d ../qt_py samples/drivers/display/ -- -DSHIELD=adafruit_neopixel_grid_bff_display

So I tried out your changes this morning and everything seems to be working as intended. I did have to add start-from-right and circulative to the device tree based on how the LEDs are oriented on the board.

Great!

I do have one request though. Can you add some kind of "power limit" to the driver? These LEDs can take up a lot of power, and I was concerned about how hot the PCB was getting. So if you're planning on using even larger displays then some kind of power limit may be a good idea to implement. One valid implementation would be to rescale the desired values with the new limit, but its ultimately up to you.

I don't think there is a way to directly limit the current. However, I believe that this should be achieved through gamma correction. In other words, the upper bound is rounded down by scaling with a translation table. (Of course, the color resolution will be reduced) There are several possible implementation methods, but I think piecewise linear implementation is preferable.

Do you think it would be a good idea to just have everything into a single device tree overlay?

If you are curious, I have everything on a new branch that I'll merge/rebase into this one when #68614 gets merged.

I think the display interface is easier to use, so it would be good to integrate it. If anyone doesn't need it, turn off CONFIG DISPLAY to be okay.

soburi avatar Mar 04 '24 14:03 soburi

I don't think there is a way to directly limit the current. However, I believe that this should be achieved through gamma correction. In other words, the upper bound is rounded down by scaling with a translation table. (Of course, the color resolution will be reduced) There are several possible implementation methods, but I think piecewise linear implementation is preferable.

Maybe less "power limit" and more "brightness limit"?

raveious avatar Mar 04 '24 19:03 raveious

Maybe less "power limit" and more "brightness limit"?

Yes it is. The higher the brightness of a color, the more power it consumes, so this is a way to limit it. In reality, there is a non-linear relationship between the brightness setting and the perceived brightness, so implementing gamma correction is good stuff.

soburi avatar Mar 05 '24 01:03 soburi

@soburi @simonguinot I have rebased this branch to get the changes from #68614. Take a look when you get a chance.

raveious avatar Mar 06 '24 11:03 raveious

#68514 also merged. You need to rebase this PR. ~With this change, you should no longer need to define led-strip in the files inside boards.~

soburi avatar Mar 06 '24 13:03 soburi

#68514 also merged. You need to rebase this PR. ~With this change, you should no longer need to define led-strip in the files inside boards.~

Rebase done. Those files are required because of the dependency on the PIO specific driver.

raveious avatar Mar 06 '24 23:03 raveious