Skip to content

feat(mixer): warn when servo mixer target exceeds rule count#2659

Open
sensei-hacker wants to merge 1 commit into
iNavFlight:maintenance-9.xfrom
sensei-hacker:feature/servo-mixer-target-validation
Open

feat(mixer): warn when servo mixer target exceeds rule count#2659
sensei-hacker wants to merge 1 commit into
iNavFlight:maintenance-9.xfrom
sensei-hacker:feature/servo-mixer-target-validation

Conversation

@sensei-hacker

Copy link
Copy Markdown
Member

Summary

Adds a warning dialog in the Mixer tab's Servo Mixer table when a rule's target servo number is invalid given the number of servo mixer rules currently defined.

Changes

  • New pure helper getServoTargetWarning() in js/servoMixerTargetWarning.js: with N servo mixer rules defined (rate != 0), only servo targets LTM - Add telemetry support #1..#N are valid (1-based numbering). Entering a higher number would skip a servo.
  • .mix-rule-servo change handler in tabs/mixer.js calls this helper and, if the entered target is invalid, shows a jBox modal warning (new servoTargetWarningModal, content in tabs/mixer.html).
  • New i18n strings servoMixRuleInvalidServoTargetTitle / servoMixRuleInvalidServoTarget in locale/en/messages.json.
  • The entered value is still applied (setTarget()) regardless of the warning - this is informational, not a hard block.

Testing

  • Added tests/servo-mixer-target-validation.test.mjs (Node's built-in test runner) - 12 tests covering: editing one of 2 existing rules, editing the first vs. second rule, the "Add new mixer rule" auto-targeted row, a single-rule mixer, unused (rate=0) rules not counting, duplicate-target rules, and entered = 0 never warning. All pass (node --test tests/*.test.mjs, 24/24 across the full suite).
  • Manually tested in the running configurator against a live mixer configuration: confirmed the warning appears/doesn't appear correctly when editing existing rows and newly-added rows, including the edge case of retargeting one of two existing rules to a value that would skip a servo.
  • Verified tabs/mixer.js and js/servoMixerTargetWarning.js pass node --input-type=module --check.

Code Review

Reviewed with the inav-code-review agent; addressed all IMPORTANT findings (verified the warning can't fire during programmatic preset-load/rebuild flows, and extracted the decision logic into a shared module so tests exercise the real code path instead of a mirror).

Adds a warning dialog in the Servo Mixer table when a rule's target
servo number is higher than the number of servo mixer rules that
exist (1-based: N rules means only servos #1..#N can be validly
targeted, so entering a higher number would skip a servo).
@qodo-code-review

Copy link
Copy Markdown
Contributor

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

Configurator test build ready — commit 7c6c148

Download build artifacts for PR #2659

Available platforms (scroll to the Artifacts section at the bottom of the run page):

  • Windows x64 (ZIP, MSI) and x32 (ZIP, MSI)
  • macOS arm64 (ZIP, DMG) and x64 (ZIP, DMG)
  • Linux x64 (DEB, RPM, ZIP) and aarch64 (DEB, RPM, ZIP)

A GitHub login is required to download artifacts. Build is for testing only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant