Clean up setGeometry (16_x)

[willmmiles]

Reorganize setGeometry to perform all checks first before applying the values. This lets us accurately detect changes before starting a transition.

16.x port of #5474

Summary by CodeRabbit

• Bug Fixes
  • Improved segment geometry validation to sanitize incoming values before applying changes, reducing the chance of invalid or inconsistent segment states.
  • Fixed edge cases where a segment could remain active with mismatched sizing or position.
  • More reliable cleanup and reset behavior when a segment is effectively reduced to zero length, including consistent pixel buffer handling and refresh behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 330fcb47-e797-40fa-8f7f-de226e01f9c2

📥 Commits

Reviewing files that changed from the base of the PR and between 85c7c5f and 3f49b3f.

📒 Files selected for processing (1)

• wled00/FX_fcn.cpp

🚧 Files skipped from review as they are similar to previous changes (1)

• wled00/FX_fcn.cpp

---

Walkthrough

Segment::setGeometry() in wled00/FX_fcn.cpp now sanitizes geometry inputs before applying state, recomputes the no-op check, and reorganizes geometry assignment plus pixel-buffer cleanup around the resulting segment length.

Changes

setGeometry Refactor

Layer / File(s)	Summary
Input sanitization and no-op check wled00/FX_fcn.cpp	Clamps and normalizes geometry inputs, preserves or constrains selected fields, applies final invalid-bounds disable checks, and rewrites the unchanged-bounds early return.
Geometry and buffer updates wled00/FX_fcn.cpp	Marks reset, stops transitions, assigns geometry fields together, then conditionally reallocates pixels or performs length-zero cleanup before refreshing light capabilities.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant Segment
    participant PixelBuffer
    participant LightCapabilities

    Caller->>Segment: setGeometry(...)
    Segment->>Segment: sanitize inputs and compute boundsUnchanged
    alt no geometry change
        Segment-->>Caller: return
    else geometry changed
        Segment->>Segment: mark reset, stop transition, set stateChanged
        Segment->>Segment: assign geometry fields
        alt length() == 0
            Segment->>PixelBuffer: free pixels and deallocate data
        else length() changed
            Segment->>PixelBuffer: reallocate pixels if needed
            Segment->>LightCapabilities: refreshLightCapabilities()
        end
    end

Loading

Possibly related PRs

• wled/WLED#5474: Refactors the same Segment::setGeometry() path to sanitize geometry inputs and recompute the no-op fast-path before applying state.

🚥 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 title is concise and accurately reflects the main change to setGeometry, with the branch note not obscuring the intent.
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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

wled00/FX_fcn.cpp (1)

520-523: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant duplicate assignment of stateChanged.

stateChanged = true; is set identically on Line 520 and Line 523; neither markForReset() nor stopTransition() clears it, so the second assignment is dead. Likely a port/merge artifact — remove one.

♻️ Proposed cleanup

-  stateChanged = true; // send UDP/WS broadcast
   markForReset();
   stopTransition(); // we can't use transition if segment dimensions changed
   stateChanged = true;      // send UDP/WS broadcast

As per path instructions: "CHECK for singleton data (defined but never used) and for dead/disabled code, and suggest to remove them" and "FLAG changes that appear unrelated".

🤖 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/FX_fcn.cpp` around lines 520 - 523, The `stateChanged` flag is being
assigned the same value twice in the segment-dimension reset flow, making the
later assignment dead. In the `FX_fcn.cpp` logic around `markForReset()` and
`stopTransition()`, remove the redundant `stateChanged = true` so the broadcast
flag is set only once and keep the existing reset/transition calls unchanged.

Source: Path instructions

🤖 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/FX_fcn.cpp`:
- Around line 520-523: The `stateChanged` flag is being assigned the same value
twice in the segment-dimension reset flow, making the later assignment dead. In
the `FX_fcn.cpp` logic around `markForReset()` and `stopTransition()`, remove
the redundant `stateChanged = true` so the broadcast flag is set only once and
keep the existing reset/transition calls unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2ca0626b-2bf1-4a75-9f5a-ab3a838c9abb

📥 Commits

Reviewing files that changed from the base of the PR and between c8a4248 and 85c7c5f.

📒 Files selected for processing (1)

• wled00/FX_fcn.cpp