Skip to content

Color Wheel LED mode & other fixes#952

Merged
pr3y merged 4 commits intoBruceDevices:mainfrom
Pablo-Ortiz-Lopez:colorwheel
Mar 21, 2025
Merged

Color Wheel LED mode & other fixes#952
pr3y merged 4 commits intoBruceDevices:mainfrom
Pablo-Ortiz-Lopez:colorwheel

Conversation

@Pablo-Ortiz-Lopez
Copy link
Contributor

This PR basically consists of the following changes:

  • Add a "Color Wheel" option to the LED Color selector
  • Instead of reserving a bunch of RMT channels both for LED TX and RF RX, avoid conflicts by reserving
    • Channels 0 and 1 for LED TX
    • Channel 6 for RF RX
  • Add calls to tft.drawPixel(0,0,0); , which prevent the T-Embed's display from ignoring commands

The only thing that I think might spark debate (or probably not) is that I used a bruceConfig.ledColor value of 1 to store the "Color Wheel" mode, so this extremely dim, almost off color, becomes unavailable.
I don't really think it's going to be an issue but maybe you'd prefer adding an extra option to bruceConfig.

@Huzzla101
Copy link

Huzzla101 commented Mar 20, 2025 via email

@rennancockles
Copy link
Collaborator

rennancockles commented Mar 20, 2025

Don't hardcode the value 1 for color wheel. That way if we decide to change this value in the future we would have to change it in a bunch of places. The best thing would be to define a constant for that like LED_COLOR_WHEEL or something. That way we would have to change the value in a single place and it's easier to understand what that value is.

@Pablo-Ortiz-Lopez
Copy link
Contributor Author

Don't hardcode the value 1 for color wheel. That way if we decide to change this value in the future we would have to change it in a bunch of places. The best thing would be to define a constant for that like LED_COLOR_WHEEL or something. That way we would have to change the value in a single place and it's easier to understand what that value is.

That is a great idea, just pushed the fix.

@pr3y pr3y merged commit af3de41 into BruceDevices:main Mar 21, 2025
5 checks passed
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.

5 participants