ConfigHWIDs: decode MAVn_DEVID parameters to human-readable strings

[Greninja44]

Summary

ArduPilot recently added MAVn_DEVID parameters (see ArduPilot/ardupilot#29762) which identify which serial/USB/network/CAN/scripting device a MAVLink channel's MAVn_* parameters correspond to. The HW ID page didn't know about this new parameter family, so those rows fell through to the IMU device-type decode and showed a meaningless raw number instead of a readable name.

This adds a mavlink_devid lookup (matching the naming used by ArduPilot/MAVProxy: SERIAL1, USB0, NET_P1, CAN_D1_UC_S1, SCR_SDEV1, etc.) and wires it into the existing DeviceInfo.DevType decode path used by the HW ID grid, following the same pattern already used for compass/IMU/baro/airspeed device types.

Fixes #3761

Mapping added

devid	Name
0	Unknown
6	USB0
14–78 (+8)	SERIAL1–SERIAL9
174–198 (+8)	NET_P1–NET_P4
334	CAN_D1_UC_S1
414	CAN_D2_UC_S1
494	SCR_SDEV1
502	SCR_SDEV2

Unrecognized values fall back to the raw number, consistent with how the other device-type enums already behave.

Test plan

• Added MissionPlannerTests/Utilities/DeviceInfoTests.cs covering all 19 mappings plus the unrecognized-value fallback.
• Verified in an isolated harness (compiling the real Device.cs/DeviceInfo.cs unmodified) that all 19 mappings, the unknown-value fallback, and the pre-existing compass/IMU/baro/airspeed/UAVCAN decode paths are unaffected.

[fallenmi]

The lookup values do not match the full device IDs that ArduPilot actually publishes. The MAVn_DEVID parameter metadata added in ArduPilot #29762 defines, for example, USB/SERIAL0 as 65542, SERIAL1 as 65798, NET_P1 as 131078, CAN_D1_UC_S1 as 196614, and SCR_SDEV1 as 262150. This patch instead tests and maps 6, 14, 174, 334, and 494.

I compiled the exact Device.cs from head 112fd6a9 into a small external consumer and fed it the real values from ArduPilot's parameter declaration. Every tested family remains undecoded and returns the raw number:

65542 -> 65542 (expected USB0)
65798 -> 65798 (expected SERIAL1)
131078 -> 131078 (expected NET_P1)
196614 -> 196614 (expected CAN_D1_UC_S1)
196622 -> 196622 (expected CAN_D2_UC_S1)
262150 -> 262150 (expected SCR_SDEV1)

The small values in the new tests omit the device-type and address fields from AP_HAL::Device::make_bus_id; they are not the values stored in the parameter. Please map the full IDs (or decode the bit fields algorithmically), update the regressions to use ArduPilot's emitted values, and include the currently omitted CAN D1/D2 UC S2/S3 and scripting SDEV3 entries. Replacing the enum constants with the upstream full values makes the same consumer pass.

Disclosure: I used OpenAI Codex to inspect the exact revisions and run this real-source mapping oracle; I verified the result locally.

[tridge]

Deprecated — see below for the updated review.

Previous review (2026-08-30, at head 3ac813d)

Automated review note — AI-generated (Claude), validated against the live diff. Please sanity-check before acting.
Full report: https://uav.tridgell.net/DevCallReviews/2026_08_30/devcall_pr_reviews.html#prMissionPlanner-3762

Reviewed at head 3ac813d3a9. Verdict: REQUEST CHANGES — the idea is good and worth doing, but the constants are wrong, so the feature doesn't currently work on any real vehicle. @fallenmi already filed CHANGES_REQUESTED on 2026-08-29 with the same diagnosis; this independently reproduces and confirms it.

1. BUG — ExtLibs/Utilities/Device.cs:225-242: all 18 non-zero values are incorrect. ArduPilot packs a devid as bus_type | bus<<3 | address<<8 | devtype<<16 (AP_HAL/Device.h), and AP_SerialManager emits make_bus_id(BUS_TYPE_SERIAL=6, can_driver, instance, DeviceType) with DeviceType{UART=1, NETWORKING=2, CANBUS=3, SCRIPTING=4}. I recomputed the values and they match ArduPilot's own published @Values in GCS_MAVLink_Parameters.cpp exactly:

   Device	Correct	This PR
   SERIAL0 (USB)	65542	6
   SERIAL1	65798	14
   NET_P1	131078	174
   CAN_D1_UC_S1	196614	334
   CAN_D2_UC_S1	196622	414
   SCR_SDEV1	262150	494
   SCR_SDEV2	262406	502

   The root cause is reproducible: every value here equals 6 | (serial_manager_index << 3) using the index defines from AP_SerialManager.h (NET_P1=21, CAN_D1=41, CAN_D2=51, SCR=61/62) — the port index went into the bus field and the devtype byte was dropped. Net effect: every MAVn_DEVID row still shows a raw number. Only Unknown = 0 is right.

2. BUG — MissionPlannerTests/Utilities/DeviceInfoTests.cs:13-33: the tests transcribe the same wrong constants into their expectation table, so they assert the enum equals itself and can never catch the above. Rebuild the table from ArduPilot's published @Values.

3. ISSUE — the strategy is fragile even once the numbers are fixed. Enumerating whole 24-bit devids means any valid-but-unlisted port falls back to a raw number; upstream documents 23 non-zero ids and this lists 18 (missing CAN_D1/D2_UC_S2/S3 and SCR_SDEV3), and future SERIAL10+/NET_P5+ would need another hand edit. Every other family in this file decodes the devtype byte and lets the existing BusType/Bus/Address columns carry the rest — e.g. devtype==3 → CAN_D{bus+1}_UC_S{address+1}, devtype==1 → address==0 ? "SERIAL0 (USB)" : "SERIAL{address}". That's closed-form and needs no maintenance.

4. NOTE — green CI proves nothing here. All three workflows only run msbuild, with no vstest/dotnet test step, and the test project has only ActiveCfg (no Build.0) in the solution config, so the solution build doesn't even compile it. Worth adding a test step alongside the fix.

5. NOTE. Upstream calls id 65542 SERIAL0 (USB on ChibiOS); USB0 here is platform-specific — SERIAL0 matches the param docs users read. And the "unknown" test case uses 12345, which decodes to an I2C device; a better negative case is an in-family but unmapped id such as SERIAL10.

Correct and unaffected: devid == 0 → "Unknown", and unmatched values render as a decimal string rather than throwing. Float precision is a non-issue — devids are ≤24 bits and exactly representable in float32, which ArduPilot documents as deliberate.

[Greninja44]

You're both right, thanks for catching this. Pushed a fix (3294d86):

• Replaced the enumerated small-literal table with a closed-form decode over the existing bus_type/bus/address/devtype bitfields (devid = bus_type | bus<<3 | address<<8 | devtype<<16), matching AP_SerialManager::UARTState::get_device_id(): devtype==1 (UART) -> SERIAL0 (USB)/SERIALn, devtype==2 (NETWORKING) -> NET_Pn, devtype==3 (CANBUS) -> CAN_D{bus+1}_UC_S{address+1}, devtype==4 (SCRIPTING) -> SCR_SDEVn.
• This also covers everything not explicitly enumerated before (CAN D1/D2 UC S2/S3, SCR_SDEV3, SERIAL10+, future NET_P5+) without further hand-maintenance, per @tridge's suggestion.
• Renamed USB0 naming to SERIAL0 (USB) to match the param docs, per the note that upstream calls it SERIAL0.
• Rebuilt DeviceInfoTests.cs with the real published devid values (65542, 65798, ... 262406) plus a SERIAL10 case (unlisted port, still decodes) and an unknown-devtype-family fallback case.

I verified the new decode logic against the exact constants from your review (65542/65798/131078/196614/196622/262150/262406) and they match. I wasn't able to get a full msbuild/vstest run working in this environment, so I'd appreciate a look at CI once it runs, and agree a vstest step would be worth adding separately.

[tridge]

Deprecated — see below for the updated review.

Previous review (2026-08-30)

Automated review note — AI-generated (Claude), validated against the live diff. Please sanity-check before acting.
Full report: https://uav.tridgell.net/DevCallReviews/followups/2026_08_30_1728/devcall_pr_reviews.html

Re-reviewed at head 3294d86bfa: both bugs from my previous comment are resolved, along with the design issue and the naming note — 4 of 5 findings closed, 1 still open. Verdict moves REQUEST CHANGES → COMMENT. My earlier comment above is superseded.

• BUG 1 (all 18 non-zero constants wrong) — RESOLVED. You deleted the enumerated table and replaced it with the closed-form decode. I re-implemented your logic independently and ran it over ArduPilot's 24 published @Values: every id resolves to the correct family and port, 0 numeric mismatches — including the five the old table omitted (CAN_D1/D2_UC_S2/S3 = 196870/197126/196878/197134, SCR_SDEV3 = 262662).
• BUG 2 (tests encoded the same wrong constants) — RESOLVED. The table is rebuilt from the published values and the tests now feed real ids through DeviceInfo.DevType instead of asserting the enum against itself.
• ISSUE (whole-devid enumeration is fragile) — RESOLVED. The decode is maintenance-free now: 68102 → SERIAL10 with no table edit, and future NET_P5+/SERIAL10+ work automatically.
• NOTE (USB0, weak negative test) — RESOLVED. Now SERIAL0 (USB), and 12345 is replaced by two genuinely useful cases (68102 in-family-unlisted, 327686 unknown family).
• NOTE (CI never runs the tests) — STILL OPEN, as you acknowledged. Re-verified here: no vstest/dotnet test step in any of the three workflows, and MissionPlanner.sln still has only ActiveCfg (no Build.0) for the test project, so it is neither compiled nor executed. Since you mentioned you couldn't run msbuild/vstest locally either, this test file has never actually been compiled. I read it for compile-correctness and it looks sound — SDK-style csproj auto-globs it, the ProjectReference is there, the int→uint literal conversions are legal — but that's inspection, not a build. Worth a separate PR adding a test step.

One thing I'd ask for before merge, and two small notes:

1. ISSUE — ExtLibs/Utilities/Device.cs was converted CRLF → LF wholesale, so the whole file reads as deleted and re-added: the base blob has CR on all 285 lines, the head blob on 0 of 294, while the real change underneath is only ~38 added lines. That destroys git blame and will conflict with anything else touching the file. A re-push preserving CRLF would fix it; the code itself is fine. (To be precise: the repo is mixed, not uniformly CRLF — DeviceInfo.cs was already LF — but Device.cs specifically was CRLF and got flipped.)
2. NOTE — string parity is 23/24, not exact. Upstream publishes 65542 as SERIAL0 (USB on ChibiOS); you return SERIAL0 (USB). Dropping the ChibiOS qualifier is defensible for a cross-platform GCS — just make it a deliberate choice, and if you'd rather match upstream verbatim, update the test expectation too.
3. NOTE — the decode switches on devtype without checking bus_type, so a synthetic id like 65536 (bus_type=0, devtype=1) yields SERIAL0 (USB) rather than the raw number. Unreachable from real firmware, which always emits BUS_TYPE_SERIAL (6); adding that to the guard would make it exact.

Test coverage is otherwise good. Two small gaps if you're touching it anyway: nothing passes a non-MAV parameter name, so the new StartsWith("MAV") branch isn't guarded against regressing the existing COMPASS/BARO/ASP/INS paths; and there's no devtype==0 case (devid 6, which correctly yields "6").

Nice turnaround — thanks for taking the algorithmic approach.

[tridge]

Automated review note — AI-generated (Claude), independently cross-checked by a second model and re-verified against the live diff. Please sanity-check before acting.

Re-reviewed at head cb4073bb57; my earlier comment above is superseded. Full report: https://uav.tridgell.net/DevCallReviews/followups/2026_09_10_2312/devcall_pr_reviews.html#prMissionPlanner-3762

Both remaining asks are done, and no code defect survives checking. No blockers. The CRLF revert is clean and the bus_type guard is exactly right. What's left is three notes, none of them code defects — but the first is worth two minutes before merge.

The delta is 24 added lines, nothing removed: the guard at ExtLibs/Utilities/Device.cs:83-84 and three test methods at MissionPlannerTests/Utilities/DeviceInfoTests.cs:69-88.

Resolved

• The CRLF→LF flip of Device.cs — the one thing I asked for before merge. CR-byte counts per blob: merge base 256 LF / 256 CR; previous head 3294d86bfa 294 LF / 0 CR; this head 297 LF / 297 CR. Every line is CRLF again, matching the base. PR-wide numstat is 134 insertions, 0 deletions across three files, and gh pr diff now contains no - line anywhere. git blame is preserved. (Correction to my own previous comment: I said the base blob had 285 lines; it has 256. The arithmetic I quoted, 256 + 38 = 294, was right — the figure wasn't.)
• The bus_type guard. ExtLibs/Utilities/Device.cs:83-84, locked by MissionPlannerTests/Utilities/DeviceInfoTests.cs:83-88 (65536 → "65536"). I verified this is exact against upstream: libraries/GCS_MAVLink/GCS_Common.cpp:161 sets devid only from AP_SerialManager::UARTState::get_device_id(), and every return path in libraries/AP_SerialManager/AP_SerialManager.cpp:913-962 passes BUS_TYPE_SERIAL, so no real firmware value is affected.
• Both test-coverage gaps. MissionPlannerTests/Utilities/DeviceInfoTests.cs:76-81 adds COMPASS_DEV_ID / 65542 → "HMC5883_OLD", guarding the StartsWith("MAV") branch against swallowing the existing COMPASS/BARO/ASP/INS paths; :69-74 adds devid 6 → "6".

Decode re-verified from scratch: I extracted every // @Values: line from the _DEVID block of ArduPilot's libraries/GCS_MAVLink/GCS_MAVLink_Parameters.cpp:224-252, re-implemented Device.cs:76-100 independently, and compared every id — 24 published ids, 23/24 exact string matches, 0 numeric / family / bus / port mismatches. All 28 ids your own tests assert agree too. The family enum at Device.cs:249-255 matches AP_SerialManager.h:234-240 exactly and completely, so no real family falls to default:.

Notes — none blocking

1. The PR description still documents the old, incorrect mapping table. The "Mapping added" table still lists 6 | USB0, 14–78 (+8) | SERIAL1–SERIAL9, 174–198 (+8) | NET_P1–NET_P4, 334 | CAN_D1_UC_S1, 414 | CAN_D2_UC_S1, 494/502 | SCR_SDEV1/2, and claims the tests cover "all 19 mappings". Every one of those constants is from the reverted first approach — the ones this PR was written to fix. Not a code defect, but this text becomes the merge commit body and is the first thing anyone reads in the history.
2. Three comments cite a symbol that doesn't exist: AP_HAL::Device::DeviceType. ExtLibs/Utilities/Device.cs:73, ExtLibs/Utilities/Device.cs:246 and MissionPlannerTests/Utilities/DeviceInfoTests.cs:60. In ArduPilot, grep -rn "DeviceType" libraries/AP_HAL/ returns exactly one hit, AP_HAL_Namespace.h:62: enum SPIDeviceType, which is unrelated. The real symbol is AP_SerialManager::UARTState::DeviceType (libraries/AP_SerialManager/AP_SerialManager.h:234). Slightly more than a typo: AP_HAL/Device.h is where the sensor devtype conventions live, and the devtype byte means something entirely different there, so a maintainer following the reference lands in the wrong header.
3. GCSViews/ConfigurationView/DeviceInfo.cs:31 short-circuits before the new MAV branch, so a synthetic MAV1_DEVID of 65539 renders as SENSOR_ID#1 rather than the raw number your guard intends. Unreachable from real firmware, and consistent with how the file already treats UAVCAN globally — flagged for completeness only.

Still open, and not yours to fix here

CI still never compiles or runs the tests. Re-verified at this head: no vstest/dotnet test in any of the three workflows, only msbuild -t:Build; and MissionPlanner.sln:551-552 still has only ActiveCfg for the test project with no Build.0 — contrast MissionPlanner.sln:547-550 for Bulb, which has both. So the 90-line test file in this PR has still never been compiled by anything. I re-read it for compile-correctness and it looks sound, but that's inspection, not a build. You already agreed this belongs in a separate PR and I don't consider it blocking.

String parity is 23/24 — upstream publishes 65542 as SERIAL0 (USB on ChibiOS), you return SERIAL0 (USB). Still the single divergence. Fine as a deliberate choice for a cross-platform GCS; just worth being a stated one.

About the two red checks — they are not yours

Build Release APK and Build Release AAB fail with error XA0031: Java SDK 11.0 or above is required. The cause is upstream of your code: the choco install microsoft-openjdk --version=11.0.11.9 step returned "Failed to fetch results from V2 feed at community.chocolatey.org ... 503 (Service Unavailable)", so the JDK was never installed. The identical command on the identical runner image (windows-2022) succeeded 10 hours earlier on another PR head, and your own previous head passed Android Build on 2026-08-30. There's no C# compile error anywhere in either log, and Build Release / Build Debug — which both run msbuild MissionPlanner.sln — are green. Re-running those two jobs should clear them.

Caveat stated plainly: blu6 has no dotnet, mono, mcs or csc, so my decode check is a faithful re-implementation of your C#, not execution of the real assembly.

Nice turnaround on the line endings — that was the fiddly one.

Verdict: COMMENT (no blockers; mergeable as-is once the description is corrected)