Skip to content

Encapsulate 10 more read-only-from-outside Tier-2 globals - #5791

Open
netmindz wants to merge 1 commit into
wled:mainfrom
netmindz:refactor/tier2-batch-accessors
Open

Encapsulate 10 more read-only-from-outside Tier-2 globals#5791
netmindz wants to merge 1 commit into
wled:mainfrom
netmindz:refactor/tier2-batch-accessors

Conversation

@netmindz

@netmindz netmindz commented Aug 8, 2026

Copy link
Copy Markdown
Member

Summary

Batch of the remaining confirmed "Do" items from the Tier-2 pass (see #5777-#5790, #12 for earlier ones). Each of these 10 was verified to be read-only from every file except its owner (checked both core files and usermods), so each gets a by-value getter rather than a mutable reference — an accidental external write is now a build error instead of a silent bug, not just a rename.

Global Owner Read-only consumer Accessor
selectedWiFi wled.cpp network.cpp getSelectedWiFi()
statusESPNow wled.cpp udp.cpp getStatusESPNow()
serialCanRX wled.cpp wled_serial.cpp isSerialCanRX()
ntpLocalPort wled.cpp set.cpp getNtpLocalPort() — unusual: nothing in the codebase writes it beyond its default, so it's a static constexpr rather than a mutable static
doCloseFile file.cpp wled.cpp isDoCloseFile()
currentLedmap FX_fcn.cpp json.cpp getCurrentLedmap()
hueError hue.cpp xml.cpp getHueError()
nightlightDelayMs led.cpp json.cpp getNightlightDelayMs()
ledMaps util.cpp json.cpp getLedMaps() (kept the existing WLED_MAX_LEDMAPS>16 uint32_t/uint16_t type split)
improvError improv.cpp wled.cpp getImprovError()

No behavior change — purely a storage/access-pattern change. Bundled as one PR since each is a small, independent, low-risk single-getter change of the same shape — not worth 10 separate review cycles.

Test plan

  • esp32dev and nodemcuv2: both build and link cleanly via pio run.
  • usermods env (builds all 59 usermods): builds and links cleanly via pio run -e usermods — confirms none of these 10 have an external usermod consumer that would have broken (two earlier PRs in this series each found one that did).
  • Repo-wide grep confirms no remaining raw references to any of the 10 identifiers outside their owning file.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Improvements
    • Improved runtime status reporting for LED maps, nightlight timing, Wi-Fi, NTP, serial connectivity, ESP-NOW, Hue, and Improv.
    • Enhanced LED-map information and active-map reporting.
    • Improved handling of network, serial, file, and notification state checks.
    • Preserved support for configurations with larger numbers of LED maps.

Batch of the remaining confirmed "Do" items from the Tier-2 pass: each
was verified to be read-only from every file except its owner (core
files and usermods both checked), so each gets a by-value getter
rather than a mutable reference - an accidental external write is now
a build error instead of a silent bug.

- selectedWiFi   (wled.cpp owns)      -> getSelectedWiFi(),  read-only in network.cpp
- statusESPNow   (wled.cpp owns)      -> getStatusESPNow(),  read-only in udp.cpp
- serialCanRX    (wled.cpp owns)      -> isSerialCanRX(),    read-only in wled_serial.cpp
- ntpLocalPort   (wled.cpp owns)      -> getNtpLocalPort(),  read-only in set.cpp - unusual
  case: nothing in the codebase writes it beyond its default, so it's a
  static constexpr instead of a mutable static
- doCloseFile      (file.cpp owns)    -> isDoCloseFile(),    read-only in wled.cpp
- currentLedmap    (FX_fcn.cpp owns)  -> getCurrentLedmap(), read-only in json.cpp
- hueError         (hue.cpp owns)     -> getHueError(),      read-only in xml.cpp
- nightlightDelayMs(led.cpp owns)     -> getNightlightDelayMs(), read-only in json.cpp
- ledMaps          (util.cpp owns)    -> getLedMaps(),       read-only in json.cpp
  (kept the existing WLED_MAX_LEDMAPS>16 uint32_t/uint16_t type split)
- improvError      (improv.cpp owns)  -> getImprovError(),   read-only in wled.cpp

No behavior change - purely a storage/access-pattern change. Bundled as
one PR since each is a small, independent, low-risk single-getter
change of the same shape - not worth 10 separate review cycles.

Verified:
- esp32dev and nodemcuv2 both build and link cleanly via `pio run`.
- usermods env (builds all 59 usermods) builds and links cleanly via
  `pio run -e usermods` - confirms none of these 10 have an external
  usermod consumer that would have broken.
- Repo-wide grep confirms no remaining raw references to any of the
  10 identifiers outside their owning file.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

WLED runtime globals were moved from public header declarations to file-private state with read-only accessors. JSON, network, serial, ESP-NOW, file, Improv, and Hue code now use those accessors.

Changes

Runtime state encapsulation

Layer / File(s) Summary
Public accessor contract
wled00/fcn_declare.h, wled00/wled.h
Added accessor declarations and removed public declarations for implementation-specific runtime state.
Private state and accessors
wled00/FX_fcn.cpp, wled00/file.cpp, wled00/hue.cpp, wled00/improv.cpp, wled00/led.cpp, wled00/util.cpp, wled00/wled.cpp
Added file-private state and read-only accessors, including conditional LED-map storage and accessors.
Accessor call-site migration
wled00/json.cpp, wled00/network.cpp, wled00/set.cpp, wled00/udp.cpp, wled00/wled.cpp, wled00/wled_serial.cpp, wled00/xml.cpp
Updated status serialization and runtime checks to use the new accessors.

Estimated code review effort: 2 (Simple) | ~15 minutes

Possibly related PRs

Suggested reviewers: softhack007, dedehai

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: encapsulating 10 Tier-2 globals behind read-only getter functions.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@wled00/wled.h`:
- Line 345: Document the intentional breaking API change for out-of-tree
usermods caused by removing the listed globals from wled.h. Add migration
guidance directing callers to getSelectedWiFi(), isSerialCanRX(),
getStatusESPNow(), getNtpLocalPort(), isDoCloseFile(), getCurrentLedmap(),
getHueError(), getImprovError(), getNightlightDelayMs(), and getLedMaps(), or
explicitly state that existing consumers must port to these replacement getters.
🪄 Autofix

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 Plus

Run ID: 9c2b6212-8c21-47cd-9db9-89752d8046fa

📥 Commits

Reviewing files that changed from the base of the PR and between 0869ef4 and b436898.

📒 Files selected for processing (15)
  • wled00/FX_fcn.cpp
  • wled00/fcn_declare.h
  • wled00/file.cpp
  • wled00/hue.cpp
  • wled00/improv.cpp
  • wled00/json.cpp
  • wled00/led.cpp
  • wled00/network.cpp
  • wled00/set.cpp
  • wled00/udp.cpp
  • wled00/util.cpp
  • wled00/wled.cpp
  • wled00/wled.h
  • wled00/wled_serial.cpp
  • wled00/xml.cpp

Comment thread wled00/wled.h
@netmindz
netmindz requested a review from willmmiles August 8, 2026 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants