Skip to content

Encapsulate hue.cpp's own runtime state as static, not global - #5777

Open
netmindz wants to merge 1 commit into
wled:mainfrom
netmindz:refactor/hue-globals
Open

Encapsulate hue.cpp's own runtime state as static, not global#5777
netmindz wants to merge 1 commit into
wled:mainfrom
netmindz:refactor/hue-globals

Conversation

@netmindz

@netmindz netmindz commented Aug 8, 2026

Copy link
Copy Markdown
Member

Summary

11 WLED_GLOBAL variables declared in wled.h turned out to be referenced only in hue.cpp — nowhere else in wled00/ or usermods/ touches them: hueXLast, hueYLast, hueHueLast, hueCtLast, hueSatLast, hueBriLast, hueLastRequestSent, hueAuthRequired, hueReceived, hueNewKey, hueClient. They were global purely because everything in wled.h lives in one big extern block regardless of who actually uses it.

Converted all 11 to file-local static in hue.cpp, with the same types and initial values as before. hueStoreAllowed, which was declared on the same source line as hueNewKey, stays global since set.cpp also writes it — verified by grep before splitting that declaration.

No behavior change — purely a storage-class change, nothing else touched.

This is a small, mechanical first step from a broader pass identifying which of WLED's ~270 WLED_GLOBAL declarations are actually only used in one file (this is the largest single-file cluster).

Test plan

  • esp32dev: builds and links cleanly via pio run -e esp32dev — 1,320,323 bytes flash, no warnings from either changed file.
  • Confirmed via repo-wide grep (wled00/, usermods/) that none of the 11 converted variables are referenced outside hue.cpp.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor
    • Improved internal handling of Hue synchronization state.
    • Reduced exposure of Hue-related implementation details while preserving existing behavior.
    • Hue synchronization continues to operate without changes to its user-facing functionality or configuration.
    • Internal state management is now more clearly separated from publicly accessible application interfaces.

11 WLED_GLOBAL variables (hueXLast, hueYLast, hueHueLast, hueCtLast,
hueSatLast, hueBriLast, hueLastRequestSent, hueAuthRequired,
hueReceived, hueNewKey, hueClient) turned out to be referenced only
in hue.cpp - nowhere else in wled00/ or usermods/ touches them. They
were global purely because everything in wled.h lives in one big
extern block regardless of who actually uses it.

Converted all 11 to file-local `static` in hue.cpp. hueStoreAllowed,
which was declared on the same line as hueNewKey, stays global since
set.cpp also writes it.

No behavior change - purely a storage-class change, same types and
initial values as before.

Verified: esp32dev builds and links cleanly via `pio run -e esp32dev`
(1,320,323 bytes flash, no warnings from the changed files).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UtCyBD91vAYWvBzaMyQSHd
@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: ede85a13-59ce-4f43-800a-b3b298420ebf

📥 Commits

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

📒 Files selected for processing (2)
  • wled00/hue.cpp
  • wled00/wled.h

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


Walkthrough

Hue synchronization state moved from public declarations in wled.h to file-private static definitions in hue.cpp. hueStoreAllowed remains globally declared.

Changes

Hue state encapsulation

Layer / File(s) Summary
Private Hue runtime state
wled00/hue.cpp, wled00/wled.h
hue.cpp now owns static Hue runtime state. wled.h removes the related state and client declarations while retaining hueStoreAllowed.

Priority: ⬇️ Low

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

Change: Refactor

Merge Risk: ⚪ Minimal · up to b853f

Hue synchronization state is now private to its implementation file without an identified behavioral or build impact.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. 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: making hue.cpp runtime state file-local static instead of globally linked.
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch refactor/hue-globals

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.

@softhack007 softhack007 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@netmindz

Copy link
Copy Markdown
Member Author

@copilot resolve the merge conflicts in this pull request

@softhack007

This comment was marked as outdated.

@coderabbitai

This comment was marked as outdated.

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