Skip to content

feat(firmware): nodes announce their board kind in HEALTH_n and _NODE_STATUS v3 (#344) #345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mottihoresh merged 3 commits into from
Oct 16, 2026
Merged

feat(firmware): nodes announce their board kind in HEALTH_n and _NODE_STATUS v3 (#344) #345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dfe06ee
feat(firmware): nodes announce their board kind in HEALTH_n and _NODE…
mottihoresh Oct 16, 2026
814656e
test(firmware): node_telem expects the node-0 row in the roster reply…
mottihoresh Oct 16, 2026
ca1691a
docs(firmware): finish the duplicate-NODE_ID test descriptions in the…
mottihoresh Oct 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 47 additions & 7 deletions Firmware/Libraries/CANProtocol/CANProtocol.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 ───────────────────────────────────────────────────────────

Expand Down Expand Up @@ -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];
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand All @@ -436,13 +457,17 @@ NodeHealthPayload makeNodeHealthPayload(uint8_t nodeId, int8_t dieTempC, NodeFau
p.faultMask = 0;
p.faultId = static_cast<uint8_t>(fault);
if (fault != NodeFaultCode::NONE) p.flags |= static_cast<uint8_t>(NodeHealthFlag::DEGRADED); // bit1
p.rsvd[0] = p.rsvd[1] = p.rsvd[2] = 0;
p.nodeKind = static_cast<uint8_t>(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
Expand All @@ -457,16 +482,31 @@ void serviceNodeStatus(uint16_t rxCount) {
_lastHbMs = now;
HeartbeatPayload hb = makeHeartbeatPayload(NODE_ID, rxCount);
send(canIdHb(NODE_ID), reinterpret_cast<const uint8_t*>(&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<const uint8_t*>(&h), sizeof(h));
STM32Board::logNodeFaultEdge(NODE_ID == 0 ? "BRIDGE" : "NODE", fault, faultDetail);
#ifdef NODE_OVERHEAT_C
Expand Down
53 changes: 44 additions & 9 deletions Firmware/Libraries/CANProtocol/CANProtocol.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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).
Expand Down Expand Up @@ -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.
Expand All @@ -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.
*/
Expand All @@ -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
Expand Down Expand Up @@ -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. */
Expand All @@ -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`.
Expand All @@ -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
24 changes: 21 additions & 3 deletions Firmware/Libraries/NodeStatus/NodeStatus.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
Expand Down Expand Up @@ -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
};

/**
Expand Down
Loading
Loading