From dfe06eeb450701b4890af879645c49e88498e437 Mon Sep 17 00:00:00 2001 From: Motti Horesh Date: Fri, 16 Oct 2026 08:29:28 -0400 Subject: [PATCH 1/3] feat(firmware): nodes announce their board kind in HEALTH_n and _NODE_STATUS v3 (#344) HEALTH_n byte 5 carries a NodeKind (unknown / panel group / PDU / bridge) set by each node library's role via a required serviceNodeStatus() argument, never from NODE_ID. PanelBridge forwards it in _NODE_STATUS v3 (28 hex), adds its own node-0 row to every roster reply, and pushes a delta when an alive node's kind changes. Every node also filters its own HB_n; a frame on it means another board has the same NODE_ID and raises DUPLICATE_NODE_ID ahead of every FaultSource, clearing 3 s after the last foreign frame. Spec synced: FirmwarePlan 02/04/06/10 + D19, TechSpec, docs. --- .../Libraries/CANProtocol/CANProtocol.cpp | 54 ++++++++++-- Firmware/Libraries/CANProtocol/CANProtocol.h | 53 +++++++++-- Firmware/Libraries/NodeStatus/NodeStatus.h | 24 ++++- .../Libraries/PanelBridge/PanelBridge.cpp | 68 ++++++++++----- Firmware/Libraries/PanelBridge/PanelBridge.h | 10 ++- Firmware/Libraries/PanelGroup/PanelGroup.cpp | 2 +- Firmware/Libraries/PanelGroup/PanelGroup.h | 4 +- Firmware/Libraries/PowerMonitor/PduNode.cpp | 2 +- Firmware/Libraries/PowerMonitor/PduNode.h | 3 +- .../ScratchPad/FirmwarePlan/00-decisions.md | 38 ++++++++ .../FirmwarePlan/02-can-protocol.md | 48 ++++++++-- .../FirmwarePlan/04-dcs-bios-integration.md | 47 +++++++--- .../FirmwarePlan/06-panelbridge-api.md | 18 ++-- .../FirmwarePlan/10-implementation-plan.md | 3 + Firmware/ScratchPad/TechSpec/CANProtocol.md | 68 +++++++++++---- Firmware/ScratchPad/TechSpec/NodeStatus.md | 31 +++++-- Firmware/ScratchPad/TechSpec/PanelBridge.md | 22 ++++- .../TechSpec/PanelGroup/PanelGroup.md | 5 +- Firmware/ScratchPad/TechSpec/PowerMonitor.md | 3 +- Firmware/Tests/CANProtocol/platformio.ini | 6 ++ .../duplicate_node_id/duplicate_node_id.cpp | 87 +++++++++++++++++++ .../tests/health_flags/health_flags.cpp | 6 +- .../tests/health_payload/health_payload.cpp | 23 +++-- .../tests/node_status_bus/node_status_bus.cpp | 16 +++- .../node_status_service.cpp | 2 +- .../tests/node_status/node_status.cpp | 60 ++++++++----- .../PanelGroup/tests/heartbeat/heartbeat.cpp | 2 +- docs/architecture/can-bus.md | 10 ++- docs/architecture/firmware-tiers.md | 9 +- docs/firmware/build-flags.md | 11 +-- docs/firmware/node-id.md | 10 +++ 31 files changed, 590 insertions(+), 155 deletions(-) create mode 100644 Firmware/Tests/CANProtocol/tests/duplicate_node_id/duplicate_node_id.cpp diff --git a/Firmware/Libraries/CANProtocol/CANProtocol.cpp b/Firmware/Libraries/CANProtocol/CANProtocol.cpp index 801d6f43..62f3d628 100644 --- a/Firmware/Libraries/CANProtocol/CANProtocol.cpp +++ b/Firmware/Libraries/CANProtocol/CANProtocol.cpp @@ -56,6 +56,7 @@ static uint32_t _txDrops = 0; static uint32_t _filterIds[MAX_FILTER_IDS]; static uint8_t _filterCount = 0; static bool _filterPassAll = false; +static bool _loopback = false; // started with startLoopback(): own frames return // Node status service (serviceNodeStatus) — #340 static constexpr uint32_t HB_PERIOD_MS = 500; @@ -72,6 +73,12 @@ static bool _masterHbPending = false; // HB_0 arrived since the l static bool _masterLost = false; static bool _overheat = false; static bool _warnAsserted = false; // what this service last wrote to the latch +// Duplicate NODE_ID check (#344): a frame on our own HB_n came from another board with our NODE_ID. +static uint32_t _lastDupMs = 0; // millis() of the last foreign own-ID HB +static bool _dupSeen = false; // a foreign own-ID HB within DUP_NODE_TIMEOUT_MS +static uint8_t _ownHbLoopPending = 0; // loopback only: own HBs sent, not yet looped back +// Last HEALTH_n built (lastNodeHealth()) — not-yet-seen until the first one: dieTempC INT8_MIN. +static NodeHealthPayload _lastHealth = { 0, INT8_MIN, 0, 0, 0, 0, { 0, 0 } }; // ── Private helpers ─────────────────────────────────────────────────────────── @@ -120,12 +127,13 @@ static void _applyFilters() { } // Build combined ID list: mandatory + caller-registered - uint32_t ids[MAX_FILTER_IDS + 4]; + uint32_t ids[MAX_FILTER_IDS + 5]; uint8_t count = 0; ids[count++] = CAN_ID_CTRL_BCAST; ids[count++] = CAN_ID_TEST_SEQ; ids[count++] = CAN_ID_SYNC_REQ; if (NODE_ID != 0) ids[count++] = canIdHb(0); // master heartbeat → serviceNodeStatus watchdog + ids[count++] = canIdHb(NODE_ID); // own heartbeat ID → duplicate NODE_ID check (#344) for (uint8_t i = 0; i < _filterCount; i++) { ids[count++] = _filterIds[i]; } @@ -222,6 +230,9 @@ static void _startInternal(uint32_t mode) { // serviceNodeStatus() arms its HB/HEALTH timers on its first call — the first loop() after // setup returns — so a restart re-arms them there too (FirmwarePlan 09 step 8). _statusArmed = false; + _loopback = (mode == CAN_MODE_SILENT_LOOPBACK); + _ownHbLoopPending = 0; + _dupSeen = false; _status = CanStatus::NORMAL; if (_statusCb) _statusCb(_status); @@ -281,7 +292,16 @@ void drain() { } else if (frame.canId == CAN_ID_TEST_SEQ) { send(canIdEcho(NODE_ID), frame.data, frame.len); } else { - if (NODE_ID != 0 && frame.canId == canIdHb(0)) { + if (frame.canId == canIdHb(NODE_ID)) { + // Our own heartbeat ID. A node never receives its own frames, so another board has + // our NODE_ID (#344) — except in loopback, where our own heartbeats come back. + if (_loopback && _ownHbLoopPending > 0) { + _ownHbLoopPending--; + } else { + _lastDupMs = millis(); + _dupSeen = true; + } + } else if (NODE_ID != 0 && frame.canId == canIdHb(0)) { // Master heartbeat: stamp it for serviceNodeStatus()'s watchdog, then still hand it // to the node's own handler like any other frame. _lastMasterMs = millis(); @@ -420,7 +440,8 @@ HeartbeatPayload makeHeartbeatPayload(uint8_t nodeId, uint16_t rxCount) { return p; } -NodeHealthPayload makeNodeHealthPayload(uint8_t nodeId, int8_t dieTempC, NodeFaultCode fault) { +NodeHealthPayload makeNodeHealthPayload(uint8_t nodeId, NodeKind kind, int8_t dieTempC, + NodeFaultCode fault) { NodeHealthPayload p; p.nodeId = nodeId; p.dieTempC = dieTempC; @@ -436,13 +457,17 @@ NodeHealthPayload makeNodeHealthPayload(uint8_t nodeId, int8_t dieTempC, NodeFau p.faultMask = 0; p.faultId = static_cast(fault); if (fault != NodeFaultCode::NONE) p.flags |= static_cast(NodeHealthFlag::DEGRADED); // bit1 - p.rsvd[0] = p.rsvd[1] = p.rsvd[2] = 0; + p.nodeKind = static_cast(kind); // board kind from the node's firmware role (#344) + p.rsvd[0] = p.rsvd[1] = 0; return p; } uint32_t txDropCount() { return _txDrops; } -void serviceNodeStatus(uint16_t rxCount) { +const NodeHealthPayload& lastNodeHealth() { return _lastHealth; } + +void serviceNodeStatus(NodeKind kind, uint16_t rxCount) { + (void)kind; // only HEALTH_n carries it — unused under -DNODE_HEALTH_TELEM=0 const uint32_t now = millis(); // after drain(): HB_0 stamps can't be ahead of `now` (#225) // Arm on the first call: that is the first loop() after setup() returned, i.e. boot complete @@ -457,16 +482,31 @@ void serviceNodeStatus(uint16_t rxCount) { _lastHbMs = now; HeartbeatPayload hb = makeHeartbeatPayload(NODE_ID, rxCount); send(canIdHb(NODE_ID), reinterpret_cast(&hb), sizeof(hb)); + if (_loopback) _ownHbLoopPending++; // will come back — not a duplicate (#344) } + // Duplicate NODE_ID (#344): live until DUP_NODE_TIMEOUT_MS after the last foreign own-ID HB. + // Signed elapsed, as for the master watchdog (#225); dropping _dupSeen keeps an old stamp from + // reading as fresh again after millis() wraps. + if (_dupSeen && (int32_t)(now - _lastDupMs) > (int32_t)DUP_NODE_TIMEOUT_MS) _dupSeen = false; + // HEALTH: half the heartbeat rate — trend data. NodeFaultCode / aggregateFaults come via // NodeStatus.h (included by CANProtocol.h) (#163). #if !defined(NODE_HEALTH_TELEM) || (NODE_HEALTH_TELEM) if (now - _lastHealthMs >= HEALTH_PERIOD_MS) { _lastHealthMs = now; + // A duplicate NODE_ID outranks every FaultSource: with two boards on one ID, nothing else + // reported under it can be trusted, so it must never be masked by a lesser fault. const char* faultDetail = nullptr; // never null after aggregateFaults() - NodeFaultCode fault = OpenSkyhawk::aggregateFaults(&faultDetail); - NodeHealthPayload h = makeNodeHealthPayload(NODE_ID, STM32Board::readDieTempC(), fault); + NodeFaultCode fault; + if (_dupSeen) { + fault = NodeFaultCode::DUPLICATE_NODE_ID; + faultDetail = "another board transmits on this NODE_ID"; + } else { + fault = OpenSkyhawk::aggregateFaults(&faultDetail); + } + NodeHealthPayload h = makeNodeHealthPayload(NODE_ID, kind, STM32Board::readDieTempC(), fault); + _lastHealth = h; send(canIdHealth(NODE_ID), reinterpret_cast(&h), sizeof(h)); STM32Board::logNodeFaultEdge(NODE_ID == 0 ? "BRIDGE" : "NODE", fault, faultDetail); #ifdef NODE_OVERHEAT_C diff --git a/Firmware/Libraries/CANProtocol/CANProtocol.h b/Firmware/Libraries/CANProtocol/CANProtocol.h index 74ba452d..4b00b84c 100644 --- a/Firmware/Libraries/CANProtocol/CANProtocol.h +++ b/Firmware/Libraries/CANProtocol/CANProtocol.h @@ -65,7 +65,10 @@ struct __attribute__((packed)) HeartbeatPayload { * - Degraded state (#163): flags bit1 + faultId — a node that is alive but has a faulted * FaultSource reporting a NodeFaultCode (NodeStatus.h). aggregateFaults() picks the primary * faultId, makeNodeHealthPayload derives DEGRADED. faultMask stays 0 (reserved, see below). - * PanelBridge caches HEALTH_1–HEALTH_63 per node and forwards them in _NODE_STATUS. + * - Board kind (#344): nodeKind — a NodeKind (NodeStatus.h) set by the node's firmware role, + * never derived from NODE_ID. Pre-#344 firmware sends 0 here = NodeKind::UNKNOWN. + * PanelBridge caches HEALTH_1–HEALTH_63 per node and forwards them in _NODE_STATUS, together with + * its own HEALTH_0 as the node-0 row. * * @note The STM32F103 internal sensor is UNCALIBRATED (no factory trim): ~±few °C absolute, * measures DIE temperature (not ambient) with a self-heat offset. Use for relative @@ -80,8 +83,10 @@ struct __attribute__((packed)) NodeHealthPayload { uint8_t flags; ///< 2: NodeHealthFlag bits (NodeStatus.h): OVERHEAT (#213), DEGRADED (#163) uint8_t faultMask; ///< 3: fault source/domain bitmap — reserved for future domain bits (0) uint8_t faultId; ///< 4: NodeFaultCode (NodeStatus.h): 0 = no fault, else primary fault — #163 - uint8_t rsvd[3]; ///< 5-7: reserved, transmit 0 — future generic health (resetCause, freeRAM, …) + uint8_t nodeKind; ///< 5: NodeKind (NodeStatus.h): board kind from the firmware role — #344 + uint8_t rsvd[2]; ///< 6-7: reserved, transmit 0 — future generic health (resetCause, freeRAM, …) }; +static_assert(sizeof(NodeHealthPayload) == 8, "NodeHealthPayload must fill one classic CAN frame"); /** * @brief 8-byte payload carried by TELEM_n frames — paged node telemetry (#216). @@ -220,7 +225,8 @@ namespace CANProtocol { * Must be called after STM32Board::begin() and after all filter and callback * registrations. Always adds CTRL_BCAST, TEST_SEQ, and SYNC_REQ to the active * filter — mandatory for all nodes, cannot be excluded — plus HB_0 on every node except - * PanelBridge (NODE_ID 0), for serviceNodeStatus()'s master-loss watchdog. + * PanelBridge (NODE_ID 0), for serviceNodeStatus()'s master-loss watchdog, and the node's own + * `HB_NODE_ID`, for its duplicate NODE_ID check (#344). * * Resets the serviceNodeStatus() timers, which arm on its first call. Fires the * onStatusChange callback with NORMAL on success. @@ -232,6 +238,8 @@ namespace CANProtocol { * * Identical to start() but uses CAN_MODE_SILENT_LOOPBACK: frames transmitted by * this node are received back internally without going on the physical bus. + * The duplicate NODE_ID check discounts the node's own looped-back heartbeats, so only an + * extra `HB_NODE_ID` a test sends itself reads as a duplicate. * * @note Never call this in production firmware. */ @@ -244,6 +252,8 @@ namespace CANProtocol { * - SYNC_REQ: fires onSyncReq(), not forwarded to onReceive(). * - TEST_SEQ: auto-replies with ECHO carrying the same payload, not forwarded. * - HB_0 on a non-bridge node: stamped for serviceNodeStatus()'s watchdog, then onReceive(). + * - The node's own `HB_NODE_ID`: stamped for serviceNodeStatus()'s duplicate NODE_ID check + * (#344), then onReceive(). * - All other frames: fires onReceive(). * * Also services sendBatched() deadlines: a half-full ControlPacketPair that has @@ -357,14 +367,16 @@ namespace CANProtocol { * STM32Board::readDieTempC()) and the node's aggregated fault (via NodeStatus::aggregateFaults()) * and packs them into a NodeHealthPayload. Sets the overheat flag (bit0) only when NODE_OVERHEAT_C * is defined and dieTempC meets it; sets the DEGRADED flag (bit1) and faultId when `fault` is not - * NONE (the enum→byte cast happens here). faultMask + reserved bytes stay 0. + * NONE (the enum→byte cast happens here), and nodeKind from `kind`. faultMask + reserved bytes + * stay 0. * * @param nodeId Node ID (1-63 PanelGroup; 0 PanelBridge). + * @param kind The node's board kind (#344) — from its firmware role, never from nodeId. * @param dieTempC Internal die temp in whole °C (INT8_MIN = unavailable). * @param fault Aggregated node fault (NodeFaultCode::NONE = healthy). Default NONE. * @return Fully populated NodeHealthPayload ready to send as HEALTH_n. */ - NodeHealthPayload makeNodeHealthPayload(uint8_t nodeId, int8_t dieTempC, + NodeHealthPayload makeNodeHealthPayload(uint8_t nodeId, NodeKind kind, int8_t dieTempC, NodeFaultCode fault = NodeFaultCode::NONE); /** @brief Return the cumulative TX queue drop count since startup. */ @@ -377,9 +389,15 @@ namespace CANProtocol { * * Every node on the bus owes the same status traffic, whatever else it does (#340): * - `HB_NODE_ID` every 500 ms (makeHeartbeatPayload), carrying `rxCount`. - * - `HEALTH_NODE_ID` every 1000 ms (makeNodeHealthPayload) with the internal die temp and the - * aggregated node fault, plus the DiagSerial fault edge log (tag `BRIDGE` on NODE_ID 0, - * `NODE` otherwise). Compiled out with `-DNODE_HEALTH_TELEM=0`. + * - `HEALTH_NODE_ID` every 1000 ms (makeNodeHealthPayload) with `kind`, the internal die temp and + * the node fault, plus the DiagSerial fault edge log (tag `BRIDGE` on NODE_ID 0, `NODE` + * otherwise). Compiled out with `-DNODE_HEALTH_TELEM=0`. + * - Duplicate NODE_ID check (#344), on every node: a node never receives its own frames, so a + * frame on its own `HB_NODE_ID` (start() adds it to the filter) means another board has the + * same NODE_ID. The node fault is then `NodeFaultCode::DUPLICATE_NODE_ID`, ahead of every + * FaultSource (the NODE_ID is wrong, so nothing else this ID reports can be trusted); it + * clears DUP_NODE_TIMEOUT_MS after the last foreign frame. aggregateFaults() supplies the + * fault otherwise. * - Master-loss watchdog, on every node except PanelBridge (NODE_ID 0): once a master * heartbeat (`HB_0`) has been seen, master loss is set after MASTER_TIMEOUT_MS of silence and * cleared by the next `HB_0`. @@ -396,18 +414,35 @@ namespace CANProtocol { * start() adds `HB_0` to the hardware filter on non-bridge nodes, so callers never * register it by hand. * + * @param kind This node's board kind (#344), sent in every HEALTH_n. Each node library + * passes its own (PanelGroup → PANEL_GROUP, PduNode → PDU, PanelBridge → + * BRIDGE), so a sketch never sets it. Required: a role can't forget it. * @param rxCount Caller-owned accepted RX count for the heartbeat (PanelGroup: accepted * CTRL_BCAST frames). Pass 0 when the node keeps no count. * * @note Samples millis() itself, after drain() has stamped any `HB_0`, and compares elapsed * time signed — so a millisecond ticking during drain() cannot read as a timeout (#225). */ - void serviceNodeStatus(uint16_t rxCount = 0); + void serviceNodeStatus(NodeKind kind, uint16_t rxCount = 0); + + /** + * @brief The HEALTH_n payload serviceNodeStatus() last built for this node (#344). + * + * PanelBridge reports it as its own node-0 `_NODE_STATUS` row. Before the first HEALTH_n + * (or with `-DNODE_HEALTH_TELEM=0`) it reads not-yet-seen: dieTempC INT8_MIN, everything else 0 + * (nodeKind UNKNOWN) — the same rule the bridge applies to a node it has no HEALTH_n from. + */ + const NodeHealthPayload& lastNodeHealth(); /** @brief Master-loss timeout: 3× the 500 ms `HB_0` cadence, matching PanelBridge's own * node timeout ratio (3000 ms / 500 ms). */ static constexpr uint32_t MASTER_TIMEOUT_MS = 1500; + /** @brief Duplicate NODE_ID clear time (#344): DUPLICATE_NODE_ID drops this long after the + * last foreign frame on our own `HB_NODE_ID` — 6× the 500 ms HB cadence, matching + * PanelBridge's own node timeout. A live duplicate re-raises within one HB period. */ + static constexpr uint32_t DUP_NODE_TIMEOUT_MS = 3000; + } // namespace CANProtocol #endif // ARDUINO_ARCH_STM32 diff --git a/Firmware/Libraries/NodeStatus/NodeStatus.h b/Firmware/Libraries/NodeStatus/NodeStatus.h index 5cde2bb1..2712e6ac 100644 --- a/Firmware/Libraries/NodeStatus/NodeStatus.h +++ b/Firmware/Libraries/NodeStatus/NodeStatus.h @@ -7,6 +7,7 @@ * names) — **the canonical source the client's `sync-a4ec.ts` parses**; * - `NodeHealthFlag` (HEALTH_n flag bits) and `NodeFaultCode` (the compact wire faultId dictionary * the client maps to labels); + * - `NodeKind` (HEALTH_n board-kind byte — what board a NODE_ID is, announced by its firmware); * - the `_NODE_TELEM` host contract and the TELEM_n page vocabulary (`TelemPage`, `PduRailFlag`, * `PduStatusFlag`) — #216; * - `FaultSource` — the interface a fault-producing object implements so a node-level aggregator @@ -27,8 +28,8 @@ // ── _NODE_STATUS DCS-BIOS host-reporting contract (#86) ─────────────────────── // PanelBridge reports connected nodes + health to the host over DCS-BIOS. Bump // NODE_STATUS_PROTO_VERSION on any _NODE_STATUS wire change so the client's sync fails loudly. -// Full 26-hex field decode: FirmwarePlan/04-dcs-bios-integration.md. -#define NODE_STATUS_PROTO_VERSION 2 +// Full 28-hex field decode: FirmwarePlan/04-dcs-bios-integration.md. +#define NODE_STATUS_PROTO_VERSION 3 #define NODE_STATUS_REQ_ADDR 0x86FE // host→device roster-request export address #define NODE_STATUS_MSG_NAME "_NODE_STATUS" #define NODE_STATUS_END_MSG_NAME "_NODE_STATUS_END" @@ -65,7 +66,24 @@ enum class NodeFaultCode : uint8_t { FUSE_OPEN = 0x06, // rail fuse blown — rail read ~0 V while the node is powered (PDU) OVER_TEMPERATURE = 0x07, // board temperature at the critical threshold (PDU NTC) SENSOR_FAULT = 0x08, // a measurement channel reads implausibly (railed / open / shorted) - // 0x09–0xFF reserved for future fault sources + DUPLICATE_NODE_ID = 0x09, // another board transmits on this node's HB_n — two boards share a NODE_ID (#344) + // 0x0A–0xFF reserved for future fault sources +}; + +/** + * @brief HEALTH_n `nodeKind` (byte 5) — what kind of board this node is (#344). + * + * Announced by the board's own firmware role (each node library passes its kind to + * CANProtocol::serviceNodeStatus()), never derived from the NODE_ID — a PDU can take any ID. + * Append-only. 0x00 is what firmware older than #344 sends (the byte was reserved), so the client + * reads it as "unknown" and never adopts on it. + */ +enum class NodeKind : uint8_t { + UNKNOWN = 0x00, // pre-#344 firmware, or no HEALTH_n seen yet + PANEL_GROUP = 0x01, // PanelGroup node (PanelGroup library) + PDU = 0x02, // power distribution / monitoring node (PowerMonitor library) + BRIDGE = 0x03, // the SimGateway + PanelBridge board — reported by its PanelBridge MCU (node 0) + // 0x04–0xFF reserved for future node types }; /** diff --git a/Firmware/Libraries/PanelBridge/PanelBridge.cpp b/Firmware/Libraries/PanelBridge/PanelBridge.cpp index b2b1b5b2..632930f9 100644 --- a/Firmware/Libraries/PanelBridge/PanelBridge.cpp +++ b/Firmware/Libraries/PanelBridge/PanelBridge.cpp @@ -45,6 +45,7 @@ struct NodeState { uint8_t healthFlags = 0; // overheat (#213) / degraded (#163) bits uint8_t faultMask = 0; // fault source/domain bitmap — reserved #163 uint8_t faultId = 0; // NodeFaultCode (NodeStatus.h) — #163 + uint8_t nodeKind = 0; // NodeKind (NodeStatus.h), 0 = unseen/unknown — #344 #endif }; @@ -59,28 +60,45 @@ static uint8_t _deadCount = 0; // tracked nodes currently tim // command messages on a reserved control name. Owned entirely by PanelBridge; // SimGateway relays the ASCII verbatim. See NodeStatus.h for the wire format. -// Emit one node's status (proto v2, 26 hex chars): -// _NODE_STATUS -// The last four come from the node's HEALTH_n frame (#221 contract). dieTempC 80 = not-yet-seen; -// hFlags (DEGRADED) + faultId are populated per node by aggregateFaults (#163); faultMask stays 0 -// (reserved for future fault-domain bits). -static void emitNode(uint8_t nodeId, bool present) { - const NodeState& ns = _nodes[nodeId - 1]; - const HeartbeatPayload& hb = ns.last; - char hex[27]; - snprintf(hex, sizeof(hex), "%02X%02X%02X%04X%04X%04X%02X%02X%02X%02X", +// Emit one status row (proto v3, 28 hex chars): +// _NODE_STATUS +// The last five come from the node's HEALTH_n frame (#221 contract). dieTempC 80 + nodeKind 00 = +// not-yet-seen; hFlags (DEGRADED) + faultId are populated per node by aggregateFaults (#163); +// faultMask stays 0 (reserved for future fault-domain bits); nodeKind is the board kind (#344). +static void emitRow(uint8_t nodeId, bool present, const HeartbeatPayload& hb, + int8_t dieTempC, uint8_t hFlags, uint8_t faultMask, uint8_t faultId, + uint8_t nodeKind) { + char hex[29]; + snprintf(hex, sizeof(hex), "%02X%02X%02X%04X%04X%04X%02X%02X%02X%02X%02X", (unsigned)nodeId, present ? 1u : 0u, (unsigned)hb.flags, (unsigned)hb.uptime, (unsigned)hb.rxCount, (unsigned)hb.esr, - (unsigned)(uint8_t)ns.dieTempC, - (unsigned)ns.healthFlags, (unsigned)ns.faultMask, (unsigned)ns.faultId); + (unsigned)(uint8_t)dieTempC, (unsigned)hFlags, (unsigned)faultMask, + (unsigned)faultId, (unsigned)nodeKind); DcsBios::sendDcsBiosMessage(NODE_STATUS_MSG_NAME, hex); } -// Emit the full roster (request response / boot seed): one _NODE_STATUS per alive -// node, then _NODE_STATUS_END so the host knows the burst is complete and -// can reconcile (prune nodes absent from it). count=0 = no panels connected. +// One CAN node's row (nodeId 1–63), from its cached HB_n + HEALTH_n. +static void emitNode(uint8_t nodeId, bool present) { + const NodeState& ns = _nodes[nodeId - 1]; + emitRow(nodeId, present, ns.last, ns.dieTempC, ns.healthFlags, ns.faultMask, ns.faultId, + ns.nodeKind); +} + +// The bridge's own row, node 0 (#344): its heartbeat state now, and the HEALTH_0 it last sent +// (kind BRIDGE once the first HEALTH_0 has gone out). Always present — it is the one reporting. +static void emitSelf() { + const HeartbeatPayload hb = CANProtocol::makeHeartbeatPayload(0, 0); + const NodeHealthPayload& h = CANProtocol::lastNodeHealth(); + emitRow(0, true, hb, h.dieTempC, h.flags, h.faultMask, h.faultId, h.nodeKind); +} + +// Emit the full roster (request response / boot seed): the bridge's own node-0 row, one +// _NODE_STATUS per alive node, then _NODE_STATUS_END so the host knows the burst is +// complete and can reconcile (prune nodes absent from it). count includes the node-0 row, so +// count=1 = bridge only, no other nodes connected. static void emitAllNodes() { - uint8_t count = 0; + emitSelf(); + uint8_t count = 1; for (uint8_t i = 0; i < MAX_NODE_ID; i++) if (_nodes[i].alive) { emitNode(i + 1, true); count++; } char arg[4]; @@ -323,6 +341,12 @@ static void onCanRx(uint32_t canId, const uint8_t* data, uint8_t len) { ns.healthFlags = h.flags; ns.faultMask = h.faultMask; ns.faultId = h.faultId; + // Board kind (#344): push a live delta when it changes — the alive delta went out on + // the node's first HB, before any HEALTH_n, with kind 00. + if (h.nodeKind != ns.nodeKind) { + ns.nodeKind = h.nodeKind; + if (ns.alive) emitNode(nodeId, true); + } } #endif return; @@ -501,7 +525,7 @@ void setup() { CANProtocol::start(); broadcastSyncReq(); #ifdef PANELBRIDGE_NODE_STATUS - emitAllNodes(); // seed the host with the current roster (empty at cold boot) (#86) + emitAllNodes(); // seed the host with the current roster (node-0 row only at cold boot) (#86) #endif } @@ -521,7 +545,7 @@ void loop() { // detect master loss even when the sim is idle/paused. The bridge is a node too: its own // FaultSources (e.g. a future host-link watchdog) roll up into HEALTH_0. No master watchdog // runs here (NODE_ID 0 is the master); the bridge keeps no RX count for its heartbeat. - CANProtocol::serviceNodeStatus(); + CANProtocol::serviceNodeStatus(NodeKind::BRIDGE); auto& diag = STM32Board::diagSerial(); while (diag.available()) { @@ -586,13 +610,17 @@ void testFeedHeartbeat(uint8_t nodeId, uint8_t flags, uint16_t uptime, } void testFeedHealth(uint8_t nodeId, int8_t dieTempC, - uint8_t hFlags, uint8_t faultMask, uint8_t faultId) { - // Mirrors the HEALTH_n branch of onCanRx: cache only, no liveness change. + uint8_t hFlags, uint8_t faultMask, uint8_t faultId, uint8_t nodeKind) { + // Mirrors the HEALTH_n branch of onCanRx: no liveness change; a kind change pushes a delta. NodeState& ns = _nodes[nodeId - 1]; ns.dieTempC = dieTempC; ns.healthFlags = hFlags; ns.faultMask = faultMask; ns.faultId = faultId; + if (nodeKind != ns.nodeKind) { + ns.nodeKind = nodeKind; + if (ns.alive) emitNode(nodeId, true); + } } void testRequestNodeStatus() { emitAllNodes(); } diff --git a/Firmware/Libraries/PanelBridge/PanelBridge.h b/Firmware/Libraries/PanelBridge/PanelBridge.h index 38a12a97..ef1a37f5 100644 --- a/Firmware/Libraries/PanelBridge/PanelBridge.h +++ b/Firmware/Libraries/PanelBridge/PanelBridge.h @@ -153,15 +153,19 @@ namespace PanelBridge { uint16_t rxCount, uint16_t esr); /** - * @brief Cache a node's HEALTH_n telemetry without changing liveness (#213/#163). + * @brief Cache a node's HEALTH_n telemetry without changing liveness (#213/#163/#344). * * Mirrors the HEALTH_n branch of onCanRx across all cached fields. hFlags/faultMask/faultId * default to 0 (healthy); pass non-zero to exercise degraded forwarding into `_NODE_STATUS`. + * nodeKind defaults to 0 (unknown); a change on an alive node emits one `_NODE_STATUS` delta, + * as a real HEALTH_n does. */ void testFeedHealth(uint8_t nodeId, int8_t dieTempC, - uint8_t hFlags = 0, uint8_t faultMask = 0, uint8_t faultId = 0); + uint8_t hFlags = 0, uint8_t faultMask = 0, uint8_t faultId = 0, + uint8_t nodeKind = 0); - /** @brief Emit the current roster (one `_NODE_STATUS` per alive node) — the request/boot path. */ + /** @brief Emit the current roster (node-0 row, one `_NODE_STATUS` per alive node, then + * `_NODE_STATUS_END`) — the request/boot path. */ void testRequestNodeStatus(); /** @brief Run the timeout sweep with an injected clock; expires nodes and emits removals. */ diff --git a/Firmware/Libraries/PanelGroup/PanelGroup.cpp b/Firmware/Libraries/PanelGroup/PanelGroup.cpp index 1c14d3df..ef7ce0d9 100644 --- a/Firmware/Libraries/PanelGroup/PanelGroup.cpp +++ b/Firmware/Libraries/PanelGroup/PanelGroup.cpp @@ -372,7 +372,7 @@ void loop() { // 6. Node status: HB_n every 500 ms, HEALTH_n every 1 s, master-loss watchdog (#340). // _rxCount = accepted CTRL_BCAST frames, carried in the heartbeat. - CANProtocol::serviceNodeStatus(_rxCount); + CANProtocol::serviceNodeStatus(NodeKind::PANEL_GROUP, _rxCount); } // ── MCP cache bridge ────────────────────────────────────────────────────────── diff --git a/Firmware/Libraries/PanelGroup/PanelGroup.h b/Firmware/Libraries/PanelGroup/PanelGroup.h index 181b8bfd..bcd663f6 100644 --- a/Firmware/Libraries/PanelGroup/PanelGroup.h +++ b/Firmware/Libraries/PanelGroup/PanelGroup.h @@ -265,8 +265,8 @@ namespace PanelGroup { * 3. poll() on all InputBase objects. * 4. CANProtocol::drain() — dispatches CTRL_BCAST, SYNC_REQ, TEST_SEQ echo. * 5. update() on all OutputBase objects. - * 6. CANProtocol::serviceNodeStatus(): HB_n every 500 ms, HEALTH_n every 1 s, - * master-loss watchdog. + * 6. CANProtocol::serviceNodeStatus(): HB_n every 500 ms, HEALTH_n every 1 s (board kind + * PANEL_GROUP, #344), master-loss watchdog. */ void loop(); diff --git a/Firmware/Libraries/PowerMonitor/PduNode.cpp b/Firmware/Libraries/PowerMonitor/PduNode.cpp index 26e0431e..bd8948f1 100644 --- a/Firmware/Libraries/PowerMonitor/PduNode.cpp +++ b/Firmware/Libraries/PowerMonitor/PduNode.cpp @@ -225,7 +225,7 @@ void setup() { void loop() { STM32Board::tick(); CANProtocol::drain(); - CANProtocol::serviceNodeStatus(0); + CANProtocol::serviceNodeStatus(NodeKind::PDU); const uint32_t us = micros(); if ((int32_t)(us - gNextScanUs) >= 0) { diff --git a/Firmware/Libraries/PowerMonitor/PduNode.h b/Firmware/Libraries/PowerMonitor/PduNode.h index 6e434be5..cb394fda 100644 --- a/Firmware/Libraries/PowerMonitor/PduNode.h +++ b/Firmware/Libraries/PowerMonitor/PduNode.h @@ -9,7 +9,8 @@ * void loop() { PduNode::loop(); } * @endcode * with `-DNODE_ID=N` in platformio.ini. What runs: - * - **Node status** — HB_n / HEALTH_n / master-loss watchdog via CANProtocol::serviceNodeStatus(). + * - **Node status** — HB_n / HEALTH_n (board kind PDU, #344) / master-loss watchdog via + * CANProtocol::serviceNodeStatus(). * Rail and thermal faults reach HEALTH_n.faultId through the FaultSource registry. * - **Sampling** — the four rail channels every PDU_SAMPLE_US (1 kHz target); the NTC and * Vrefint every 100 ms. Faults are held clear for PDU_BOOT_BLANK_MS after boot. diff --git a/Firmware/ScratchPad/FirmwarePlan/00-decisions.md b/Firmware/ScratchPad/FirmwarePlan/00-decisions.md index 3e9ebf3c..51cca2b5 100644 --- a/Firmware/ScratchPad/FirmwarePlan/00-decisions.md +++ b/Firmware/ScratchPad/FirmwarePlan/00-decisions.md @@ -449,3 +449,41 @@ stays at v2. The first pages belong to the PDU monitoring node: `PDU_RAIL_12V`, **Affects:** `02-can-protocol.md`, `04-dcs-bios-integration.md`, `06-panelbridge-api.md`, `08-hardware-firmware-contracts.md`, `10-implementation-plan.md`, TechSpec (`CANProtocol`, `NodeStatus`, `PanelBridge`, `PowerMonitor`), `docs/architecture/can-bus.md`, `docs/architecture/power.md`. + +--- + +## D19 — Every board announces its kind; nodes detect a duplicate NODE_ID (#344) + +**Decision:** `HEALTH_n` byte 5 (reserved until now) carries a `NodeKind` — `0x00` unknown, `0x01` +panel group, `0x02` PDU, `0x03` bridge, append-only in `NodeStatus.h` — set by the board's firmware +role: `serviceNodeStatus(kind, …)` takes it as a required argument and each node library passes its +own. PanelBridge caches it and forwards it in `_NODE_STATUS` **v3** (28 hex), together with a node-0 +row for itself (its heartbeat state + its own `HEALTH_0`, kind bridge), and pushes a delta when an +alive node's kind changes. Separately, every node filters its own `HB_n`; a frame on it raises +`NodeFaultCode::DUPLICATE_NODE_ID`, ahead of every `FaultSource`, clearing 3 s after the last +foreign frame. + +**Rationale:** +- *The ID is not a type.* A PDU can take any `NODE_ID`, and silence proves nothing (a panel group + sends no telemetry), so the client had no positive signal for a panel group and couldn't let one + be adopted. Only the board's own firmware knows what it is. +- *A required argument, from the library.* Sketches never call `serviceNodeStatus()` — the node + library does — so the library is the role. Making `kind` required means a new node library won't + compile until it says what it is; nothing ambiguous needs a per-sketch override today. +- *Byte 5 is backward-safe.* Pre-#344 firmware already sends 0 there, which reads as unknown — an old + node stays un-adoptable until re-flashed, which is intended. The proto bump makes a v2 client fail + loudly instead of misreading 28 hex as 26. +- *The bridge reports itself.* Every board reports, for consistency; the bridge row is the client's + own link, never a new node. The bridge stays on node 0 — the master address every node watches. +- *Kind delta.* The alive delta leaves on a node's first `HB_n`, ~1 s before its first `HEALTH_n`, + so without a push the kind would wait for the client's next roster poll. +- *Duplicate IDs hide.* Two boards on one ID blend into one node and collide on identical CAN IDs + (error frames, error-passive), while inputs mostly still work — a "flaky board". Filtering one's + own HB costs one filter slot and no traffic. The duplicate outranks every other fault because + nothing reported under a shared ID can be trusted, and it auto-clears so fixing the other board + needs no reboot of this one. It is not a registry `FaultSource`: registry order is construction + order, which across translation units can't guarantee its priority. + +**Affects:** `02-can-protocol.md`, `04-dcs-bios-integration.md`, `06-panelbridge-api.md`, +`10-implementation-plan.md`, TechSpec (`CANProtocol`, `NodeStatus`, `PanelBridge`, `PanelGroup`, +`PowerMonitor`), `docs/architecture/can-bus.md`, `docs/firmware/node-id.md`. diff --git a/Firmware/ScratchPad/FirmwarePlan/02-can-protocol.md b/Firmware/ScratchPad/FirmwarePlan/02-can-protocol.md index 66a4fdba..e4ba057b 100644 --- a/Firmware/ScratchPad/FirmwarePlan/02-can-protocol.md +++ b/Firmware/ScratchPad/FirmwarePlan/02-can-protocol.md @@ -111,7 +111,23 @@ registers directly, and — since #340 — the sending itself: every node calls `CANProtocol::serviceNodeStatus()` once per loop, which sends `HB_n` every 500 ms and `HEALTH_n` every 1000 ms and, on every node except PanelBridge, runs the master-loss watchdog (WARNING after 1500 ms without `HB_0`, once a master has been seen). On those nodes `start()` adds `HB_0` to the -hardware filter automatically. +hardware filter automatically. Each node passes its board kind (`NodeKind`, #344) to +`serviceNodeStatus()` from its library role — see *Node Health Payload* below. + +**Duplicate NODE_ID check (#344).** `start()` also adds the node's **own** `HB_n` to its filter, on +every node including PanelBridge. bxCAN never receives its own transmissions, so a frame on that ID +means a second board was flashed with the same `NODE_ID` — which otherwise only shows up as one +blended node (uptime/rxCount jumping between two boards, error frames from identical-ID +collisions). The node then reports `NodeFaultCode::DUPLICATE_NODE_ID` in its `HEALTH_n` (DEGRADED + +`faultId`, plus the DiagSerial fault edge log): +- **Top priority** — it is chosen ahead of every registered `FaultSource`, because nothing else + reported under a shared ID can be trusted; a lesser fault can never mask it. +- **Auto-clears** `DUP_NODE_TIMEOUT_MS` (3000 ms) after the last foreign frame, so removing or + re-flashing the other board clears it without rebooting this one. A live duplicate re-raises + within one 500 ms heartbeat. +- In `startLoopback()` test mode the node's own heartbeats do come back; the check discounts one + looped-back frame per heartbeat it sent, so only extra frames read as a duplicate. +- The own-ID frame is still passed to `onReceive()`. ```cpp struct __attribute__((packed)) HeartbeatPayload { @@ -137,7 +153,8 @@ REC in bits [31:24]. After the shift: low byte = TEC, high byte = REC. health that PanelBridge reports to the host. No new CAN frame is added — PanelBridge caches the last `HB_n` per node and re-serializes it into a DCS-BIOS `_NODE_STATUS` message (see `04-dcs-bios-integration.md` and `06-panelbridge-api.md`). As of `_NODE_STATUS` proto **v2** -the message also carries the node's cached `dieTempC` + health/fault fields from `HEALTH_n` (below). +the message also carries the node's cached `dieTempC` + health/fault fields from `HEALTH_n` (below); +proto **v3** (#344) adds the node's board kind and the bridge's own node-0 row. --- @@ -157,7 +174,8 @@ struct __attribute__((packed)) NodeHealthPayload { uint8_t flags; // 2: NodeHealthFlag bits (NodeStatus.h): OVERHEAT, DEGRADED — #213/#163 uint8_t faultMask; // 3: fault source/domain bitmap — reserved for future domain bits (0) uint8_t faultId; // 4: NodeFaultCode (NodeStatus.h): 0 = no fault, else primary fault — #163 - uint8_t rsvd[3]; // 5–7: reserved, transmit 0 — future generic health (resetCause, freeRAM, …) + uint8_t nodeKind; // 5: NodeKind (NodeStatus.h): board kind from the firmware role — #344 + uint8_t rsvd[2]; // 6–7: reserved, transmit 0 — future generic health (resetCause, freeRAM, …) }; ``` @@ -165,16 +183,28 @@ This is the **shared node-health wire contract**. Distinct features populate the within the fixed 8 bytes and never collide: temperature (#213) owns `dieTempC` / `flags` bit0; the degraded-state feature (#163) owns `flags` bit1 / `faultId`, populated per node by `OpenSkyhawk::aggregateFaults()` (the primary fault, first non-`NONE` in `FaultSource` registry -order). A healthy node ships all-zero; a faulted node ships `flags` DEGRADED + a nonzero `faultId`, +order) — or `DUPLICATE_NODE_ID`, which outranks them all (see *Duplicate NODE_ID check* above); +the board-kind feature (#344) owns `nodeKind`. A healthy node ships all-zero; a faulted node ships `flags` DEGRADED + a nonzero `faultId`, but `faultMask` **stays 0** — it is reserved for future fault-domain bits. `faultId` is a `NodeFaultCode` (NodeStatus.h) the client maps to a label — no fault strings on the wire. **Rail voltage/current is deliberately absent** — that is the PDU's dedicated power telemetry (`TELEM_n` below, #216; INA180A2 current sense per console, #202), not generic per-node health; the reserved bytes are for future *generic* health fields (reset cause, free RAM, I2C error count). +**nodeKind (#344)** — what kind of board this `NODE_ID` is, as a `NodeKind` (NodeStatus.h, +append-only): `0x00` unknown, `0x01` panel group, `0x02` PDU, `0x03` bridge. It is announced by +the board's own firmware **role**, never derived from the `NODE_ID` (a PDU can take any ID): +`serviceNodeStatus(kind, …)` takes it as a required argument, and each node library passes its +own — `PanelGroup` → panel group, `PduNode` (PowerMonitor) → PDU, `PanelBridge` → bridge — so a +sketch never sets it and can't forget it. "Bridge" is the SimGateway + PanelBridge board: one board, +two MCUs; the PanelBridge MCU reports it in `HEALTH_0`, and the SimGateway (not on the bus) has no +kind of its own. Firmware older than #344 sent `0` in this byte (it was reserved), so `0x00` reads +as unknown — the client never adopts a node on it. + **Default-on**, compile out with `-DNODE_HEALTH_TELEM=0`. PanelBridge caches each node's fields -(under `PANELBRIDGE_NODE_STATUS`) and forwards all of them in `_NODE_STATUS` (proto v2, so the -degraded feature needs no further wire/proto change). A `HEALTH_n` frame is cache-only — it never -changes node liveness (`HB_n` owns that). +(under `PANELBRIDGE_NODE_STATUS`) and forwards all of them in `_NODE_STATUS` (proto v3, #344), plus +its own `HEALTH_0` as the node-0 row. A `HEALTH_n` frame never changes node liveness (`HB_n` owns +that); the only delta it triggers is a `_NODE_STATUS` push when the node's cached `nodeKind` +changes (see `04-dcs-bios-integration.md`). **flags bitmask:** - bit 0 (`0x01`): overheat — `dieTempC >= NODE_OVERHEAT_C`. **Opt-in:** the flag is computed @@ -312,7 +342,9 @@ filter. **Strategy:** use a **pass-all mask filter** (accepts every ID) with software-side validation inside the CAN RX interrupt handler. Frames whose ID does not fall in one of the accepted ranges -are silently discarded. `HB_0` (`0x100`) is PanelBridge's own transmission, not a child-node liveness frame. This is safe +are silently discarded. `HB_0` (`0x100`) is PanelBridge's own transmission, not a child-node liveness +frame — so an `HB_0` that does arrive came from another board on node 0 and trips the bridge's +duplicate NODE_ID check (#344), like any node's. This is safe because PanelGroup nodes never transmit outside their assigned ranges. --- diff --git a/Firmware/ScratchPad/FirmwarePlan/04-dcs-bios-integration.md b/Firmware/ScratchPad/FirmwarePlan/04-dcs-bios-integration.md index e10b1678..916d8312 100644 --- a/Firmware/ScratchPad/FirmwarePlan/04-dcs-bios-integration.md +++ b/Firmware/ScratchPad/FirmwarePlan/04-dcs-bios-integration.md @@ -22,7 +22,8 @@ points: **PanelBridge** (DCS-BIOS inputs) and **SimGateway** (HID inputs). ## Node-Status Reporting (#86) -PanelBridge surfaces connected PanelGroup nodes + their health to the host (OpenSkyhawk Client) +PanelBridge surfaces every board on the bus — connected nodes and, since v3 (#344), the bridge +itself — with its health and board kind to the host (OpenSkyhawk Client) **over the existing DCS-BIOS protocol**, using reserved A-4E-C-namespace identifiers. No bespoke sideband; SimGateway relays both directions verbatim (the request is binary export it forwards host→device; the response is ASCII it forwards device→host). PanelBridge owns it. @@ -30,34 +31,52 @@ forwards host→device; the response is ASCII it forwards device→host). PanelB | Reserved ID | Direction | Purpose | |-------------|-----------|---------| | Export address `0x86FE` (`NODE_STATUS_REQ_ADDR`) | host→device | Client writes it (value ignored) to request the roster. Above every real A-4E-C output (~`0x8554`), so DCS never exports it. Excluded from the CAN broadcast in `handleDcsBiosExport()`. | -| Command name `_NODE_STATUS` (`NODE_STATUS_MSG_NAME`) | device→host | One DCS-BIOS command message per node. Leading underscore — no A-4E-C control collides; DCS-BIOS ignores it as an unknown control if a copy leaks. | -| Command name `_NODE_STATUS_END` (`NODE_STATUS_END_MSG_NAME`) | device→host | Burst terminator for request/boot replies; argument = node count. Lets the client know a roster reply is complete and reconcile/prune. | +| Command name `_NODE_STATUS` (`NODE_STATUS_MSG_NAME`) | device→host | One DCS-BIOS command message per node (the bridge's own node-0 row included). Leading underscore — no A-4E-C control collides; DCS-BIOS ignores it as an unknown control if a copy leaks. | +| Command name `_NODE_STATUS_END` (`NODE_STATUS_END_MSG_NAME`) | device→host | Burst terminator for request/boot replies; argument = row count, node 0 included. Lets the client know a roster reply is complete and reconcile/prune. | **Request** — client writes a DCS-BIOS export frame to `0x86FE`. PanelBridge's `NodeStatusReqListener` (an `ExportStreamListener`) fires → emits the full roster. -**Response** — `_NODE_STATUS ` where `` is **26 chars** (proto **v2**, #221), **each field its -numeric value as fixed-width uppercase hex (most-significant nibble first)**: -`nodeId(2) present(2) flags(2) uptime(4) rxCount(4) esr(4) dieTempC(2) hFlags(2) faultMask(2) faultId(2)` +**Response** — `_NODE_STATUS ` where `` is **28 chars** (proto **v3**, #344; v2 was 26 chars +without `nodeKind`), **each field its numeric value as fixed-width uppercase hex +(most-significant nibble first)**: +`nodeId(2) present(2) flags(2) uptime(4) rxCount(4) esr(4) dieTempC(2) hFlags(2) faultMask(2) faultId(2) nodeKind(2)` (the 8-byte `HeartbeatPayload` plus `present`, then the node's cached `HEALTH_n` fields). `present`: `01` alive, `00` removed. `flags`: bit0 BOFF, bit1 EPVF. `esr`: low byte TEC, high byte REC. -`nodeId` 1–63. `uptime`/`rxCount` are uint16 — wrap at 65535 (~18 h / 65 k frames); treat as -health indicators, not monotonic counters. +`nodeId` 0–63: 0 is the bridge's own row, 1–63 the CAN nodes. `uptime`/`rxCount` are uint16 — +wrap at 65535 (~18 h / 65 k frames); treat as health indicators, not monotonic counters. -The last four fields come from the node's `HEALTH_n` frame (see `02-can-protocol.md`): +**Node-0 row (v3).** Every roster reply starts with the bridge's own row: `present` `01`, its +heartbeat state (`flags`/`uptime`/`esr` read live; `rxCount` `0000` — the bridge keeps no RX +count), and the health fields of the `HEALTH_0` it last sent, so `nodeKind` `03` (bridge). Before +its first `HEALTH_0` (~1 s after boot — e.g. the boot seed) the row reads not-yet-seen like any +node: `dieTempC` `80`, `nodeKind` `00`. The row only appears in roster replies, never as a live +delta: the bridge is the one reporting, so it is present whenever anything arrives. + +The last five fields come from the node's `HEALTH_n` frame (see `02-can-protocol.md`): - `dieTempC`: int8 two's-complement whole °C (internal MCU sensor — UNCALIBRATED, die not ambient). `80` = `INT8_MIN` = not-yet-seen → render unknown. - `hFlags`: node health bits — bit0 overheat, bit1 degraded. - `faultId`: `NodeFaultCode` — `00` = no fault, else the node's primary fault, populated per node by `aggregateFaults()` (#163); the client maps it to a label (no strings on the wire). - `faultMask`: fault source/domain bitmap — stays `00`, reserved for future fault-domain bits. +- `nodeKind` (#344): `NodeKind` — `00` unknown, `01` panel group, `02` PDU, `03` bridge (append-only). + Announced by the board's firmware role, never derived from `nodeId`. `00` before the node's first + `HEALTH_n` (same rule as `dieTempC` `80`) and from pre-#344 firmware. The client adopts a new node + only on `01` or `02`. A `faultId` of `09` (`DUPLICATE_NODE_ID`) means two boards share this + `nodeId`; every other field in the row is then a blend of both boards. **Emission semantics:** - A single bare `_NODE_STATUS` is a **live delta** — emitted on each node alive/dead transition - (`present` 01/00); the host applies it immediately. -- A **request/boot reply** is N `_NODE_STATUS` messages followed by `_NODE_STATUS_END `. That - set is the **authoritative present-roster** — the host reconciles (prunes nodes absent from - it). `count=0` = no panels connected. + (`present` 01/00), and (v3) when an alive node's cached `nodeKind` changes. The alive delta goes + out on a node's first `HB_n`, before its first `HEALTH_n`, so it reads kind `00`; the kind delta + follows ~1 s later, when that `HEALTH_n` arrives. A node re-flashed to another role pushes one + too. A cached kind is kept when a node times out and comes back. The host applies a delta + immediately. +- A **request/boot reply** is the node-0 row, then one `_NODE_STATUS` per alive node, then + `_NODE_STATUS_END `. That set is the **authoritative present-roster** — the host + reconciles (prunes nodes absent from it). `count` includes the node-0 row: `count=1` = the + bridge alone, no other nodes connected. - **Silent death** (node yanked / CAN bus-off) is reported as `present=00` by PanelBridge's existing 3 s heartbeat timeout (`checkNodeTimeouts`). A periodic client request reconciles any delta lost on the wire. @@ -70,7 +89,7 @@ change). The whole feature is gated behind `-DPANELBRIDGE_NODE_STATUS` (default ### Node telemetry — `_NODE_TELEM` (#216) Same transport, separate contract (`NODE_TELEM_*` in `NodeStatus.h`, `NODE_TELEM_PROTO_VERSION` 1 — -independent of `_NODE_STATUS`, which stays v2). PanelBridge forwards **every** `TELEM_n` CAN frame +independent of `_NODE_STATUS`, now v3). PanelBridge forwards **every** `TELEM_n` CAN frame (`0x181`–`0x1BF`, see `02-can-protocol.md`) to the host **unread**, one DCS-BIOS command message per frame: diff --git a/Firmware/ScratchPad/FirmwarePlan/06-panelbridge-api.md b/Firmware/ScratchPad/FirmwarePlan/06-panelbridge-api.md index 685a5235..9d318dd6 100644 --- a/Firmware/ScratchPad/FirmwarePlan/06-panelbridge-api.md +++ b/Firmware/ScratchPad/FirmwarePlan/06-panelbridge-api.md @@ -18,7 +18,8 @@ PanelBridge is the single STM32F103 CAN master that: - `0x8000 <= controlId <= 0x86FF` → binary search in generated input map → `sendDcsBiosMessage()` - `controlId < 0x8000` → wrap in HID frame → UART → SimGateway 4. Tracks node liveness from `HB_1`-`HB_63`, and sends its own `HB_0` master beacon and - `HEALTH_0` through `CANProtocol::serviceNodeStatus()` like every node (#93/#340). + `HEALTH_0` through `CANProtocol::serviceNodeStatus()` like every node (#93/#340), announcing + board kind `BRIDGE` (#344). --- @@ -53,15 +54,20 @@ emitted in response to `SYNC_REQ` are the state snapshot. **Node-status reporting to the host (#86, behind `-DPANELBRIDGE_NODE_STATUS`, default off):** PanelBridge surfaces the roster + per-node health to OpenSkyhawk Client over DCS-BIOS (see `04-dcs-bios-integration.md`). It caches each node's last `HeartbeatPayload` (`NodeState::last`, -populated in the `HB_n` handler) plus its last `HEALTH_n` fields (`dieTempC` + health/fault, cached -in the `HEALTH_n` handler, #221) and emits a `_NODE_STATUS ` DCS-BIOS command message: +populated in the `HB_n` handler) plus its last `HEALTH_n` fields (`dieTempC` + health/fault + +`nodeKind`, cached in the `HEALTH_n` handler, #221/#344) and emits a `_NODE_STATUS ` DCS-BIOS +command message (proto v3): - on every alive/dead transition — a single bare `_NODE_STATUS` (a live delta; the 3 s heartbeat - timeout in `checkNodeTimeouts` emits `present=00` for a silently-dead node), and + timeout in `checkNodeTimeouts` emits `present=00` for a silently-dead node), +- when an alive node's cached `nodeKind` changes (#344) — normally its first `HEALTH_n`, ~1 s after + the alive delta, which went out with kind `00`, and - at boot, and in response to a host request — a DCS-BIOS export write to `NODE_STATUS_REQ_ADDR` (`0x86FE`), handled by a dedicated `ExportStreamListener`, excluded from the CAN broadcast. A - request/boot reply is the full roster followed by `_NODE_STATUS_END `, so the host can tell - the burst is complete and prune nodes absent from it. + request/boot reply is the bridge's own node-0 row (its live heartbeat state + + `CANProtocol::lastNodeHealth()`, the `HEALTH_0` it last sent), then the full roster, then + `_NODE_STATUS_END ` (count includes node 0), so the host can tell the burst is complete + and prune nodes absent from it. SimGateway is unchanged — the ASCII messages and the binary request pass through transparently. diff --git a/Firmware/ScratchPad/FirmwarePlan/10-implementation-plan.md b/Firmware/ScratchPad/FirmwarePlan/10-implementation-plan.md index 45e8a310..2a73c21b 100644 --- a/Firmware/ScratchPad/FirmwarePlan/10-implementation-plan.md +++ b/Firmware/ScratchPad/FirmwarePlan/10-implementation-plan.md @@ -225,6 +225,9 @@ covered; the only uncovered outputs were the five light-intensity values (`LIGHT - [ ] On the PDU itself (with #202 bring-up): 1 kHz scan, `CAL VDDA` / `CAL ZERO` per board, thresholds tuned against the real load, calibration survives a reflash. - [ ] Client: `_NODE_TELEM` decode + per-rail display (OpenSkyHawk/SkyHawkClient#61). +- [x] Board kind in `HEALTH_n` byte 5 + `_NODE_STATUS` v3 with the bridge's node-0 row, and + duplicate NODE_ID detection (#344, D19). Bench on PanelBridge + a panel group (+ a PDU) and + the client resync (SkyHawkClient#63/#77) pending. --- diff --git a/Firmware/ScratchPad/TechSpec/CANProtocol.md b/Firmware/ScratchPad/TechSpec/CANProtocol.md index fc4c1de3..f2bab8a1 100644 --- a/Firmware/ScratchPad/TechSpec/CANProtocol.md +++ b/Firmware/ScratchPad/TechSpec/CANProtocol.md @@ -75,18 +75,23 @@ Firmware/Tests/CANProtocol/ │ after start(); callback not called again without a status change ├── heartbeat_payload.cpp — makeHeartbeatPayload() fills nodeId, uptime, flags, rxCount, │ and ESR-derived TEC/REC without callers touching HAL registers - ├── health_payload.cpp — makeNodeHealthPayload() packs nodeId/dieTempC + faultId/DEGRADED, - │ faultMask/reserved stay 0; on-target readDieTempC()/readVddMv() range (#213) + ├── health_payload.cpp — makeNodeHealthPayload() packs nodeId/dieTempC + faultId/DEGRADED + + │ nodeKind in byte 5 (#344), faultMask/reserved 6-7 stay 0; on-target + │ readDieTempC()/readVddMv() range (#213) ├── health_flags.cpp — overheat+degraded coexistence: -DNODE_OVERHEAT_C=0, asserts a faulted │ hot payload sets BOTH flag bits (|= never clobbers overheat) (#163) ├── node_status_service.cpp — serviceNodeStatus() (#340), loopback: start() auto-accepts HB_0 and │ drain() still forwards it; HB_n every 500 ms (first ~500 ms after │ start); HEALTH_n every 1000 ms; master-loss WARNING absent before a │ master is seen, raised after HB_0 stops > 1500 ms, cleared on return + ├── duplicate_node_id.cpp — duplicate NODE_ID check (#344), loopback: own looped-back HBs never + │ trip it; extra HB_n frames the test sends raise DUPLICATE_NODE_ID + + │ DEGRADED in lastNodeHealth(), held while they flow, clear ~3 s after └── node_status_bus.cpp — the same service on a physical bus against a master (PanelBridge or a USB-CAN adapter sending HB_0 every 500 ms): DiagSerial logs every - LED state change + a 10 s summary (warnRaises / hb0Seen / TEC / REC) - for watchdog timing, flapping-master and link-drop runs, and soaks + LED state change + a 10 s summary (warnRaises / hb0Seen / TEC / REC / + fault / kind) for watchdog timing, flapping-master and link-drop runs, + and soaks; on two boards with the same NODE_ID, the duplicate bench ``` All runtime scenarios use `startLoopback()` — single board, no second node or physical bus @@ -325,7 +330,7 @@ static constexpr uint16_t CTRL_ID_DCS_MIN = 0x8000; ///< DCS-BIOS range start static constexpr uint16_t CTRL_ID_DCS_MAX = 0x86FF; ///< DCS-BIOS range end // Node-status host contract + fault vocabulary (NODE_STATUS_*, NodeHealthFlag, NodeFaultCode, -// FaultSource) live in the neutral `NodeStatus` library (see NodeStatus.md), not here. +// NodeKind, FaultSource) live in the neutral `NodeStatus` library (see NodeStatus.md), not here. // CANProtocol owns only NodeHealthPayload (the CAN HEALTH_n frame struct). // ── HID axis/button controlIds — from HIDControls.h (included above) ──────── @@ -393,7 +398,8 @@ namespace CANProtocol { * registrations. Always adds CTRL_BCAST (0x010), TEST_SEQ (0x011), and * SYNC_REQ (0x012) to the active filter — these are mandatory for all nodes * and cannot be excluded. On every node except PanelBridge (NODE_ID 0) it also adds - * HB_0 (0x100) for the serviceNodeStatus() master-loss watchdog (#340). + * HB_0 (0x100) for the serviceNodeStatus() master-loss watchdog (#340), and on every node + * its own HB_NODE_ID for the duplicate NODE_ID check (#344). * * Resets the serviceNodeStatus() timers, which arm on its first call. Sets CanStatus to * NORMAL on success and fires the onStatusChange callback. @@ -406,7 +412,8 @@ namespace CANProtocol { * Identical to start() but uses CAN_MODE_SILENT_LOOPBACK: frames transmitted * by this node are received back internally without going on the physical bus. * Enables single-board testing of filter dispatch, RX callbacks, and TX queue - * behaviour without a second node. + * behaviour without a second node. The duplicate NODE_ID check discounts one looped-back + * own HB_n per heartbeat sent, so only extra own-ID frames read as a duplicate (#344). * * @note Never call this in production firmware. */ @@ -419,7 +426,8 @@ namespace CANProtocol { * call. For each frame: fires onSyncReq() for SYNC_REQ, auto-replies with * ECHO carrying the same 8-byte payload for TEST_SEQ, fires onReceive() for * all other frames. On a non-bridge node an HB_0 is first stamped for the - * serviceNodeStatus() watchdog, then still passed to onReceive(). + * serviceNodeStatus() watchdog, then still passed to onReceive(). A frame on the node's own + * HB_NODE_ID is first stamped for the duplicate NODE_ID check (#344), then passed on too. * * Also services sendBatched() deadlines: any half-full ControlPacketPair * that has waited two owning firmware loop iterations is flushed with @@ -557,14 +565,16 @@ namespace CANProtocol { * overheat flag (bit0) only when NODE_OVERHEAT_C is defined at build time and dieTempC meets * it; otherwise the overheat bit is 0 (pure telemetry). This is the packing boundary that owns * the NodeFaultCode->byte cast: it writes faultId and OR-derives the DEGRADED flag (bit1) when - * fault != NONE (never clobbering the overheat bit). faultMask + reserved bytes stay 0 (#163). + * fault != NONE (never clobbering the overheat bit), and writes nodeKind (byte 5, #344). + * faultMask + reserved bytes 6-7 stay 0 (#163). * * @param nodeId Node ID to place in the payload; 1-63 PanelGroup, 0 PanelBridge. + * @param kind Board kind (NodeKind) from the node's firmware role — never from nodeId. * @param dieTempC Internal die temp in whole °C (INT8_MIN = unavailable). - * @param fault Primary NodeFaultCode from aggregateFaults(); NONE (default) = healthy node. + * @param fault Primary node fault; NONE (default) = healthy node. * @return Fully populated NodeHealthPayload ready to send as HEALTH_n. */ - NodeHealthPayload makeNodeHealthPayload(uint8_t nodeId, int8_t dieTempC, + NodeHealthPayload makeNodeHealthPayload(uint8_t nodeId, NodeKind kind, int8_t dieTempC, NodeFaultCode fault = NodeFaultCode::NONE); /** @@ -579,8 +589,12 @@ namespace CANProtocol { * @brief Run the node-generic status duties. Call once per loop(), after drain() (#340). * * - HB_NODE_ID every 500 ms (makeHeartbeatPayload), carrying rxCount. - * - HEALTH_NODE_ID every 1000 ms (makeNodeHealthPayload) + the DiagSerial fault edge log - * (tag BRIDGE on NODE_ID 0, NODE otherwise). Compiled out with -DNODE_HEALTH_TELEM=0. + * - HEALTH_NODE_ID every 1000 ms (makeNodeHealthPayload, carrying `kind`) + the DiagSerial + * fault edge log (tag BRIDGE on NODE_ID 0, NODE otherwise). Compiled out with + * -DNODE_HEALTH_TELEM=0. The fault is DUPLICATE_NODE_ID while the duplicate check is live, + * else aggregateFaults(). + * - Duplicate NODE_ID check (#344), every node: a frame on the node's own HB_NODE_ID raises + * DUPLICATE_NODE_ID, ahead of every FaultSource; clears DUP_NODE_TIMEOUT_MS after the last. * - Master-loss watchdog on every node except PanelBridge (NODE_ID 0): once HB_0 has been * seen, master loss is set after MASTER_TIMEOUT_MS of silence and cleared by the next HB_0. * - WARNING on a non-bridge node = master loss OR overheat (NODE_OVERHEAT_C builds only), @@ -589,14 +603,23 @@ namespace CANProtocol { * Timers arm on the first call (first loop() after setup()), so the first HB/HEALTH go out * one period after boot completes. * + * @param kind Board kind for HEALTH_n (#344), required — each node library passes its role's + * (PanelGroup PANEL_GROUP, PduNode PDU, PanelBridge BRIDGE). * @param rxCount Caller-owned accepted RX count for the heartbeat; 0 if the node keeps none. * @note Samples millis() after drain() and compares signed (#225). */ - void serviceNodeStatus(uint16_t rxCount = 0); + void serviceNodeStatus(NodeKind kind, uint16_t rxCount = 0); + + /** @brief The HEALTH_n payload serviceNodeStatus() last built (PanelBridge's node-0 row, #344). + * Not-yet-seen (dieTempC INT8_MIN, the rest 0) before the first one. */ + const NodeHealthPayload& lastNodeHealth(); /** @brief Master-loss timeout — 3× the 500 ms HB_0 cadence. */ static constexpr uint32_t MASTER_TIMEOUT_MS = 1500; + /** @brief Duplicate NODE_ID clear time — 6× the 500 ms HB cadence (#344). */ + static constexpr uint32_t DUP_NODE_TIMEOUT_MS = 3000; + } // namespace CANProtocol ``` @@ -610,12 +633,24 @@ dependency. | Duty | Cadence | Who | |------|---------|-----| | `HB_n` (`makeHeartbeatPayload`) | 500 ms | every node; PanelBridge's is `HB_0` | -| `HEALTH_n` (`makeNodeHealthPayload`) + fault edge log | 1000 ms | every node, unless `-DNODE_HEALTH_TELEM=0` | +| `HEALTH_n` (`makeNodeHealthPayload`, with the caller's `NodeKind`) + fault edge log | 1000 ms | every node, unless `-DNODE_HEALTH_TELEM=0` | +| Duplicate NODE_ID check → `DUPLICATE_NODE_ID` in `HEALTH_n` | checked every call | every node, PanelBridge included | | Master-loss watchdog + overheat → `STM32Board::setWarning()` | checked every call | every node except NODE_ID 0 (bridge: overheat raise-only) | - **`HB_0` intake.** On a non-bridge node `start()` adds `canIdHb(0)` to the hardware filter list (no-op under `filterAcceptAll()`), and `drain()` stamps each `HB_0` for the watchdog *before* passing it to `onReceive()` as usual — node code may still observe it. +- **Board kind (#344).** `kind` is a required argument: the node library is the role (sketches never + call the service), so a new node library won't build until it says what it is. It never comes + from `NODE_ID`. +- **Duplicate NODE_ID (#344).** `start()` adds the node's own `canIdHb(NODE_ID)` to the filter on + every node (PanelBridge is pass-all anyway). bxCAN never receives its own frames, so `drain()` + stamps any frame on that ID as foreign, then passes it to `onReceive()`. While a stamp is younger + than `DUP_NODE_TIMEOUT_MS` the `HEALTH_n` fault is `DUPLICATE_NODE_ID` — chosen before + `aggregateFaults()`, so no `FaultSource` can mask it (registry order is construction order, which + can't guarantee a priority). It drives no LED, like every DEGRADED fault. In `startLoopback()` the + node's own heartbeats come back, so the service counts the heartbeats it sent and `drain()` + discounts that many own-ID frames. - **WARNING rules.** The LED has one shared WARNING latch, so on a non-bridge node the service keeps its two causes apart and drives the latch from their OR, writing it only when the OR changes: - *master loss* — set when `HB_0` has been silent for more than `MASTER_TIMEOUT_MS` after a master @@ -763,7 +798,8 @@ overridden. `start()` unconditionally adds CTRL_BCAST (0x010), TEST_SEQ (0x011), and SYNC_REQ (0x012) to the active filter regardless of what the caller registered. These are global — every -node must receive them. If `filterAcceptAll()` was called, the hardware is pass-all and +node must receive them. It also adds the node's own `HB_n` (duplicate NODE_ID check, #344) and, on +every node except PanelBridge, `HB_0` (master-loss watchdog, #340). If `filterAcceptAll()` was called, the hardware is pass-all and these additions are redundant but harmless. ### ControlPacketPair payload diff --git a/Firmware/ScratchPad/TechSpec/NodeStatus.md b/Firmware/ScratchPad/TechSpec/NodeStatus.md index 88f79744..ec093aeb 100644 --- a/Firmware/ScratchPad/TechSpec/NodeStatus.md +++ b/Firmware/ScratchPad/TechSpec/NodeStatus.md @@ -1,7 +1,7 @@ # NodeStatus — Technical Specification **Status:** Done -**FirmwarePlan ref:** `FirmwarePlan/04-dcs-bios-integration.md#node-status-reporting`, `FirmwarePlan/00-decisions.md#d14` +**FirmwarePlan ref:** `FirmwarePlan/04-dcs-bios-integration.md#node-status-reporting`, `FirmwarePlan/00-decisions.md#d14`, `#d19` **Depends on:** `` (enum base type) — otherwise platform-agnostic --- @@ -20,7 +20,11 @@ Owns: the client loudly. - **`NodeHealthFlag`** — HEALTH_n flag bits (OVERHEAT, DEGRADED) mirroring `NodeHealthPayload.flags`. - **`NodeFaultCode`** — the compact `faultId` dictionary (cross-node: I2C_PERIPHERAL, OVER_VOLTAGE, - UNDER_VOLTAGE, SHORT_CIRCUIT, HOST_LINK_LOST, …). Client maps id → label (SkyHawkClient#40). + UNDER_VOLTAGE, SHORT_CIRCUIT, HOST_LINK_LOST, DUPLICATE_NODE_ID, …). Client maps id → label + (SkyHawkClient#40). +- **`NodeKind`** (#344) — the `HEALTH_n` byte-5 board kind (UNKNOWN / PANEL_GROUP / PDU / BRIDGE), + append-only, forwarded as `_NODE_STATUS` v3's last field. Set by each node library's role via + `CANProtocol::serviceNodeStatus(kind, …)`, never from the NODE_ID. - **`FaultSource`** — the interface a fault-producing object implements + a self-registering list. - **`NODE_TELEM_*` + the TELEM_n page vocabulary (#216)** — `NODE_TELEM_MSG_NAME` (`_NODE_TELEM`) and `NODE_TELEM_PROTO_VERSION` (1, independent of `_NODE_STATUS`); `TelemPage` (global, append-only @@ -37,7 +41,7 @@ strings** stay local (DiagSerial) and never go on the wire or into any shared/HI ``` Firmware/Libraries/NodeStatus/ -├── NodeStatus.h ← NODE_STATUS_* + NodeHealthFlag + NodeFaultCode + FaultSource +├── NodeStatus.h ← NODE_STATUS_* + NodeHealthFlag + NodeFaultCode + NodeKind + FaultSource ├── NodeStatus.cpp ← FaultSource intrusive-list statics └── library.json ← platforms: * ``` @@ -47,7 +51,7 @@ Firmware/Libraries/NodeStatus/ ## Public API ```cpp -#define NODE_STATUS_PROTO_VERSION 2 // bump on ANY _NODE_STATUS wire change +#define NODE_STATUS_PROTO_VERSION 3 // bump on ANY _NODE_STATUS wire change (v3: nodeKind + node-0 row, #344) #define NODE_STATUS_REQ_ADDR 0x86FE #define NODE_STATUS_MSG_NAME "_NODE_STATUS" #define NODE_STATUS_END_MSG_NAME "_NODE_STATUS_END" @@ -59,7 +63,12 @@ enum class NodeHealthFlag : uint8_t { OVERHEAT = 0x01, DEGRADED = 0x02 }; enum class NodeFaultCode : uint8_t { NONE = 0, I2C_PERIPHERAL = 1, OVER_VOLTAGE = 2, UNDER_VOLTAGE = 3, SHORT_CIRCUIT = 4, HOST_LINK_LOST = 5, - FUSE_OPEN = 6, OVER_TEMPERATURE = 7, SENSOR_FAULT = 8, // PDU (#216); 0x09+ reserved + FUSE_OPEN = 6, OVER_TEMPERATURE = 7, SENSOR_FAULT = 8, // PDU (#216) + DUPLICATE_NODE_ID = 9, // another board on this node's HB_n (#344); 0x0A+ reserved +}; + +enum class NodeKind : uint8_t { // HEALTH_n byte 5 (#344), append-only + UNKNOWN = 0, PANEL_GROUP = 1, PDU = 2, BRIDGE = 3, // 0x04+ reserved }; namespace OpenSkyhawk { @@ -90,7 +99,9 @@ typed `NodeFaultCode`; the cast to `uint8_t` happens only at the CAN/DCS-BIOS pa **Aggregation & priority:** `aggregateFaults()` returns the **first non-`NONE`** `faultCode()` in **registry iteration order, which is REVERSE construction order** — the intrusive list pushes new sources at `head()`, so the *last-constructed* source is visited first. One fault is on the wire at -a time; multi-fault priority is a later concern. Each node's health-TX calls it, packs the result +a time; multi-fault priority is a later concern. The one exception sits outside the registry: +`DUPLICATE_NODE_ID` is raised by `CANProtocol::serviceNodeStatus()` itself and outranks every +`FaultSource` (D19) — registry order can't guarantee a priority. Each node's health-TX calls it, packs the result into `HEALTH_n` (via `makeNodeHealthPayload`, which derives DEGRADED), and edge-logs `faultDetail()` via the shared `STM32Board::logNodeFaultEdge(tag, fault, detail)` helper (one prev-fault static; DiagSerial only, on change). **Degraded drives no status-LED** — frame + DiagSerial only @@ -100,7 +111,8 @@ DiagSerial only, on change). **Degraded drives no status-LED** — frame + DiagS `DrumDisplay` is *one* `FaultSource` (I2C); a PDU rail monitor and a PanelBridge host-link watchdog are others. Implementers report **cached** state only (cheap/const, no blocking I/O) — the aggregator runs on the periodic health path. **Adding a fault code:** append a `NodeFaultCode` value + mirror -the client label. **Changing the `_NODE_STATUS` wire shape:** bump `NODE_STATUS_PROTO_VERSION`. +the client label. **Adding a node kind:** append a `NodeKind` value and pass it from the new node +library's `serviceNodeStatus()` call — no proto bump (the field already exists). **Changing the `_NODE_STATUS` wire shape:** bump `NODE_STATUS_PROTO_VERSION`. **Adding a TELEM page:** append a `TelemPage` value (never renumber) and document its layout in FirmwarePlan 02 — additive, no bump; changing an existing page's layout bumps `NODE_TELEM_PROTO_VERSION`. @@ -114,8 +126,9 @@ FirmwarePlan 02 — additive, no bump; changing an existing page's layout bumps its detail, null `faultDetail()` normalized to `""`, `nullptr` detailOut accepted. Also verified indirectly: `DrumDisplay` breaker test asserts `faultCode()`/`faultDetail()` via the `FaultSource` override; `CANProtocol` `health_payload`/`health_flags` tests assert the faultId/DEGRADED packing -(incl. overheat+degraded coexistence); `PanelBridge` node_status test exercises the `_NODE_STATUS` -emission; `PanelBridge` node_telem test exercises `_NODE_TELEM` forwarding; `PowerMonitor` tests +(incl. overheat+degraded coexistence) and the `nodeKind` byte; `CANProtocol` duplicate_node_id test +covers `DUPLICATE_NODE_ID` raise/hold/clear; `PanelBridge` node_status test exercises the v3 +`_NODE_STATUS` emission (kind `00` before HEALTH, kind delta, node-0 row); `PanelBridge` node_telem test exercises `_NODE_TELEM` forwarding; `PowerMonitor` tests cover the PDU fault sources and page flags; the client `reference.test.ts` asserts the proto-version contract. diff --git a/Firmware/ScratchPad/TechSpec/PanelBridge.md b/Firmware/ScratchPad/TechSpec/PanelBridge.md index a73e601b..2434db04 100644 --- a/Firmware/ScratchPad/TechSpec/PanelBridge.md +++ b/Firmware/ScratchPad/TechSpec/PanelBridge.md @@ -227,7 +227,7 @@ namespace PanelBridge { * @brief Run PanelBridge work that is not owned by DcsBios::loop(). * * Drains CANProtocol RX, dispatches EVT packets, services CANProtocol batching deadlines, - * checks node heartbeat timeouts, sends its own HB_0 / HEALTH_0 through + * checks node heartbeat timeouts, sends its own HB_0 / HEALTH_0 (kind BRIDGE, #344) through * CANProtocol::serviceNodeStatus() (#340), and handles DiagSerial TEST_SEQ trigger bytes. */ void loop(); @@ -361,14 +361,30 @@ are child-node liveness frames. The RX handler validates IDs in software: | Range | Accepted by PanelBridge | Action | |-------|--------------------------|--------| | `0x101`-`0x13F` | `HB_1`-`HB_63` | Update node heartbeat/liveness | -| `0x141`-`0x17F` | `HEALTH_1`-`HEALTH_63` | Cache die temp + health/fault for `_NODE_STATUS` (not liveness) | +| `0x141`-`0x17F` | `HEALTH_1`-`HEALTH_63` | Cache die temp + health/fault + `nodeKind` for `_NODE_STATUS` (not liveness); push a `_NODE_STATUS` delta when an alive node's kind changes (#344) | | `0x181`-`0x1BF` | `TELEM_1`-`TELEM_63` | Forward unread as `_NODE_TELEM` (#216; not liveness; page 0 / short dropped) | | `0x201`-`0x23F` | `EVT_1`-`EVT_63` | Dispatch each non-null ControlPacket slot | | `0x301`-`0x33F` | `ECHO_1`-`ECHO_63` | Consume TEST_SEQ RTT diagnostic response | | `0x401`-`0x43F` | `READY_1`-`READY_63` | Mark node ready and broadcast `SYNC_REQ` | All other IDs are discarded silently. `HB_0` (`0x100`) is PanelBridge's own transmission, never -a child-node heartbeat. +a child-node heartbeat — one that does arrive is another board on node 0, which +`CANProtocol::drain()` flags as a duplicate NODE_ID before `onCanRx` ignores it (#344). + +### Node-status reporting (#86, proto v3 #344) + +Behind `-DPANELBRIDGE_NODE_STATUS`. `NodeState` caches each node's last `HeartbeatPayload` and its +`HEALTH_n` fields (`dieTempC`, `healthFlags`, `faultMask`, `faultId`, `nodeKind` — not-yet-seen: +`INT8_MIN` / `0`). One `emitRow()` formats a 28-hex `_NODE_STATUS` row; `emitNode(id, present)` +feeds it from the cache and `emitSelf()` from the bridge's own state — `makeHeartbeatPayload(0, 0)` +plus `CANProtocol::lastNodeHealth()` (its last `HEALTH_0`, kind `BRIDGE`). `emitAllNodes()` (boot +seed + host request) sends the node-0 row, every alive node, then `_NODE_STATUS_END ` with +node 0 counted. Live deltas: alive/dead transitions, and a `nodeKind` change on an alive node. +Contract: FirmwarePlan `04-dcs-bios-integration.md`. + +Test hooks (`PANELBRIDGE_TEST`): `testFeedHeartbeat`, `testFeedHealth(nodeId, dieTempC, hFlags, +faultMask, faultId, nodeKind)` (mirrors the `HEALTH_n` branch, incl. the kind delta), +`testRequestNodeStatus`, `testCheckTimeouts`. ### READY and Recovery SYNC_REQ diff --git a/Firmware/ScratchPad/TechSpec/PanelGroup/PanelGroup.md b/Firmware/ScratchPad/TechSpec/PanelGroup/PanelGroup.md index a448a7f5..5b6d5ee1 100644 --- a/Firmware/ScratchPad/TechSpec/PanelGroup/PanelGroup.md +++ b/Firmware/ScratchPad/TechSpec/PanelGroup/PanelGroup.md @@ -15,6 +15,7 @@ dispatch of input events via `CANProtocol::sendBatched()`; reception and distrib inputs); and the `READY_n` frame on boot completion. The node-generic status traffic — the 500 ms `HB_n` heartbeat, the 1 s `HEALTH_n` frame and the master-loss watchdog — is not PanelGroup's: PanelGroup calls `CANProtocol::serviceNodeStatus()` once per loop and passes its +board kind (`NodeKind::PANEL_GROUP`, #344 — so a PanelGroup sketch never sets it) and its accepted-`CTRL_BCAST` count for the heartbeat (#340). `TEST_SEQ` echo is owned by `CANProtocol::drain()` and is not forwarded to PanelGroup callbacks. @@ -376,8 +377,8 @@ namespace PanelGroup { * then ignored by PanelGroup's receive callback. * anything else → ignored by PanelGroup's receive callback. * 5. Call update() on all OutputBase objects. - * 6. CANProtocol::serviceNodeStatus(rxCount) — HB_n every 500 ms, HEALTH_n every - * 1 s, master-loss watchdog (#340). + * 6. CANProtocol::serviceNodeStatus(NodeKind::PANEL_GROUP, rxCount) — HB_n every 500 ms, + * HEALTH_n every 1 s (kind PANEL_GROUP, #344), master-loss watchdog (#340). */ void loop(); diff --git a/Firmware/ScratchPad/TechSpec/PowerMonitor.md b/Firmware/ScratchPad/TechSpec/PowerMonitor.md index 649fbca4..5f5e84be 100644 --- a/Firmware/ScratchPad/TechSpec/PowerMonitor.md +++ b/Firmware/ScratchPad/TechSpec/PowerMonitor.md @@ -14,7 +14,8 @@ The firmware of the **PDU monitoring node** (#202 hardware, #216 firmware) — o NODE_IDs 3/4/5. It samples the 12 V and 5 V rails (voltage + current) and the board NTC, turns them into engineering units with the board's own calibration, raises rail / thermal conditions, publishes them as `TELEM_n` pages, and feeds its faults into `HEALTH_n` through the FaultSource -registry. Node plumbing (HB / HEALTH / master watchdog) comes from `CANProtocol::serviceNodeStatus()`; +registry. Node plumbing (HB / HEALTH / master watchdog) comes from `CANProtocol::serviceNodeStatus()`, +called with `NodeKind::PDU` so every PDU announces its kind in `HEALTH_n` whatever its NODE_ID (#344); there is **no PanelGroup dependency** (the PDU has no inputs or outputs). Does **not** own: the TELEM wire contract (`TelemPayload` in CANProtocol, page vocabulary in diff --git a/Firmware/Tests/CANProtocol/platformio.ini b/Firmware/Tests/CANProtocol/platformio.ini index a566e9af..a04bdfa5 100644 --- a/Firmware/Tests/CANProtocol/platformio.ini +++ b/Firmware/Tests/CANProtocol/platformio.ini @@ -76,6 +76,12 @@ extends = env_base build_flags = ${env_base.build_flags} -DSTM32BOARD_TEST -DNODE_OVERHEAT_C=0 build_src_filter = -<*> + +; Duplicate NODE_ID check (#344): single board, loopback. The test sends extra HB_1 frames itself to +; stand in for a second board with NODE_ID 1; own looped-back heartbeats must not trip the check. +[env:test_duplicate_node_id] +extends = env_base +build_src_filter = -<*> + + ; Same service on a physical bus: one board (NODE_ID=1) + a master — PanelBridge, or a USB-CAN ; adapter sending HB_0 (0x100) every 500 ms. DiagSerial logs LED state changes for the ; master-loss watchdog and a 10 s summary (warnRaises / TEC / REC) for soaks. diff --git a/Firmware/Tests/CANProtocol/tests/duplicate_node_id/duplicate_node_id.cpp b/Firmware/Tests/CANProtocol/tests/duplicate_node_id/duplicate_node_id.cpp new file mode 100644 index 00000000..d79322bb --- /dev/null +++ b/Firmware/Tests/CANProtocol/tests/duplicate_node_id/duplicate_node_id.cpp @@ -0,0 +1,87 @@ +// CANProtocol — duplicate NODE_ID check (#344) +// +// Single board, silent loopback (own frames come back). NODE_ID=1. A node never receives its own +// frames on a real bus, so a frame on its own HB_1 means another board has NODE_ID 1. In loopback +// the node's own heartbeats DO come back; serviceNodeStatus() discounts those, so only the extra +// HB_1 frames this test sends itself (standing in for a second board) read as a duplicate. +// +// Timeline (t = ms since boot complete; HEALTH_1 every 1000 ms from t≈1000): +// 0..1500 own HB/HEALTH only → HEALTH faultId 0, kind 1, not DEGRADED +// 1500..2500 foreign HB_1 every 500 ms → HEALTH at ~2000 + ~3000 carry DUPLICATE_NODE_ID +// 2500.. foreign HB_1 stops (last ~2500) → clears 3000 ms later; HEALTH at ~6000 is clean +// The node's real FaultSource set is empty here, so any fault seen is the duplicate check's. +// +// Expected output (DiagSerial 115200), all PASS: +// [NODE] degraded: another board transmits on this NODE_ID (fault 9) +// [NODE] recovered +// --- duplicate_node_id results --- +// no false duplicate from own HBs: PASS +// kind PANEL_GROUP in HEALTH: PASS +// foreign HB_1 -> DUPLICATE_NODE_ID: PASS +// foreign HB_1 -> DEGRADED: PASS +// held while foreign HB_1 flows: PASS +// cleared ~3 s after it stops: PASS + +#include +#include + +static uint32_t _startMs = 0; + +static void pass(const __FlashStringHelper* label, bool ok) { + auto& d = STM32Board::diagSerial(); + d.print(label); + d.println(ok ? F("PASS") : F("FAIL")); +} + +static bool isDup(const NodeHealthPayload& h) { + return h.faultId == (uint8_t)NodeFaultCode::DUPLICATE_NODE_ID; +} + +void setup() { + STM32Board::setDebug(true); + STM32Board::begin(); + STM32Board::log("=== duplicate_node_id ==="); + + CANProtocol::onStatusChange(STM32Board::onCanStatus); + CANProtocol::startLoopback(); // own HB_1 is added to the filter by start() itself + _startMs = millis(); // boot complete +} + +void loop() { + STM32Board::tick(); + CANProtocol::drain(); + CANProtocol::serviceNodeStatus(NodeKind::PANEL_GROUP); + + const uint32_t t = millis() - _startMs; + + // Second board stand-in: an extra HB_1 every 500 ms in [1500, 2500]. + static uint32_t lastForeign = 0; + if (t >= 1500 && t <= 2500 && (lastForeign == 0 || t - lastForeign >= 500)) { + lastForeign = t; + HeartbeatPayload hb = CANProtocol::makeHeartbeatPayload(NODE_ID, 0); + CANProtocol::send(canIdHb(NODE_ID), reinterpret_cast(&hb), sizeof(hb)); + } + + // Probes on the last HEALTH_1 built, each latched once. + static NodeHealthPayload before, during, held, after; + static bool p1 = false, p2 = false, p3 = false, p4 = false; + const NodeHealthPayload& h = CANProtocol::lastNodeHealth(); + if (!p1 && t >= 1400) { p1 = true; before = h; } // HEALTH ~1000: no foreign frame yet + if (!p2 && t >= 2400) { p2 = true; during = h; } // HEALTH ~2000: foreign since 1500 + if (!p3 && t >= 3400) { p3 = true; held = h; } // HEALTH ~3000: last foreign ~2500 + if (!p4 && t >= 6400) { p4 = true; after = h; } // HEALTH ~6000: > 2500 + 3000 + + static bool reported = false; + if (!reported && t >= 6500) { + reported = true; + auto& d = STM32Board::diagSerial(); + d.println(F("--- duplicate_node_id results ---")); + pass(F("no false duplicate from own HBs: "), before.faultId == 0 && before.flags == 0); + pass(F("kind PANEL_GROUP in HEALTH: "), before.nodeKind == (uint8_t)NodeKind::PANEL_GROUP); + pass(F("foreign HB_1 -> DUPLICATE_NODE_ID: "), isDup(during)); + pass(F("foreign HB_1 -> DEGRADED: "), + (during.flags & (uint8_t)NodeHealthFlag::DEGRADED) != 0); + pass(F("held while foreign HB_1 flows: "), isDup(held)); + pass(F("cleared ~3 s after it stops: "), after.faultId == 0 && after.flags == 0); + } +} diff --git a/Firmware/Tests/CANProtocol/tests/health_flags/health_flags.cpp b/Firmware/Tests/CANProtocol/tests/health_flags/health_flags.cpp index 09fdf98e..71588afb 100644 --- a/Firmware/Tests/CANProtocol/tests/health_flags/health_flags.cpp +++ b/Firmware/Tests/CANProtocol/tests/health_flags/health_flags.cpp @@ -27,18 +27,18 @@ void setup() { const uint8_t DEGRADED = (uint8_t)NodeHealthFlag::DEGRADED; // 0x02 // Hot + healthy: overheat only. - NodeHealthPayload hot = CANProtocol::makeNodeHealthPayload(NODE_ID, /*dieTempC*/ 40); + NodeHealthPayload hot = CANProtocol::makeNodeHealthPayload(NODE_ID, NodeKind::PANEL_GROUP, /*dieTempC*/ 40); check("hot healthy -> OVERHEAT only", hot.flags == OVERHEAT); // Hot + faulted: both bits, faultId carries the code. NodeHealthPayload both = CANProtocol::makeNodeHealthPayload( - NODE_ID, /*dieTempC*/ 40, NodeFaultCode::I2C_PERIPHERAL); + NODE_ID, NodeKind::PANEL_GROUP, /*dieTempC*/ 40, NodeFaultCode::I2C_PERIPHERAL); check("hot + fault -> OVERHEAT|DEGRADED", both.flags == (OVERHEAT | DEGRADED)); check("hot + fault -> faultId set", both.faultId == (uint8_t)NodeFaultCode::I2C_PERIPHERAL); // Sentinel temp (unavailable) never trips overheat; fault alone -> DEGRADED only. NodeHealthPayload deg = CANProtocol::makeNodeHealthPayload( - NODE_ID, INT8_MIN, NodeFaultCode::OVER_VOLTAGE); + NODE_ID, NodeKind::PDU, INT8_MIN, NodeFaultCode::OVER_VOLTAGE); check("no-temp + fault -> DEGRADED only", deg.flags == DEGRADED); d.println(pass ? F("=== ALL PASS ===") : F("=== FAIL ===")); diff --git a/Firmware/Tests/CANProtocol/tests/health_payload/health_payload.cpp b/Firmware/Tests/CANProtocol/tests/health_payload/health_payload.cpp index 5d89b804..18df15bc 100644 --- a/Firmware/Tests/CANProtocol/tests/health_payload/health_payload.cpp +++ b/Firmware/Tests/CANProtocol/tests/health_payload/health_payload.cpp @@ -1,8 +1,8 @@ -// CANProtocol — makeNodeHealthPayload + STM32Board internal-temp test (#213) +// CANProtocol — makeNodeHealthPayload + STM32Board internal-temp test (#213, nodeKind #344) // // Two parts: -// 1. Packing: makeNodeHealthPayload() copies nodeId/dieTempC through and zeroes flags + -// fault + reserved bytes (no NODE_OVERHEAT_C defined in this build). +// 1. Packing: makeNodeHealthPayload() copies nodeId/dieTempC through, puts the NodeKind in +// byte 5, and zeroes flags + fault + reserved bytes 6-7 (no NODE_OVERHEAT_C in this build). // 2. Live read: STM32Board::readDieTempC() returns a plausible value from the F103 internal // sensor (readVddMv() is read on-node for the temp calc; it is not transmitted). // UNCALIBRATED — this only sanity-checks the range, not accuracy. @@ -12,6 +12,8 @@ // dieTempC correct: PASS // flags == 0 (no trip): PASS // fault/reserved zeroed: PASS +// nodeKind in byte 5: PASS +// kind UNKNOWN packs 0: PASS // live temp in range: PASS (-40..125 °C, or sentinel if unavailable) // live Vdd in range: PASS (2700..3600 mV, or 0 if unavailable) // liveTemp=C liveVdd=mV @@ -25,10 +27,15 @@ void setup() { STM32Board::log("=== health_payload ==="); const int8_t kTemp = 37; - NodeHealthPayload p = CANProtocol::makeNodeHealthPayload(NODE_ID, kTemp); // healthy (fault defaults NONE) + // healthy (fault defaults NONE), panel-group kind + NodeHealthPayload p = CANProtocol::makeNodeHealthPayload(NODE_ID, NodeKind::PANEL_GROUP, kTemp); + const uint8_t* raw = reinterpret_cast(&p); // wire bytes, as sent on HEALTH_n // Faulted payload: faultId carries the code, DEGRADED flag derived, faultMask stays 0 (#163). - NodeHealthPayload pf = CANProtocol::makeNodeHealthPayload(NODE_ID, kTemp, NodeFaultCode::I2C_PERIPHERAL); + NodeHealthPayload pf = CANProtocol::makeNodeHealthPayload(NODE_ID, NodeKind::PDU, kTemp, + NodeFaultCode::I2C_PERIPHERAL); + // Pre-#344 equivalent: kind UNKNOWN is the 0 old firmware already sent in byte 5. + NodeHealthPayload pu = CANProtocol::makeNodeHealthPayload(NODE_ID, NodeKind::UNKNOWN, kTemp); int8_t liveTemp = STM32Board::readDieTempC(); uint16_t liveVdd = STM32Board::readVddMv(); @@ -42,7 +49,11 @@ void setup() { d.print(F("dieTempC correct: ")); d.println(p.dieTempC == kTemp ? F("PASS") : F("FAIL")); d.print(F("flags == 0 (no trip): ")); d.println(p.flags == 0 ? F("PASS") : F("FAIL")); d.print(F("fault/reserved zeroed: ")); - d.println((p.faultMask | p.faultId | p.rsvd[0] | p.rsvd[1] | p.rsvd[2]) == 0 ? F("PASS") : F("FAIL")); + d.println((p.faultMask | p.faultId | p.rsvd[0] | p.rsvd[1] | raw[6] | raw[7]) == 0 ? F("PASS") : F("FAIL")); + d.print(F("nodeKind in byte 5: ")); + d.println(raw[5] == (uint8_t)NodeKind::PANEL_GROUP && pf.nodeKind == (uint8_t)NodeKind::PDU + ? F("PASS") : F("FAIL")); + d.print(F("kind UNKNOWN packs 0: ")); d.println(pu.nodeKind == 0 ? F("PASS") : F("FAIL")); d.print(F("fault: faultId set: ")); d.println(pf.faultId == (uint8_t)NodeFaultCode::I2C_PERIPHERAL ? F("PASS") : F("FAIL")); d.print(F("fault: DEGRADED flag: ")); diff --git a/Firmware/Tests/CANProtocol/tests/node_status_bus/node_status_bus.cpp b/Firmware/Tests/CANProtocol/tests/node_status_bus/node_status_bus.cpp index 7e80afd1..2b7a3fce 100644 --- a/Firmware/Tests/CANProtocol/tests/node_status_bus/node_status_bus.cpp +++ b/Firmware/Tests/CANProtocol/tests/node_status_bus/node_status_bus.cpp @@ -8,8 +8,8 @@ // DiagSerial (115200) logs every status-LED state change with a timestamp, so the master-loss // watchdog can be checked from the log instead of by eye: // t= LED -// plus a summary line every 10 s: -// t= summary warnRaises= hb0Seen= tec= rec= +// plus a summary line every 10 s, with the fault + kind of the last HEALTH_1 sent: +// t= summary warnRaises= hb0Seen= tec= rec= fault= kind= // // Expected with an adapter that sends HB_0, then stops for a few seconds, then resumes: // - no WARNING before the first HB_0 @@ -17,6 +17,12 @@ // - LED back to NORMAL on the first HB_0 after the gap // - tec / rec stay 0 (frames are being ACKed) // Soak: leave HB_0 running; warnRaises must stay 0. +// +// Duplicate NODE_ID (#344): flash this env onto TWO boards on the same bus (both NODE_ID=1). Each +// receives the other's HB_1, so within ~1 s both log +// [NODE] degraded: another board transmits on this NODE_ID (fault 9) +// and report fault=9 (DUPLICATE_NODE_ID) in the summary. Unplug one: the other logs +// "[NODE] recovered" ~3 s later and its summary returns to fault=0. kind=1 (PANEL_GROUP) throughout. #include #include @@ -54,7 +60,7 @@ void setup() { void loop() { STM32Board::tick(); CANProtocol::drain(); - CANProtocol::serviceNodeStatus(0); + CANProtocol::serviceNodeStatus(NodeKind::PANEL_GROUP); auto& d = STM32Board::diagSerial(); const uint32_t now = millis(); @@ -74,6 +80,8 @@ void loop() { d.print(F(" summary warnRaises=")); d.print(_warnRaises); d.print(F(" hb0Seen=")); d.print(_hb0Seen); d.print(F(" tec=")); d.print(CANProtocol::tec()); - d.print(F(" rec=")); d.println(CANProtocol::rec()); + d.print(F(" rec=")); d.print(CANProtocol::rec()); + d.print(F(" fault=")); d.print(CANProtocol::lastNodeHealth().faultId); + d.print(F(" kind=")); d.println(CANProtocol::lastNodeHealth().nodeKind); } } diff --git a/Firmware/Tests/CANProtocol/tests/node_status_service/node_status_service.cpp b/Firmware/Tests/CANProtocol/tests/node_status_service/node_status_service.cpp index 780a5d41..b729185d 100644 --- a/Firmware/Tests/CANProtocol/tests/node_status_service/node_status_service.cpp +++ b/Firmware/Tests/CANProtocol/tests/node_status_service/node_status_service.cpp @@ -94,7 +94,7 @@ void setup() { void loop() { STM32Board::tick(); CANProtocol::drain(); - CANProtocol::serviceNodeStatus(0); + CANProtocol::serviceNodeStatus(NodeKind::PANEL_GROUP); const uint32_t t = millis() - _startMs; diff --git a/Firmware/Tests/PanelBridge/tests/node_status/node_status.cpp b/Firmware/Tests/PanelBridge/tests/node_status/node_status.cpp index 0d982e73..28ce467d 100644 --- a/Firmware/Tests/PanelBridge/tests/node_status/node_status.cpp +++ b/Firmware/Tests/PanelBridge/tests/node_status/node_status.cpp @@ -1,9 +1,10 @@ -// PanelBridge — node_status test (#86) +// PanelBridge — node_status test (#86, v3 #344) // -// Purpose: verify node-status reporting. PanelBridge surfaces connected PanelGroup -// nodes + health to the host as `_NODE_STATUS` DCS-BIOS command messages, emitted on: +// Purpose: verify node-status reporting. PanelBridge surfaces connected nodes + health to the +// host as `_NODE_STATUS` DCS-BIOS command messages (proto v3, 28 hex), emitted on: // - node alive (dead/unseen → alive transition), -// - host request (full roster), and +// - a node's board kind changing (its first HEALTH_n after the alive delta) (#344), +// - host request (full roster: the bridge's own node-0 row first), and // - node removal (heartbeat timeout, present=00, cached health retained). // Exercises the PANELBRIDGE_NODE_STATUS path via PANELBRIDGE_TEST hooks — no CAN // bus or second board required (heartbeats are injected directly). @@ -17,33 +18,41 @@ // 1. pio run -e test_node_status -t upload // 2. Open USB-UART @ 250000 (the _NODE_STATUS messages) and DiagSerial @ 115200 (phase log). // -// Also at boot: _NODE_STATUS_END 0 (empty roster seed). +// Also at boot (before the bridge's first HEALTH_0, so its row reads not-yet-seen 80 / kind 00): +// _NODE_STATUS 0001ffuuuu0000eeee8000000000 +// _NODE_STATUS_END 1 // // Expected on USB-UART (250000), repeating each cycle. Node 1 is fed a DEGRADED HEALTH frame -// (42 °C → dieTempC 2A; hFlags 02 DEGRADED; faultId 01 I2C_PERIPHERAL; faultMask 00 reserved) — -// this proves the #163 degraded fields forward through the cache into _NODE_STATUS. Node 2 gets no -// HEALTH, so its temp stays at the not-yet-seen sentinel 80 with all-zero health fields: +// before it goes alive (42 °C → dieTempC 2A; hFlags 02 DEGRADED; faultMask 00 reserved; +// faultId 01 I2C_PERIPHERAL; nodeKind 01 PANEL_GROUP) — this proves the #163 degraded fields and +// the #344 kind forward through the cache into _NODE_STATUS. Node 2 goes alive with no HEALTH +// (temp 80, kind 00 — not yet seen), then gets its first HEALTH (35 °C, kind 02 PDU), which pushes +// one delta: // Phase A — two nodes go alive (bare delta emits, no terminator): -// _NODE_STATUS 010100000A001200002A020001 -// _NODE_STATUS 02010200140034000280000000 -// Phase B — host request (full roster, terminated): -// _NODE_STATUS 010100000A001200002A020001 -// _NODE_STATUS 02010200140034000280000000 -// _NODE_STATUS_END 2 +// _NODE_STATUS 010100000A001200002A02000101 +// _NODE_STATUS 0201020014003400028000000000 +// Phase B — node 2's first HEALTH (kind 00 → 02 delta), then host request (full roster, +// node-0 row first, terminated; count includes node 0): +// _NODE_STATUS 0201020014003400022300000002 +// _NODE_STATUS 0001ffuuuu0000eeeett00000003 +// _NODE_STATUS 010100000A001200002A02000101 +// _NODE_STATUS 0201020014003400022300000002 +// _NODE_STATUS_END 3 // Phase C — heartbeat timeout (~3 s after Phase A) removes both (present=00 deltas, -// cached health retained, incl. node 1's degraded fields): -// _NODE_STATUS 010000000A001200002A020001 -// _NODE_STATUS 02000200140034000280000000 +// cached health retained, incl. node 1's degraded fields and both kinds): +// _NODE_STATUS 010000000A001200002A02000101 +// _NODE_STATUS 0200020014003400022300000002 // // hex = nodeId(2) present(2) flags(2) uptime(4) rxCount(4) esr(4) dieTempC(2) hFlags(2) -// faultMask(2) faultId(2); each field a fixed-width hex number (MSN first). -// dieTempC int8 two's-complement (80 = unseen). +// faultMask(2) faultId(2) nodeKind(2); each field a fixed-width hex number (MSN first). +// dieTempC int8 two's-complement (80 = unseen). Node-0 row: ff = the bridge's own CAN flags, +// uuuu = its uptime in s, eeee = its ESR, tt = its die temp, kind 03 BRIDGE — live values. #define DCSBIOS_DEFAULT_SERIAL #include #include #include -#include // NodeHealthFlag / NodeFaultCode for the degraded HEALTH injection (#163) +#include // NodeHealthFlag / NodeFaultCode / NodeKind for the HEALTH injections (#163/#344) static uint8_t _phase = 0; static uint32_t _lastMs = 0; @@ -69,16 +78,21 @@ void loop() { switch (_phase) { case 0: // Phase A — inject heartbeats; both nodes transition to alive d.println(F("[TEST] A: feed HB node 1 & 2 + DEGRADED HEALTH node 1 -> expect 2x _NODE_STATUS ...01...")); - // dieTempC 2A + DEGRADED/I2C_PERIPHERAL — cache before the alive edge emits (#163 forwarding) + // Node 2 back to not-yet-seen (it is dead here, so no delta), so every cycle repeats. + PanelBridge::testFeedHealth(2, INT8_MIN); + // dieTempC 2A + DEGRADED/I2C_PERIPHERAL + PANEL_GROUP — cache before the alive edge emits PanelBridge::testFeedHealth(1, 42, (uint8_t)NodeHealthFlag::DEGRADED, 0, - (uint8_t)NodeFaultCode::I2C_PERIPHERAL); + (uint8_t)NodeFaultCode::I2C_PERIPHERAL, + (uint8_t)NodeKind::PANEL_GROUP); PanelBridge::testFeedHeartbeat(1, 0x00, 10, 18, 0x0000); PanelBridge::testFeedHeartbeat(2, 0x02, 20, 52, 0x0002); // node 2: no HEALTH → temp 80 _phase = 1; break; case 1: // Phase B — host roster request - d.println(F("[TEST] B: request -> expect 2x _NODE_STATUS roster")); + d.println(F("[TEST] B: node 2 first HEALTH (kind PDU) -> expect 1 delta")); + PanelBridge::testFeedHealth(2, 35, 0, 0, 0, (uint8_t)NodeKind::PDU); + d.println(F("[TEST] B: request -> expect node-0 row + 2x _NODE_STATUS + _NODE_STATUS_END 3")); PanelBridge::testRequestNodeStatus(); d.println(F("[TEST] C: stop feeding -> ~3 s timeout removes both (present=00)")); _phase = 2; diff --git a/Firmware/Tests/PanelGroup/tests/heartbeat/heartbeat.cpp b/Firmware/Tests/PanelGroup/tests/heartbeat/heartbeat.cpp index 45154a7a..4e558d9c 100644 --- a/Firmware/Tests/PanelGroup/tests/heartbeat/heartbeat.cpp +++ b/Firmware/Tests/PanelGroup/tests/heartbeat/heartbeat.cpp @@ -47,7 +47,7 @@ void loop() { // Same order as PanelGroup::loop(): drain, then the node status service CANProtocol::drain(); - CANProtocol::serviceNodeStatus(0); + CANProtocol::serviceNodeStatus(NodeKind::PANEL_GROUP); if (now - setupDoneMs < 1200) return; diff --git a/docs/architecture/can-bus.md b/docs/architecture/can-bus.md index 7e4d5305..e622e65e 100644 --- a/docs/architecture/can-bus.md +++ b/docs/architecture/can-bus.md @@ -45,8 +45,14 @@ helper functions — **never hard-code these values**, always use the helper. - `HB_n` is a 500 ms heartbeat with CAN health (TEC/REC, flags, uptime). Every node — PanelBridge included, as `HB_0` — sends `HB_n` and `HEALTH_n` from one shared CANProtocol routine, which also runs each node's watch on the master heartbeat. -- `HEALTH_n` carries `NodeHealthPayload` — internal die temperature, and node - degraded/fault state as that lands. +- `HEALTH_n` carries `NodeHealthPayload` — internal die temperature, the node's + degraded/fault state, and its **board kind** (panel group, PDU or bridge). The kind comes from + the board's own firmware, never from its `NODE_ID`, so the host app can tell what a newly seen + node is before anyone adopts it. PanelBridge reports every node's kind to the host, its own + included. +- Each node also listens on its own heartbeat ID. A board never hears its own frames, so a + heartbeat there means a second board was flashed with the same `NODE_ID`; the node then flags a + duplicate-ID fault in `HEALTH_n`, which the host app shows. - `TELEM_n` carries paged telemetry: byte 0 says which page the other seven bytes are. The power-distribution node uses it for per-rail voltage and current and its board temperature, one frame a second plus an immediate frame whenever a rail condition changes. PanelBridge passes every diff --git a/docs/architecture/firmware-tiers.md b/docs/architecture/firmware-tiers.md index 657e149b..ad332c8e 100644 --- a/docs/architecture/firmware-tiers.md +++ b/docs/architecture/firmware-tiers.md @@ -69,10 +69,11 @@ The CAN master, and the only tier that runs the DCS-BIOS library. It receives th stream from SimGateway over UART, fires the DCS-BIOS export listener, and broadcasts every DCS output onto the CAN bus as `CTRL_BCAST` frames. In the other direction it collects input events (`EVT_n`) from the nodes and routes each one: DCS-BIOS inputs go out as ASCII commands -over UART; HID inputs are wrapped in HID frames for SimGateway. It also tracks which PanelGroup -nodes are alive (from their heartbeats) and — behind a build flag — reports that roster + health -to the host over DCS-BIOS itself (`_NODE_STATUS` messages; request address `0x86FE`), so the client -can show connected panels without any SimGateway change +over UART; HID inputs are wrapped in HID frames for SimGateway. It also tracks which nodes are +alive (from their heartbeats) and — behind a build flag — reports that roster to the host over +DCS-BIOS itself (`_NODE_STATUS` messages; request address `0x86FE`): each node's health and board +kind, plus a row for the bridge itself. The client can show connected boards without any +SimGateway change ([#86](https://github.com/OpenSkyHawk/OpenSkyhawk/issues/86)). **What it does *not* do:** diff --git a/docs/firmware/build-flags.md b/docs/firmware/build-flags.md index 20048a39..5504e5b5 100644 --- a/docs/firmware/build-flags.md +++ b/docs/firmware/build-flags.md @@ -60,10 +60,10 @@ actually does. | Flag | Default | Set where | Read at | |---|---|---|---| -| `NODE_ID` | none — omitting it fails to compile | every STM32 env that links our libraries; `WiringCheck` omits it deliberately | `CANProtocol.cpp:217`, `PanelGroup.cpp:300`, `STM32Board.h:29` | +| `NODE_ID` | none — omitting it fails to compile | every STM32 env that links our libraries; `WiringCheck` omits it deliberately | `CANProtocol.cpp:225`, `PanelGroup.cpp:300`, `STM32Board.h:29` | | `PANELBRIDGE_NODE_STATUS` | off | on in `Templates/PanelBridge` | `PanelBridge.cpp:41` + ~10 more | -| `NODE_HEALTH_TELEM` | **on** | never set | `CANProtocol.cpp:464` (`serviceNodeStatus`, every node) | -| `NODE_OVERHEAT_C` | **unset — by design** | bench envs only | `CANProtocol.cpp:428`, `CANProtocol.cpp:472` | +| `NODE_HEALTH_TELEM` | **on** | never set | `CANProtocol.cpp:495` (`serviceNodeStatus`, every node) | +| `NODE_OVERHEAT_C` | **unset — by design** | bench envs only | `CANProtocol.cpp:449`, `CANProtocol.cpp:512` | | `FORCE_CLOCK_FALLBACK` | off | `Tests/STM32Board` env `test_adc_clock_fallback` | `STM32Board.cpp:179` | | `PINREF_DEBUG` | off | never set | `PinRef.cpp:8` + ~9 more | | `SHIFTBUS_SCK` | `PB3` | never set | `ShiftBus.cpp:13` | @@ -76,8 +76,9 @@ actually does. **What they do, briefly.** `NODE_ID` is the board's CAN address — see [NODE_ID & CAN Addressing](node-id.md). `PANELBRIDGE_NODE_STATUS` makes PanelBridge report which -PanelGroup nodes it can see, up to the host over DCS-BIOS. `NODE_HEALTH_TELEM` is the periodic -health frame (die temperature, fault bitmap) at half the heartbeat rate. +nodes it can see — with their health and board kind, and itself — up to the host over DCS-BIOS. +`NODE_HEALTH_TELEM` is the periodic health frame (die temperature, fault, board kind) at half the +heartbeat rate. `FORCE_CLOCK_FALLBACK` exercises the dead-crystal fault path without a dead crystal; the `test_adc_clock_fallback` env uses it to prove `SystemClock_Config` sets the ADC prescaler on its fault path as well as its verified-72 MHz path (#263). diff --git a/docs/firmware/node-id.md b/docs/firmware/node-id.md index a02e9a34..b6f0e617 100644 --- a/docs/firmware/node-id.md +++ b/docs/firmware/node-id.md @@ -46,3 +46,13 @@ for the full workflow — claiming the ID is step one. `Firmware/Panels/` against this registry on each firmware PR: it fails the build if a board's NODE_ID is unregistered, duplicated, or out of range. The table above is transcluded directly from `Firmware/NODE_IDS.md`, so the docs can't drift from the registry. + +!!! warning "Two boards on one NODE_ID" + CI only checks the sketches in the repo. A board flashed by hand with an ID that is already on + the bus is caught at runtime instead: every node listens for its own heartbeat ID, and the + moment another board transmits on it, both report a **duplicate NODE_ID** fault, which the host + app shows against that node. Re-flash one of them with its own registered ID; the fault clears + a few seconds after the other board stops transmitting. + +A NODE_ID is an address, not a board type — a PDU can take any free ID. Each board announces what +it is (panel group, PDU or bridge) in its health frame, from its own firmware. From 814656e0277cf3ba2d3461ba76dfad994737a266 Mon Sep 17 00:00:00 2001 From: Motti Horesh Date: Fri, 16 Oct 2026 10:17:04 -0400 Subject: [PATCH 2/3] test(firmware): node_telem expects the node-0 row in the roster reply (#344) --- Firmware/Tests/PanelBridge/tests/node_telem/node_telem.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Firmware/Tests/PanelBridge/tests/node_telem/node_telem.cpp b/Firmware/Tests/PanelBridge/tests/node_telem/node_telem.cpp index 416b4c97..ac77028b 100644 --- a/Firmware/Tests/PanelBridge/tests/node_telem/node_telem.cpp +++ b/Firmware/Tests/PanelBridge/tests/node_telem/node_telem.cpp @@ -13,7 +13,9 @@ // _NODE_TELEM 030100E02E182EBC02 ← node 3 PDU_RAIL_12V: 12000 mV avg, 11800 min, 700 mA // _NODE_TELEM 050303FA0010273412 ← node 5 PDU_STATUS: flags 03, 25.0 °C, peaks 10000 / 4660 mA // (nothing for the page-0 frame, the 4-byte frame, or canIdTelem(0)) -// _NODE_STATUS_END 0 ← roster request: TELEM made no node alive +// _NODE_STATUS 0001…03 ← roster request: the bridge's own node-0 row (#344; kind +// 03, or 00 before its first HEALTH_0) +// _NODE_STATUS_END 1 ← node 0 only: TELEM made no CAN node alive // // hex = nodeId(2) then the 8 frame bytes in wire order (multi-byte page fields little-endian). @@ -54,7 +56,7 @@ void loop() { PanelBridge::testFeedCanFrame(canIdTelem(3), page0, 8); PanelBridge::testFeedCanFrame(canIdTelem(3), rail12, 4); PanelBridge::testFeedCanFrame(canIdTelem(0), rail12, 8); - d.println(F("[TEST] roster request -> expect _NODE_STATUS_END 0 (TELEM is not liveness)")); + d.println(F("[TEST] roster request -> expect node-0 row + _NODE_STATUS_END 1 (TELEM is not liveness)")); PanelBridge::testRequestNodeStatus(); d.println(F("[TEST] --- cycle ---")); } From ca1691a37b3304aa492f818da583390df7739e68 Mon Sep 17 00:00:00 2001 From: Motti Horesh Date: Fri, 16 Oct 2026 10:18:02 -0400 Subject: [PATCH 3/3] docs(firmware): finish the duplicate-NODE_ID test descriptions in the CANProtocol TechSpec (#344) --- Firmware/ScratchPad/TechSpec/CANProtocol.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Firmware/ScratchPad/TechSpec/CANProtocol.md b/Firmware/ScratchPad/TechSpec/CANProtocol.md index f2bab8a1..890c08f4 100644 --- a/Firmware/ScratchPad/TechSpec/CANProtocol.md +++ b/Firmware/ScratchPad/TechSpec/CANProtocol.md @@ -87,11 +87,14 @@ Firmware/Tests/CANProtocol/ ├── duplicate_node_id.cpp — duplicate NODE_ID check (#344), loopback: own looped-back HBs never │ trip it; extra HB_n frames the test sends raise DUPLICATE_NODE_ID + │ DEGRADED in lastNodeHealth(), held while they flow, clear ~3 s after + │ they stop └── node_status_bus.cpp — the same service on a physical bus against a master (PanelBridge or a USB-CAN adapter sending HB_0 every 500 ms): DiagSerial logs every LED state change + a 10 s summary (warnRaises / hb0Seen / TEC / REC / fault / kind) for watchdog timing, flapping-master and link-drop runs, - and soaks; on two boards with the same NODE_ID, the duplicate bench + and soaks; flashed on two boards with the same NODE_ID, it is also + the duplicate-NODE_ID bench (fault=9, then fault=0 once one board is + unplugged) ``` All runtime scenarios use `startLoopback()` — single board, no second node or physical bus