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

Various usability problems in the LEDs settings page - Type not changeable, GPIO, Length and mA/LED settings sometimes lost upon save #4280

Closed
1 task done
Fonzi03 opened this issue Nov 14, 2024 · 17 comments
Labels
bug confirmed The bug is reproducable and confirmed

Comments

@Fonzi03
Copy link

Fonzi03 commented Nov 14, 2024

What happened?

After saving, only ports 1-3 applied properly. Ports 4-8 (in my instance) changed from the following settings:
mA/LED: 55mA (default setting)
Data GPIO:

changed to:
mA/LED: Custom
Data GPIO: 0

To Reproduce Bug

WT32-ETH01
(installed ethernet version)

flash a new board, apply wifi settings.
navigate to settings --> Usermods
Under Digitalmic:
Change Pin I2S WS to unused from default
Change Pin I2S SCK to unused from default
Save
navigate to settings --> LED Preferences
Add 8 strings ( I used sk6812 )
Each string set to Length: 1

GPIO in order of ports 1-8 :
2, 4, 12, 14, 15, 17, 5, 33

Save

Go back to LED Preferences and the mA/LED, Length (sometimes only some ports are affect not all, never 1-3), and Data GPIO ports are not as they were set previously.

Removing the strings and re-adding them saving again seems to have a similar issue where mA/LED clears but GPIO assignments stay.

Change the incorrect mA/LED from custom back to the default 55mA
Save

Now settings remain

Start changing the length of strings
1: 145
Save

2: 283
Save

3: 55
Save
Crash

Rebooting board does nothing.
The only recovery option at this point is to re-flash

This is repeatable. (except sometimes the GPIO stays and sometimes does not, every time it loses the GPIO it only loses them after port 3)

I first started with putting the appropriate pixels and strings in order then save but to test for reporting the bug I did them individually and only 1 pixel each.

This renders the software/ board unusable for me as I cannot get it set to the strings that I need it consistently locks up the board.

This does not occur in 14.4 (however applying the above settings, then selecting the Ethernet type in 14.4 it goes into the same bricked state)

Expected Behavior

I can set all pixel settings without issues.

Install Method

Binary from WLED.me

What version of WLED?

0.15.0-b7

Which microcontroller/board are you seeing the problem on?

ESP32

Relevant log/trace output

No response

Anything else?

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@Fonzi03 Fonzi03 added the bug label Nov 14, 2024
@Fonzi03 Fonzi03 changed the title Ports >4 lose GPIO and mA/LED settings upon save Ports >3 lose GPIO and mA/LED settings upon save Nov 14, 2024
@Fonzi03 Fonzi03 changed the title Ports >3 lose GPIO and mA/LED settings upon save Ports >3 lose GPIO, Length and mA/LED settings upon save Nov 14, 2024
@k7bbr
Copy link
Contributor

k7bbr commented Nov 15, 2024 via email

@softhack007
Copy link
Collaborator

As this affects usability of the LEDs settings page, it would be good to understand what's going on before 0.15.0-RC1 #4287

@netmindz netmindz added this to the 0.15.0-final candidate milestone Nov 24, 2024
@softhack007
Copy link
Collaborator

softhack007 commented Nov 28, 2024

Another note: Is it supposed to be the behavior of the LED Preferences page now that you can only change the pixel type of the last item in the list of LED outputs?

I can confirm this part of the problem description - only the last output type can be modified, all others are grayed out.
wled_cfg1.json

image

@softhack007
Copy link
Collaborator

GPIO in order of ports 1-8 : 2, 4, 12, 14, 15, 17, 5, 33
Save
Start changing the length of strings
1: 145 Save
2: 283 Save
3: 55 Save Crash

I could not reproduce the crash any more - most likely it was fixed with #4312, #4327, or #4321

@softhack007
Copy link
Collaborator

Can get Ethernet to work on initial installation of 0.15.0-b7

  • This needs to verified by someone wo has the WT32-ETH board - might be related to a pin conflict, because many pins broken out on the WT32-ETH are actually not usable.

@softhack007
Copy link
Collaborator

softhack007 commented Nov 28, 2024

GPIO in order of ports 1-8 : 2, 4, 12, 14, 15, 17, 5, 33
Save

Go back to LED Preferences and the mA/LED, Length (sometimes only some ports are affected but not all, never 1-3), and Data GPIO ports are not as they were set previously.

I can reproduce the behaviour with this simple cfg.json
wled_cfg2.json

  • enter LEDs settings
  • add two additional outputs, save
  • re-enter LEDs settings
  • --> last output ABL is reset to 0

Before Save:
image

When re-opening the page:
image

Note: I did not test if adding 3, 4, or 5 outputs also leaves the last 2,3, or 4 ABL values as custom/0, but this is very likely.

@softhack007 softhack007 changed the title Ports >3 lose GPIO, Length and mA/LED settings upon save Various usability problems in the LEDs settings page - Type not changeable, GPIO, Length and mA/LED settings lost upon save Nov 28, 2024
@softhack007 softhack007 added the confirmed The bug is reproducable and confirmed label Nov 28, 2024
@softhack007 softhack007 changed the title Various usability problems in the LEDs settings page - Type not changeable, GPIO, Length and mA/LED settings lost upon save Various usability problems in the LEDs settings page - Type not changeable, GPIO, Length and mA/LED settings sometimes lost upon save Nov 28, 2024
@softhack007
Copy link
Collaborator

