feat(dronecan): GPS node-ID/battery-ID filtering and node health guard - #11698
Draft
daijoubu wants to merge 68 commits into
Draft
feat(dronecan): GPS node-ID/battery-ID filtering and node health guard#11698daijoubu wants to merge 68 commits into
daijoubu wants to merge 68 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.
- Make processCanardTxQueue, shouldAcceptTransfer, onTransferReceived static - Replace ISR LOG calls with volatile counters (txErrCount, rxDropCount); log and clear at 1Hz from main loop via canardSTM32GetAndClearRxDropCount() - Add canardSTM32GetAndClearRxDropCount() to driver interface; F7 implements ring-buffer drop counter, H7/SITL return 0 (no SW ring buffer) - Check canardBroadcast/canardRequestOrRespond return values; log on OOM - H7: set RxBuffersNbr=0 (dedicated buffer was unused, wasted message RAM) - H7: reject unmatched standard-ID frames (FDCAN_REJECT for NonMatchingStd) - F7: move NVIC_EnableIRQ calls after all ActivateNotification calls succeed - Fix F7 bxCAN Doxygen: correct @brief, remove wrong @PARAM, fix @RetVal - Fix all @RetVal docs: ret==0 is OK (CANARD_OK), not ret==1 - Cast pid_t to uint32_t before bit-shifting in SITL unique ID generation
- Wrap processCanardTxQueue in #if STM32H7||STM32F7 — function is ISR-only and has no callers in SITL builds; static + no callers caused -Werror=unused-function on CI - Atomic read-clear of txErrCount: hold dronecanMaskTxISR across snapshot+zero to prevent ISR increment being silently dropped - Atomic read-clear of rxDropCount: disable CAN1_RX0_IRQn across snapshot+zero in canardSTM32GetAndClearRxDropCount (F7) - Make all eight file-local handler functions static: send_NodeStatus, handle_NodeStatus, handle_GNSSAuxiliary, handle_GNSSFix, handle_GNSSFix2, handle_GNSSRCTMStream, handle_BatteryInfo, handle_GetNodeInfo - H7 global filter: FDCAN_REJECT_REMOTE for both RTR frame params (was FDCAN_FILTER_REMOTE which passed RTR frames to normal filter) - H7 Receive: add comment that DataLength==byte count holds only in FDCAN_FRAME_CLASSIC mode (FDCAN_DLC_BYTES_0..8 equal 0..8) - F7 ring buffer: use local index snapshots in push/pop to avoid double volatile re-reads of writeIndex/readIndex
- H7 Transmit: add FDCAN_FRAME_CLASSIC comment on DataLength assignment matching the comment already on the receive path - F7 init: add comment explaining pre-shifted timing register values - SITL: map RTR flag in sitlCANFrameToLinux/FromLinux, consistent with hardware drivers - SITL canardSTM32Transmit: add ERR frame guard matching hardware drivers - handle_GNSSRCTMStream: remove dead decode, add comment that RTCM forwarding is not yet implemented - dronecanInit: add default case to bitrate switch for EEPROM corruption - vendor_specific_status_code: explicit (uint16_t) cast with comment acknowledging bits 16-30 of armingFlags are not transmitted - fport.c: remove dead static volatile frameErrors counter (written but never read; triggered -Werror=unused-but-set-variable on GCC 16)
- H7 Receive: add bounds guard on DataLength before assignment to data_len (uint8_t); mirrors existing TX path guard; safe no-op in FDCAN_FRAME_CLASSIC mode but prevents silent truncation if DLC ever exceeds 8 - F7: fix stale comment hfdcan->ErrorCode -> hcan1.ErrorCode - dronecanInit: fix mixed tab/space indentation in canardInit() call - dronecanGetBitrateKbps: return 500 for DRONECAN_BITRATE_COUNT and default cases, matching what dronecanInit actually selects - Fix typo: incremeneted -> incremented - SITL GetRxFifoFillLevel: add comment explaining FIONREAD on SOCK_RAW returns next-datagram size only, so result is 0 or 1
FDCAN_FILTER_DUAL with FilterID1=0x0, FilterID2=0x1FFFFFFF only matched those two exact IDs. Replace with FDCAN_FILTER_MASK (pattern=0, mask=0) which accepts any extended ID. Also fix stale comments in H7 driver and dronecanInit.
…on bxCan (F7) targets. Set max quanta per bit to 18 unconditionally to fix 1MBps on bxCan.
17 tests covering canardSTM32ComputeTimings() algorithm at standard bitrates (125k/250k/500k/1M) for PCLK 54 MHz and 48 MHz. Includes an 18-quanta regression test that would fail against the old 10-quanta limit (prescaler=6, 9 tq/bit) and passes with the new unconditional 18-quanta path (prescaler=3, 18 tq/bit at 1 Mbps / 54 MHz).
- Wrap canard.c with USE_DRONECAN guard via platform.h include so the libcanard protocol engine is excluded from non-CAN targets (e.g. F722). Add comment noting both lines must be preserved on library updates. - Replace USE_GPS_PROTO_DRONECAN with USE_DRONECAN in gps.c and gps_dronecan.c; remove the define from common.h. GPS DroneCAN callbacks only make sense when CAN hardware is present. - Add IOCFG_AF_PP_FAST_UP (VERY_HIGH speed + PULLUP) to io.h for F7/H7, matching ST CubeF7 example recommendation for CAN GPIO pins. - Use IOCFG_AF_PP_FAST_UP for CAN1_TX and CAN1_RX in both the F7 bxCAN and H7 FDCAN drivers, replacing the previous LOW speed / no-pull config. - Add bus-off counter and canard pool allocator stats to dronecan driver and expose both in the CLI dronecan command.
Adds #include "platform.h" + #ifdef USE_DRONECAN to both canard_stm32f7xx_driver.c and canard_stm32h7xx_driver.c, preventing CAN1_RX0_IRQHandler and related code from linking into non-CAN targets.
2 tasks
|
Test firmware build ready — commit Download firmware for PR #11698 244 targets built. Find your board's
|
…sembly too Replace hand-rolled NVIC_DisableIRQ/EnableIRQ pairs with the existing ATOMIC_BLOCK(NVIC_PRIO_CAN) macro, which saves/restores the actual prior BASEPRI value instead of a manual counter, so nested critical sections (e.g. handle_GetNodeInfo's own masking, reached synchronously from within canardHandleRxFrame) compose correctly without risk of leaving the TX interrupt permanently masked. Also wrap canardHandleRxFrame() itself, which was previously unmasked and could race the TX-complete ISR's freeBlock() calls on the shared canard pool allocator during multi-frame RX reassembly.
canardSTM32ComputeTimings() was duplicated verbatim (aside from the PCLK source, SJW value, and BS1/BS2 register-offset convention) between the F7 bxCAN and H7 FDCAN drivers. Extract the HAL-free quanta/prescaler solve into canard_stm32_timing.c, shared by both; each driver now applies only its own peripheral-specific glue on top. bxcan_timing_unittest.cc previously hand-copied this algorithm into the test file, hardcoding max_quanta_per_bit=18 with a "keep in sync" comment. It drifted from the driver's real (target_bitrate >= 1000000) ? 10 : 17 within days of being written and was never updated. Rewritten to call the real, now-shared canardComputeCanTimingSolution() directly, so there's no separate copy to keep in sync.
Previously only checked that crcAddByte() changed its input at all (!= 0xFFFF, != 0x0000), which a subtly wrong polynomial or bit order would still satisfy. Assert the actual CRC-16/CCITT-FALSE value for a single byte instead.
# Conflicts: # src/main/io/gps.c # src/main/target/common.h
Add on-demand GetNodeInfo support: request a target node's software/ hardware version and name over DroneCAN, decode the response into the node table, and expose it to the configurator/CLI via MSP (MSP2_INAV_DRONECAN_NODE_INFO). This is the first on-demand (as opposed to broadcast-only) DroneCAN service request INAV makes - GetNodeInfo uses canardRequestOrRespond() under ATOMIC_BLOCK(NVIC_PRIO_CAN), matching the ISR masking the H7/F7 driver rework established for TX. MSP2_INAV_DRONECAN_NODE_INFO response grew to 71 bytes to carry the decoded version/name fields (docs/msp regenerated to match; MSP2_DRONECAN_NODE_INFO_SIZE replaces an inline field-count literal wherever the response size was checked, including the msp_protocol_v2_inav.h constant's own location, which moved out of fc_msp.c). Also: per-node transfer_id (rather than one shared counter across all nodes) so concurrent/overlapping GetNodeInfo requests to different nodes don't cross-contaminate; node name storage extended to 80 bytes with overflow logging; dronecanGetNodeByID() added to eliminate node-table lookups duplicated across handlers; bus-off recovery now gives up after 50 attempts and enters STATE_DRONECAN_FAILED instead of retrying forever. Full unit test suite passes: GetNodeInfo/SoftwareVersion/HardwareVersion/ RTCMStream response decode, shouldAcceptTransfer dispatch (GAP-S1/S2), and node-table tests. Squashed from the original feature/dronecan-getnodeinfo commit sequence (24 commits - the initial multi-phase implementation, several rounds of code-review fixups, and one rebase-artifact cleanup, "fixup: remove orphaned TX loop and duplicate process1HzTasks from rebase artifact" - into this single commit for a clean PR diff. No functional changes from the squash itself.
…ed async slot Extend on-demand DroneCAN service requests beyond GetNodeInfo to param GetSet, ExecuteOpcode, and RestartNode. All four services now share a single in-flight async request slot (dronecanAsyncSlot) rather than per-service state, since only one on-demand request is ever outstanding at a time in practice: dronecanAsyncRequest() encodes and sends whichever service's request (masked under ATOMIC_BLOCK against the CAN TX ISR), and one response handler decodes whichever service's response arrives, guarded by service_id/node_id/transfer_id matching so a stale or mismatched response can't be misattributed to the wrong in-flight request. A timeout (DRONECAN_ASYNC_TIMEOUT_MS) expires a request that never gets a response, so the slot can't wedge waiting forever. GetSet: full int/float/bool/string value union plus min/max NumericValue range, exposed through MSP so the configurator can read/write a remote node's parameters and see their valid range. ExecuteOpcode/RestartNode: simple ok/fail response, for triggering a remote node's save/erase opcodes or a restart. Full unit test suite passes: response-decode coverage for GetSet (int/float/bool/string/empty), ExecuteOpcode, and RestartNode, plus the async-slot dispatch tests (GAP-S2) updated for the new shared-slot architecture. Squashed from the original feature/dronecan-param-getset commit sequence (20 commits - the initial async-slot/GetSet/ExecuteOpcode/RestartNode implementation plus several rounds of code-review fixups) into this single commit for a clean PR diff. No functional changes from the squash itself.
…odule Split dronecanAsyncRequest() and the response handler (GetNodeInfo, ParamGetSet, ExecuteOpcode, RestartNode - a single shared slot serialising all on-demand service requests) out of dronecan.c into dronecan_async.c/.h. dronecanAsyncSlot's definition and the response handler move too; dronecan.h keeps declaring dronecanAsyncRequest()/ dronecanAsyncSlot since fc_msp.c is an external caller of both. dronecan.c's onTransferReceived() now calls dronecanAsyncHandleServiceResponse() (renamed from the static handle_AsyncServiceResponse for external linkage), and STATE_DRONECAN_NORMAL calls the new dronecanAsyncCheckTimeout() instead of carrying the timeout-expiry check inline. Also flips the file-scope `canard` CanardInstance from static to plain external linkage, since dronecan_async.c needs `extern CanardInstance canard` to reach it. (On the branch this was originally authored on, that linkage change had already landed earlier, as part of the DNA server work - rebasing this extraction back to sit directly on param-getset instead means picking it up here.) Cherry-picked from feature/dronecan-actuator-control (original commit e577393) onto feature/dronecan-param-getset: this is general dronecan.c restructuring in async-request/GetNodeInfo/ParamGetSet/ ExecuteOpcode/RestartNode territory - this branch's own scope - not actuator-control-specific, so it belongs here rather than riding along with unrelated actuator-output work. Full unit test suite (29 tests in dronecan_application_unittest, full suite otherwise unchanged) passes. SITL builds clean with -Werror.
Add a DNA (Dynamic Node Allocation) server so peripheral nodes with no configured node ID (anonymous, node ID 0) can request one dynamically over DroneCAN, instead of every device on the bus needing a fixed ID set by hand. Implements the standard three-stage UID handshake (uavcan.protocol.dynamic_node_id.Allocation broadcasts assembling a peripheral's 16-byte unique ID across up to three frames), matching already-assigned peripherals back to their stored allocation so a peripheral doesn't get handed a different ID every boot, and honouring a peripheral's preferred node ID when it's free. Allocation starts from node ID 125 downward (126/127 reserved for network maintenance nodes, per spec) and re-checks the live node table before handing out an ID so it won't collide with a node already active on the bus. Node allocations persist to EEPROM (via Parameter Groups) so a peripheral keeps the same node ID across FC power cycles, not just within a session; persistence writes happen on disarm rather than on every allocation. Gated behind a new dronecan_use_dna_server CLI setting so it's opt-in. Extracted into its own dronecan_dna_server.c/.h module (mirroring the file-per-concern pattern already used elsewhere in the driver) rather than living inline in dronecan.c. Full unit test suite passes: DNA-1 through DNA-9 covering the stage handshake, timeout/reset behavior, preferred-ID honouring, existing- peripheral re-matching, allocation table exhaustion, and the FC's-own-node-ID skip guard. Squashed from the original feature/dronecan-dna-server commit sequence (30 commits - the initial multi-pass implementation, spec-correctness fixes, EEPROM persistence and settings-gate additions, several rounds of code-review fixups, and a cluster of commits restoring code (NVIC masking, txErrCount/bc_res, vendor_specific_status_code masking, memory_pool's static qualifier, busOffCount and its accessors) that had been silently dropped by an earlier rebase-conflict resolution against param-getset - into this single commit for a clean PR diff. No functional changes from the squash itself; the single-frame full-UID stage-1 delivery fix (a distinct, later-discovered regression with its own dedicated differential test) is kept as a separate commit rather than folded in here.
…allocation The DNA stage-detection logic (detectRequestStage()) assumed a peripheral's 16-byte unique ID always arrives split across the standard three-frame handshake (stage 1: first partial chunk, stage 2: middle chunk, stage 3: final chunk). Some peripherals - and CAN FD frames, which have enough payload capacity - deliver the complete 16-byte UID in a single stage-1 frame instead. That single-frame case was being rejected as a malformed/out-of-sequence request rather than accepted as a valid one-shot allocation, so those peripherals could never complete DNA allocation at all. Regression test (differential: fails against the pre-fix logic, passes against the fix) added alongside the DNA-1..9 suite. Also removes a duplicate MSP2_INAV_DRONECAN_ASYNC_RESULT case statement found while touching this code, and stubs _logf in the DNA server unit test (needed once the fix's added logging pulled the symbol into that test binary's link).
Split the node presence/heartbeat concern out of dronecan.c into dronecan_node_status.c/.h: the live node table (activeNodeCount/ nodeTable[], consumed by dronecan_dna_server.c's isNodeAvailable() via dronecanGetNodeCount()/dronecanGetNode() to avoid handing out node IDs already seen on the bus), the incoming NodeStatus broadcast handler (renamed handle_NodeStatus -> dronecanNodeStatusHandleBroadcast for external linkage), and our own 1Hz NodeStatus heartbeat send. dronecan.c's handle_GetNodeInfo() now calls the new dronecanGetOwnNodeStatus() accessor instead of touching the node_status struct directly, and process1HzTasks() delegates to dronecanNodeStatusUpdate() after its own stale-transfer cleanup. Also drops a dead duplicate stale-node pruning loop: process1HzTasks() had two back-to-back loops removing nodes past their last-seen timeout, one using DRONECAN_NODE_STALE_TIMEOUT_MS (10000) and one hardcoding the same value - the second could never find anything left to prune since the first already removed it. Cherry-picked from feature/dronecan-actuator-control (original commit 34aefbc) directly onto feature/dronecan-dna-server's existing tip: this is general dronecan.c restructuring in NodeStatus/node-table territory - driven by dna_server.c's own need for the dronecanGetNodeCount()/dronecanGetNode() accessors this introduces - not actuator-control-specific, so it belongs here rather than riding along with unrelated actuator-output work. Applied as a plain cherry- pick on top of dna-server's unmodified history (not a full rebase of its 34 commits onto the updated param-getset branch) to avoid replaying real pre-existing history irregularities found partway through an initial rebase attempt. Note: the include-alphabetization part of the original commit's title wasn't carried over - dna-server's current header block already has a different structure (several includes above the USE_DRONECAN guard, predating this change) than what the original diff assumed, so only the new dronecan_node_status.h include was added without reordering the rest. Full unit test suite (dronecan_application_unittest 29/29, dronecan_dna_server_unittest 16/16, no failures suite-wide) passes. SITL builds clean with -Werror.
…guard
Add dronecan_gps_node_id and dronecan_battery_id settings so users with
multiple identical DroneCAN sensors (e.g. two batteries, two GPS units)
can pin the FC to a specific source node instead of accepting whichever
one happens to broadcast.
GPS side (new dronecanGpsAcceptSource() gate in gps_dronecan.c, shared
by the Fix2 and Auxiliary receive paths):
- With dronecan_gps_node_id unset (0), the first node to report Fix2
data locks in as the active source ("first-over-fence"); all other
nodes are rejected until the active one is evicted from the node
table (stale NodeStatus timeout) or the filter is set.
- With dronecan_gps_node_id set, only that node's data is accepted,
bypassing the lock so reconfiguring at runtime to a different node
takes effect immediately rather than getting stuck on the old lock.
- A node reporting NodeStatus health >= ERROR is rejected even before
it would otherwise lock in.
- dronecanGpsIsHealthy() (wired into io/gps.c's isGPSHealthy() for the
GPS_DRONECAN provider) reports whether the currently-selected node
(locked-in or statically filtered) is present and healthy, so
arming/OSD reflect a degraded or absent CAN GPS.
- dronecanGpsOnNodeEvicted(), called from the stale-node purge in
dronecan_node_status.c's dronecanNodeStatusUpdate(), releases the
lock when the active node drops off the bus.
- No automatic failover to a second GPS node if the locked-in one
degrades but keeps broadcasting NodeStatus: INAV has no general
redundant-sensor story, and picking a winner between two live nodes
needs its own design. Deliberately out of scope here (documented in
dronecanGpsAcceptSource()'s comment).
Battery side: handle_BatteryInfo in dronecan.c now drops BatteryInfo
messages whose battery_id doesn't match dronecan_battery_id (0 = any).
dronecanConfig_t grows two fields (batteryId, gpsNodeId); PG version
bumped 0->1 per the struct-layout-change rule in
docs/development/settings/versioning-rules.md.
dronecanState and dronecanUpdate()'s 1Hz scheduler deadline
(next_1hz_service_at) move from function-local/static to file-scope,
exposed non-static under UNIT_TEST alongside the existing
activeNodeCount/nodeTable pattern - needed so
gps_dronecan_unittest.cc's end-to-end eviction test can reset them in
SetUp() for deterministic sequencing regardless of test order.
New gps_dronecan_unittest.cc links the real gps_dronecan.c against the
real dronecan.c (unlike dronecan_application_unittest.cc, which stubs
GNSS receive as no-ops) to test the filtering/lock/health-guard logic
end-to-end, including through the real dronecanUpdate() 1Hz task path.
15 tests (GPS-1..8, GPS-14..20 in the file's numbering - GPS-9..13,
covering parseGnssTime(), land in a separate commit alongside the
covariance/time-parsing fix they test).
Reconstructed against the current dronecan.c (which already has the
async-client, NodeStatus, and actuator-output extractions applied)
rather than cherry-picked, since the original commits' diffs were
obscured by dronecan_gps-health-guard's divergent copy of that
extraction work. Full relevant suite passes: gps_dronecan_unittest
15/15, dronecan_application_unittest 29/29, dronecan_dna_server_unittest
16/16, full suite 0 failures.
Add logNodeHealth(), called from dronecanNodeStatusHandleBroadcast() whenever a node's health changes (OK/WARNING/ERROR/CRITICAL, mapped to LOG_INFO/WARNING/ERROR/ERROR) and once when a node is first seen. Pure observability - no behavior change to node tracking, GPS filtering, or anything else; the health value written to nodeTable is unchanged. Full relevant suite passes unchanged: dronecan_application_unittest 29/29, gps_dronecan_unittest 15/15, dronecan_dna_server_unittest 16/16.
…rsing Two independent data-quality fixes to dronecanGPSReceiveGNSSFix2(), unrelated to node filtering: Covariance (EPH/EPV): the DSDL doesn't specify Fix2's covariance array layout. The old code treated it as a 6-element upper-triangular position covariance matrix (indices [0]/[2]/[5] as x/y/z variance, summing x+y for horizontal). AP_Periph - the dominant DroneCAN peripheral firmware - actually packs it as [0]=[1]=hacc², [2]=vacc², [3]=[4]=[5]=sacc² (verified against live hardware data: [0]==[1] and [3]==[4]==[5] as expected from AP_Periph's source). Read index [0] directly for horizontal accuracy and [2] for vertical, and lower the length guard from >=6 to >=3 to match. GNSS time: gpsSolDRV.flags.validTime was hardcoded to 0 with the actual field-population code commented out as dead TODOs. Add parseGnssTime(), which converts the DSDL's gnss_timestamp (UTC/GPS/TAI standard, microseconds since each standard's own epoch) to gpsSolDRV.time via gmtime(). GPS and TAI standards need num_leap_seconds to compute the UTC offset (GPS: UTC = GPS - leap_seconds + 9; TAI: UTC = TAI - leap_seconds - 10, per the DSDL comment) and reject rather than guess when num_leap_seconds is UNKNOWN; UAVCAN_TIMESTAMP_UNKNOWN rejects immediately regardless of standard. Adds GPS-9..13 to gps_dronecan_unittest.cc: UTC/GPS/TAI conversion, both leap-second-unknown rejection paths, and the unknown-timestamp rejection - independently deriving expected calendar fields via gmtime() on the same epoch formula rather than hand-transcribing dates, so a rederivation error can't accidentally match a matching bug in parseGnssTime() itself. Full suite: gps_dronecan_unittest 20/20 (all of GPS-1..20 now present), dronecan_application_unittest 29/29, dronecan_dna_server_unittest 16/16, 0 failures suite-wide.
…handler Legacy uavcan.equipment.gnss.Fix is superseded by Fix2, which every node we support already sends. Rather than maintaining two parallel decode paths, drop Fix entirely: keep accepting the transfer (so a node still sending it doesn't trip an unhandled-transfer error) but log a one-time deprecation warning instead of decoding it. dronecanGPSReceiveGNSSFix() and its stale covariance/time-TODO code are removed along with it. Also drop the uavcan.equipment.gnss.RTCMStream handler. It decoded inbound RTCM correction data but never did anything with it - the FC transmits RTCM corrections to GPS nodes, it does not receive them, so the handler was dead code from the start. Removing both handlers means dronecan.c and gps_dronecan.c no longer reference the Fix/RTCMStream DSDL codecs, so the corresponding extra_sources entries come out of the two unit test targets that link dronecan.c/gps_dronecan.c directly (dronecan_messages_unittest and dronecan_getnodeinfo_unittest still need them - they exercise the DSDL codec itself, independent of dronecan.c).
daijoubu
force-pushed
the
fix/dronecan-gps-health-guard
branch
from
August 18, 2026 02:55
56f4b0a to
6ab50f8
Compare
…entries Settings.md was never regenerated after these two settings were added, so the settings_md CI check (which diffs a fresh update_cli_docs.py run against the committed file) was failing on this PR.
|
RAM / Flash usage vs. base branch — commit
|
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
dronecan_gps_node_idanddronecan_battery_idsettings so a specific DroneCAN GPS/battery node can be pinned, rejecting messages from any other node (0 = accept any, matching existing DroneCAN filter conventions)gps_dronecan.c: enforces a single active GPS node, evicts stale nodes from the stale-node cleanup path, and logs node health transitions (OK/WARNING/ERROR/CRITICAL) viahandle_NodeStatusFix(v1) message — nodes sending it are logged once with a "deprecated, node must send Fix2" warning and otherwise ignored. OnlyFix2is parsed:gnss_timestamp(UTC/GPS/TAI, with leap-second handling), covariance, and related fields, split out into their own header contentRTCMStreammessage stub fromshouldAcceptTransferUSE_GPS_PROTO_DRONECANtoUSE_DRONECANand bumps thedronecanConfig_tPG version for the new fieldsgps_dronecan_unittest.ccplus expandeddronecan_application_unittest.cccoverageStacked on
feature/dronecan-param-getset(#11683) andfeature/dronecan-dna-server(#11688) — do not merge until both land.Configurator branch:
fix/dronecan-gps-health-guard(UI for the new node-ID/battery-ID filter fields) — iNavFlight/inav-configurator#2673Test plan