Skip to content

Clean up global variables namespace, save a few 100 bytes of flash#5368

Merged
softhack007 merged 6 commits intomainfrom
fix_variable_scope
Feb 11, 2026
Merged

Clean up global variables namespace, save a few 100 bytes of flash#5368
softhack007 merged 6 commits intomainfrom
fix_variable_scope

Conversation

@softhack007
Copy link
Member

@softhack007 softhack007 commented Feb 11, 2026

Reduces the scope of some variables and functions to "static" - if not used anywhere else.

Making them static avoids name conflicts, cleans up the global scope and allows for better optimization by the compiler.
We also save a few bytes, because the compiler does not need to keep copies of previously global functions.

Summary by CodeRabbit

  • Refactor
    • Tightened internal scope across multiple modules by restricting many globals and helper functions to file-local visibility and removing a few shared declarations.
    • Cleaned up internal helper declarations and loop variable naming to avoid shadowing.
    • No user-facing behavior, features, or public API changes; runtime behavior and error handling remain unchanged.

these are not used anywhere else. Making them static avoid name conflicts, cleans up the global scope and in some cases allows for better  optimization by the compiler.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 11, 2026

Walkthrough

Restricts linkage by converting many globals and helper functions to internal (static), adds local forward declarations, removes a few extern declarations, and renames some loop indices; no algorithmic or control-flow changes.

Changes

Cohort / File(s) Summary
Bus manager
wled00/bus_manager.cpp
Removed three extern declarations: cmDNS[], cctICused, useParallelI2S; made several small helper bit-manipulation functions static inline.
Global state -> static
wled00/ir.cpp, wled00/ntp.cpp, wled00/wled_serial.cpp, wled00/ws.cpp
Converted multiple global variables to static internal linkage (IR state, NTP tz/tzCurrent, serial state, websocket live client/time); added/marked internal forward declarations.
Helper functions -> static
wled00/e131.cpp, wled00/FX.cpp, wled00/overlay.cpp, wled00/improv.cpp, wled00/remote.cpp, wled00/udp.cpp, wled00/wled_server.cpp, wled00/xml.cpp, wled00/json.cpp, wled00/ntp.cpp, wled00/ws.cpp
Added/updated forward declarations and marked many helper functions as static (Artnet handlers, FX helpers, overlay helpers, improv WiFi parser, preset helper, TPM2 ack, server edit handler, XML helpers, JSON palette setters, NTP helpers, WS live send).
Header surface reduced
wled00/fcn_declare.h
Removed public declarations for functions now local to their .cpp files and replaced them with commented/local notes.
Usermod extern removed
usermods/Analog_Clock/Analog_Clock.cpp
Removed extern Timezone* tz; declaration.
Loop index rename (shadow avoidance)
wled00/set.cpp, wled00/xml.cpp
Renamed local loop counter i to ii in specific blocks to avoid shadowing; adjusted increments accordingly.
Minor cleanup
wled00/util.cpp, wled00/cfg.cpp
Removed an unused local variable and made an internal utility static inline (getStringFromJson).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • netmindz
  • DedeHai
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.83% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the primary change: reducing global variable scope by marking internal symbols as static to clean up the namespace and save flash memory.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix_variable_scope

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 and usage tips.

@softhack007

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

@softhack007
Copy link
Member Author

@netmindz is there a way to trigger a "manual" run of the usermods CI job on this PR branch? Just to be sure that no usermod breaks due to my changes.

@softhack007

This comment was marked as resolved.

@coderabbitai

This comment was marked as outdated.

@softhack007

This comment was marked as outdated.

@coderabbitai

This comment was marked as outdated.

Safe to make static
* declared in fcn_declare.h, only used locally in one file
* not declared in fcn_declare.h, only used locally
make bit array functions "static inline"
-> better for optimization, saves some bytes because the compiler does not need to preserve a non-inline function copy for external references.
@softhack007
Copy link
Member Author

Flash Size

board before after delta
esp32dev 1295769 1295657 -112
nodemcuv2_160 910111 909731 -380

@softhack007 softhack007 changed the title Clean up global variables namespace Clean up global variables namespace, save a few 100 bytes of flash Feb 11, 2026
@softhack007

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

as suggested by the rabbit.
@softhack007
Copy link
Member Author

Flash Size - update

board before after delta
esp32dev 1295769 1295609 -160
nodemcuv2_160 910111 909619 -492

@softhack007
Copy link
Member Author

@coderabbitai please add the following to you regular review checks:

When "C-style" functions or variables are added or modified, check if they are visible globally. If the function is not part of any global header file, suggest to:

  • either make the function "static" (visible only in one source code file) - possibly with a forward declaration,
  • or add a comment to clarify that the function is intended to be a global function.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 11, 2026

Tip

For best results, initiate chat on the files or code changes.

@softhack007, I've added this check to my regular review guidelines. Going forward, when reviewing code with C-style functions or variables, I will:

  1. Check global visibility: Verify if the function/variable is globally visible (not static)
  2. Check header declarations: Confirm if it's declared in a global header file (like fcn_declare.h)
  3. Suggest improvements if globally visible but not in a header:
    • Recommend making it static (with forward declaration if needed), OR
    • Suggest adding a comment clarifying the intent for global linkage

