WLED icon indicating copy to clipboard operation
WLED copied to clipboard

Preset Navigation usermod

Open obar opened this issue 1 year ago • 17 comments

A simple usermod called ~Preset Array~ Preset Navigation (edit: it occurred to me since making the request, this name could be better! And this is my only chance to fix it! Sorry if anyone was in the midst of dealing with this while I made the change). It lets you group effect presets together however you see fit, and cycle between the groups ("banks") without going through every single effect preset you've saved. When you've gotten to the bank you want, you can then cycle through just the presets of that bank.

This is primarily focused for controlling WLED in ways other than the web UI.

Preset banks are one one line of preset IDs, comma separated:

Screenshot 2024-12-06 at 16-04-47 Usermod Settings

Cycling between banks in this example takes you through preset 10, 20, 30, 40, then back to 10. If you are at 30, cycling within the bank takes you to 31, 32, 33, and then back to 30.

For a specific example: presets in a bank can have any logical connection you want, in this video demo of the usermod I show how the third bank (presets 30,31,32,33) are all different color variations of a flame effect. When cycling between banks you don't have to cycle through every color variation if you aren't going for flame. When you do get to that flame effect, you can cycle just within that bank through the color variations.

Another benefit of this usermod over normal preset cycling is that it isn't sequence-sensitive. I've used a numerical pattern above but a new preset can be squeezed in anywhere in the cycle, whereas the typical preset cycling API call needs preset IDs to be in order without gaps.

This usermod uses a textarea in the config for multiline input, so it depends on #4217

Summary by CodeRabbit

  • New Features
    • Introduced a "Preset Navigation" usermod for organizing effect presets into user-defined banks and navigating them easily, ideal for non-touchscreen interfaces.
    • Users can cycle through banks and presets with both wrapping and non-wrapping commands via the JSON API.
    • Supports configuration of banks and presets, with customizable limits.
  • Documentation
    • Added detailed usage instructions and configuration guidelines for the new "Preset Navigation" usermod.

obar avatar Dec 06 '24 21:12 obar

Ninja edit: Preset Navigation is a better name for this. Updated accordingly but the usermod is the same as described.

obar avatar Dec 07 '24 04:12 obar

IMO this is why playlists exist.

blazoncek avatar Dec 07 '24 15:12 blazoncek

Thanks for mentioning it. Can you say more about that? It wouldn't be the first time my mental model about WLED would be wrong ;)

obar avatar Dec 07 '24 21:12 obar

@obar is this a revival of the "textarea" mod https://github.com/Aircoookie/WLED/pull/4217 ? I think we already decided to not add large textarea support.

softhack007 avatar Dec 11 '24 21:12 softhack007

@softhack007 Not a revival of anything, it's a feature to let a user create a menu of many presets.

There were concerns about textareas in the usermods page initially voiced but they were all addressed, and nobody has suggested any further issues since.

obar avatar Dec 11 '24 22:12 obar

Handling presets is not a thing usermod would need to be invented for IMO. If anything it should be a core feature.

blazoncek avatar Dec 12 '24 05:12 blazoncek

I could adapt most of this code for a core feature.

If WLED were desktop-first, I'd think about a drag-and-drop reordering interface for the preset cards themselves... but that demands a lot of screen real estate, and drag-to-reorder implementations are only barely good enough for a single list on mobile—this is much more complicated as a list of lists . A text area was the simplest idea that I could think up. An enhancement might be a JS parser to check for properly formed input/no undefined preset IDs.

Should a text area like this live in a heading under Macros, or would it be a better fit under User Interface?

obar avatar Dec 13 '24 02:12 obar

I think the utility is there for the users who change their WLED effects with buttons, a remote control, an encoder, or other interesting interfaces that aren't web. Not every user but also not the tiniest minority either. Fortunately it won't add much to the binary.

