fix: The Alignment Tool's sliders are not linked to the Value fields

[RomanLut]

Fixes #2589

[github-actions]

Branch Targeting Suggestion

You've targeted the master branch with this PR. Please consider if a version branch might be more appropriate:

• maintenance-9.x - If your change is backward-compatible and won't create compatibility issues between INAV firmware and Configurator 9.x versions. This will allow your PR to be included in the next 9.x release.

• maintenance-10.x - If your change introduces compatibility requirements between firmware and configurator that would break 9.x compatibility. This is for PRs which will be included in INAV 10.x

If master is the correct target for this change, no action is needed.

---

This is an automated suggestion to help route contributions to the appropriate branch.

[qodo-code-review]

Review Summary by Qodo

Fix alignment tool sliders not syncing with value fields

🐞 Bug fix

Walkthroughs

Description

• Fix slider-value field synchronization using noUiSlider API
  - Replace jQuery .val() with noUiSlider.set() method
  - Add _settingSlider flag to prevent circular updates
• Prevent slider event handlers from triggering value updates
  - Wrap update functions with flag checks in event listeners
• Optimize 3D rendering with requestAnimationFrame throttling
  - Add _renderPending flag to limit renders to one per frame
• Minor formatting cleanup (trailing whitespace removal)

Diagram

flowchart LR
  A["Value Field Update"] -->|"Set _settingSlider=true"| B["Call noUiSlider.set()"]
  B -->|"Set _settingSlider=false"| C["Update Complete"]
  D["Slider Event"] -->|"Check _settingSlider flag"| E{"Flag is false?"}
  E -->|"Yes"| F["Update Value Field"]
  E -->|"No"| G["Skip Update"]
  H["render3D() Called"] -->|"Check _renderPending"| I{"Pending?"}
  I -->|"No"| J["Schedule requestAnimationFrame"]
  I -->|"Yes"| K["Skip Render"]

Loading

File Changes

1. tabs/magnetometer.js 🐞 Bug fix +25/-17

Fix slider-value synchronization and optimize rendering

• Added _settingSlider flag to prevent circular update loops between sliders and value fields
• Replaced jQuery .val() calls with noUiSlider.set() API for proper slider synchronization in
 six update functions (updateBoardRollAxis, updateBoardPitchAxis, updateBoardYawAxis,
 updateRollAxis, updatePitchAxis, updateYawAxis)
• Wrapped slider event handlers with flag checks to prevent cascading updates when values are set
 programmatically
• Added _renderPending flag and requestAnimationFrame throttling to optimize 3D rendering
 performance
• Removed trailing whitespace for code formatting consistency

tabs/magnetometer.js

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2)   📘 Rule violations (1)   📎 Requirement gaps (0)   🖥 UI issues (0)   🎨 UX Issues (0)

🐞\ ☼ Reliability (2)

📘\ ☼ Reliability (1)

1. Unchecked *_slider[0] access 📘 ☼

Description

The new slider-sync code directly dereferences self.pageElements.*_slider[0].noUiSlider without
first verifying the jQuery collection has an element at index 0. If a slider element is missing/not
yet initialized, this can throw at runtime and break the tab.

Code

tabs/magnetometer.js[274]

+        if (self.pageElements.board_roll_slider[0].noUiSlider && !_settingSlider) { _settingSlider = true; self.pageElements.board_roll_slider[0].noUiSlider.set(self.boardAlignmentConfig.roll); _settingSlider = false; }

Evidence

PR Compliance ID 6 requires defensive guards before accessing nested properties. The added lines
access ...[0].noUiSlider (nested property chain) while only guarding noUiSlider after already
dereferencing [0], which can be undefined.

tabs/magnetometer.js[274-274]
tabs/magnetometer.js[283-283]
tabs/magnetometer.js[292-292]
tabs/magnetometer.js[317-317]
tabs/magnetometer.js[326-326]
tabs/magnetometer.js[335-335]
Best Practice: Learned patterns

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 code dereferences `self.pageElements.*_slider[0].noUiSlider` without verifying the slider collection contains an element at index 0, which can throw if the element is missing or not initialized.

## Issue Context
Although `noUiSlider` is checked, the check happens after dereferencing `[0]`. Per the defensive-guards requirement, the code should validate existence/shape before nested access.

## Fix Focus Areas
- tabs/magnetometer.js[274-274]
- tabs/magnetometer.js[283-283]
- tabs/magnetometer.js[292-292]
- tabs/magnetometer.js[317-317]
- tabs/magnetometer.js[326-326]
- tabs/magnetometer.js[335-335]

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

