Use live cfg json instead of file for usermod settings page

[willmmiles]

Ensure that the usermods settings page is using a correct and complete set of configuration items for the currently loaded firmware, regardless of what may have been left behind in cfg.json from previous firmware installs.

Implemented by:

• Separating the config the config JSON construction from the filesystem operation;
• Arranging that the json/cfg endpoint uses live constructed JSON output instead of returning the disk file;
• Modifying the usermod settings page to request json/cfg instead.

Reference: #4597

Summary by CodeRabbit

• Refactor
  • Redesigned the system’s configuration persistence workflow to improve modularity and ensure that all settings changes (such as WiFi credentials and presets) are reliably saved.
  • Updated the settings retrieval endpoint to streamline data fetching, enhancing consistency and performance.
  • Refined internal processes governing settings updates to further minimize potential inconsistencies, resulting in a more stable operation.

[coderabbitai]

Walkthrough

The changes refactor the configuration serialization process and update variable names across several modules. The primary adjustment is the renaming of the serialization function from serializeConfig() to serializeConfigToFS() along with adding an overload that accepts a JsonObject. In parallel, the flag variable used to trigger configuration writes is renamed from doSerializeConfig to configNeedsWrite in multiple files. Other adjustments include an update to the URL endpoint in the settings page and a shift from JSON path constants to an enum class for improved type safety in JSON handling.

Changes

Files	Change Summary
wled00/cfg.cpp, wled00/fcn_declare.h, wled00/improv.cpp	- Renamed function serializeConfig() to serializeConfigToFS() and added a new overload serializeConfig(JsonObject root). - Encapsulated file handling within serializeConfigToFS(), updating calls accordingly.
wled00/dmx_input.cpp, wled00/presets.cpp, wled00/set.cpp, wled00/wled.cpp, wled00/wled.h, wled00/wled_server.cpp	- Renamed the flag variable from doSerializeConfig to configNeedsWrite across callbacks, control loops, and global declarations to signal that configuration writing is required.
wled00/data/settings_um.htm	- Updated the fetch URL endpoint from '/cfg.json' to '/json/cfg' within the settings retrieval logic.
wled00/json.cpp	- Replaced JSON path constants with an enum class json_target and updated the conditional logic and switch-case structure, including removal of the dedicated configuration file handling check.

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 86393e0 and 36cb1ca.

📒 Files selected for processing (11)

• wled00/cfg.cpp (3 hunks)
• wled00/data/settings_um.htm (1 hunks)
• wled00/dmx_input.cpp (2 hunks)
• wled00/fcn_declare.h (1 hunks)
• wled00/improv.cpp (1 hunks)
• wled00/json.cpp (2 hunks)
• wled00/presets.cpp (1 hunks)
• wled00/set.cpp (1 hunks)
• wled00/wled.cpp (2 hunks)
• wled00/wled.h (1 hunks)
• wled00/wled_server.cpp (1 hunks)

🧰 Additional context used

🧬 Code Definitions (2)

wled00/improv.cpp (2)

wled00/cfg.cpp (2)

• serializeConfigToFS (691-708)
• serializeConfigToFS (691-691)

wled00/fcn_declare.h (1)

• serializeConfigToFS (31-31)

wled00/wled.cpp (2)

wled00/fcn_declare.h (1)

• serializeConfigToFS (31-31)

wled00/cfg.cpp (2)

• serializeConfigToFS (691-708)
• serializeConfigToFS (691-691)

⏰ Context from checks skipped due to timeout of 90000ms (5)

• GitHub Check: wled_build / Build Environments (esp32_wrover)
• GitHub Check: wled_build / Build Environments (usermods)
• GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
• GitHub Check: wled_build / Build Environments (esp32s3dev_16MB_opi)
• GitHub Check: wled_build / Build Environments (lolin_s2_mini)

🔇 Additional comments (20)

wled00/data/settings_um.htm (1)

16-16: LGTM! API endpoint change for configuration retrieval.

The change updates the fetch URL from /cfg.json to /json/cfg, which aligns with the PR objective of using live constructed JSON instead of a potentially outdated static file. This ensures the usermod settings page always displays the correct configuration for the currently loaded firmware.

wled00/wled.h (1)

880-880: Improved variable naming for better code clarity.

The global variable has been renamed from doSerializeConfig to configNeedsWrite, which better reflects its purpose as a flag that indicates when the configuration needs to be written to storage, rather than the action that will be performed.

wled00/presets.cpp (1)

245-245: LGTM! Consistent variable naming.

Changed from doSerializeConfig = true to configNeedsWrite = true, maintaining consistency with the variable renaming in wled.h.

wled00/improv.cpp (1)

275-275: LGTM! Updated function call for clearer separation of concerns.

Changed from serializeConfig() to serializeConfigToFS() to explicitly indicate that this operation involves filesystem I/O. This change is part of the separation of configuration JSON construction from filesystem operations mentioned in the PR objectives.

wled00/dmx_input.cpp (2)