This will help maintain a clean global namespace and make the codebase more maintainable, as demonstrated by the benefits in this PR (-160 bytes flash on esp32dev, -492 bytes on nodemcuv2_160).

🐰✨
/_/
( o.o )
> ^ <
"Noted for future reviews!"


✏️ Learnings added
Learnt from: softhack007
URL: https://github.com/wled/WLED/pull/5368

Timestamp: 2026-02-11T17:17:04.646Z
Learning: In WLED code reviews, when C-style functions or variables are added or modified, check if they are globally visible (not static). If the function/variable is not declared in any global header file (like fcn_declare.h), suggest either: (1) making it static (visible only within the translation unit) with a forward declaration if needed, or (2) adding a comment to clarify that the function/variable is intentionally global. This helps reduce global namespace pollution and enables better compiler optimizations.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5293
File: usermods/user_fx/user_fx.cpp:167-171
Timestamp: 2026-01-13T03:07:20.560Z
Learning: In WLED effects (usermods/user_fx/user_fx.cpp and similar), when storing future timestamps, assignments like `stored_time = now + delay` are correct and handle uint32_t wraparound properly. Only comparison statements need to use subtraction-based elapsed-time checks (e.g., `(now - stored_time) >= delay`). Do not flag assignment statements as needing fixes for wraparound handling.

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-02-11T11:14:24.380Z
Learning: In WLED code reviews, verify that all global variables in .cpp files are properly declared: check that they are defined via .h file or via "extern" declaration, verify that declarations are not duplicated, and ensure they are not implicitly declared as "int" (which can happen when the type is omitted).

Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-03T14:38:09.949Z
Learning: In WLED code reviews, check that FastLED noise function aliases (`inoise8`, `inoise16`, etc.) are not used. These should be replaced with the proper WLED functions (`perlin8`, `perlin16`, etc.) as the `inoise*` functions are only aliases kept for compatibility reasons.

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.

Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR `#4798`, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.

Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-03T14:38:09.949Z
Learning: In WLED code reviews, check that FastLED trigonometric function aliases are not used. Replace `sin8()` and `cos8()` with `sin8_t()` and `cos8_t()`. Replace `sin16()` and `cos16()` with `sin16_t()` and `cos16_t()`. For float versions, replace `sinf()` and `cosf()` with `sin_approx()` and `cos_approx()`. See wled_math.cpp for the proper WLED implementations.

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.

Learnt from: softhack007
Repo: wled/WLED PR: 5276
File: wled00/file.cpp:0-0
Timestamp: 2026-01-05T18:35:03.147Z
Learning: In WLED's file.cpp, the static File object `f` cannot be nullptr (it's not a pointer). The `if (f)` check only tests whether the file handle is open, and this check is already performed inside the `f.close()` function. Therefore, there is no need for an explicit `if (f)` check before calling `f.close()` - double-checking is redundant.

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, verify that file operations (especially file.open()) respect LittleFS filename limitations. Assume default WLED configuration with LittleFS default filename limit of 255 bytes. Do not assume extreme configuration values like WLED_MAX_SEGNAME_LEN = 512 which would not be standard configurations.

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, file operations (especially file.open()) should be checked to ensure they respect LittleFS filename limitations. The default LittleFS filename limit is 255 bytes (LFS_NAME_MAX). Reviews should assume default WLED configuration defines and not extreme edge-case values (e.g., WLED_MAX_SEGNAME_LEN = 512 would not be standard). File paths should be validated to stay within the 255-byte limit.

Learnt from: willmmiles
Repo: wled/WLED PR: 4853
File: wled00/util.cpp:779-781
Timestamp: 2025-08-29T01:34:34.358Z
Learning: On ESP8266 systems, avoid adding no-op stub functions across translation units due to limited code memory constraints, as the compiler cannot inline away the function calls, resulting in wasteful memory usage.

Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.

Learnt from: BobLoeffler68
Repo: wled/WLED PR: 4891
File: wled00/FX.cpp:3333-3349
Timestamp: 2025-08-31T03:38:14.114Z
Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.

Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-13T21:23:35.514Z
Learning: In WLED, the global `paletteBlend` variable (wled.h:603) and the `WS2812FX::paletteBlend` member (FX.h:940) are duplicates without synchronization code. The global is loaded/saved in cfg.cpp and set via UI in set.cpp, but never copied to the strip member. This is the only such case in the codebase; other settings are either strip-only members (autoSegments, correctWB, cctFromRgb, isMatrix) or global-only (gammaCorrectCol/Bri/Val, blendingStyle).

@softhack007 softhack007 merged commit f830ea4 into main Feb 11, 2026
55 checks passed
@softhack007 softhack007 deleted the fix_variable_scope branch February 11, 2026 21:24
@netmindz
Copy link
Member

@netmindz is there a way to trigger a "manual" run of the usermods CI job on this PR branch? Just to be sure that no usermod breaks due to my changes.

The normal build includes the usermods env that triggers every usermod with an esp32 so that should hopefully catch you changes as they don't relate to the compatibility of specific chips

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.

3 participants

Comments