LED strip: add rainbow overlay GUI controls by HereComesWhitey · Pull Request #2718 · iNavFlight/inav-configurator

HereComesWhitey · 2026-08-25T08:00:44Z

Adds GUI support for the new rainbow overlay ('V') introduced in the companion FC PR.

Changes

Adds rainbow overlay toggle to the overlays section in the LED strip tab
Adds Rainbow Overlay Settings panel with sweep rate and colour delta inputs
Inputs are enabled only when the rainbow overlay toggle is active and rainbow LEDs are selected
Inputs are greyed out and non-interactive when the toggle is off
Registers 'V' in the overlay letters array in msp.js so the overlay is correctly parsed and transmitted
Save path safely handles NaN inputs with fallback to FC defaults and clamps values to valid ranges

Settings

ledstrip_rainbow_sweep_rate (0-255) — controls sweep speed. Higher values sweep faster. 0 freezes the rainbow.
ledstrip_rainbow_delta_deg (0-359) — hue offset in degrees between adjacent rainbow LEDs.

Testing

Verified on SKYSTARSH743HD with 14 LEDs configured with the rainbow overlay active. Sweep rate and colour delta changes save correctly to EEPROM and persist across power cycles.

Companion FC PR

iNavFlight/inav#11820

Maintenance 9.x to master

… 1-255

qodo-free-for-open-source-projects · 2026-08-25T08:00:52Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Sweep rate 0 blocked~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

The Sweep Rate input is constrained to min=1 and the UI label says "1-255", but the save path clamps
to 0-255, making 0 a supported value that cannot be entered via the GUI.

Code

tabs/led_strip.html[R147-148]

+                                                        <input type="number" id="rainbowFreqInput" min="1" max="255" step="1" disabled style="width: 40px; text-align: right;" />
+                            <span style="margin-left: 6px;" i18n="ledStripRainbowFreqUnit"></span>

Evidence
The UI restricts the numeric input to a minimum of 1 and the unit label says 1–255, but the save
logic clamps to include 0, so the UI and the persisted range are inconsistent.
tabs/led_strip.html[143-148]
locale/en/messages.json[5958-5960]
tabs/led_strip.js[829-838]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The rainbow sweep rate is clamped and sent as 0–255, but the HTML input and localized unit text restrict users to 1–255. This prevents setting sweep rate 0 via the UI.
### Issue Context
The save path already supports 0 (it clamps to 0..255), so the UI should match.
### Fix Focus Areas
- tabs/led_strip.html[147-148]
- locale/en/messages.json[5958-5960]
- tabs/led_strip.js[833-834]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Inconsistent enable conditions~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

Rainbow inputs are enabled/disabled using three different conditions (checkbox state vs selected
LEDs having function-v), which can leave the inputs enabled even when no rainbow LEDs are selected
(or disabled even when the toggle is on).

Code

tabs/led_strip.js[R629-633]

+                var rainbowEnabled = !!$('.ui-selected').filter(function() {
+                    return $(this).is('.function-v');
+                }).length;
+                $('#rainbowFreqInput, #rainbowDeltaInput').prop('disabled', !rainbowEnabled).css('pointer-events', rainbowEnabled ? 'auto' : '');
+

Evidence

One code path enables inputs based on the checkbox (.toggle.function-v), another enables based on
selection containing .function-v, and a third runs on checkbox changes regardless of selection
presence.

tabs/led_strip.js[223-226]
tabs/led_strip.js[629-636]
tabs/led_strip.js[763-767]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The rainbow settings inputs are enabled/disabled in multiple places using different predicates:
- based on the rainbow checkbox state
- based on whether selected LEDs have `.function-v`
This can produce inconsistent UI state.
### Issue Context
There is already a helper `syncRainbowInputState()` but it only checks the checkbox state and is not used as the single source of truth.
### Fix Focus Areas
- tabs/led_strip.js[223-226]
- tabs/led_strip.js[629-633]
- tabs/led_strip.js[763-766]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~Rainbow title not localized~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The "Rainbow Overlay Settings" panel title is hard-coded in HTML instead of using the newly-added
i18n key, so it will not translate and will drift from message definitions.

Code

tabs/led_strip.html[R143-145]

+                    <div class="rainbowSettings" id="rainbowSettings" style="margin-top: 8px; margin-left: 4px;">
+                        <div style="font-weight: bold; margin-bottom: 6px; color: #1dacf2;">Rainbow Overlay Settings</div>
+                        <div class="rainbowSettingRow" style="display: flex; align-items: center; margin-bottom: 4px;">

