seven_segment_display_reloaded_v2: bound MQTT-set brightness/lux values to prevent uint16_t wraparound - #30
Open
KrX3D wants to merge 1 commit into
Conversation
…es to prevent uint16_t wraparound _cmpIntSetting_P() stored strtol()'s raw output into the target via static_cast<T> with no range check. For the auto-brightness/lux min/max settings (uint16_t), a negative MQTT payload (e.g. "-1") wraps to 65535 instead of clamping, which then silently pins the display at full brightness regardless of ambient light (loop()'s spanBright/spanLux interpolation has no way to recover from a corrupted min/max pair). - Added optional inclusive minVal/maxVal bounds (defaulting to the full 32-bit range, so every other existing call site is unaffected) to _cmpIntSetting_P, and pass them for the four settings that feed the brightness interpolation: brightness min/max to WLED's 0-255 scale, lux min/max to the full non-negative uint16_t range. - Added a matching spanBright<0 guard next to the existing spanLux<=0 guard in loop(), for the case where Min/Max are misconfigured (not just malformed) via the settings page. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J6bJtispewBKR2dXBcNWpw
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
2 participants
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.
Summary
Review of
seven_segment_display_reloaded_v2(this one hadn't had a dedicated review/fix PR yet, unlike the others — #1/#2/#19/#20 covered the originalseven_segment_display_reloadedbefore the v2 rewrite). Found one real bug:_cmpIntSetting_P()storesstrtol()'s raw output straight into the target field viastatic_cast<T>with no range check. The four settings that feed the auto-brightness interpolation (umSSDRBrightnessMin/Max,umSSDRLuxMin/Max) areuint16_t, so a negative payload (e.g. publishing"-1"to.../auto-brightness-min/set) wraps to65535instead of clamping to0.loop()'s brightness interpolation already guards against an invalidspanLuxbut had no equivalent guard forspanBright, so a single malformed/negative MQTT payload permanently pins the display at full brightness regardless of ambient light, with nothing surfaced except a debug log line.Fix: added optional inclusive
minVal/maxValbounds to_cmpIntSetting_P(defaulting to the full 32-bit range, so the four other call sites — the boolean flag settings — are unaffected), and passed brightness-scale bounds (0-255) and non-negativeuint16_tbounds (0-65535) for the four settings above. Also added aspanBright < 0guard next to the existingspanLux <= 0one inloop(), for the case where Min/Max end up inverted via the settings page rather than MQTT.Test plan
🤖 Generated with Claude Code
https://claude.ai/code/session_01J6bJtispewBKR2dXBcNWpw
Generated by Claude Code