frontend: fix compass priority ordering and drag behavior#3874
Merged
patrickelectric merged 1 commit intobluerobotics:masterfrom Apr 13, 2026
Merged
frontend: fix compass priority ordering and drag behavior#3874patrickelectric merged 1 commit intobluerobotics:masterfrom
patrickelectric merged 1 commit intobluerobotics:masterfrom
Conversation
Load initial compass order from COMPASS_PRIO*_ID parameters instead of COMPASS_DEV_ID name order. Only write PRIO_ID params on user drag interactions, not on page load. Fix tab content to follow reordered list using deviceIdNumber for correct parameter mapping, and select the drop position after reordering. Made-with: Cursor
Reviewer's GuideThis PR changes the compass setup UI to initialize and persist compass priority based on COMPASS_PRIO*_ID parameters, ensures the detail tabs follow the reordered list using deviceIdNumber-based indexing, and only writes priority parameters in response to explicit drag-and-drop interactions while keeping the active tab aligned to the dropped position. Sequence diagram for compass drag-and-drop priority updatesequenceDiagram
actor User
participant CompassSetupUI
participant Draggable
participant AutopilotData
User->>CompassSetupUI: Drag compass tab handle
CompassSetupUI->>Draggable: Initialize drag operation
Draggable-->>Draggable: Reorder reordered_compasses
Draggable-->>CompassSetupUI: end(evt)
CompassSetupUI->>CompassSetupUI: onDragEnd(evt)
CompassSetupUI->>CompassSetupUI: $nextTick
CompassSetupUI-->>CompassSetupUI: applyCompassPriority()
loop For each compass in reordered_compasses
CompassSetupUI->>AutopilotData: parameter(COMPASS_PRIOi_ID)
AutopilotData-->>CompassSetupUI: Parameter
CompassSetupUI->>AutopilotData: setParamValue(Parameter, compass.paramValue)
end
CompassSetupUI-->>CompassSetupUI: tab = evt.newIndex
CompassSetupUI-->>User: Active tab follows dropped compass
Class diagram for ArdupilotMavlinkCompassSetup compass priority logicclassDiagram
class ArdupilotMavlinkCompassSetup {
+number tab
+Compass[] compasses
+Compass[] reordered_compasses
+Parameter edited_param
+mounted()
+onDragEnd(evt)
+applyCompassPriority()
+openParameterEditor(parameter)
+printParam(parameter)
}
class Compass {
+number deviceIdNumber
+string deviceName
+number paramValue
}
class Parameter {
+string name
+number value
}
class AutopilotData {
+parameter(name) Parameter
+setParamValue(parameter, value) void
}
ArdupilotMavlinkCompassSetup "*" o-- "*" Compass : displays
ArdupilotMavlinkCompassSetup "*" --> "*" Parameter : uses
ArdupilotMavlinkCompassSetup --> AutopilotData : reads_writes_priority
ArdupilotMavlinkCompassSetup : mounted() loads COMPASS_PRIO_ID order
ArdupilotMavlinkCompassSetup : reordered_compasses sorted by priority
ArdupilotMavlinkCompassSetup : onDragEnd(evt) calls applyCompassPriority()
ArdupilotMavlinkCompassSetup : applyCompassPriority() updates COMPASS_PRIO_ID
ArdupilotMavlinkCompassSetup : UI indexing uses Compass.deviceIdNumber
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- All array accesses now use
compass.deviceIdNumber - 1as an index; consider centralizing this mapping in a small helper (with bounds checking) to avoid duplicated logic and potential off‑by‑one errors ifdeviceIdNumberassumptions change. - The PRIO slots are hardcoded as
[1, 2, 3]inmounted; if the number of priority slots can change or differ between firmware versions, consider deriving this list dynamically (e.g., by probing availableCOMPASS_PRIO*_IDparameters) or defining a shared constant.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- All array accesses now use `compass.deviceIdNumber - 1` as an index; consider centralizing this mapping in a small helper (with bounds checking) to avoid duplicated logic and potential off‑by‑one errors if `deviceIdNumber` assumptions change.
- The PRIO slots are hardcoded as `[1, 2, 3]` in `mounted`; if the number of priority slots can change or differ between firmware versions, consider deriving this list dynamically (e.g., by probing available `COMPASS_PRIO*_ID` parameters) or defining a shared constant.
## Individual Comments
### Comment 1
<location path="core/frontend/src/components/vehiclesetup/configuration/compass/ArdupilotMavlinkCompassSetup.vue" line_range="174" />
<code_context>
<br />
- <template v-if="compass_extern_param[index]?.value ?? 0 === 0">
+ <template v-if="compass_extern_param[compass.deviceIdNumber - 1]?.value ?? 0 === 0">
<b>Mounting Rotation:</b>
- <v-btn class="ml-4" fab x-small @click="openParameterEditor(compass_orient_param[index])">
</code_context>
<issue_to_address>
**issue (bug_risk):** The `??` and `===` precedence likely makes this condition always treat the fallback as `true` instead of comparing the numeric value to zero.
This is parsed as `(compass_extern_param[...]?.value) ?? (0 === 0)`, i.e. effectively `?? true`. So whenever the left side is `null`/`undefined`, the condition is always `true`. If you want “treat undefined as 0, then compare to 0”, it should be `(compass_extern_param[compass.deviceIdNumber - 1]?.value ?? 0) === 0` instead.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
patrickelectric
approved these changes
Apr 13, 2026
Member
|
@Williangalvani do we need to cherry-pick to 1.4 ? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Load initial compass order from COMPASS_PRIO*_ID parameters instead of COMPASS_DEV_ID name order. Only write PRIO_ID params on user drag interactions, not on page load. Fix tab content to follow reordered list using deviceIdNumber for correct parameter mapping, and select the drop position after reordering.
fix #3873
Summary by Sourcery
Align compass configuration UI with compass priority parameters and correct behavior when reordering devices.
Bug Fixes:
Enhancements: