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

crude hack in presets.cpp #2681

Closed
softhack007 opened this issue May 30, 2022 · 3 comments · Fixed by #2737
Closed

crude hack in presets.cpp #2681

softhack007 opened this issue May 30, 2022 · 3 comments · Fixed by #2737

Comments

@softhack007
Copy link
Collaborator

softhack007 commented May 30, 2022

I just saw that preset.cpp contains a somewhat 'crude hack' - see below.

It might be dangerous:

  • the observation that user code runs on core 1, while system events (like network requests) run on core 0 may not always be true.
  • core allocation can be changed at compile time; try adding -DARDUINO_RUNNING_CORE=0 -DARDUINO_EVENT_RUNNING_CORE=1 in your platformio.ini
  • there are ESP32 chips without second core - both"classic" ESP32 and some of the newer "-C3" and "-S2" variants sometimes have only one single core. On single core, the variable core will always have the value false (i.e. 0).

Please consider if there are other options to (more reliably) detect where a file request came from.

https://github.com/Aircoookie/WLED/blob/19c8b4fe2d0d86887d49abd5c965fd1d46502679/wled00/presets.cpp#L14-L24

@blazoncek
Copy link
Collaborator

The upcoming changes I proposed introduce a asynchronous preset loading which always happens within main loop().
Unfortunately this leads to all sorts of different problems (making some presets with HTTP API no longer work correctly and UI not getting a correct preset ID on return, etc).
If you want to test please use my fork, dev branch.

@softhack007
Copy link
Collaborator Author

softhack007 commented Aug 10, 2022

In SR WLED, we found a solution by

  1. using a patched version of Lorol LittleFS, which is more thread-safe (https://github.com/softhack007/LITTLEFS-threadsafe.git#master)

  2. replacing the "coreID" checks with a check for task names:
    https://github.com/atuline/WLED/blob/153e29b927f2afaec4fcd96f4cbe2c71506eeab0/wled00/presets.cpp#L13-L31

image

Users feedback was very positive after these changes. No more "lost preset" problems seen.

@blazoncek blazoncek linked a pull request Aug 18, 2022 that will close this issue
@softhack007
Copy link
Collaborator Author

Solved in 0_14 branch by using new async presets implementation. Can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants