[PX4] AM32 EEPROM settings support - #14013
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive AM32 ESC EEPROM settings support to QGroundControl for PX4 autopilots, enabling wireless configuration of AM32 ESCs via MAVLink. The implementation includes schema-based settings management with version-aware field visibility, per-ESC selection with majority-match detection, and a complete UI for reading/writing ESC parameters.
Changes:
- Implemented AM32 EEPROM read/write via MAVLink ESC_EEPROM messages with schema-based field definitions
- Added PX4 AutoPilot plugin component with QML UI for ESC settings configuration
- Refactored ESC status indicators to use per-ESC online status instead of shared bitmask, with grid layout support for >4 ESCs
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
Vehicle.h/cc |
Integrated AM32EepromFactGroupListModel into Vehicle class |
eeprom.json |
Added bundled AM32 EEPROM schema with field definitions and version-specific metadata |
EscStatusFactGroupListModel.cc |
Fixed per-ESC online status extraction from bitmask instead of using shared value |
AM32EepromSchema.h/cc |
Implemented schema loader with remote fetch capability and version-aware field filtering |
AM32EepromFactGroupListModel.h/cc |
Created MAVLink message handling for ESC EEPROM with write masks and setting comparison |
AM32EepromFact.json |
Defined fact metadata for AM32 EEPROM version and firmware info fields |
EscIndicator.qml/EscIndicatorPage.qml |
Updated to per-ESC online status and grid layout for multiple ESCs |
PX4AutoPilotPlugin.h/cc |
Added conditional AM32Component instantiation based on DSHOT_ESC_TYPE parameter |
AM32Component.h/cc/qml |
Created new vehicle component for AM32 ESC configuration |
AM32SettingsComponent.qml |
Implemented full settings UI with ESC selection and grouped setting controls |
AM32SettingSlider.qml |
Created reusable slider control with pending change indicators |
AM32ComponentSummary.qml |
Added summary view showing ESC detection status |
Build ResultsPlatform Status
All builds passed. Pre-commit
Pre-commit hooks: 2 passed, 46 failed, 7 skipped. Test Resultslinux-coverage: 90 passed, 0 skipped Code CoverageCoverage: 61.8% No baseline available for comparison Artifact Sizes
Updated: 2026-06-02 00:22:02 UTC • Triggered by: Linux |
|
@DonLakeFlyer do you have any feedback on the design? |
|
I'll try to to get to this. I'm swamped with my own wildlife drone testing right now. |
Okay no rush, I just wasn't sure if you saw it. Let me know if I can help. |
|
First quick comment: With regards to the new |
|
For the places you are using sliders, what about using FactTextFieldSlider. Then you get both slider and type in support: |
There's a couple of reasons I went with the message "package" structure
There's a lot more context in the original RFC mavlink/rfcs#27 |
I'm all good as long as there is some way to determine whether it worked or not. |
|
@dakejahl Can you squash to a reasonable set of commits. Rebase and then I'll submit it for another CoPilot review. |
e039772 to
98595e2
Compare
98595e2 to
a00368e
Compare
|
I'm also going to run some Claude /review on this. Let's hold off on copilot since it can really thrash the PR discussion with noise. |
a00368e to
9f39bae
Compare
|
I guess the thing I wonder about with this is why it's valuable to increase the surface area of QGC for something which is already supported by an external tool. Why not go the same route as ArduPilot which has a mode to make the regular configurator? Also means you need to chase changes made to the original tool. I always worry about biggish new things since I tend to get left holding the bag on them for years to come... |
AM32 configurator requires a physical USB connection.
The eeprom data has a
Totally understand that. I can always make a separate Qt tool for AM32 over MAVLink, but it's nice to have everything in one tool. The impact to other modules is low, so can be easily removed if it becomes a burden. We get this complaint very often:
And PX4 doesn't support "ESC passthrough" (the USB protocol) so indeed they need to use an FC that runs betaflight. So they have to disassemble their airframe just configure settings or flash fw or they need an FC that supports both PX4 and Betaflight. And the counter argument is
But a lot of people don't know this and end up frustrated. All of this work is coming from real world user complaints around setting up and updating AM32 firmware. |
|
Good reasons to add this then... |
Yup. I'll work with Claude today/tomorrow and try to get this as good as possible before you review so that you don't have to spend too much time on it 😃 |
0ed1f59 to
a011d5f
Compare
|
@dakejahl Is this working and what's blocking this? |
|
@PetervdPerk-NXP it does indeed work! There is one issue which needs to be fixed, we need to create a whitelist for mismatched settings and add |
ESC_EEPROM write messages were encoded with the vehicle's system/component IDs as the source, causing mavlink-router to drop them since they appeared to originate from the autopilot rather than the GCS. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Re-syncs the bundled schema to the canonical AM32 PR295 artifact: stallProtection and telemetry30ms become uint8 magnitudes, motorPoles (max 64) and canEscIndex (max 31) ranges, motorKv unit, temperature/current limit sentinels, the new inputType DroneCAN value, and startupMelody as a bluejay blob.
…ghting Motor direction alternates CW/CCW across a multirotor, so a mismatch there is expected. A whitelist (isMismatchExempt) skips such settings in settingsMatch() (ESC border color) and forces them to report as matching in _updateMajorityMatches() (per-setting dots), so only critical settings like motor poles or KV flag a mismatch.
…ecision - determineValueType now scans version/firmware overrides so timingAdvance's 0.9375 deg / -9.375 conversion (defined only under versions.eeprom:3+) is no longer dropped on EEPROM v3+, where the Fact was frozen as uint8. - toRaw rounds to nearest (std::lround) instead of truncating, removing a systematic downward bias when writing every number field. - AM32Setting now sets decimalPlaces and rawIncrement from the schema so sliders show the right precision and each step maps to one EEPROM raw count.
Brings the QGC AM32 settings page to parity with the am32-configurator, verified against AM32 firmware as ground truth. - New AM32SettingCombo (the enum control QGC lacked) and AM32SliderCell. Enum options come from the Fact metadata, now populated via setEnumInfo() from the schema and refreshed when version overrides apply. - variablePwmFreq and lowVoltageCutoff become version-gated combos, fixing the silent corruption where a checkbox clobbered the third enum value (By RPM / Absolute) to 0/1. - Expose previously-missing settings: brakeOnStop and inputType combos; directionReversed, bidirectionalMode, rcCarReversing, sineStartup checkboxes; servo, brake and sine sliders. - Dependent-field gating: lowVoltageThreshold (mode 1), absoluteVoltageCutoff (mode 2), activeBrakePower (brake mode 2). - AM32SettingSlider honors the schema disabledValue: a 'Disabled' label, plus a toggle for out-of-range sentinels (temperatureLimit 255). inputType intentionally includes DroneCAN (firmware-valid; QGC's MAVLink/PX4 context differs from the configurator's USB-only curation). stallProtection and telemetry30ms stay checkboxes to mirror the configurator. Configurator-stale ranges/sentinels (motorPoles 36, temperatureLimit 141, currentLimit 202) are not copied; QGC follows the firmware.
Mirrors the upstream Inc/eeprom.json fix: the firmware documents active_brake_power as 1-5 percent duty cycle, but the schema allowed up to 10. Keeps the bundled schema byte-identical to AM32 PR295.
|
So what's the state of this? |
|
I wanted to wait until the AM32 PR is in am32-firmware/AM32#295 and get the metadata hosted so that we're not relying on the built-in version of the schema. Just waiting on the maintainers over there to do something. |
|
Ok, this is going to miss next stable then. I'm wrapping up now. |
|
Should we have a blocked label or something that shows a PR is waiting on something else first |
Summary
This is effectively the AM32 Configurator built into QGC. This enables a wireless method to check and edit ESC settings.
The accompanying PX4 PR is here PX4/PX4-Autopilot#26263
Details
New files:
AM32Component— PX4 auto-pilot plugin component for the AM32 settings pageAM32SettingsComponent.qml— Full settings UI with read/write controls, unsaved change indicators, and setting slidersAM32EepromFactGroupListModel— MAVLink message handling for ESC EEPROM read/write, including write masks and majority matching across selected ESCsAM32EepromSchema— Remote schema fetching withCachedFileDownload, JSON parsing, version overrides, and conversion factor supportModified:
EscIndicator.qml/EscIndicatorPage.qml— Per-ESC online status instead of shared bitmask; grid layout for motor count >4EscStatusFactGroupListModel.cc— Fix index-to-bitmask mapping for 4-per-message groupsVehicle.cc/h— Integrate AM32 EEPROM modelTest plan
Screenshots
All settings match
One ESC beep volume is different
ESC indicator grid layout
