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

New LED Color menu + some code optimization #1853

Merged
merged 12 commits into from
Apr 22, 2021

Conversation

digant73
Copy link
Contributor

@digant73 digant73 commented Apr 19, 2021

NEW FEATURES:

  • added custom LED menu:
  1. it is now possible to set all the parameters for M150 gcode (machine RGB LED)
  2. the set LED color is previewed at runtime. Any change is scheduled to be applied every 1 second at maximun, just to avoid useless traffic (to send M150 gcode) every time a component is changed
  3. RGB component colors and RGB color are also displayed just as color pickup. Obviously RGB color on display is limited to RGB 565 format while machine LED uses the wider range 0-255 for each component
  4. the LED main menu allows to set and save standard/custom color and switch to on/off the saved color
  5. rotary encoder can be used to change the value for the control holding the focus

IMPROVEMENTS:

  • some code optimization on Terminal, Mesh editor and LED menus. They now reuse some common code
  • on terminal menu, the page box is no more clickable now

resolves #1687

PR STATE: ready to be merged

IMG_20210418_210420

IMG_20210418_201621

IMG_20210418_201653

@Guilouz
Copy link
Contributor

Guilouz commented Apr 19, 2021

Hi, I have a warning with your PR :

Capture d’écran 2021-04-19 à 15 04 11

Capture d’écran 2021-04-19 à 15 04 24

@Guilouz
Copy link
Contributor

Guilouz commented Apr 19, 2021

And just try your PR, not working for me.
I have Neopixels connected to my SKR 1.4 Turbo (not on my TFT), at startup I have the color cycle and then the switching on in white (so far everything is fine) but when I go in LED menu, I can't change anything, buttons blue, green, red, white, on and off doesn't work anymore. Any button turns off my leds and I can't turn them back on. Custom color have no effect no more.

M150 commands do not seem to be sent correctly to the motherboard.

@digant73
Copy link
Contributor Author

digant73 commented Apr 19, 2021

The warning is not a problem (the define on LEDColor overrides the one provided in another .h file).
The standard colors (red, green etc...) in the LED Color menu were maintained compatible with the previous ones (so P and I for M150 are set to 0). From the custom menu you have the possibility to set also W, P and I.
The M150 command used is:

M150 R%d U%d B%d W%d P%d I%d

where %d are the 6 components set from the custom menu

Try to set I properly. The Red, blue, ON, OFF etc... can be easily fixed. yust not setting the I component to 0

@Guilouz
Copy link
Contributor

Guilouz commented Apr 19, 2021

Ah we need to set W, P and I in custom menu to use red, green, blue and white button ? Not really intuitive.

@digant73
Copy link
Contributor Author

Ah we need to set W, P and I in custom menu to use red, green, blue and white button ? Not really intuitive.

currently pressing on RED, BLUE etc... will always set to 0 P and I. It could be easily fixed. Use the custom menu setting the proper value (also I) and it should work

@Guilouz
Copy link
Contributor

Guilouz commented Apr 19, 2021

Ah we need to set W, P and I in custom menu to use red, green, blue and white button ? Not really intuitive.

currently pressing on RED, BLUE etc... will always set to 0 P and I. It could be easily fixed. Use the custom menu setting the proper value (also I) and it should work

Just try, when i set W, P and I my leds light up, but after when I click on red, blue or green button my leds light off instead of changing color and value P and I go back to 0.

@digant73
Copy link
Contributor Author

digant73 commented Apr 19, 2021

Ah we need to set W, P and I in custom menu to use red, green, blue and white button ? Not really intuitive.

currently pressing on RED, BLUE etc... will always set to 0 P and I. It could be easily fixed. Use the custom menu setting the proper value (also I) and it should work

Just try, when i set W, P and I my leds light up, but after when I click on red, blue or green button my leds light off instead of changing color and value P and I go back to 0.

