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]

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.