feat(dronecan): on-demand GetNodeInfo, GetSet, ExecuteOpcode, RestartNode via async slot

[daijoubu]

Summary

Adds an on-demand async service slot (dronecanAsyncSlot) that handles GetNodeInfo, param GetSet, ExecuteOpcode, and RestartNode requests through a single IDLE → PENDING → READY/ERROR state machine — one request in flight at a time rather than a queue per service. All four are exposed over MSP (MSP2_INAV_DRONECAN_ASYNC_REQUEST / MSP2_INAV_DRONECAN_ASYNC_RESULT), which is what the companion configurator PR uses to read/write node parameters and trigger restarts without any custom DroneCAN tooling on the host side.

Also extends the node table MSP response with last_seen_ms as a delta (ms since the node's last NodeStatus) instead of an absolute FC timestamp, and updates docs/DroneCAN-Driver.md/docs/DroneCAN.md to cover the new client and fix a few things that had drifted out of sync with the code (stale field names, a settings table that no longer matched settings.yaml).

Companion UI: daijoubu:feature/dronecan-configurator-tab (iNavFlight/inav-configurator#2671) — review/merge together.

29 unit tests: node table management, the shouldAcceptTransfer filter, all four guard-rejection paths, all five param.Value types with min/max range fields, ExecuteOpcode and RestartNode ok/fail, and the re-entry guard.

Test plan

• Full build matrix green: F4/F7/H7/AT32/SITL (SITL built with -Werror)
• Both unit test suites pass (29/29)
• Hardware (static node ID, no DNA server dependency): read a parameter by index — decoded name/type/value matched the node's actual config
• Hardware: wrote a parameter and read it back — confirmed the node applied it, then ExecuteOpcode(SAVE) to persist
• Confirmed writes need name set alongside index — index-only writes are silently ignored by this node (and DroneCAN param.GetSet nodes generally); index-only reads work fine. The configurator always sends name on writes; raw MSP testing won't.
• MSP edge case: a second request while one's already pending correctly returns busy
• MSP edge case: reading a READY result resets the slot to IDLE
• MSP edge case: a request that never gets a response times out to IDLE/ERROR after ~2s
• RestartNode: confirmed with a CAN sniffer that it can time out even when the node restarted fine, if the peripheral doesn't flush its ACK before resetting — the response frame never makes it onto the bus. Noted in docs/DroneCAN.md.

[github-actions]

Test firmware build ready — commit c3bbcc5

Download firmware for PR #11683

244 targets built. Find your board's .hex file by name on that page (e.g. MATEKF405SE.hex). Files are individually downloadable — no GitHub login required.

Development build for testing only. Use Full Chip Erase when flashing.

[daijoubu]

Companion configurator PR: iNavFlight/inav-configurator#2671 (draft) — adds the DroneCAN node tab UI for GetNodeInfo, GetSet, ExecuteOpcode, and RestartNode. The two should be reviewed/merged together.

[github-actions]

RAM / Flash usage vs. base branch — commit c3bbcc5

Target	Flash Δ	RAM Δ
MATEKF405	±0 B (±0.00%)	±0 B (±0.00%)
MATEKF722	±0 B (±0.00%)	±0 B (±0.00%)
MATEKF765	±0 B (±0.00%)	±0 B (±0.00%)
MATEKH743 ⚠️	+4256 B (+0.59%)	-928 B (-0.61%)

See RAM/flash optimization guide for techniques to reduce usage.

[qodo-code-review]

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

[qodo-free-for-open-source-projects]

PR Summary by Qodo

DroneCAN: add single-slot async service client + MSP async request/result

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Add single-slot async DroneCAN client for GetNodeInfo, Param GetSet, ExecuteOpcode, RestartNode
• Expose async request/result over MSP2 and extend node list with uptime/vendor/last-seen delta
• Add unit tests and refresh DroneCAN/MSP documentation to match new wire formats

Diagram

graph TD
  CFG["Configurator / GCS"] --> MSP["MSP2 Async cmds"] --> FCMSP["fc_msp.c"] --> ASYNC["dronecan_async.c"] --> CAN["libcanard / CAN"] --> NODE["Peripheral node"]
  NODE --> CAN --> DRV["dronecan.c"] --> SLOT[("dronecanAsyncSlot")]
  DRV --> TABLE[("nodeTable[]") ] --> FCMSP --> MSP --> CFG

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Per-service (or per-node) request queues

• ➕ Supports true concurrency (e.g., fetch params from multiple nodes in parallel)
• ➕ Avoids a single busy slot blocking unrelated operations
• ➖ More RAM/state complexity (queues, expiry, matching)
• ➖ Harder MSP surface area (need handles/IDs per request)
• ➖ Doesn’t match current configurator behavior (sequential requests)

2. Synchronous MSP request that blocks until response/timeout

• ➕ Simpler host API (single request/response)
• ➕ No polling loop on the host
• ➖ Blocks MSP processing / main loop while waiting
• ➖ Hard to set safe timeouts without harming flight loop responsiveness
• ➖ Still needs response matching and bus-off resilience

3. Pass CanardInstance via API instead of extern global

• ➕ Cleaner dependency boundaries than making canard non-static
• ➕ Easier to test/multi-instance in the future
• ➖ Requires threading the instance pointer through multiple call sites
• ➖ Bigger refactor for limited immediate benefit

Recommendation: The single shared async slot is the right trade-off for the intended UX (configurator drives one request at a time) and keeps both MSP and driver complexity low. If future use cases require concurrency, evolve toward per-request handles/queues; for now, keep the slot model but consider the API-boundary alternative (passing CanardInstance) if canard global access becomes contentious.

Files changed (14) +2251 / -209

Enhancement (5) +685 / -96

dronecan.cIntegrate async response handling, node aging, and stricter bus-off recovery +117/-54

Integrate async response handling, node aging, and stricter bus-off recovery

• Makes the CanardInstance non-static for async module access, routes CanardTransferTypeResponse frames into the async response handler, and adds periodic timeout checks. Improves node table management (find-by-ID helper, stale node removal, full-table logging) and changes bus-off recovery to fail permanently after 50 attempts.

src/main/drivers/dronecan/dronecan.c

dronecan.hDefine async slot state machine and param/result wire structs +90/-12

Define async slot state machine and param/result wire structs

• Removes cached node name fields from dronecanNodeInfo_t and introduces async slot types/constants (services, timeout, param encoding, result unions). Exposes dronecanAsyncSlot and the dronecanAsyncRequest API, plus a new dronecanGetNodeByID helper.

src/main/drivers/dronecan/dronecan.h

dronecan_async.cImplement single-slot DroneCAN async request/response client +294/-0

Implement single-slot DroneCAN async request/response client

• Adds request encoding and dispatch for GetNodeInfo, param.GetSet, ExecuteOpcode, and RestartNode via canardRequestOrRespond under an atomic block. Implements strict response matching (service_id/node_id/transfer_id) and decodes results into the shared slot, including param min/max range extraction and a 2s timeout to ERROR.

src/main/drivers/dronecan/dronecan_async.c

dronecan_async.hExpose async response + timeout entry points +16/-0

Expose async response + timeout entry points

• Declares the async response handler and periodic timeout checker used by the core driver, guarded under USE_DRONECAN.

src/main/drivers/dronecan/dronecan_async.h

fc_msp.cAdd MSP2 async request/result and extend node list records +168/-30

Add MSP2 async request/result and extend node list records

• Extends MSP2_INAV_DRONECAN_NODES output to include uptime/vendor status and reports last_seen_ms as an elapsed delta. Replaces MSP2_INAV_DRONECAN_NODE_INFO with MSP2_INAV_DRONECAN_ASYNC_REQUEST/RESULT: dispatches service requests (with Param GetSet write-value parsing) and serializes READY results, consuming them by resetting the slot to IDLE.

src/main/fc/fc_msp.c

Tests (3) +1263 / -0

CMakeLists.txtWire in new DroneCAN unit test targets and DSDL sources +48/-0

Wire in new DroneCAN unit test targets and DSDL sources

• Adds build rules for new GetNodeInfo-focused tests and application-layer DroneCAN tests, including necessary generated DSDL .c sources and compile-time definitions.

src/test/unit/CMakeLists.txt

dronecan_application_unittest.ccAdd application-layer tests for node table and async response guards +853/-0

Add application-layer tests for node table and async response guards

• Introduces extensive unit tests that compile dronecan.c with stubs to validate node table insertion/update/overflow behavior, shouldAcceptTransfer filtering, async slot guard rejection paths, and decoding for GetNodeInfo/Param GetSet/ExecuteOpcode/RestartNode responses.

src/test/unit/dronecan_application_unittest.cc

dronecan_getnodeinfo_unittest.ccAdd DSDL encode/decode tests for GetNodeInfo and RTCMStream +362/-0

Add DSDL encode/decode tests for GetNodeInfo and RTCMStream

• Adds round-trip encode/decode coverage for GetNodeInfoResponse, SoftwareVersion flag behavior, HardwareVersion unique_id, and RTCMStream message payloads, plus checks of key signatures/IDs/sizes.

src/test/unit/dronecan_getnodeinfo_unittest.cc

Documentation (4) +299 / -112

DroneCAN-Driver.mdDocument async service client and updated node/MSP layouts +147/-32

Document async service client and updated node/MSP layouts

• Adds a new section describing the single-slot async state machine and supported services (GetNodeInfo, Param GetSet, ExecuteOpcode, RestartNode). Updates the node table definition to remove cached names and corrects MSP node list sizing/semantics (including last_seen_ms as a delta).

docs/DroneCAN-Driver.md

DroneCAN.mdUpdate settings name and add Param Get/Set user workflow +23/-4

Update settings name and add Param Get/Set user workflow

• Marks Parameter Get/Set as supported, updates bitrate setting naming to dronecan_bitrate_kbps, and adds configurator-driven parameter read/write instructions with restart/ack timeout caveat.

docs/DroneCAN.md

README.mdRegenerate MSP docs for async DroneCAN request/result and node list +36/-20

Regenerate MSP docs for async DroneCAN request/result and node list

• Replaces the old NODE_INFO command documentation with MSP2_INAV_DRONECAN_ASYNC_REQUEST/RESULT, and expands the node list record to include uptime and vendor status with last_seen_ms as an elapsed delta.

docs/development/msp/README.md

msp_messages.jsonUpdate MSP JSON spec for new DroneCAN async commands +93/-56

Update MSP JSON spec for new DroneCAN async commands

• Modifies the DroneCAN node list payload schema and replaces MSP2_INAV_DRONECAN_NODE_INFO with the async request/result definitions, including variable-length service-specific encoding notes.

docs/development/msp/msp_messages.json

Other (2) +4 / -1

CMakeLists.txtBuild DroneCAN async client sources +2/-0

Build DroneCAN async client sources

• Adds dronecan_async.c and dronecan_async.h to the common source set so they’re compiled into supported targets.

src/main/CMakeLists.txt

msp_protocol_v2_inav.hDefine new DroneCAN MSP command IDs +2/-1

Define new DroneCAN MSP command IDs

• Repurposes 0x2043 as MSP2_INAV_DRONECAN_ASYNC_REQUEST and adds 0x2044 as MSP2_INAV_DRONECAN_ASYNC_RESULT.

src/main/msp/msp_protocol_v2_inav.h

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Timeout checked before RX ✓ Resolved 🐞 Bug ☼ Reliability

Description

dronecanUpdate() calls dronecanAsyncCheckTimeout() before draining the CAN RX FIFO, so a response
already queued can be discarded as ERROR when the millis() delta crosses the threshold in that tick.
Because the response handler ignores non-PENDING slots, this can produce false timeouts even when
the response arrived within DRONECAN_ASYNC_TIMEOUT_MS.

Code

src/main/drivers/dronecan/dronecan.c[R148-151]

+            dronecanAsyncCheckTimeout();
+
            for (numMessagesToProcess = canardSTM32GetRxFifoFillLevel(); numMessagesToProcess > 0; numMessagesToProcess--)
            {

Evidence

The PR adds the timeout check ahead of the RX FIFO drain; the async response handler immediately
returns unless the slot is still PENDING, so once the pre-RX timeout flips the state to ERROR, the
queued response cannot be processed in the same tick.

src/main/drivers/dronecan/dronecan.c[145-168]
src/main/drivers/dronecan/dronecan_async.c[159-168]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`dronecanAsyncCheckTimeout()` is executed before processing queued RX frames in `dronecanUpdate()`. If a response arrives just before the timeout boundary but the next `dronecanUpdate()` tick occurs at/after the boundary, the slot can be marked `ERROR` and the queued response will then be ignored by `dronecanAsyncHandleServiceResponse()` (it only processes when `state==PENDING`).
## Issue Context
The timeout logic uses `millis()` granularity, so boundary effects are realistic. A correct implementation should always give queued RX frames a chance to resolve the slot before expiring it.
## Fix Focus Areas
- src/main/drivers/dronecan/dronecan.c[145-168]
- src/main/drivers/dronecan/dronecan_async.c[159-168]
## Suggested fix
Reorder the NORMAL-state loop so it drains RX (and any resulting TX) first, then calls `dronecanAsyncCheckTimeout()` after RX handling. Optionally, call the timeout check once per loop iteration after RX+TX drain, so a response in the FIFO always wins over expiring the slot.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Truncated MSP writes accepted ⊘ Outdated 🐞 Bug ≡ Correctness

Description

The MSP2_INAV_DRONECAN_ASYNC_REQUEST parser accepts PARAM_GETSET write requests even when the
payload is truncated (missing value_type/value bytes or missing the declared string/name bytes),
resulting in requests being sent with zero/garbage-filled values/names. This can unintentionally
write incorrect parameter values or target the wrong parameter name.

Code

src/main/fc/fc_msp.c[R4651-4654]

+                if (req.is_write && sbufBytesRemaining(src) >= 1) {
+                    req.value_type = sbufReadU8(src);
+                    switch (req.value_type) {
+                        case DRONECAN_PARAM_TYPE_INT:

Evidence

The MSP handler only conditionally reads value bytes when available but still dispatches the
request; the async request encoder blindly uses the parsed lengths and buffers, so any short payload
results in a valid UAVCAN request containing zeroed/uninitialized fields rather than rejecting the
MSP command.

src/main/fc/fc_msp.c[4643-4695]
src/main/drivers/dronecan/dronecan_async.c[52-90]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`MSP2_INAV_DRONECAN_ASYNC_REQUEST` parses service-specific fields but does not fail the command when required bytes are missing. For PARAM_GETSET writes, this can leave `req.value_type` unset, leave numeric values at 0, or leave `value_str`/`req_name` buffers zero-filled while still dispatching `dronecanAsyncRequest()`.
## Issue Context
`sbufReadData()` does not advance the buffer pointer and the code conditionally reads/advances only when enough bytes remain, but it does not return an error on short payloads. `dronecanAsyncRequest()` then encodes whatever is in `dronecanParamRequest_t` (including zeroed bytes) into a UAVCAN GetSet request.
## Fix Focus Areas
- src/main/fc/fc_msp.c[4643-4695]
- src/main/drivers/dronecan/dronecan_async.c[52-90]
## Suggested fix
1. In the PARAM_GETSET path, enforce strict minimum lengths:
 - If `is_write==1`, require presence of `value_type` byte.
 - For INT require 8 bytes; FLOAT require 4 bytes; BOOL require 1 byte; STRING require 1 byte length + that many bytes.
2. For STRING and req_name parsing, if the declared length exceeds remaining bytes, return `MSP_RESULT_ERROR` (don’t dispatch).
3. Consider also rejecting `is_write==1` with `value_type==EMPTY` (unless explicitly supported) to prevent silent “write empty” requests.
4. Only call `dronecanAsyncRequest()` when the payload has been fully and consistently parsed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