yes that is the bug. an easy fix. but you will need to use the custom menu to set the proper NEO pixel setting. Then P and I will be maintained pressing on RED, BLUE... ON and OFF. I'm going to fix the bug

@Guilouz
Copy link
Contributor

Guilouz commented Apr 19, 2021

@digant73 Let me know as soon as it's done so I can test.

@digant73
Copy link
Contributor Author

digant73 commented Apr 19, 2021

@digant73 Let me know as soon as it's done so I can test.

fixed. Now the P and I components (used by NEOPixel) are maintained once they were set from the custom menu. So switching to standard colors (red, green...) or to ON/OFF will avoid to loose the NEOPixel components

@Guilouz
Copy link
Contributor

Guilouz commented Apr 19, 2021

@digant73 This issue is now fixed but another issue : P and I components are not saved after restarting the printer, it's really annoying if you have to redefine them every time.

Maybe it would be better to set P and I components to 255 by default.

@GregSKR
Copy link
Contributor

GregSKR commented Apr 19, 2021

@digant73 : Thanks for your work! 👍
I'm in the same boat here : NeoPixel LEDs connected to my SKR board.
For NeoPixels, W value has no effect, but P is used for intensity (eg: green = M150 R0 U255 B0 P255) so I suggest this change:

  if (skipNeopixel)
    storeCmd("M150 R%d U%d B%d P%d\n", (*led)[0], (*led)[1], (*led)[2], (*led)[4]);

Just tried your lastest PR : Red / Green / Blue buttons don't work "out of the box", I have to set manually P to 255 first.
If I is set to 0, only the first LED will lit, so I have to set I to 255 manually, too.
After that, Red / Green / Blue buttons are OK.
Liked @Guilouz said, having P and I set to 255 by default for NeoPixels would be great

@Guilouz
Copy link
Contributor

Guilouz commented Apr 19, 2021

@GregSKR Yes when I (index) is set to 0 only the first led is controlled, if you set 1 the second, etc.. depending of number of LED you have.

Personally, I don't really see the importance of this "I" parameter, I don't change the color of my LEDs one by one but all at the same time.

@digant73
Copy link
Contributor Author

digant73 commented Apr 19, 2021

ok so it is enough to use 255 as default value for P and I instead of 0.
The LED color is not saved to flash. You always need to select it after a reboot. Check the next commit for the fix

@Guilouz
Copy link
Contributor

Guilouz commented Apr 19, 2021

@digant73 Just try your fix and value for P and I are always 0 at startup. Not working, I have the same like first issue, buttons red, green, blue, white shutdown my leds.

And for LED Off, not needed to be 255 for P and I, no ?

const LED_VECT ledOff = {0x00, 0x00, 0x00, 0x00, 0xFF, 0xFF};

Here, a video of the issue : https://www.youtube.com/watch?v=h85AneYQ2C4

@digant73
Copy link
Contributor Author

digant73 commented Apr 19, 2021

should work now.

for LED OFF, at least I should be 255 in order it is applied to all LEDs

const LED_VECT ledOff = {0x00, 0x00, 0x00, 0x00, 0x00, 0xFF};

@Guilouz
Copy link
Contributor

Guilouz commented Apr 19, 2021

@digant73 Just try your last fix, all is working as expected now. Thanks.

@GregSKR
Copy link
Contributor

GregSKR commented Apr 20, 2021

@digant73 Thanks, I confirm it's all fine now for NeoPixels, everything works as expected! 👍

@digant73
Copy link
Contributor Author

good. thanks to both for the testing

@bigtreetech bigtreetech merged commit 3c6d135 into bigtreetech:master Apr 22, 2021
@oldman4U oldman4U mentioned this pull request May 9, 2021
jeffeb3 pushed a commit to V1EngineeringInc/BIGTREETECH-TouchScreenFirmware that referenced this pull request Nov 10, 2021
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.

[FR] Make the LED/Lights menu in TFT mode more feature complete
4 participants