feat(dronecan): add Dynamic Node Allocation (DNA) server#11688
Draft
daijoubu wants to merge 127 commits into
Draft
feat(dronecan): add Dynamic Node Allocation (DNA) server#11688daijoubu wants to merge 127 commits into
daijoubu wants to merge 127 commits into
Conversation
SJW=8 was overly conservative (80% of bit time at 1Mbps with 10 quanta). SJW=3 is the standard value also used by the F7 driver. Tested with 6037 arm/disarm cycles at 500kbps: TEC=0, REC=0, zero errors.
…DCAN PLL2Q was 3 (266 MHz, invalid for FDCAN ≤ 80 MHz). Fix to 10 (80 MHz). Extend PLL2 guard from USE_SDCARD_SDIO to USE_SDCARD_SDIO || USE_DRONECAN so H7 boards with CAN but no SD card get PLL2 configured. Adopt upstream PLL2M/N formula (VCI=1.6 MHz, VCO=800 MHz) and error check on HAL_RCCEx_PeriphCLKConfig.
…pport Remove redundant PeriphClkInitStruct clock config from canardSTM32CAN1_Init. system_stm32h7xx.c already configures FDCAN to use PLL2Q (80 MHz) when USE_DRONECAN is defined; duplicating it in the driver overwrites with PLL1. Also add CAN1 pin definitions and USE_DRONECAN to KAKUTEH7WING target (PD0/PD1, CAN1_STANDBY PD3 disabled by default).
Use HAL_RCCEx_GetPeriphCLKFreq(RCC_PERIPHCLK_FDCAN) instead of HAL_RCC_GetPCLK1Freq() for bit timing calculation. FDCAN is clocked from PLL2Q (80 MHz) configured in system_stm32h7xx.c; using PCLK1 (100 MHz) produced a ~25% baud rate error causing immediate bus-off. Restore SJW to 3 for better synchronisation tolerance.
Remove high-frequency LOG_DEBUG messages from GNSS Fix/Fix2/Auxiliary handlers, onTransferReceived, dronecanInit, and gps_dronecan HDOP path that fired at 25 Hz and flooded the log. Fix PLL2 VCO input to target 1.6 MHz (PLL2M = HSE/1600000, PLL2N = 500) rather than 2.0 MHz, keeping the operating point clearly within VCIRANGE_0 (1-2 MHz) as the original SDCARD-only code did with PLL2M=5. VCO output remains 800 MHz; FDCAN (80 MHz via PLL2Q=10) and SDMMC (200 MHz via PLL2R=4) outputs are unchanged.
Drop high-frequency and verbose-but-low-value LOG_DEBUG(CAN messages: - dronecan.c: Battery Info (x2), GetNodeInfo, NodeStatus, TX success, RX loop, commented-out debug blocks - canard_stm32h7xx_driver.c: timing computation intermediates (Baudrate, Max Quanta, Prescaler BS, Prescaler, Timings summary) - canard_stm32f7xx_driver.c: same timing intermediates, TX success, In CAN Init, commented-out clock and RX blocks Retain error-path messages (decode failed, TX/RX error, init failures) and the single-line Prescaler/SJW/BS summary logged at init.
… operation Tested at 1 Mbps on KAKUTEH7WING hardware and confirmed bus operational.
…ivers Remove CubeMX boilerplate markers, commented-out dead code, and development-time question comments from both drivers.
Fix: DroneCAN GNSS messages were being applied to gpsSolDRV regardless of the configured GPS provider. Guard added in gps_dronecan.c where it belongs, keeping CAN transport layer unaware of GPS config.
…g difference F7 bxCAN HAL writes SJW directly to BTR register where hardware adds 1, so stored value 3 gives 4 tq. This wider SJW is needed for reliable bus operation on F7 targets and is different from H7 where SJW=1 is actual tq.
Prevents state machine from continuing in INIT state when the CAN peripheral fails to initialize.
Prevents out-of-bounds access when STATE_DRONECAN_FAILED is active.
Prevents stale pre-bus-off frames from storming the bus on recovery.
With AutoRetransmission=ENABLE, frames that fail on a degraded bus occupy FIFO slots indefinitely. All 32 slots fill, HAL_FDCAN_AddMessage returns HAL_ERROR, and all outgoing traffic stalls permanently with no indication until full bus-off. DroneCAN reliability is handled at the application layer via periodic republishing.
Matches the H7 driver pattern. Previously the return value was silently discarded; if timing computation failed, uninitialized stack bytes were passed to HAL_CAN_Init.
The H7 FDCAN 128x11 recessive-bit recovery sequence takes up to 11.264ms at 125kbps. The 1ms delay was restarting the counter before it could complete, preventing the node from ever exiting bus-off. 20ms gives safe margin above worst-case and allows time to detect immediate re-entry.
Guard against non-DroneCAN GPS provider at the transport boundary (handle_GNSS* functions) rather than in each leaf function in gps_dronecan.c. Also adds the guard to handle_GNSSRCTMStream which had none. Removes stale UNUSED(pgnssAux) and placeholder comment from dronecanGPSReceiveGNSSAuxiliary.
canardSTM32GetProtocolStatus() was called on every dronecanUpdate() invocation (~500Hz) to detect bus-off. Moved into the existing 1Hz task block — bus-off detection latency of up to 1s is acceptable. Adds LOG_DEBUG to report BusOff and ErrorPassive flags each second for bench diagnostics.
AutoBusOff=ENABLE handles the 128x11 recovery sequence automatically, but ESR.BOFF is a sticky read-only flag that is NOT cleared when hardware recovery completes. GetProtocolStatus() reads this flag, so the state machine was permanently stuck in STATE_DRONECAN_BUS_OFF after any bus-off event on F7 targets. Stop/Start re-enters init mode which clears ESR.BOFF, allowing recovery detection to work correctly.
Comment incorrectly stated '25MHz' as a supported HSE value — 25MHz fails the assert. CMake always provides HSE_VALUE per-target via -DHSE_VALUE=<n> so the stm32h7xx_hal_conf.h fallback of 25MHz is never used. Current targets use 8MHz (default) or 16MHz (KAKUTEH7WING).
…adence GetProtocolStatus() was called every dronecanUpdate() cycle (~500Hz) in BUS_OFF state. Moved inside the 20ms recovery timer block so it runs at the same cadence as RecoverFromBusOff() — still detects recovery within 20ms but reduces MMIO reads from ~500/sec to ~50/sec.
HAL_CAN_Stop/Start called from the scheduler context with CAN interrupts active caused a full FC lockup. Reverted to empty stub pending investigation of a safe mechanism to clear the sticky ESR.BOFF flag on F7.
Unconditional 1Hz LOG_DEBUG was flooding the bootlog with healthy status messages. Now only logs when an error condition is actually present.
HAL_CAN_AddTxMessage returns non-OK when all mailboxes are busy — a normal transient condition at startup. The log was noise. Matches the H7 driver which already handles this path silently.
DroneCAN float16 optional fields encode NaN when unpopulated. Without a guard, NaN * 100 converts to 0 on Cortex-M (ARM VCVT saturation), permanently blocking the HDOP fallback path. Also passes values through gpsConstrainHDOP() to prevent uint16_t overflow for extreme DOP values.
…ompatible gpsSolDRV has hdop but no vdop field. VDOP and EPV are not interchangeable (different units, conversion requires receiver UERE). lastVDOP was a dead store with no valid consumer.
…ames size Guards the CLI state name array against future enum additions — if a new state is added without updating the array, the build fails immediately.
…G_WARNING Error conditions (decode failures, transmit errors, receive errors, node table full, anonymous node ID, bus-off permanent fault) were logged at DEBUG level, making them invisible in normal operation. Raise all to WARNING so they surface without requiring debug logging enabled. Bus-off permanent fault raised to ERROR as it disables the CAN bus entirely.
Add bool dronecanUseDNAServer to dronecanConfig_t with a default of ON. Guard the DNA allocation handler behind the setting so users can disable plug-and-play node assignment without rebuilding firmware. Regenerate docs/Settings.md from settings.yaml.
Tests cover the full 3-stage UID accumulation handshake, node ID lookup and assignment, table-full rejection, non-broadcast source rejection, stage mismatch and followup timeout handling, and the FC node ID skip. DNA-9 is a known-failing test documenting the unimplemented preferred node ID feature (peripheral requests a specific ID in Stage 1).
Capture the requested node_id from Stage 1 (the only stage where the peripheral sets it) into a static accumulator variable. Pass it to dnaLookupOrAssignNode at Stage 3 completion; the lookup uses it if the ID is in the valid range and not already taken, falling back to sequential assignment otherwise. Reset the accumulator on timeout and after each completed handshake. All 9 unit tests pass (DNA-1 through DNA-9).
DroneCAN.md: - Mark Dynamic Node Assignment as Supported (was Planned) - Fix setting names: dronecan_bitrate_kbps, correct default node_id=1 - Add dronecan_use_dna_server setting - Update peripheral setup note: DNA handles node IDs automatically - Update Example 4 to show DNA workflow - Add Dynamic Node Allocation section covering the 3-stage handshake, preferred node IDs, allocation table persistence, and enable/disable DroneCAN-Driver.md: - Update header (date, branch) - Fix settings table: remove non-existent dronecan_mode/dronecan_baudrate, add correct dronecan_bitrate_kbps and dronecan_use_dna_server entries - Add DNA allocation message to Supported Message Types table - Add DNA Server section: handshake stages, assignment priority, table persistence, entry point, unit test coverage - Update document history
…are reserved for network maintenance nodes.
…the network. If a stored assignment is in use by another node, assign a new id and update the stored value.
- Assign node IDs from top down (125→1); 126-127 reserved for network maintenance tools. Add DRONECAN_DNA_MAX_NODE_ID = 125. - Check live node table (dronecanGetNodeCount/dronecanGetNode) in isNodeAvailable so static-ID nodes on the bus are not double-assigned. - On re-negotiation, detect if a previously stored node ID is now in use by another device; if so, fall through to assign a new ID and overwrite the existing table entry rather than creating a duplicate. - Remove assigned flag; write path now returns directly from the table slot loop with LOG_ERROR + return 0 for table-full case. - Add dronecanGetNodeCount/dronecanGetNode stubs to unit test. All 9 unit tests pass.
dronecan_dna_server.c: - Wrap uidStr/sprintf block in #ifdef USE_LOG — buffer and loop no longer execute in production builds where LOG_DEBUG is a no-op - Add DNA_STAGE3_UID_LEN named constant and _Static_assert to catch UID length constant mismatches at compile time - Replace raw unsigned subtraction in detectRequestStage with the named constant - Remove misplaced post-response LOG_DEBUG (first_part flag) dronecan_dna_server.h: - Wrap contents in #ifdef USE_DRONECAN to prevent hard build errors on targets that include this header without DroneCAN support dronecan_dna_server_unittest.cc: - Make live node table stubs controllable (mock_node_table/count) so DNA-13 can simulate static-ID nodes on the network - Reset mock node table in SetUp - Update DNA-8 comment to reflect top-down assignment - Update DNA-9 comment (feature is implemented, not failing) - Add DNA-10: first sequential assignment is 125 (top-down) - Add DNA-11: preferred ID in reserved range (126-127) falls back - Add DNA-12: preferred ID already taken falls back to sequential - Add DNA-13: stored ID conflicting with live node → reassigned and table entry updated in place All 13 unit tests pass.
Per UAVCAN spec: when a peripheral requests a preferred node ID, search upward from that ID first; if nothing is found upward, search downward from preferred-1. Only fall back to top-down from 125 when no preference is given or the preferred range is fully exhausted. Update DNA-12 assertion to verify the upward search direction explicitly (preferred=60 taken → expect 61, not an arbitrary valid ID). All 13 unit tests pass.
Replace the implicit 'len < MAX_LENGTH' condition with an exact match against DNA_STAGE3_UID_LEN so the check remains correct if either constant is ever changed.
Targets without USE_DRONECAN still compile the file but the header guard excluded all type definitions, causing build failures on non-CAN targets (e.g. SPEEDYBEEF405WING).
Remove chatty DEBUG/VERBOSE messages that fired on every allocation request. Keep INFO for successful assignments, WARNING/ERROR for failure paths. Add warning when a stored node ID conflict forces re-assignment. Drop unused stdio.h include.
…etPoolStats These were present in feature/dronecan-param-getset but were lost when the first DNA commit's conflict resolution used --theirs for the entire dronecan.c file. Restoring the variable declaration, its increment on BUS_OFF entry, and the two getter functions declared in dronecan.h and called from cli.c. No logic change — pure restoration of param-getset infrastructure needed for the build to link.
…ution The first DNA server commit's conflict resolution used --theirs for the entire dronecan.c, reverting to a pre-masking version. Restore the three mask/unmask pairs that were present in feature/dronecan-param-getset: - dronecanAsyncRequest(): around canardRequestOrRespond - handle_GetNodeInfo(): around canardRequestOrRespond (GetNodeInfo response) - send_NodeStatus(): around canardBroadcast Also mask the dronecanDnaHandleAllocation() call site in the broadcast dispatch (option b — keeps masking out of the user-written DNA server file).
dronecan_dna_server.c calls saveConfig() on disarm to persist node allocations. Add a no-op stub so the test binary links. Also add dronecan_dna_server.c to depends and Allocation DSDL source to extra_sources so the application unittest compiles and links all DNA server code paths. All 186 unit tests pass.
…param-getset rebase The rebase of feature/dronecan-dna-server onto the current feature/dronecan-param-getset tip discarded two declarations that were lost as stale-duplicate rebase artifacts: txErrCount (used in dronecanUpdate/processCanardTxQueue but never declared, volatile since it's touched from ISR context) and send_NodeStatus's canardBroadcast return value (bc_res, referenced but never captured). Both are build-breaking on every target. Also: dronecan_dna_server.h now includes config/parameter_group.h directly instead of relying on include order for PG_DECLARE; log dnaSendResponse's broadcast failures instead of discarding them; and upgrade NodeStatus broadcast failure logging to LOG_WARNING to match its sibling failure paths.
…_pool static Second review pass caught two more regressions reintroduced during the param-getset rebase: vendor_specific_status_code lost its 16-bit mask (and explanatory comment) picked up from param-getset's own fix, and memory_pool lost its static qualifier as a side effect of making canard extern for the DNA server (memory_pool itself is never referenced outside this file). Also: trim trailing whitespace at end of settings.yaml, and correct docs/DroneCAN-Driver.md's DNA Server section, which described sequential fallback as "1 upward" when the implementation actually scans top-down from 125 with a preferred-ID up/down search first; also fixed the stated unit test count (13, not 9).
FcNodeIdNeverAssigned previously ran 5 handshakes under top-down assignment starting at 125 -- structurally incapable of ever reaching down to FC_NODE_ID (5), since the allocation table (32 slots) plus the live node table (32 slots) can occupy at most 64 IDs, nowhere near the 120 needed to force the scan that far down. The test passed without ever exercising the skip guard it claimed to cover. Request FC_NODE_ID as the peripheral's preferred ID instead: the preferred-ID search starts at the requested value and scans upward, so skipping the FC's own ID on a clean table must land on ID 6. This directly and deterministically exercises isNodeAvailable(canard.node_id) in a single handshake.
… allocation detectRequestStage() previously accepted first_part_of_unique_id=true for any of the three structurally-valid unique_id lengths (4, 6, 16), including a 4-byte length that isn't a legitimate stage-1 length under either the classic or single-frame delivery scheme. Tighten it to accept exactly two: 6 bytes (classic multi-frame handshake) or 16 bytes (the full UID in one message, e.g. a transport like CAN-FD that can carry it in a single frame). The existing accumulation logic already handles a single 16-byte stage-1 message correctly -- it completes allocation immediately since currentUniqueId.len jumps straight to 16. Add DNA-14 (single-frame full UID completes allocation immediately) and DNA-15 (the same length without the stage-1 flag is rejected) to cover both paths. Update docs/DroneCAN-Driver.md to describe the single-frame delivery option and correct the test count.
…-1 fix DNA-14 and DNA-15 (added in 9a48d9b) don't actually depend on that commit's detectRequestStage() fix -- both of their (length, first_part) combinations were already handled correctly before it, confirmed by running them against 9a48d9b~1 where they pass unchanged. Add DNA-16, which reproduces the actual regression the fix guards against: a malformed first_part=true, len=4 message was previously accepted as stage 1, setting currentUniqueId.len=4. getExpectedStage(4) falls through to DNA_INVALID_STAGE (4 is neither 0, nor >=12, nor >=6), so any legitimate follow-up message within the 500ms followup window was rejected as a stage mismatch -- silently blocking real allocation until the timeout reset the accumulator. Verified as a genuine differential test: fails against dronecan_dna_server.c from 9a48d9b~1 (pre-fix), passes against the current fixed version, with no effect on the other 15 tests either way. Also fix a missing break in DNA-14's table-scan loop for consistency with the equivalent loop elsewhere in the file, and update the doc test count to 16.
Draft
4 tasks
|
Test firmware build ready — commit Download firmware for PR #11688 240 targets built. Find your board's
|
This was referenced Jul 8, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Adds a non-redundant (single-master) Dynamic Node Allocation (DNA) server per UAVCAN Specification §6.4.9, so peripherals with no configured node ID can request one automatically instead of requiring manual
dronecan_node_idassignment on each device.PG_DRONECAN_DNA_SERVER) — same peripheral gets the same node ID across power cycles, saved only while disarmeddronecan_use_dna_serversetting (default ON)Dependencies
feature/dronecan-param-getset.Configurator companion PR: iNavFlight/inav-configurator#2672 (enable/disable toggle + node ID validation).
Test plan
-DWARNINGS_AS_ERRORS=ON) — all pass, zero warningsdronecan_dna_server_unittest(DNA-1 through DNA-16 — full handshake, UID re-use, table-full rejection, non-broadcast rejection, stage ordering/mismatch rejection, followup timeout reset, FC-node-ID exclusion, preferred-ID honouring including reserved-range and taken-ID fallback, top-down sequential assignment, live-node conflict reassignment, single-frame full-UID delivery, and a differential regression test proving a malformed 4-byte stage-1 message can't poison the accumulator against a legitimate follow-up handshake);dronecan_application_unittest(29/29) unaffectedstaticqualifier, a stale doc description of the assignment order and test count), and a third pass caught that two newly-added tests didn't actually depend on the code change they were meant to cover — replaced with a differential test independently confirmed to fail against the pre-fix code and pass against the fixReference
daijoubu/inav #4