Skip to content
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

boards: shields: Adding Adafruit 5x5 Neopixel Grid BFF #67610

Merged
merged 1 commit into from Mar 11, 2024

Conversation

raveious
Copy link
Contributor

@raveious raveious commented Jan 15, 2024

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.

@zephyrbot zephyrbot added the area: Shields Shields (add-on boards) label Jan 15, 2024
@raveious raveious force-pushed the adafruit_rgb_bff branch 3 times, most recently from 1a61387 to 1ba9845 Compare January 18, 2024 16:41
@raveious
Copy link
Contributor Author

@soburi @simonguinot Would you two mind reviewing this?

@raveious
Copy link
Contributor Author

raveious commented Mar 1, 2024

@soburi @simonguinot Any thoughts on this one?

Copy link
Collaborator

@simonguinot simonguinot left a comment

Choose a reason for hiding this comment

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

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 ?

boards/shields/adafruit_neopixel_grid_bff/doc/index.rst Outdated Show resolved Hide resolved
@raveious
Copy link
Contributor Author

raveious commented Mar 1, 2024

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
Copy link
Contributor Author

raveious commented Mar 1, 2024

@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?

@soburi
Copy link
Member

soburi commented Mar 3, 2024

@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

@raveious
Copy link
Contributor Author

raveious commented Mar 4, 2024

@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.

@soburi
Copy link
Member

soburi commented Mar 4, 2024

@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.

@raveious
Copy link
Contributor Author

raveious commented Mar 4, 2024

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"?

@soburi
Copy link
Member

soburi commented Mar 5, 2024

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.

@raveious
Copy link
Contributor Author

raveious commented Mar 6, 2024

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

@soburi
Copy link
Member

soburi commented Mar 6, 2024

#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.

@raveious
Copy link
Contributor Author

raveious commented Mar 6, 2024

#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.

soburi
soburi previously approved these changes Mar 6, 2024
Copy link
Member

@soburi soburi left a comment

Choose a reason for hiding this comment

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

Wonderful!
Thank you for your continued support.

This is a 5x5 grid of WS2812 RGB LEDs.

Signed-off-by: Ian Wakely <raveious.irw@gmail.com>
Copy link
Collaborator

@simonguinot simonguinot left a comment

Choose a reason for hiding this comment

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

Thanks !

@fabiobaltieri fabiobaltieri merged commit 96d9390 into zephyrproject-rtos:main Mar 11, 2024
21 checks passed
@raveious raveious deleted the adafruit_rgb_bff branch March 23, 2024 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Shields Shields (add-on boards)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants