Extended Wordclock#5586
Open
Zippochonda wants to merge 13 commits intowled:mainfrom
Open
Conversation
Added a Link to the source of the SVG
Mapping wrong at 11x11 ---> eins
Added map for a 2d Matrix in Meanderwiring
Added meander wiring for a 2d Matrix 11x11
Add the Status of the usermod in JSON, so that you can switch off and on the Usermod remotely for Example with Home Assistant
Added the integration into Homeassist
Added HA Configuration
Added a new configuration option for meander wiring and updated documentation to reflect changes.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe Word Clock usermod now supports 11x11 grid wiring modes alongside existing configurations. New LED mask lookup tables enable four wiring combinations (normal, meander, 11x11, 11x11-meander). Display logic branches on configuration flags to select appropriate tables. Config serialization restructures state persistence to use nested JSON objects with ChangesWord Clock 11x11 Grid & Meander Wiring
🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
usermods/usermod_v2_word_clock/usermod_v2_word_clock.cpp (2)
459-463: ⚡ Quick winMove repeated JSON keys to
PROGMEMconstants and avoid RAM string literals.Line 460/471/474 adds repeated raw key strings (
"WordClockUsermod","active"). In usermods, these should be sharedstatic const char[] PROGMEMkeys (used viaFPSTR/F) to reduce RAM pressure.Suggested change
+ static const char kWordClockUsermod[] PROGMEM; + static const char kActive[] PROGMEM; ... - JsonObject usermodJson = root.createNestedObject("WordClockUsermod"); - usermodJson["active"] = usermodActive; + JsonObject usermodJson = root.createNestedObject(FPSTR(kWordClockUsermod)); + usermodJson[FPSTR(kActive)] = usermodActive; ... - JsonObject usermodJson = root["WordClockUsermod"]; + JsonObject usermodJson = root[FPSTR(kWordClockUsermod)]; ... - if (usermodJson.containsKey("active")) { - usermodActive = usermodJson["active"].as<bool>(); + if (usermodJson.containsKey(FPSTR(kActive))) { + usermodActive = usermodJson[FPSTR(kActive)].as<bool>();As per coding guidelines: "
usermods/**/*.{cpp,h,hpp}: Store repeated strings asstatic const char[] PROGMEMin usermods" and "UseF(\"string\")for string constants in C++ to save RAM on ESP8266".Also applies to: 471-477
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@usermods/usermod_v2_word_clock/usermod_v2_word_clock.cpp` around lines 459 - 463, The JSON keys "WordClockUsermod" and "active" are repeated as RAM string literals when creating usermodJson (root.createNestedObject) and assigning usermodJson["active"] = usermodActive; replace these raw literals with shared PROGMEM string constants (e.g. static const char[] PROGMEM) and use FPSTR(...) or F("...") when passing them to createNestedObject and JsonObject access to avoid RAM usage; update all occurrences (including the other instances around lines where usermodJson is built/updated) to reference the new constants.
30-38: ⚡ Quick winReplace new mask-size macros with typed
constexprconstants.The added
#definemask sizes on Line 30 and Line 33-38 should bestatic constexprto keep type safety and scope local to the class.Suggested change
- `#define` maskSizeLeds 121 - `#define` maskSizeMinutes11 12 - `#define` maskSizeMinutes11Mea 12 - `#define` maskSizeHours11 6 - `#define` maskSizeHours11Mea 6 + static constexpr uint16_t maskSizeLeds = 121; + static constexpr uint8_t maskSizeMinutes11 = 12; + static constexpr uint8_t maskSizeMinutes11Mea = 12; + static constexpr uint8_t maskSizeHours11 = 6; + static constexpr uint8_t maskSizeHours11Mea = 6;As per coding guidelines: "Prefer
constexprover#definefor compile-time constants in C++".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@usermods/usermod_v2_word_clock/usermod_v2_word_clock.cpp` around lines 30 - 38, Replace the preprocessor macros (maskSizeLeds, maskSizeMinutes, maskSizeMinutesMea, maskSizeMinutes11, maskSizeMinutes11Mea, maskSizeHours, maskSizeHoursMea, maskSizeHours11, maskSizeHours11Mea) with typed constexpr variables (e.g., static constexpr std::size_t) declared in the appropriate scope (class or anonymous namespace) so they are type-safe and have limited scope; set maskSizeLeds to 121 and the minute/hour masks to 12/6 respectively and ensure they are marked static constexpr to keep linkage local.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@usermods/usermod_v2_word_clock/readme.md`:
- Line 8: The README contains spelling/parameter typos: rename the parameter
label "diplayItIs" to "displayItIs" and correct the words "integreated" to
"integrated" and "Grafics" to "Graphics" in the document so users won't copy
incorrect names; update every occurrence (e.g., the README entries around the
"displayItIs" description and the section headers/text mentioning "integreated"
and "Grafics") to the corrected spellings to keep docs and config names
consistent.
- Around line 55-56: The fenced code block starting with the line "# 1. REST
command to send on/off commands to the WLED usermod" should include a YAML
language tag; update the opening fence from ``` to ```yaml so the block is
```yaml followed by that header line, ensuring proper rendering and lint
compliance in readme.md.
In `@usermods/usermod_v2_word_clock/usermod_v2_word_clock.cpp`:
- Around line 559-560: The code now reads the new key F("meanderWiring") but has
no fallback for older configs; update the load so getJsonValue is attempted
against the new key and, if that fails or is missing, fall back to the original
key (e.g., F("meander") or the old name used previously) when populating the
meander variable — use the same pattern around top[...] and getJsonValue so
configComplete is combined correctly; reference getJsonValue, top,
F("meanderWiring"), and the meander variable to locate where to add the fallback
read and preserve backward compatibility.
---
Nitpick comments:
In `@usermods/usermod_v2_word_clock/usermod_v2_word_clock.cpp`:
- Around line 459-463: The JSON keys "WordClockUsermod" and "active" are
repeated as RAM string literals when creating usermodJson
(root.createNestedObject) and assigning usermodJson["active"] = usermodActive;
replace these raw literals with shared PROGMEM string constants (e.g. static
const char[] PROGMEM) and use FPSTR(...) or F("...") when passing them to
createNestedObject and JsonObject access to avoid RAM usage; update all
occurrences (including the other instances around lines where usermodJson is
built/updated) to reference the new constants.
- Around line 30-38: Replace the preprocessor macros (maskSizeLeds,
maskSizeMinutes, maskSizeMinutesMea, maskSizeMinutes11, maskSizeMinutes11Mea,
maskSizeHours, maskSizeHoursMea, maskSizeHours11, maskSizeHours11Mea) with typed
constexpr variables (e.g., static constexpr std::size_t) declared in the
appropriate scope (class or anonymous namespace) so they are type-safe and have
limited scope; set maskSizeLeds to 121 and the minute/hour masks to 12/6
respectively and ensure they are marked static constexpr to keep linkage local.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 18ad6de6-2a54-4793-98c6-497e9302c684
⛔ Files ignored due to path filters (1)
usermods/usermod_v2_word_clock/de_DE_s1.svgis excluded by!**/*.svg
📒 Files selected for processing (2)
usermods/usermod_v2_word_clock/readme.mdusermods/usermod_v2_word_clock/usermod_v2_word_clock.cpp
| There are 3 parameters that control behavior: | ||
|
|
||
| active: enable/disable usermod | ||
| diplayItIs: enable/disable display of "Es ist" on the clock |
Contributor
There was a problem hiding this comment.
Fix parameter/name and spelling typos in docs.
Line 8 (diplayItIs) and Line 23 (integreated, Grafics) should be corrected to avoid copy/paste mistakes in user setups.
Also applies to: 23-23
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@usermods/usermod_v2_word_clock/readme.md` at line 8, The README contains
spelling/parameter typos: rename the parameter label "diplayItIs" to
"displayItIs" and correct the words "integreated" to "integrated" and "Grafics"
to "Graphics" in the document so users won't copy incorrect names; update every
occurrence (e.g., the README entries around the "displayItIs" description and
the section headers/text mentioning "integreated" and "Grafics") to the
corrected spellings to keep docs and config names consistent.
Corrected typos and improved clarity in readme.
Added backward compatibility for meander wiring configuration.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extend the wordclock to 11X11 LEDs
Summary by CodeRabbit
New Features
Documentation