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

Inconsistent preset cycling behavior between web GUI and physical buttons #4154

Closed
1 task done
timnehring opened this issue Sep 22, 2024 · 6 comments
Closed
1 task done
Labels
bug fixed in source This issue is unsolved in the latest release but fixed in master

Comments

@timnehring
Copy link

What happened?

The preset behavior for cycling presets seems to be different for triggering presets via the web GUI versus using a physical button/ touch sensor. The state of the currently running preset doesn't seem to be updated correctly, if triggered via a physical button.

If I trigger the cycling preset via web GUI or physical button it goes 1-2-3-4-1-2-3-4… as expected.
The problem arises when triggering presets directly and then go back to using the preset for cycling.

Via GUI if I cycle to preset 3 and then hit preset 1, cycling again would put preset 2 next (1-2-3-1-2).
If I do the same via physical button after activating preset 1 via long press, cycling via short press continues at preset 4 and not 2 (1-2-3-1-4).

To Reproduce Bug

Create 5 presets in total.
Presets 1-4: solid colors to tell them apart
Preset 5: API Call to cycle through presets 1-4 ({"ps":"1~ 4~"})
The physical button should be configured to trigger preset 5 on short press and trigger preset 1 on long press.

Expected Behavior

I would expect both ways to trigger presets to have the same outcome.

Triggering a preset directly and then using the cycling preset again should continue from the currently selected preset and not jump to the next one from the last time the cycling preset got triggered.

Install Method

Binary from WLED.me

What version of WLED?

WLED 0.15.0-b5 (build 2409100)

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
@timnehring timnehring added the bug label Sep 22, 2024
blazoncek added a commit that referenced this issue Oct 7, 2024
@blazoncek blazoncek added the fixed in source This issue is unsolved in the latest release but fixed in master label Oct 7, 2024
@timnehring
Copy link
Author

@blazoncek I just tested the changes you made and now the cycling isn't working at all. The Web GUI part works as before, but cycling through the presets via touch sensor isn't working anymore. It just keeps activating the preset with the id 1. I made sure that the touch gets registered every time via a print message.

Here is the serial monitor output when I trigger preset 5 via touch button on short press:

-> SHORT PRESS
Playlist unloaded.
Request to apply preset: 5
JSON buffer locked. (9)
Applying preset: 5
Playlist unloaded.
Request to apply preset: 1
JSON buffer released. (9)
JSON buffer locked. (9)
Applying preset: 1
JSON buffer released. (9)
JSON buffer locked. (12)
JSON buffer size: 2633 for WS request (1505).
heap 177552
Sending WS data to multiple clients.
JSON buffer released. (12)

Another touch shows shows this output:

-> SHORT PRESS
Playlist unloaded.
Request to apply preset: 5
JSON buffer locked. (9)
Applying preset: 5
JSON buffer released. (9)

Touches after that just alternates between the outputs above.

To me it seems as if presetCycCurr should be updated in json.cpp before it, since we have access to presetId there. But I only have limited understanding of C++ and the code base, so that's more like a wild guess.

If you need someone to test further changes before commiting, I would be happy to test them.

@blazoncek
Copy link
Collaborator

I found this comment in the code:
remove load request for presets to prevent recursive crash (if not called by button and contains preset cycling string "1~5~")

It seems like handling preset cycling from buttons presents an unsolvable issue ATM.

@blazoncek blazoncek reopened this Oct 9, 2024
@timnehring
Copy link
Author

Thanks for having another look at it.

I had been trying to fix this one by myself and came up with something that worked out. But the downside is, that it wasn't just called for button presses, but also for GUI presses. That would need some more refining, but maybe it could be a workaround.

Is there a save way to know in deserializeState that it was called from via button press?

@blazoncek
Copy link
Collaborator

I think the main issue is recursive calls to apply preset and asynchronous preset handling.

@blazoncek
Copy link
Collaborator

I've tested my latest change and it behaves as expected. I.e. when I click in the UI on preset (98) with {"ps":"1~10~"} it will cycle between presets 1 and 10, if I do the same with a button press (button 0 in my case; short press assigned to 98) each press will advance preset by 1 wrapping at 10 (between 1 and 10).

I am using a slightly modified development version though. When I get the chance I'll downgrade to plain 0_15 and retest.

softhack007 pushed a commit to MoonModules/WLED that referenced this issue Oct 11, 2024
blazoncek added a commit that referenced this issue Oct 13, 2024
@blazoncek
Copy link
Collaborator

It appears to be working correctly so I am closing as completed. If you still experience issues you can reopen. In such case, please use debug build and post serial traces as I added two additional debug prints to help with diagnosis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fixed in source This issue is unsolved in the latest release but fixed in master
Projects
None yet
Development

No branches or pull requests

2 participants