Adds an API parameter to allow the user to skip to the next preset in a playlist at any time

[freakintoddles2]

Why is this needed?

There is no good way to create a playlist, and then skip to the next preset in that playlist whenever you'd like, especially if you ever need to modify that playlist. This would be nice to have especially when building a fixture that will primarily be controlled by a remote control (IR/wizmote) since it will make it easy to have a simple button to skip through a selection of presets in a playlist.

Many users have expressed an interest in this and so I decided to implement it as my first PR to WLED

Updates made

Initially I created a separate function in playlist.cpp which was essentially a copy of most of the code inside handlePlaylist() but then I realized I could just update handlePlaylist() to accept an optional boolean parameter skipNext which would then trigger the existing code to go to the next preset within handlePlaylist().

This reduced the overall code changes and I think provides a more elegant solution since it makes it compatible with any playlist even if the playlist uses shuffle mode.

I also added a new API command np (stands for Next Preset) to both the JSON and HTTP APIs (http is important IMO because it's simpler for novice/non-coders to understand and use).

[freakintoddles2]

Not sure if I need to do anything to indicate this, but it's ready to be re-reviewed with the suggested changes.

[blazoncek]

You mentioned that the "fix" was simple. This take is far from it. 😄

My suggestion:

• just add JSON (and perhaps HTTP) and set "skip to next" variable (bool)
• in handlePlaylist() read that variable and unset (false) it when moving to next item

That's 4 files changed: wled.h, set.cpp, json.cpp and plalist.cpp. Nothing else.
handlePlaylist() is executed in each iteration of loop().

Once you've done that follow: https://github.com/Aircoookie/WLED/wiki/How-to-properly-submit-a-PR

[freakintoddles2]

You mentioned that the "fix" was simple. This take is far from it. 😄

My suggestion:

• just add JSON (and perhaps HTTP) and set "skip to next" variable (bool)
• in handlePlaylist() heed that variable and unset (false) it when moving to next item

That's 4 files changed: wled.h, set.cpp, json.cpp and plalist.cpp. Nothing else. handlePlaylist() is executed in each iteration of loop().

Once you've done that follow: https://github.com/Aircoookie/WLED/wiki/How-to-properly-submit-a-PR

@blazoncek I think my original PR is along the lines of what you are suggesting and only touched four files (I was following your guidance from Discord) but then I changed the approach based on the feedback from softhack above. Is there some version of what you are saying that addresses his concerns?

[blazoncek]

@blazoncek I think my original PR is along the lines of what you are suggesting and only touched four files (I was following your guidance from Discord) but then I changed the approach based on the feedback from softhack above. Is there some version of what you are saying that addresses his concerns?

Unfortunately that's not the case. Json.cpp and set.cpp only need to set global doAdvancePlaylist and there is no need to add parameter to handlePlaylist() as you can access globals from within.

What @softhack007 said was that you must not call handlePlaylist() from within json.cpp as that is invoked from async web callback. That is also what I am saying above - let the flow continue, don't interrupt it.

Example:
json.cpp

doAdvancePlaylist = root[”np”].as<bool>();

handlePlaylist()

if (millis()-presetCycleTime > ... || doAdvancePlaylist) {
  doAdvancePlaylist = false;
...
}

And that's all there is to it.

[softhack007]

deserializeState(...)

doAdvancePlaylist = root[”np”].as<bool>();

handlePlaylist()

if (millis()-presetCycleTime > ... || doAdvancePlaylist) {
  doAdvancePlaylist = false;
...
}

And that's all there is to it.

I fully agree - that's what I had in mind, too.

[freakintoddles2]

ok thanks I'll update and resubmit once I get a chance.

[freakintoddles2]

@blazoncek @softhack007 ok I believe this is now updated with all requested changes. I read through https://github.com/Aircoookie/WLED/wiki/How-to-properly-submit-a-PR and figured out how to push changes from platform.io into this PR, but I just can't tell if I've missed any instructions. Please forgive any mistakes as this is my first attempt submitting a PR to WLED.

[blazoncek]

Your solution will achieve that doAdvancePlaylist will not become false but IMO it is not the the best way.
Overloaded operator | is technically the same as if (!root["np"].isNull())) doAdvancePlaylist = root["np"];

[softhack007]

Looks good for me now 👍

[blazoncek]

Looks good to me too.
I'm sorry this took so many iterations, but I think it was worth it.

[blazoncek]

@freakintoddles2 if you would be so kind, could you update WLED Docs, as well? Please.

[freakintoddles2]

@freakintoddles2 if you would be so kind, could you update WLED Docs, as well? Please.

Sure - I'll look at how to do that next