output assignment: firmware-authoritative MSP2 READ/QUERY API#11564
Open
sensei-hacker wants to merge 9 commits into
Open
output assignment: firmware-authoritative MSP2 READ/QUERY API#11564sensei-hacker wants to merge 9 commits into
sensei-hacker wants to merge 9 commits into
Conversation
Exposes the firmware's post-boot output assignment as an MSP message so the Configurator no longer needs to mirror the assignment algorithm in JS. ## New MSP messages MSP2_INAV_OUTPUT_ASSIGNMENT (0x210E) — READ Response: [(u8 outputIndex, u8 type, u8 number)] per assigned output type: 1=MOTOR 2=SERVO 3=LED; number: 1-indexed ordinal Sent once on Mixer tab load; Configurator uses this for display instead of running getTimerMap() locally. MSP2_INAV_QUERY_OUTPUT_ASSIGNMENT (0x210F) — QUERY/preview Request: (u8 timerCount, [u8 timerId, u8 outputMode] × N) Response: same format as READ Lets Configurator preview assignments for proposed timer overrides without save+reboot. Firmware runs its own algorithm on the proposed overrides (save/restore approach, no hardware side-effects) and returns the result. ## Implementation - pwm_mapping.h: expose timMotorServoHardware_t; add OUTPUT_ASSIGNMENT_TYPE_* defines; declare pwmGetOutputAssignment() and pwmCalculateAssignment() - pwm_mapping.c: promote timOutputs to static; add getter and pure-function calculator (saves/restores timerHardware flags and timerOverrides config) - fc_msp.c: READ handler in mspFcProcessOutCommand (#ifndef SITL_BUILD); QUERY handler in mspFCProcessInOutCommand (#ifndef SITL_BUILD) - msp_protocol_v2_inav.h: reserve 0x210E and 0x210F ## Fallback Old firmware returns error/no-data; Configurator falls back to getTimerMap() (the existing JS algorithm). No behaviour change for users on older firmware. Companion configurator PR: iNavFlight/inav-configurator#XXXX
Contributor
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
MAX_PWM_OUTPUTS expands to MAX_PWM_OUTPUT_PORTS which is not defined in SITL targets. The struct and its dependent function declarations are hardware-only; wrap them in #ifndef SITL_BUILD to match the existing pattern used in fc_msp.c for the MSP handlers.
INAV has no runtime ASSERT macro — STATIC_ASSERT is compile-time only. The ptrdiff_t bounds checks were unnecessary since timMotors/timServos pointers are guaranteed to point within timerHardware[] by construction in pwmBuildTimerOutputList(). Replace with the direct cast used in the READ handler.
|
Test firmware build ready — commit Download firmware for PR #11564 234 targets built. Find your board's
|
The isMixerUsingServos parameter was marked UNUSED; the function populated timServos[] with all servo-capable hardware pins regardless of how many servos the mixer actually needs. This caused MSP2_INAV_OUTPUT_ASSIGNMENT to report phantom servo assignments — e.g., reporting 4 servo outputs on a quadcopter with only 1 servo in the mixer. Fix mirrors the existing motor pattern: only add a servo slot when maxTimServoCount < servoCount, and skip all servo slots when isMixerUsingServos is false.
pwmBuildTimerOutputList() previously used getServoCount() which reads mixerProfiles->ServoMixers via computeServoCount(). That PG array is never cleared by the Configurator's MSP2_INAV_SET_SERVO_MIXER handler (which writes only to customServoMixers). Stale rules in mixerProfiles->ServoMixers survive settings-preserving firmware flashes and cause phantom servo outputs even with an empty servo mixer. Fix: count servo outputs directly from customServoMixers — the same PG source that loadCustomServoMixer() and the CLI smix command use for the current profile. The inactive profile (dual-profile setups) continues to use mixerServoMixersByIndex(nextMixerProfileIndex) which is populated when the user saves a profile via the Configurator.
…tList The inactive profile's mixerProfiles(j)->ServoMixers is a separate PG that the Configurator never writes to via MSP2_INAV_SET_SERVO_MIXER, so it can contain stale data from old configurations that survives settings-preserving firmware upgrades. Checking it caused phantom servo outputs whenever that stale data had any non-zero rate rule. Use only customServoMixers — the Configurator-facing PG — to determine how many servo outputs to allocate. Dual-profile output reallocation is handled at runtime by outputProfileHotSwitch() when the user switches mixer profiles.
Remove the isMixerUsingServos gate: that flag reads mixerProfiles->ServoMixers which can disagree with customServoMixers in both directions (stale non-zero data → phantom servos; clean data when real rules exist → zero servos). Scanning customServoMixers directly and unconditionally is always correct — an empty mixer (all rate=0) naturally produces servoCount=0 without needing the flag.
The isMixerUsingServos parameter was never read inside the function body — only the isMixerUsingServos() function was called. Remove the dead parameter and update both call sites to fix -Werror=unused-parameter CI failure.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds firmware-authoritative MSP2 APIs for output assignment so Configurator can read and preview motor/servo pin assignments without duplicating firmware logic.
Changes:
- Adds MSP2 opcode definitions for output assignment READ and QUERY.
- Exposes timer motor/servo assignment structures and calculation functions.
- Implements READ/QUERY handlers and a save/restore-based assignment simulation path.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/main/msp/msp_protocol_v2_inav.h |
Reserves new MSP2 output assignment message IDs. |
src/main/fc/fc_msp.c |
Adds MSP response handlers for finalized and previewed output assignments. |
src/main/drivers/pwm_mapping.h |
Exposes output assignment types and APIs outside the PWM mapping module. |
src/main/drivers/pwm_mapping.c |
Stores finalized assignments and adds assignment recalculation using proposed timer overrides. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pwmBuildTimerOutputList now scans all mixer profiles (matching computeServoCount/getServoCount) and uses 1+max-min formula so the assignment simulation returns the same servo count as the real pwmInitServos() path on multi-profile targets. Also zero-initialise tempOut in the QUERY MSP handler so an early return from pwmCalculateAssignment cannot expose uninitialised memory.
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
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.
Problem
The Configurator's Mixer/Outputs tab re-implements the firmware's output assignment algorithm in JavaScript (
getTimerMap()). When firmware and Configurator versions differ, the tab shows wrong pin assignments. On targets with explicit timer overrides (e.g.OUTPUT_MODE_MOTORS/OUTPUT_MODE_SERVOSset per-timer), the actual motor/servo pin mapping silently changes when firmware is upgraded independently of the Configurator.Solution
Two new MSP messages make the firmware the sole authority on assignments:
MSP2_INAV_OUTPUT_ASSIGNMENT(0x210E) — READOn tab load, the Configurator requests the finalized post-boot output assignment. Response:
[(u8 outputIndex, u8 type, u8 number)]per assigned output, wheretypeis 1=MOTOR or 2=SERVO andnumberis the 1-indexed motor/servo ordinal.MSP2_INAV_QUERY_OUTPUT_ASSIGNMENT(0x210F) — QUERY/previewRequest:
(u8 timerCount, [u8 timerId, u8 outputMode] × N). The Configurator sends proposed timer overrides; the firmware runs its assignment algorithm on those overrides (save/restore approach — no hardware side-effects) and returns the resulting assignment in the same format as READ. Enables live preview when the user changes a timer override dropdown, without save+reboot.Old Configurator connected to new firmware: ignores the new messages, uses existing
getTimerMap()(no regression).New Configurator connected to old firmware: falls back to
getTimerMap()automatically when the messages return error.Changes
src/main/drivers/pwm_mapping.h: exposetimMotorServoHardware_t; addOUTPUT_ASSIGNMENT_TYPE_*defines; declarepwmGetOutputAssignment()andpwmCalculateAssignment()src/main/drivers/pwm_mapping.c: promotetimOutputstostatic timMotorServoHardware_t timOutputsStatic; addpwmGetOutputAssignment()getter; addpwmCalculateAssignment()— simulates assignment for proposed overrides using save/restore oftimerHardware[].usageFlagsandtimerOverridesconfigsrc/main/fc/fc_msp.c: READ handler inmspFcProcessOutCommand(#ifndef SITL_BUILD); QUERY handler inmspFCProcessInOutCommand(#ifndef SITL_BUILD) withtimerCountclamped toHARDWARE_TIMER_DEFINITION_COUNTsrc/main/msp/msp_protocol_v2_inav.h: reserve 0x210E and 0x210FTesting
0x210EREAD: returns 8 valid tuples — Motor 1–4 on outputs 0–3, Servo 1–4 on outputs 4–70x210FQUERY: sending current overrides returns result matching READ exactly$X>response frames)["Motor 1", ..., "Servo 1", ...]Code Review
Code review performed. Issues addressed:
savedFlags[]sized toTIMER_HW_MAX = 64(notMAX_PWM_OUTPUTS) to cover all timer hardware entries; early-return guard addedtimerCountclamped in QUERY handler to prevent excess loop iterationsOUTPUT_ASSIGNMENT_TYPE_LEDremoved (LED outputs conveyed via existingMSP2_INAV_OUTPUT_MAPPING_EXT2usageFlags)pwmCalculateAssignment()must be called only from the main loop / MSP taskNotes
MSP2_INAV_OUTPUT_MAPPING_EXT2(0x210D)pwmCalculateAssignment()is not ISR-safe; it must only be called from the MSP task (main loop context)