To other maintainers:
I think the problems that are still reproducible are annoying but not blocking.

We could list them as "know problems" in the 0.15.0-RC1 release notes and take the time to find a fix later.

@willmmiles
Copy link
Collaborator

Thanks for making a clear reproducing case. Smells to me like a bug in the settings JSON response - if you can, can you quickly check that the response looks sane?

@softhack007
Copy link
Collaborator

softhack007 commented Nov 28, 2024

Thanks for making a clear reproducing case. Smells to me like a bug in the settings JSON response - if you can, can you quickly check that the response looks sane?

I can try that tomorrow - actually i need to use MSEdge dev tools, right? Or is there a place where DEBUG_PRINTF still works, to dump out the response string?

@willmmiles
Copy link
Collaborator

I usually use my browser's dev panel, yes. For non-websocket requests like this, you can curl or wget at the command line too.

@softhack007
Copy link
Collaborator

Smells to me like a bug in the settings JSON response - if you can, can you quickly check that the response looks sane?

I've checked the "settingsScript" part s.js?p=2 looks meaningful and not truncated.

@willmmiles
Copy link
Collaborator

Yup. Looks like the problem is with the POST - the form data is empty for the 'LA3' and 'LA4' fields, we just get 'LAsel3' and 'LAsel4', so the value read back is 0. I think something is going wrong with the initialization of some of the hidden fields.

@blazoncek
Copy link
Collaborator

I can confirm this part of the problem description - only the last output type can be modified, all others are grayed out.

FYI this is now a behaviour to allow correct selection of bus types (i.e. if ESP only has 3 RMT you cannot select more than 3 WS281x outputs or if you select PWM with only 4 LEDC available you will not be able to select PWM+CCT). Switching LED type after it was set up would need to have implementation to account for that. This seemed too complex to me so I opted for locking LED type except for the last one.

softhack007 added a commit that referenced this issue Dec 5, 2024
settings_leds: always initialize current limiter field (partly solves #4280 )
@Ferqli
Copy link

Ferqli commented Dec 13, 2024

I also already experienced this kind of bug, my solution to that was just to update wled to a new version.

@Fonzi03 Fonzi03 closed this as completed Jan 5, 2025
@andrenrwn
Copy link

andrenrwn commented Jan 7, 2025

I can confirm this part of the problem description - only the last output type can be modified, all others are grayed out.

FYI this is now a behaviour to allow correct selection of bus types (i.e. if ESP only has 3 RMT you cannot select more than 3 WS281x outputs or if you select PWM with only 4 LEDC available you will not be able to select PWM+CCT). Switching LED type after it was set up would need to have implementation to account for that. This seemed too complex to me so I opted for locking LED type except for the last one.

This makes it very difficult to deploy and test multiple LEDs and makes the LED settings user interface practically unusable. I have SK6812 7-8 LED strips with different lengths and wattages. It's very onerous to have to delete all the configuration every time and re-add them to make changes to the length or wattage of earlier LED strips once you have configured them all. I imagine this would frustrate a majority of WLED users accustomed to making these changes without having to tear down their entire configuration manually for every little change or fix they add to one of their strips.

The implementation to check and account for the above restrictions should be done on the serverside, not clientside. If a setting violates the restrictions, the serverside should just return an error and refuse to set the entire POST request leds payload.

Is this the changes that was made to limit the LED changes except the last one?
https://github.com/Aircoookie/WLED/blame/2e06f5b1e8dbcb36f8f3634bb601d8fca34a5cb9/wled00/data/settings_leds.htm#L255
0ff4016#diff-138cb5fc9bf0400d2faa6aecd7a41956e8cad2ea491eca232b9c2d01e480807fR129

If we still need to validate it on the clientside as the path of least resistance, perhaps instead of greying out the array of LED settings except the last one, we could just do a custom validator to check the validity of the input form upon submission, i.e

https://stackoverflow.com/questions/21003342/how-do-i-invoke-custom-constraint-validation-as-part-of-the-native-validation-ev
https://jsfiddle.net/lexicality/VK99U/

@DedeHai
Copy link
Collaborator

DedeHai commented Jan 7, 2025

The implementation to check and account for the above restrictions should be done on the serverside, not clientside. If a setting violates the restrictions, the serverside should just return an error and refuse to set the entire POST request leds payload.

if instead of graying out the settings, allowing to change them but then refuse to save is a much worse approach as there is no feedback on why it wont save.
also the hardware setup in normal use of WLED does not change after installation, running tests on many different led strips, constantly changing the hardware is an edge case.

@blazoncek
Copy link
Collaborator

@andrenrwn you are welcome to implement your suggestions, however consider: #4168 #4117 #4310 #4301

Also, I concur with @DedeHai regarding (silently) dropping incorrect configuration and setting up LED configuration is relatively infrequent operation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug confirmed The bug is reproducable and confirmed
Projects
None yet
Development

No branches or pull requests

9 participants