For now, how about I do this: text area for a preset array under Macros (because it's fundamentally a macro functionality), no JS input checking but still use the C++ input checking shown in my branch, and write it up for the docs. In the future this feature can get fancier as needed, or even move places.

Sounds good?

obar avatar Dec 18 '24 03:12 obar

I like the idea, I just fail see the difference between this and a playlist. can you elaborate?

DedeHai avatar Dec 18 '24 06:12 DedeHai

All of the OP requests can (as already mentioned) be achieved by carefully crafted presets, playlists and button/IR assignments. Please see KB and sources (json.cpp) as KB may lack complete details for information on JSON API. I see no benefit in implementing this PR.

blazoncek avatar Dec 18 '24 07:12 blazoncek

Sorry team, I do try to look at internet messages generously but I'm struggling to view this discussion and the related one on textareas as something other than animosity. I've answered challenge questions to no response, and I've asked questions that sometimes don't get a response either. I feel like I've caught some snark, but it might just be cultural differences in communication. In the defense of you all, obviously this isn't the only issue or feature being discussed or worked on, and I know you all do a lot to support complete strangers with projects: often with the very basics of how to wire things up. That's incredibly generous. Just noting how this specific discussion feels to me. I did ask how this feature is the same as playlists when it was brought up. The reply I eventually got to that question was that preset handling should be a core feature rather than a usermod, not that it already exists as a core feature. That's why I went forward with asking how this could best fit in.

WLED is an open source project of course, and nobody owes anyone anything in open source, but perhaps if someone looks like they're willing to be a new contributor it's worth meeting them with extra kindness and patience, understanding that they aren't as knowledgeable about the code base as seasoned contributors. Anyway, that's a side suggestion and I'd be excited to put it behind, start fresh.


@dedehai that's a great way to put it in a question. My understanding of the distinctions follow.

A playlist is a 1D list, and you can navigate it in one direction: "np": "t". This proposed interface is a 2D list-of-lists, where you can navigate in 4 directions:

"pn": "b+"
"pn": "b-"
"pn": "p+"
"pn": "p-"

A playlist is composed in a largely linear way: it's possible to delete or change single entries anywhere in the list, but there isn't rearranging and you can only insert at the end, not at the beginning or middle. This proposed interface makes it very easy to rearrange the banks and the presets within the banks.

I believe most people use a playlist to automatically cycle their LEDs through a set of effects. This interface lets you build a menu of effects where the input is likely tactile and the graphical feedback of where you are in the menu is the LED state.

To me they aren't the same feature. But as I said before: I'd love to hear more, my understanding could be flawed. Thanks for your time!

obar avatar Dec 19 '24 17:12 obar

Hi @obar,

I think the key contentions are:

  • We feel that there should be one and exactly one abstraction in WLED for defining a list of presets: a playlist. We do not want a second interface for entering a list of presets. As I understand things, you've identified two weaknesses in the playlist implementation as they pertain to your objectives:

    • There's no "previous entry in playlist" operation. I think this is also a weakness and I'd be happy to consider a patch that adds this feature; it does need some thought as to how to handle shuffling, but that's a solvable problem. Note that the 'next preset in playlist' API feature is itself quite recent (#3946)!
    • The web UI doesn't provide support for easily re-ordering entries in a playlist. This is a definite gap. I have a vague recollection that someone else had put some effort towards this feature, though I can't find the reference here. This would also be a valuable feature in and of itself.
  • Playlists are, themselves, presets. We feel there should be one and exactly one abstraction in WLED for defining a list of presets: a playlist. It is my understanding that a playlist that is a "list of playlists" can already be expressed in the presets.json file. Again, though, there are indeed missing features to get to the behaviour you'd like:

    • Playlists would need to be able to remember their previous state, eg. that preset 17 last loaded index 5 of its playlist. I think this would also be a useful feature everywhere, independent of your specific use case. I'd probably suggest making it opt-in for each playlist ('resume when reloaded', etc.). Once this is available, "ps": 17, "np": "t" could be composed to implement "next bank" as you describe.
    • There is no web UI support for constructing a playlist of playlists (as it wouldn't be very useful right now). With the above features, this could be unlocked.
    • Some care must be taken to clarify that the current implementation would support only the "innermost" playlist's auto-cycling behaviour -- the timing properties of meta-playlists would be ignored. A "playlist stack" feature could also be implemented, though it probably isn't necessary for your use case.

The interesting point to me is that each one of those features would be a valuable addition on its own -- and then together they combine to make something even greater.

I'm sorry it's been a challenge to get a clear direction - we're all working on this in our spare time. Does this provide more clarity as to a path forward?

willmmiles avatar Dec 19 '24 22:12 willmmiles

Thanks for clarifying some things @willmmiles. I think the biggest point that hadn't been conveyed to me was this:

We feel that there should be one and exactly one abstraction in WLED for defining a list of presets: a playlist.

So it's not so much that what I was proposing === playlists, but rather that because it contains multiple presets it should be a playlist.

You've outlined a great list of changes that need to happen to generalize playlists further, and would allow this interface proposal to be reproduced in the playlists model.

obar avatar Dec 19 '24 23:12 obar

@obar your contribution is much appreciated, maybe there was some misunderstanding/communication breakdown. Regarding this UM vs playlists I agree with @willmmiles that instead of adding a new feature on top to patch current shortcomings it would be better to improve the playlists and preset handling in general towards a more "natural" user interaction, which I believe should not be editing fields of numbers but rather moving items in a list. Your approach however does a great job in showing the benefits of improved preset handling, the option to re-order or make nested playlists.

DedeHai avatar Dec 20 '24 06:12 DedeHai

  • There is no web UI support for constructing a playlist of playlists (as it wouldn't be very useful right now). With the above features, this could be unlocked.

I already made that. The problem, as you already mentioned, is how long does a nested playlist run? Until it is finished or until parent timer runs out? Currently implemented as a 1st option.

blazoncek avatar Dec 20 '24 06:12 blazoncek

Hey! This pull request has been open for quite some time without any new comments now. It will be closed automatically in a week if no further activity occurs. Thank you for contributing to WLED! ❤️

github-actions[bot] avatar Apr 19 '25 12:04 github-actions[bot]

Walkthrough

This update introduces a new usermod called "Preset Navigation" for the WLED project. The core logic is implemented in a new header file that defines a class for managing navigation through user-defined banks of effect presets, with support for JSON API commands and configuration persistence. Documentation for the usermod is provided in a new README file, detailing setup, usage, and configuration instructions. Integration with the WLED usermod system is achieved by conditionally including and registering the new usermod in the usermods list source file, based on a build flag.

Changes

File(s) Change Summary
usermods/preset_navigation/preset_navigation.h Added a new header file defining the UsermodPresetNavigation class, which manages preset navigation via banks and supports JSON-based configuration and state commands. Implements methods for cycling through banks and presets, parsing configuration, and applying presets.
usermods/preset_navigation/readme.md Added documentation for the Preset Navigation usermod, including setup instructions, usage details, API commands, and configuration guidance. No code or public entity changes.
wled00/usermods_list.cpp Modified to conditionally include and register the new UsermodPresetNavigation usermod when the USERMOD_PRESET_NAVIGATION macro is defined. This includes a new header inclusion and an instantiation/registration line in the usermod registration function.
✨ Finishing Touches
  • [ ] 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar Apr 19 '25 12:04 coderabbitai[bot]

Hey! This pull request has been open for quite some time without any new comments now. It will be closed automatically in a week if no further activity occurs. Thank you for contributing to WLED! ❤️

github-actions[bot] avatar Aug 18 '25 12:08 github-actions[bot]