Skip to content

Fix brighness not being applied properly in nightlight mode#5625

Open
DedeHai wants to merge 1 commit into
wled:mainfrom
DedeHai:FixNightlighDimming
Open

Fix brighness not being applied properly in nightlight mode#5625
DedeHai wants to merge 1 commit into
wled:mainfrom
DedeHai:FixNightlighDimming

Conversation

@DedeHai
Copy link
Copy Markdown
Collaborator

@DedeHai DedeHai commented May 17, 2026

fix for #5620

Summary by CodeRabbit

  • Bug Fixes
    • Fixed brightness finalization during nightlight color transitions, ensuring final brightness values are correctly applied after color updates complete
    • Improved state update handling by eliminating unnecessary operations when transitions are inactive, resulting in more reliable lighting state management

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 17, 2026

Walkthrough

This PR refactors brightness finalization in LED state updates. The stateUpdated() function now delegates final updates to applyFinalBri() when no transition is active, and handleNightlight() now explicitly calls applyFinalBri() after color updates to finalize brightness during nightlight fading, establishing a consistent finalization pattern.

Changes

LED brightness finalization refactor

Layer / File(s) Summary
Brightness finalization via applyFinalBri() in state and nightlight updates
wled00/led.cpp
stateUpdated() removes direct strip.trigger() call when no transition is active and relies on applyFinalBri() instead; handleNightlight() adds applyFinalBri() after colorUpdated(CALL_MODE_NO_NOTIFY) to finalize brightness despite color updates restarting transitions.

Sequence Diagram

sequenceDiagram
  participant stateUpdated as stateUpdated()
  participant applyFinalBri as applyFinalBri()
  participant strip as strip
  participant handleNightlight as handleNightlight()
  participant colorUpdated as colorUpdated()
  
  stateUpdated->>strip: check getTransition()
  rect rgba(100, 150, 200, 0.5)
    Note over stateUpdated,applyFinalBri: No transition case
    stateUpdated->>applyFinalBri: finalize brightness
    applyFinalBri->>strip: trigger final update
  end
  
  handleNightlight->>colorUpdated: apply interpolated colors
  rect rgba(150, 100, 200, 0.5)
    Note over handleNightlight,applyFinalBri: Color fade case
    handleNightlight->>applyFinalBri: finalize brightness after color update
    applyFinalBri->>strip: trigger final update
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% 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 pull request title clearly and specifically describes the main change: fixing brightness application in nightlight mode, which aligns with the code changes modifying brightness handling in handleNightlight().
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.

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


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.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
wled00/led.cpp (1)

234-235: 💤 Low value

Nightlight brightness fix is correct; minor optimization opportunity.

The applyFinalBri() call successfully prevents transitions from blocking brightness updates during nightlight fade. This fixes the reported issue where repeated colorUpdated() calls every 100ms would restart transitions, preventing bri from changing.

However, when strip.getTransition() == 0, stateUpdated() (called by colorUpdated()) already invokes applyFinalBri() at line 128, so line 235 results in a redundant second call. While harmless, you could optimize by guarding the call:

if (strip.getTransition() > 0) applyFinalBri();

This ensures the bypass only happens when transitions are enabled. Given the 100ms rate limit and nightlight-only scope, the current unconditional approach is acceptable and keeps the fix simple.

♻️ Optional optimization to avoid redundant call
 colorUpdated(CALL_MODE_NO_NOTIFY);
-applyFinalBri(); // colorUpdated() re-starts transition (if enabled) every 100ms, preventing brightness from changing correctly (fix for `#5620`)
+if (strip.getTransition() > 0) applyFinalBri(); // bypass transition restart to allow brightness updates (fix for `#5620`)
🤖 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 `@wled00/led.cpp` around lines 234 - 235, colorUpdated() followed by an
unconditional applyFinalBri() causes a redundant call when no transition is
active because stateUpdated() already calls applyFinalBri(); update the code
around colorUpdated(CALL_MODE_NO_NOTIFY) so you only call applyFinalBri() when a
transition is enabled (check strip.getTransition() > 0) to avoid the duplicate
call while preserving the nightlight fix; reference colorUpdated(),
applyFinalBri(), strip.getTransition(), and stateUpdated() when locating the
change.
🤖 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.

Nitpick comments:
In `@wled00/led.cpp`:
- Around line 234-235: colorUpdated() followed by an unconditional
applyFinalBri() causes a redundant call when no transition is active because
stateUpdated() already calls applyFinalBri(); update the code around
colorUpdated(CALL_MODE_NO_NOTIFY) so you only call applyFinalBri() when a
transition is enabled (check strip.getTransition() > 0) to avoid the duplicate
call while preserving the nightlight fix; reference colorUpdated(),
applyFinalBri(), strip.getTransition(), and stateUpdated() when locating the
change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 12f2b97d-bcac-40fd-922f-ae044ec9bc59

📥 Commits

Reviewing files that changed from the base of the PR and between 0a19027 and a8eb397.

📒 Files selected for processing (1)
  • wled00/led.cpp

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.

1 participant