Clean up setGeometry (for 0.15.x only)

[willmmiles]

Reorganize setGeometry to perform all checks first before applying the values. This lets us accurately detect if the geometry was, in fact, actually changed before inducing a transition.

This should fix #5427 by not blanking the lights when the segment geometry hasn't actually changed.

Summary by CodeRabbit

• Bug Fixes
  • Improved validation and sanitization of LED segment geometry configurations to prevent invalid settings and ensure stable operation across various LED strip and matrix setups.
  • Enhanced bounds checking and parameter normalization to eliminate edge cases that could cause unexpected behavior.

[coderabbitai]

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% 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.
Linked Issues check	✅ Passed	The PR directly addresses issue #5427 by fixing unnecessary transitions/blanking when geometry unchanged, which is the root cause of random flickers reported in 0.15.4.
Out of Scope Changes check	✅ Passed	All changes are focused on the setGeometry function refactoring to improve validation and change detection logic, staying within the stated PR objectives.
Title check	✅ Passed	The title 'Clean up setGeometry (for 0.15.x only)' is directly related to the main change, which refactors the setGeometry() function to reorganize validation logic and improve change detection, fixing flickers caused by unnecessary transitions.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@wled00/FX_fcn.cpp`:
- Around line 488-490: The stop-path currently flips i2Y < i1Y but only sets i2
= 0, leaving startY/stopY written with the reversed (invalid) Y bounds; update
the code handling the stop case (references: i1Y, i2Y, i2, startY, stopY) to
normalize or clear the Y pair before storing—either swap i1Y/i2Y so startY <=
stopY or set startY/stopY to a safe default (e.g., zeroed) when you set i2 = 0;
apply the same normalization/clearing logic in the other analogous stop branch
around the second occurrence mentioned (lines ~529-530) so future partial
updates don’t reuse reversed bounds.
- Around line 461-472: The sanitisation currently forces i2 to 1, breaking the
inactive/deletion sentinel i2==0 used by
setGeometry(0,0)/Segment::deactivate()/seg.stop logic; change the clamping so
i2==0 is preserved as the inactive path (e.g. if i2 == 0 leave it alone,
otherwise apply the lower-bound clamp that sets i2 to 1 when 0 < i2 < 1), so
modify the block around the i2 clamps in FX_fcn.cpp (the i2/i1 sanitisation
code) to allow i2==0 to pass through unchanged before any final disable checks.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 76518a47-085f-4ca5-b991-967c1a9a0161

📥 Commits

Reviewing files that changed from the base of the PR and between c753658 and 83816b4.

📒 Files selected for processing (1)

• wled00/FX_fcn.cpp

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@wled00/FX_fcn.cpp`:
- Around line 496-501: The preserved ofs (set when ofs == UINT16_MAX) can be >=
the new segment length after shrinking and will cause
setPixelColor()/getPixelColor() to read/write out of bounds; after computing the
final segment bounds (using i1, i2, i1Y, i2Y) and before using ofs, renormalize
it relative to the new segment length: compute segLen = i2 - i1 (or the
appropriate Y-length if using i1Y/i2Y) and if segLen == 0 disable the segment
(as done with i2 = 0), otherwise if ofs >= segLen replace ofs with ofs % segLen
(or clamp to 0) so subsequent calls to setPixelColor()/getPixelColor() wrap
correctly within the new length.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e01058bf-9754-495f-add7-ca21ea5f93de

📥 Commits

Reviewing files that changed from the base of the PR and between 83816b4 and 49d826d.

📒 Files selected for processing (1)

• wled00/FX_fcn.cpp

[softhack007]

@willmmiles is this PR ready for merging into 0_15 ?

[willmmiles]

@willmmiles is this PR ready for merging into 0_15 ?

It works for me, but there's been no human reviews. I think 0_15_x is probably defunct now anyways, I'll make sure it gets ported upstream.

[willmmiles]

@willmmiles is this PR ready for merging into 0_15 ?

It works for me, but there's been no human reviews. I think 0_15_x is probably defunct now anyways, I'll make sure it gets ported upstream.

Spoke too soon, found a bug. :( Nope, just second-guessing myself.