fix(mavlink): correct BODY_NED position setpoint parsing and bitmasks#27686
fix(mavlink): correct BODY_NED position setpoint parsing and bitmasks#27686PuRe-ToX wants to merge 4 commits into
Conversation
Previously, SET_POSITION_TARGET_LOCAL_NED messages using MAV_FRAME_BODY_NED with position-only or partial-velocity bitmasks were incorrectly rejected. This fix properly rotates body position offsets to local Earth coordinates and replaces the bitwise OR with AND for velocity/acceleration ignore masks.
🔎 FLASH Analysispx4_fmu-v5x [Total VM Diff: 248 byte (0.01 %)]px4_fmu-v6x [Total VM Diff: 248 byte (0.01 %)]Updated: 2026-06-17T19:57:36 |
|
Thanks for digging into #27617. Stepping back from the diff: this isn't actually a regression, and I don't think a code change is the right first move here. BODY_NED position setpoints have never worked in PX4. Before the v1.12 offboard overhaul the coordinate frame was only ever applied to velocity, and the 2021 rework (#16739) kept exactly that: BODY_NED rotates velocity/acceleration and intentionally NANs position, because per the MAVLink spec BODY_NED is a rotated frame and "position in body frame" is ambiguous unless you mean an offset from the current position, which is a different frame (BODY_OFFSET_NED) that PX4 also never supported. Both frames are deprecated in MAVLink in favor of BODY_FRD. The real bug was in our docs, which listed the supported input combinations and the supported coordinate frames as two independent lists. That's what misled the reporter in #27617, and it's fixed in #27806. What this PR adds, "current position + yaw-rotated offset, re-anchored on every message", is a new feature with invented semantics, not a bugfix. Since offboard requires streaming setpoints above 2 Hz, a constant "2 m forward" target never converges and effectively becomes an unbounded velocity command. It would also silently flip existing BODY_NED velocity users into position hold if their type_mask doesn't set the position ignore bits. Whether PX4 should support body-relative position stepping at all, and on which frame (BODY_FRD being the non-deprecated candidate), is a design decision the maintainers need to make first. The one piece of #27617 that's a genuine code quirk is the grouped velocity ignore bits in the BODY_NED branch (vx-only gets rejected). If you want to carve that out into a small focused PR we can discuss the right per-axis semantics there, keeping in mind that "ignore vy" isn't representable after rotating body to earth frame, and that ignored axes should stay NAN rather than become commanded zeros. Closing this one: the position half needs a maintainers design decision before any implementation, and the mask fix deserves its own focused PR where basically none of this diff would carry over. Would genuinely welcome that PR if you're up for it, and the body-relative position question is worth raising, I'll bring it up with the maintainers. That said, this was a well put together first contribution: clean diff, clear description, tested before submitting. Good job, and I hope to see you back for the mask fix. |
Previously, 'SET_POSITION_TARGET_LOCAL_NED' messages using 'MAV_FRAME_BODY_NED' with position-only or partial-velocity bitmasks were incorrectly rejected. This fix properly rotates body-position offsets to local Earth coordinates and replaces the bitwise OR with AND for the velocity/acceleration ignore masks.
Solved Problem
Fixes #27617
The current implementation of 'MavlinkReceiver::handle_message_set_position_target_local_ned' incorrectly rejects valid 'MAV_FRAME_BODY_NED' messages from companion computers.
Solution
Changelog Entry
Bugfix: MavlinkReceiver now correctly parses position-only and partial-velocity
SET_POSITION_TARGET_LOCAL_NEDmessages in theBODY_NEDframe.Alternatives
N/A. Without this fix, offboard companion computers cannot reliably use the BODY_NED frame for targeted positional movements or partial velocity vectors.
Test coverage
Tested locally in PX4 SITL (Gazebo gz_x500). Wrote a custom PyMAVLink script to send position-only MAV_FRAME_BODY_NED messages with a type_mask ignoring velocity and acceleration.
Before: Firmware rejected the command with WARN [mavlink] SET_POSITION_TARGET_LOCAL_NED invalid.
After: Firmware successfully parsed the bitmask, transformed the position coordinates, and accepted the command silently without throwing warnings.
Context
This is highly relevant for edge-AI companion computers (such as the Jetson Nano) that attempt to send relative coordinate movements to the flight controller via MAVLink.