NED velocities are added into GPS data #14762
Merged
+45
−19
Conversation
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
|
Do you want to test this code? You can flash it directly from the Betaflight App:
WARNING: It may be unstable. Use only for testing! |
Contributor
WalkthroughGPS north/east/down velocities are captured from UBLOX NAV‑VELNED / NAV‑PVT, stored in a new Changes
Sequence Diagram(s)sequenceDiagram
participant UBX as UBLOX Receiver
participant GPS as src/main/io/gps.c
participant Store as gpsSolutionData (gps.h)
participant Blackbox as src/main/blackbox/blackbox.c
participant MAVLink as src/main/telemetry/mavlink.c
UBX->>GPS: NAV-VELNED / NAV-PVT messages (velocities + related data)
GPS->>GPS: parse, scale (NAV-PVT ÷10), cast (NAV-VELNED → int16)
GPS->>Store: store velN, velE, velD in gpsSol.velned
rect rgb(220,235,255)
Store->>Blackbox: read velned
Blackbox->>Blackbox: emit GPS G-frame including velN/E/D
end
rect rgb(235,245,220)
Store->>MAVLink: read velned, dop, acc, alt
MAVLink->>MAVLink: populate GPS_RAW_INT / global payloads (replace placeholders with real values)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (9)📚 Learning: 2025-08-24T19:16:35.413ZApplied to files:
📚 Learning: 2025-09-30T20:29:28.877ZApplied to files:
📚 Learning: 2025-10-26T08:47:16.557ZApplied to files:
📚 Learning: 2025-10-05T10:00:10.464ZApplied to files:
📚 Learning: 2025-10-29T16:03:23.891ZApplied to files:
📚 Learning: 2025-09-28T08:42:08.229ZApplied to files:
📚 Learning: 2025-10-29T16:03:51.429ZApplied to files:
📚 Learning: 2025-07-09T09:22:27.106ZApplied to files:
📚 Learning: 2025-07-06T15:59:35.310ZApplied to files:
🧬 Code graph analysis (1)src/main/telemetry/mavlink.c (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/blackbox/blackbox.c(2 hunks)src/main/io/gps.c(2 hunks)src/main/io/gps.h(1 hunks)src/main/telemetry/mavlink.c(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-05T10:00:10.464Z
Learnt from: demvlad
Repo: betaflight/betaflight PR: 0
File: :0-0
Timestamp: 2025-10-05T10:00:10.464Z
Learning: In Betaflight MAVLink telemetry implementation (src/main/telemetry/mavlink.c), all mavlink_msg_*_pack() calls must use component ID 1 (MAV_COMP_ID_AUTOPILOT1) instead of 200 (MAV_COMP_ID_MISSIONPLANNER) for telemetry to be visible in ELRS TX modules and ground control stations like MissionPlanner. Component ID 200 is for ground station software, not flight controllers.
Applied to files:
src/main/telemetry/mavlink.c
🧬 Code graph analysis (1)
src/main/telemetry/mavlink.c (3)
src/test/unit/flight_failsafe_unittest.cc (2)
micros(730-733)micros(730-730)src/test/unit/osd_unittest.cc (2)
micros(1306-1308)micros(1306-1306)src/platform/SIMULATOR/sitl.c (1)
micros(443-446)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: CI / Build (TBS_LUCID_FC)
- GitHub Check: CI / Build (SPRACINGH7EXTREME)
- GitHub Check: CI / Build (AIRBOTG4AIO)
- GitHub Check: CI / Build (SPRACINGH7RF)
- GitHub Check: CI / Build (CRAZYBEEF4FR)
- GitHub Check: CI / Build (APM32F405)
- GitHub Check: CI / Build (IFLIGHT_BLITZ_F722)
- GitHub Check: CI / Build (NUCLEOF446)
- GitHub Check: CI / Build (STM32H750)
- GitHub Check: CI / Build (CRAZYBEEF4SX1280)
- GitHub Check: CI / Build (AT32F435G)
- GitHub Check: CI / Build (RP2350A)
- GitHub Check: CI / Build (STM32H735)
- GitHub Check: CI / Build (STM32F405)
- GitHub Check: CI / Build (STM32F7X2)
- GitHub Check: CI / Build (STM32H723)
- GitHub Check: CI / Build (APM32F407)
- GitHub Check: CI / Build (STM32H725)
- GitHub Check: CI / Build (STM32H743)
- GitHub Check: CI / Build (STM32F745)
- GitHub Check: CI / Build (STM32H730)
- GitHub Check: CI / Build (STM32F446)
- GitHub Check: CI / Build (RP2350B)
Contributor
|
Good work. Just as a note, GPS velocities reported by GPS tend to be more accurate than the global position reported by GPS, so this will be quite worth while to implement into any of our GPS related code. |
Contributor
Author
@Quick-Flash |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/telemetry/mavlink.c(3 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: ledvinap
Repo: betaflight/betaflight PR: 14578
File: src/main/telemetry/jetiexbus.c:547-556
Timestamp: 2025-08-12T18:26:17.016Z
Learning: For Betaflight Jeti ExBus backport PR #14578: ledvinap prefers to keep the PR focused on backporting fixes from iNavFlight rather than expanding into broader architectural improvements. Additional improvements to TX/RX state machine should be handled in separate PRs.
📚 Learning: 2025-08-24T19:16:35.413Z
Learnt from: haslinghuis
Repo: betaflight/betaflight PR: 14620
File: src/main/telemetry/motor_sensor.c:0-0
Timestamp: 2025-08-24T19:16:35.413Z
Learning: In src/main/telemetry/motor_sensor.c DSHOT path, voltage, consumption, and dataAge fields are intentionally left uninitialized since they are not currently used by consumers. Initialization of these fields should be deferred to a separate PR when they become needed.
Applied to files:
src/main/telemetry/mavlink.c
📚 Learning: 2025-09-30T20:29:28.877Z
Learnt from: haslinghuis
Repo: betaflight/betaflight PR: 14683
File: src/main/cli/cli.c:5011-5014
Timestamp: 2025-09-30T20:29:28.877Z
Learning: In src/main/fc/rc_controls.h, the rcSmoothingFilter_t struct fields setpointCutoffFrequency and throttleCutoffFrequency are uint16_t types, not float. They should be printed directly without lrintf() conversion in CLI output.
Applied to files:
src/main/telemetry/mavlink.c
📚 Learning: 2025-10-26T08:47:16.557Z
Learnt from: demvlad
Repo: betaflight/betaflight PR: 0
File: :0-0
Timestamp: 2025-10-26T08:47:16.557Z
Learning: In Betaflight MAVLink telemetry (src/main/telemetry/mavlink.c), DEBUG fields are used as follows: DEBUG[0] indicates whether telemetry should transmit (actual free TX buffer space > mavlink_min_txbuffer), DEBUG[1] shows actual free TX buffer space, and DEBUG[2] through DEBUG[7] are data transmit counters used to determine actual stream data rates.
Applied to files:
src/main/telemetry/mavlink.c
📚 Learning: 2025-10-05T10:00:10.464Z
Learnt from: demvlad
Repo: betaflight/betaflight PR: 0
File: :0-0
Timestamp: 2025-10-05T10:00:10.464Z
Learning: In Betaflight MAVLink telemetry implementation (src/main/telemetry/mavlink.c), all mavlink_msg_*_pack() calls must use component ID 1 (MAV_COMP_ID_AUTOPILOT1) instead of 200 (MAV_COMP_ID_MISSIONPLANNER) for telemetry to be visible in ELRS TX modules and ground control stations like MissionPlanner. Component ID 200 is for ground station software, not flight controllers.
Applied to files:
src/main/telemetry/mavlink.c
📚 Learning: 2025-10-29T16:03:23.891Z
Learnt from: nerdCopter
Repo: betaflight/betaflight PR: 0
File: :0-0
Timestamp: 2025-10-29T16:03:23.891Z
Learning: In Betaflight firmware reviews, do not suggest adding DEBUG_PRINTF, logging, or diagnostic output. Betaflight follows embedded systems best practices with silent error handling using return codes and graceful fallbacks. Debug output was explicitly removed per Betaflight standards (see commit 12b2813ac in PR #14733).
Applied to files:
src/main/telemetry/mavlink.c
📚 Learning: 2025-09-28T08:42:08.229Z
Learnt from: demvlad
Repo: betaflight/betaflight PR: 0
File: :0-0
Timestamp: 2025-09-28T08:42:08.229Z
Learning: In Betaflight, duplicate #define statements in common_pre.h can cause the configurator to not properly detect available telemetry protocols. When USE_TELEMETRY_MAVLINK is defined multiple times, it can break the protocol registration system, causing MAVLink telemetry to disappear from the configurator's Port tab even when properly defined.
Applied to files:
src/main/telemetry/mavlink.c
📚 Learning: 2025-10-29T16:03:51.429Z
Learnt from: nerdCopter
Repo: betaflight/betaflight PR: 0
File: :0-0
Timestamp: 2025-10-29T16:03:51.429Z
Learning: In Betaflight embedded drivers, error handling should use silent failure with bool return values and graceful fallbacks, not DEBUG_PRINTF or console logging. DEBUG_PRINTF is inappropriate for production flight controller code due to code bloat, performance impact, and lack of console output in production hardware. The standard pattern is: return false on error, provide fallback behavior where possible, and avoid string formatting in drivers.
Applied to files:
src/main/telemetry/mavlink.c
📚 Learning: 2025-07-09T09:22:27.106Z
Learnt from: jianpingwu1
Repo: betaflight/betaflight PR: 14519
File: lib/main/GD32F4/Drivers/GD32F4xx_standard_peripheral/Source/gd32f4xx_can.c:110-110
Timestamp: 2025-07-09T09:22:27.106Z
Learning: In Betaflight GD32F4 firmware library integration, typos and minor issues in vendor-supplied code from GigaDevice Semiconductor are kept as-is to maintain synchronization with upstream library updates, avoiding local modifications that could complicate future library version updates.
Applied to files:
src/main/telemetry/mavlink.c
📚 Learning: 2025-07-06T15:59:35.310Z
Learnt from: ledvinap
Repo: betaflight/betaflight PR: 14517
File: src/main/cms/cms_menu_firmware.c:191-192
Timestamp: 2025-07-06T15:59:35.310Z
Learning: In the Betaflight codebase, the constants MAX_MANUFACTURER_ID_LENGTH and MAX_BOARD_NAME_LENGTH already include space for the null terminator, so arrays should be sized exactly to these constants without adding +1. The previous +1 additions were over-allocating by one byte.
Applied to files:
src/main/telemetry/mavlink.c
🧬 Code graph analysis (1)
src/main/telemetry/mavlink.c (1)
src/platform/SIMULATOR/sitl.c (1)
micros(443-446)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: CI / Build (SPRACINGH7RF)
- GitHub Check: CI / Build (STM32H750)
- GitHub Check: CI / Build (STM32H725)
- GitHub Check: CI / Build (STM32H735)
- GitHub Check: CI / Build (STM32H743)
- GitHub Check: CI / Build (STM32H723)
- GitHub Check: CI / Build (STM32H730)
- GitHub Check: CI / Build (RP2350B)
- GitHub Check: CI / Build (STM32F745)
- GitHub Check: CI / Build (RP2350A)
- GitHub Check: CI / Build (APM32F405)
- GitHub Check: CI / Test
🔇 Additional comments (4)
src/main/telemetry/mavlink.c (4)
313-313: LGTM - Heading accuracy sentinel value is correct.Using
UINT32_MAXfor unknown heading/track uncertainty is the correct approach, consistent with MAVLink's pattern for other accuracy fields. This properly signals to ground stations that heading accuracy data is unavailable, rather than falsely advertising perfect accuracy with a zero value.
332-336: LGTM - NED velocity implementation looks correct.The NED (North-East-Down) velocity components are now populated from
gpsSol.velnedfields, which according to the PR objectives are converted from UBLOX NAV-PVT mm/s to cm/s units. This matches the MAVLink GLOBAL_POSITION_INT specification that expects ground speed components in m/s × 100 (cm/s). This is a valuable improvement as GPS-reported velocities are typically more accurate than derived position velocities.
349-351: LGTM - GPS origin altitude and timestamp improvements.Replacing the placeholder zeros with actual GPS home position altitude (
GPS_home_llh.altCm * 10for mm units) and real timestamps (micros()) provides more accurate and useful data to ground stations. The unit conversions are correct per the MAVLink GPS_GLOBAL_ORIGIN specification.
304-311: GPS accuracy field units are correctly handled.Verification confirms that
gpsSol.acc.hAcc,gpsSol.acc.vAcc, andgpsSol.acc.sAccare already in millimeter and mm/s units as expected by MAVLink:
- Structure (gps.h): Fields are documented as mm and mm/s
- MSP source (msp.c lines 3680-3682): Explicitly converts cm to mm with
* 10- UBX source (gps.c lines 2257-2259): Uses u-blox native units (already mm/s)
- Virtual GPS (gps_virtual.c lines 36-38): Hardcoded in correct units
No conversions are needed in telemetry/mavlink.c; the values are already correct.
blckmn
approved these changes
Nov 14, 2025
blckmn
approved these changes
Nov 14, 2025
haslinghuis
approved these changes
Nov 14, 2025
Contributor
Author
|
My self made GCS sees the new GPS data by MAVLink well.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
NED velocities are added into GPS data.
GPS NED velocities are used in MAVLink telemetry and stored in black box log.
The GPS accuracy is added into MAVLink telemetry.
Summary by CodeRabbit
New Features
Improvements