---

2. RAF render after cleanup 🐞 ☼

Description

render3D now schedules renderer.render() via requestAnimationFrame, but magnetometer.cleanup doesn’t
cancel pending frames. When switching tabs, #content is emptied (canvas removed), so a queued frame
can render after teardown (wasted work and potential runtime errors depending on renderer/canvas
state).

Code

tabs/magnetometer.js[R693-700]

+        // draw — throttled to one render per animation frame
+        if (camera != null && !_renderPending) {
+            _renderPending = true;
+            requestAnimationFrame(() => {
+                _renderPending = false;
+                renderer.render(scene, camera);
+            });
+        }

Evidence

Tab switching clears the #content container (removing the magnetometer canvas) while
magnetometer.cleanup only unbinds the window resize handler; with the new requestAnimationFrame
scheduling, a render can still run after the DOM has been cleared.

tabs/magnetometer.js[670-701]
tabs/magnetometer.js[823-827]
js/configurator_main.js[165-169]

Agent prompt

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

### Issue description
`tabs/magnetometer.js` now throttles drawing by scheduling `renderer.render()` inside `requestAnimationFrame`. If the user switches tabs while a frame is pending, the tab switch logic clears `#content` (removing the canvas), but `TABS.magnetometer.cleanup` does not cancel the pending RAF. This can cause a render to occur after teardown.

### Issue Context
Tab switching empties `#content`, so any async work that assumes the canvas is still present should be canceled/guarded.

### Fix Focus Areas
- tabs/magnetometer.js[670-701]
- tabs/magnetometer.js[823-827]
- js/configurator_main.js[165-169]

### Suggested change
- Track the RAF id (e.g., `_rafId`) and cancel it in `cleanup` using `cancelAnimationFrame`.
- Add a disposed flag (e.g., `_disposed`) and early-return inside the RAF callback if disposed.
- Optionally guard render with `renderer.domElement?.isConnected` (or equivalent) before calling `renderer.render`.

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

---

3. Slider guard can stick 🐞 ☼

Description

The new _settingSlider reentrancy guard is set/reset without try/finally around noUiSlider.set()
and around slider update handlers. If any call inside the guarded region throws, _settingSlider
remains true and further slider/value synchronization is disabled until reload.

Code

tabs/magnetometer.js[R273-275]

        self.boardAlignmentConfig.roll = Number(value);
-        self.pageElements.board_roll_slider.val(self.boardAlignmentConfig.roll);
+        if (self.pageElements.board_roll_slider[0].noUiSlider && !_settingSlider) { _settingSlider = true; self.pageElements.board_roll_slider[0].noUiSlider.set(self.boardAlignmentConfig.roll); _settingSlider = false; }
        self.pageElements.orientation_board_roll.val(self.boardAlignmentConfig.roll);

Evidence

Multiple sites set _settingSlider = true and then clear it afterward with no exception-safety. The
repo already treats noUiSlider operations as potentially throwing (e.g., wrapping
noUiSlider.create in try/catch), so leaving global UI state in a stuck 'in-progress' mode is a
realistic failure mode if an error occurs.

tabs/magnetometer.js[258-339]
tabs/magnetometer.js[480-489]
tabs/magnetometer.js[578-587]
tabs/receiver_msp.js[160-187]

Agent prompt

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

### Issue description
`_settingSlider` is used as a global reentrancy guard, but it is toggled via `true; ...; false;` without `try/finally`. If any guarded operation throws (e.g., slider API issues), the flag can remain `true` and permanently block subsequent updates.

### Issue Context
This pattern appears both when calling `noUiSlider.set(...)` and in the `noUiSlider.on('update', ...)` handlers.

### Fix Focus Areas
- tabs/magnetometer.js[258-339]
- tabs/magnetometer.js[480-489]
- tabs/magnetometer.js[578-587]

### Suggested change
- Introduce a small helper like:
 ```js
 function withSettingSlider(fn) {
   if (_settingSlider) return;
   _settingSlider = true;
   try { fn(); } finally { _settingSlider = false; }
 }
 ```
- Use it consistently around both `noUiSlider.set(...)` calls and the `'update'` handlers.
- (Optional) If nesting is required, track a counter instead of a boolean.

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

---

ⓘ The new review experience is currently in Beta. Learn more

[sonarqubecloud]

Quality Gate passed

Issues
0 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