Evidence
A localized string for the settings title was added, but the HTML uses a literal string rather than
the i18n key.
tabs/led_strip.html[143-145]
locale/en/messages.json[5952-5954]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The rainbow settings panel title is hard-coded text even though an i18n message key was added for it. This prevents translation and duplicates the source of truth.
### Issue Context
Localization is performed by i18n.localize() on elements with an `i18n` attribute.
### Fix Focus Areas
- tabs/led_strip.html[143-145]
- locale/en/messages.json[5952-5954]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. ~~Debug logging left in~~ ✓ Resolved 🐞 Bug ◔ Observability

Description

The LED strip tab logs rainbow setting reads to the console (including a successful read), which
adds noise in production and is inconsistent with the other getSetting call that silently ignores
errors.

Code

tabs/led_strip.js[R234-240]

+        mspHelper.getSetting('ledstrip_rainbow_delta_deg').then(function (result) {
+            console.log('[Rainbow] ledstrip_rainbow_delta_deg =', result);
+            if (result && result.value !== null && result.value !== undefined) {
+                $('#rainbowDeltaInput').val(result.value);
+            }
+        }).catch(function (err) {
+            console.error('[Rainbow] getSetting ledstrip_rainbow_delta_deg FAILED:', err);

Evidence

The code logs the delta setting result on success and logs errors only for delta, while the
sweep_rate catch is empty.

tabs/led_strip.js[228-241]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
There are unconditional console logs for rainbow setting retrieval. This will spam devtools in normal usage.
### Issue Context
Other settings retrieval in the same block uses an empty catch; either both should be silent, or both should log only on failure / behind a debug flag.
### Fix Focus Areas
- tabs/led_strip.js[228-241]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. ~~Redundant updateBulkCmd call~~ ✓ Resolved 🐞 Bug ➹ Performance

Description

The selection handler calls updateBulkCmd() twice back-to-back, adding unnecessary work on every
selection change.

Code

tabs/led_strip.js[R632-636]

+                $('#rainbowFreqInput, #rainbowDeltaInput').prop('disabled', !rainbowEnabled).css('pointer-events', rainbowEnabled ? 'auto' : '');
+
+
+                updateBulkCmd();

Evidence

In the same handler block, updateBulkCmd() is called once before the rainbow-enabled calculation and
then again immediately after it.

tabs/led_strip.js[623-636]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
updateBulkCmd() is invoked twice consecutively in the same selection-change flow. This duplicates DOM parsing and FC.LED_STRIP reconstruction.
### Fix Focus Areas
- tabs/led_strip.js[623-636]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Tip of the day

💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

qodo-free-for-open-source-projects · 2026-08-25T08:02:05Z

PR Summary by Qodo

LED strip: add Rainbow overlay controls and settings persistence

✨ Enhancement 🕐 20-40 Minutes

AI Description

• Add Rainbow overlay toggle and settings inputs to LED Strip tab UI.
• Load/save rainbow sweep rate and hue delta via MSP settings with validation.
• Register overlay letter 'v' so Rainbow overlay is parsed and transmitted correctly.

Diagram

graph TD
U[User] --> UI["LED Strip tab (led_strip.js/html)"] --> MSPH["MSPHelper save pipeline"] --> FC[("Flight Controller")] --> EEPROM[("EEPROM write")]
UI --> I18N["locale/en messages"]
UI --> MSP["MSP overlay letters (msp.js)"]

High-Level Assessment

The following are alternative approaches to this PR:

1. Only write Rainbow settings when Rainbow overlay is enabled

➕ Avoids unintended setting changes when users click Save for unrelated LED edits
➕ Reduces MSP setting writes and EEPROM churn
➖ Users may expect settings to be saved even before enabling the overlay
➖ Requires clear UX (e.g., prompt/auto-save on toggle enable)

2. Skip writes on NaN by reusing last-read FC values (or omit setSetting)

➕ Prevents fallback constants from overwriting FC defaults when reads fail
➕ Keeps behavior consistent with 'FC is source of truth'
➖ Requires retaining loaded values and handling partial read failures
➖ Slightly more state management in the tab code

Recommendation: The current approach (load settings on tab init, then write validated values during Save) is reasonable and keeps the save flow serialized behind existing LED strip MSP writes. Consider tightening the write conditions: either gate setting writes behind the Rainbow toggle and/or avoid overwriting with hard-coded fallbacks when inputs are NaN due to a failed getSetting, to minimize surprising changes to FC-side defaults.

Files changed (4) +107 / -12

Enhancement (4) +107 / -12

msp.jsRegister Rainbow overlay letter 'v' for LED strip overlays +1/-1

Register Rainbow overlay letter 'v' for LED strip overlays

• Extends the LED overlay letter table to include 'v'. This allows the configurator to correctly parse and transmit the Rainbow overlay bit in MSP LED strip data.

js/msp.js

messages.jsonAdd English strings for Rainbow overlay UI and units +18/-0

Add English strings for Rainbow overlay UI and units
• Introduces new i18n keys for the Rainbow overlay toggle, settings panel title, and labels/units for sweep rate and color delta.
locale/en/messages.json

led_strip.htmlAdd Rainbow overlay toggle and settings panel markup +17/-1

Add Rainbow overlay toggle and settings panel markup

• Adds a new overlay checkbox for Rainbow and a settings panel containing numeric inputs for sweep rate and hue delta. Inputs are initially disabled and the panel is shown/hidden by the tab logic.

tabs/led_strip.html

led_strip.jsWire Rainbow overlay state, setting load/save, and input enabling +71/-10

Wire Rainbow overlay state, setting load/save, and input enabling

• Adds 'v' to the overlays list, loads rainbow sweep rate and delta settings on tab init, and enables/disables inputs based on toggle state and current LED selection. Extends the save pipeline to persist rainbow settings with NaN fallback handling and value clamping before issuing EEPROM write.

tabs/led_strip.js

qodo-code-review · 2026-08-25T08:06:11Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. ~~Sweep rate 0 blocked~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

The Sweep Rate input is constrained to min=1 and the UI label says "1-255", but the save path clamps
to 0-255, making 0 a supported value that cannot be entered via the GUI.

Code

tabs/led_strip.html[R147-148]

+                                                        <input type="number" id="rainbowFreqInput" min="1" max="255" step="1" disabled style="width: 40px; text-align: right;" />
+                            <span style="margin-left: 6px;" i18n="ledStripRainbowFreqUnit"></span>

Evidence
The UI restricts the numeric input to a minimum of 1 and the unit label says 1–255, but the save
logic clamps to include 0, so the UI and the persisted range are inconsistent.
tabs/led_strip.html[143-148]
locale/en/messages.json[5958-5960]
tabs/led_strip.js[829-838]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The rainbow sweep rate is clamped and sent as 0–255, but the HTML input and localized unit text restrict users to 1–255. This prevents setting sweep rate 0 via the UI.

### Issue Context
The save path already supports 0 (it clamps to 0..255), so the UI should match.

### Fix Focus Areas
- tabs/led_strip.html[147-148]
- locale/en/messages.json[5958-5960]
- tabs/led_strip.js[833-834]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Inconsistent enable conditions~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

Rainbow inputs are enabled/disabled using three different conditions (checkbox state vs selected
LEDs having function-v), which can leave the inputs enabled even when no rainbow LEDs are selected
(or disabled even when the toggle is on).

Code

tabs/led_strip.js[R629-633]

+                var rainbowEnabled = !!$('.ui-selected').filter(function() {
+                    return $(this).is('.function-v');
+                }).length;
+                $('#rainbowFreqInput, #rainbowDeltaInput').prop('disabled', !rainbowEnabled).css('pointer-events', rainbowEnabled ? 'auto' : '');
+

Evidence

One code path enables inputs based on the checkbox (.toggle.function-v), another enables based on
selection containing .function-v, and a third runs on checkbox changes regardless of selection
presence.

tabs/led_strip.js[223-226]
tabs/led_strip.js[629-636]
tabs/led_strip.js[763-767]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The rainbow settings inputs are enabled/disabled in multiple places using different predicates:
- based on the rainbow checkbox state
- based on whether selected LEDs have `.function-v`
This can produce inconsistent UI state.

### Issue Context
There is already a helper `syncRainbowInputState()` but it only checks the checkbox state and is not used as the single source of truth.

### Fix Focus Areas
- tabs/led_strip.js[223-226]
- tabs/led_strip.js[629-633]
- tabs/led_strip.js[763-766]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~Rainbow title not localized~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The "Rainbow Overlay Settings" panel title is hard-coded in HTML instead of using the newly-added
i18n key, so it will not translate and will drift from message definitions.

Code

tabs/led_strip.html[R143-145]

+                    <div class="rainbowSettings" id="rainbowSettings" style="margin-top: 8px; margin-left: 4px;">
+                        <div style="font-weight: bold; margin-bottom: 6px; color: #1dacf2;">Rainbow Overlay Settings</div>
+                        <div class="rainbowSettingRow" style="display: flex; align-items: center; margin-bottom: 4px;">

Evidence
A localized string for the settings title was added, but the HTML uses a literal string rather than
the i18n key.
tabs/led_strip.html[143-145]
locale/en/messages.json[5952-5954]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The rainbow settings panel title is hard-coded text even though an i18n message key was added for it. This prevents translation and duplicates the source of truth.

### Issue Context
Localization is performed by i18n.localize() on elements with an `i18n` attribute.

### Fix Focus Areas
- tabs/led_strip.html[143-145]
- locale/en/messages.json[5952-5954]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. ~~Redundant updateBulkCmd call~~ ✓ Resolved 🐞 Bug ➹ Performance

Description

The selection handler calls updateBulkCmd() twice back-to-back, adding unnecessary work on every
selection change.

Code

tabs/led_strip.js[R632-636]

+                $('#rainbowFreqInput, #rainbowDeltaInput').prop('disabled', !rainbowEnabled).css('pointer-events', rainbowEnabled ? 'auto' : '');
+
+
+                updateBulkCmd();

Evidence

In the same handler block, updateBulkCmd() is called once before the rainbow-enabled calculation and
then again immediately after it.

tabs/led_strip.js[623-636]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
updateBulkCmd() is invoked twice consecutively in the same selection-change flow. This duplicates DOM parsing and FC.LED_STRIP reconstruction.

### Fix Focus Areas
- tabs/led_strip.js[623-636]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. ~~Debug logging left in~~ ✓ Resolved 🐞 Bug ◔ Observability

Description

The LED strip tab logs rainbow setting reads to the console (including a successful read), which
adds noise in production and is inconsistent with the other getSetting call that silently ignores
errors.

Code

tabs/led_strip.js[R234-240]

+        mspHelper.getSetting('ledstrip_rainbow_delta_deg').then(function (result) {
+            console.log('[Rainbow] ledstrip_rainbow_delta_deg =', result);
+            if (result && result.value !== null && result.value !== undefined) {
+                $('#rainbowDeltaInput').val(result.value);
+            }
+        }).catch(function (err) {
+            console.error('[Rainbow] getSetting ledstrip_rainbow_delta_deg FAILED:', err);

Evidence

The code logs the delta setting result on success and logs errors only for delta, while the
sweep_rate catch is empty.

tabs/led_strip.js[228-241]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
There are unconditional console logs for rainbow setting retrieval. This will spam devtools in normal usage.

### Issue Context
Other settings retrieval in the same block uses an empty catch; either both should be silent, or both should log only on failure / behind a debug flag.

### Fix Focus Areas
- tabs/led_strip.js[228-241]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Context sources

Review mode: ⚖️ Balanced

Tip of the day

💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

- Fix sweep rate input min to 0 to match save path range (0-255) - Consolidate rainbow input enable/disable into syncRainbowInputState() - Use i18n key for rainbow settings panel title - Fix duplicate case block for overlay letters i and v - Remove redundant updateBulkCmd() call in selection handler - Use optional chaining for getSetting result checks

sonarqubecloud · 2026-08-25T08:58:55Z

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

sensei-hacker and others added 5 commits July 7, 2026 21:07

Merge pull request iNavFlight#2674 from iNavFlight/maintenance-9.x

f680269

Maintenance 9.x to master

Merge pull request iNavFlight#2684 from iNavFlight/maintenance-9.x

07b87c0

Maintenance 9.x to master

LED strip: add RGB sweep/rainbow overlay GUI controls

00c5a76

LED strip: rename rainbow freq setting to sweep_rate, update range to…

0dc3923

… 1-255

LED strip: fix NaN handling in rainbow settings save

5ee91ce

qodo-code-review Bot reviewed Aug 25, 2026

View reviewed changes

Comment thread tabs/led_strip.html Outdated

Comment thread tabs/led_strip.html

Comment thread tabs/led_strip.js Outdated

Comment thread tabs/led_strip.js Outdated

Comment thread tabs/led_strip.js Outdated

HereComesWhitey mentioned this pull request Aug 25, 2026

LED Strip: add rainbow overlay iNavFlight/inav#11820

Open

Navigation Menu

Uh oh!

LED strip: add rainbow overlay GUI controls - #2718

LED strip: add rainbow overlay GUI controls#2718
HereComesWhitey wants to merge 6 commits into
iNavFlight:maintenance-10.xfrom
HereComesWhitey:rgb-sweep

HereComesWhitey commented Aug 25, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 25, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 25, 2026 •

edited by qodo-code-review Bot

Loading

Uh oh!

qodo-code-review Bot commented Aug 25, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Aug 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

HereComesWhitey commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Settings

Testing

Companion FC PR

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 25, 2026 • edited by qodo-code-review Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary by Qodo

Uh oh!

qodo-code-review Bot commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Aug 25, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

HereComesWhitey commented Aug 25, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Aug 25, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Aug 25, 2026 •

edited by qodo-code-review Bot

Loading

qodo-code-review Bot commented Aug 25, 2026 •

edited

Loading