25-25: Variable renamed for consistency across codebase.

The doSerializeConfig variable has been renamed to configNeedsWrite to provide better clarity on its purpose and maintain consistency with other files in the project.

---

43-43: Variable renamed for consistency across codebase.

The doSerializeConfig variable has been renamed to configNeedsWrite to provide better clarity on its purpose and maintain consistency with other files in the project.

wled00/wled_server.cpp (1)

331-332: Variable renamed for consistency across codebase.

The doSerializeConfig variable has been renamed to configNeedsWrite to better reflect its purpose and maintain consistency with other files. This happens in the JSON configuration handler, properly flagging when new settings need to be saved to the filesystem.

wled00/fcn_declare.h (1)

30-31: Function declaration updated to separate concerns.

The configuration serialization process has been improved by:

1. Modifying serializeConfig to accept a JsonObject parameter
2. Adding a new serializeConfigToFS() function

This change separates the responsibilities between JSON object construction and filesystem operations, which supports the PR's objective of providing live configuration JSON for the usermod settings page.

wled00/wled.cpp (3)

196-196: Variable renamed for consistency across codebase.

The doSerializeConfig variable has been renamed to configNeedsWrite to better reflect its purpose and maintain consistency with the changes in other files.

---

203-203: Updated function call to match new architecture.

Changed to use the new serializeConfigToFS() function which handles writing the configuration to the filesystem, in line with the separation of JSON construction from filesystem operations.

---

226-226: Variable renamed in conditional check.

Updated the reboot condition to use the renamed configNeedsWrite variable. This maintains the same logic - only rebooting when there are no pending configuration writes.

wled00/set.cpp (1)

808-809: Variable rename to better reflect its purpose

The variable doSerializeConfig has been renamed to configNeedsWrite, which more accurately describes that this flag indicates when configuration needs to be written to persistent storage, rather than being a command to perform serialization. This change aligns with the separation of configuration JSON construction from filesystem operations.

wled00/json.cpp (4)

1030-1034: Improved type safety with enum class instead of constants

Converting from numeric constants to an enum class for JSON target types improves type safety and code readability. This makes the code more maintainable by preventing errors from implicit type conversions and clearly documenting the valid target values.

---

1036-1044: Updated URL parsing to use enum values

The URL parsing code has been updated to assign enum values from json_target instead of numeric values, matching the type safety improvement. This change maintains the same functionality while making the code more robust.

---

1066-1066: Updated LockedJsonResponse constructor to use enum type

The LockedJsonResponse constructor now properly uses the enum class for conditional array creation, maintaining type safety throughout the function.

---

1072-1100: Updated switch statement to use enum values and added config serialization

The switch statement now uses enum values from json_target instead of numeric constants, matching the type safety improvements. Most notably, this change includes the addition of the serializeConfig(lDoc) case for the config target, implementing the live construction of JSON configuration instead of serving a static file.

This change is central to the PR's goal of ensuring the usermod settings page uses the current firmware configuration state rather than potentially outdated information from a static file.

wled00/cfg.cpp (4)

674-674: Function call properly renamed.

Updated call to the renamed function to maintain correct behavior after the refactoring.

---

688-688: Function call properly renamed.

Updated call to the renamed function to maintain correct behavior after the refactoring.

---

691-708: Well-structured refactoring for better code organization.

The separation of serialization logic from file I/O operations is a good design improvement. The function has been renamed from serializeConfig() to serializeConfigToFS() to clearly indicate its purpose of writing to the filesystem, while the actual serialization work is delegated to the new serializeConfig(JsonObject) function.

The variable rename from doSerializeConfig to configNeedsWrite also improves code clarity.

---

710-1125: Clean extraction of serialization logic.

The serialization logic has been properly extracted into its own function that accepts a JsonObject parameter. This separation of concerns allows the configuration to be serialized for various use cases (file storage, API endpoints, etc.) without duplicating code.

This change aligns well with the PR objective to provide live constructed JSON output at the /json/cfg endpoint.

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

🪧 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.

[blazoncek]

One thing of note: The original idea for settings rewrite was to abandon config processing using cfg.cpp/xml.cpp and rely on cfg.json parsing in JavaScript. Usermod setting page was the first implementation of that but further development stalled as @Aircoookie couldn't find time to do it.

AFAIK /json/cfg was intended to allow WLED configuration from external sources like LedFx or xLights, etc.

[willmmiles]

One thing of note: The original idea for settings rewrite was to abandon config processing using cfg.cpp/xml.cpp and rely on cfg.json parsing in JavaScript. Usermod setting page was the first implementation of that but further development stalled as @Aircoookie couldn't find time to do it.

Parsing the configuration JSON directly in the settings pages is a good idea. The difference here is only in the source: this PR changes it to come from the live memory state, instead of whatever was last written to the filesystem. Any existing code need only point to /json/cfg to get the live state instead, and it will work. (And of course anyone who wants to read the filesystem state can always continue to use /cfg.json.)

[blazoncek]

Approved from my side.