Skip to content

refactor(firmware): name the comment-only protocol values so the client sync can read them (#346) #347

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 1 commit into from
Oct 16, 2026
Merged

refactor(firmware): name the comment-only protocol values so the client sync can read them (#346) #347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 12 additions & 10 deletions Firmware/Libraries/CANProtocol/CANProtocol.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ static bool _warnAsserted = false; // what this service last w
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 } };
// Last HEALTH_n built (lastNodeHealth()) — not-yet-seen until the first one.
static NodeHealthPayload _lastHealth = { 0, DIE_TEMP_UNAVAILABLE, 0, 0, 0, 0, { 0, 0 } };

// ── Private helpers ───────────────────────────────────────────────────────────

Expand Down Expand Up @@ -132,7 +132,7 @@ static void _applyFilters() {
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
if (NODE_ID != GATEWAY_NODE_ID) ids[count++] = canIdHb(GATEWAY_NODE_ID); // 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 @@ -301,7 +301,7 @@ void drain() {
_lastDupMs = millis();
_dupSeen = true;
}
} else if (NODE_ID != 0 && frame.canId == canIdHb(0)) {
} else if (NODE_ID != GATEWAY_NODE_ID && frame.canId == canIdHb(GATEWAY_NODE_ID)) {
// 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 @@ -433,10 +433,12 @@ HeartbeatPayload makeHeartbeatPayload(uint8_t nodeId, uint16_t rxCount) {
HeartbeatPayload p;
uint32_t esr = CAN1->ESR;
p.nodeId = nodeId;
p.flags = (uint8_t)(((esr >> 2) & 0x01) | (esr & 0x02)); // bit0=BOFF, bit1=EPVF
p.flags = (uint8_t)(((esr & CAN_ESR_BOFF) ? (uint8_t)HeartbeatFlag::BOFF : 0u)
| ((esr & CAN_ESR_EPVF) ? (uint8_t)HeartbeatFlag::EPVF : 0u));
p.uptime = (uint16_t)(millis() / 1000);
p.rxCount = rxCount;
p.esr = (uint16_t)(esr >> 16); // low byte=TEC, high byte=REC
p.esr = (uint16_t)((((esr >> 16) & 0xFFu) << HB_ESR_TEC_SHIFT) // ESR[23:16] = TEC
| (((esr >> 24) & 0xFFu) << HB_ESR_REC_SHIFT)); // ESR[31:24] = REC
return p;
}

Expand All @@ -449,7 +451,7 @@ NodeHealthPayload makeNodeHealthPayload(uint8_t nodeId, NodeKind kind, int8_t di
#ifdef NODE_OVERHEAT_C
// Overheat trip is opt-in: the internal sensor is uncalibrated, so a sane threshold
// needs field data first. Only compute the flag when a build defines NODE_OVERHEAT_C.
if (dieTempC != INT8_MIN && dieTempC >= (int)(NODE_OVERHEAT_C))
if (dieTempC != DIE_TEMP_UNAVAILABLE && dieTempC >= (int)(NODE_OVERHEAT_C))
p.flags |= static_cast<uint8_t>(NodeHealthFlag::OVERHEAT); // bit0 — OR in, never overwrite
#endif
// Degraded fault (#163): faultId carries the code, DEGRADED bit is derived. faultMask stays 0
Expand Down Expand Up @@ -508,17 +510,17 @@ void serviceNodeStatus(NodeKind kind, uint16_t rxCount) {
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);
STM32Board::logNodeFaultEdge(NODE_ID == GATEWAY_NODE_ID ? "BRIDGE" : "NODE", fault, faultDetail);
#ifdef NODE_OVERHEAT_C
_overheat = (h.flags & static_cast<uint8_t>(NodeHealthFlag::OVERHEAT)) != 0;
// The bridge's latch is also PanelBridge's dead-node WARNING, which this service can't see —
// so on the bridge overheat stays raise-only rather than risk clearing that.
if (NODE_ID == 0 && _overheat) STM32Board::setWarning(true);
if (NODE_ID == GATEWAY_NODE_ID && _overheat) STM32Board::setWarning(true);
#endif
}
#endif

if (NODE_ID == 0) return; // PanelBridge is the master: no master-loss watchdog
if (NODE_ID == GATEWAY_NODE_ID) return; // PanelBridge is the master: no master-loss watchdog

// Master-loss watchdog. Signed elapsed so a stamp a hair ahead of `now` reads as fresh, never as
// overdue (#225).
Expand Down
18 changes: 9 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 @@ -49,10 +49,10 @@ struct __attribute__((packed)) ControlPacketPair {
*/
struct __attribute__((packed)) HeartbeatPayload {
uint8_t nodeId; ///< Node ID — redundant with CAN ID, aids logging
uint8_t flags; ///< bit0=BOFF, bit1=EPVF (Error Passive; TEC >= 128)
uint8_t flags; ///< HeartbeatFlag bits (NodeStatus.h): BOFF, EPVF
uint16_t uptime; ///< Seconds since boot, little-endian (wraps at ~18 h)
uint16_t rxCount; ///< Caller-owned accepted RX count, little-endian
uint16_t esr; ///< (CAN1->ESR >> 16): low byte=TEC, high byte=REC
uint16_t esr; ///< TEC at HB_ESR_TEC_SHIFT, REC at HB_ESR_REC_SHIFT (NodeStatus.h)
};

/**
Expand All @@ -79,7 +79,7 @@ struct __attribute__((packed)) HeartbeatPayload {
*/
struct __attribute__((packed)) NodeHealthPayload {
uint8_t nodeId; ///< 0: node ID — redundant with CAN ID, aids logging
int8_t dieTempC; ///< 1: internal die temp, whole °C (INT8_MIN = unavailable) — #213
int8_t dieTempC; ///< 1: internal die temp, whole °C (DIE_TEMP_UNAVAILABLE) — #213
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
Expand Down Expand Up @@ -121,7 +121,7 @@ static constexpr uint32_t CAN_ID_SYNC_REQ = 0x012; ///< Request all nodes to

// ── CAN ID functions (per-node IDs computed from NODE_ID) ─────────────────────

/** @brief Heartbeat frame ID for node n. Range 0x100-0x13F; n=0 is PanelBridge. */
/** @brief Heartbeat frame ID for node n. Range 0x100-0x13F; n=GATEWAY_NODE_ID (0) is PanelBridge. */
constexpr uint32_t canIdHb(uint8_t n) { return 0x100 + n; }

/** @brief Node-health/thermal telemetry frame ID for node n. Range 0x140-0x17F; n=0 is PanelBridge.
Expand Down Expand Up @@ -354,7 +354,7 @@ namespace CANProtocol {
*
* Fills uptime, CAN health flags, and ESR-derived TEC/REC from CANProtocol state.
*
* @param nodeId Node ID (1-63 PanelGroup; 0 reserved for PanelBridge).
* @param nodeId Node ID (1-63 PanelGroup; GATEWAY_NODE_ID for PanelBridge).
* @param rxCount Caller-owned receive counter to embed in the payload.
* @return Fully populated HeartbeatPayload ready to send as HB_n.
*/
Expand All @@ -370,9 +370,9 @@ namespace CANProtocol {
* 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 nodeId Node ID (1-63 PanelGroup; GATEWAY_NODE_ID 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 dieTempC Internal die temp in whole °C (DIE_TEMP_UNAVAILABLE = unavailable).
* @param fault Aggregated node fault (NodeFaultCode::NONE = healthy). Default NONE.
* @return Fully populated NodeHealthPayload ready to send as HEALTH_n.
*/
Expand Down Expand Up @@ -429,8 +429,8 @@ namespace CANProtocol {
* @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.
* (or with `-DNODE_HEALTH_TELEM=0`) it reads not-yet-seen: dieTempC DIE_TEMP_UNAVAILABLE,
* everything else 0 (nodeKind UNKNOWN) — the same rule the bridge applies to a node it has no HEALTH_n from.
*/
const NodeHealthPayload& lastNodeHealth();

Expand Down
115 changes: 106 additions & 9 deletions Firmware/Libraries/NodeStatus/NodeStatus.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,17 @@
*
* Owns the cross-node **status API** — not just health:
* - the `_NODE_STATUS` DCS-BIOS host-reporting contract (proto version, request address, message
* 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);
* names, `GATEWAY_NODE_ID`, row layout `NodeStatusField`) — **the canonical source the client's
* `sync-a4ec.ts` parses**;
* - `HeartbeatFlag` + the `HB_ESR_*` split (HB_n flags / esr fields);
* - `NodeHealthFlag` (HEALTH_n flag bits), `DIE_TEMP_UNAVAILABLE`, 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;
* `PduStatusFlag`, the per-page field offsets, `NTC_TEMP_UNAVAILABLE`) — #216;
*
* Every wire value is a named `enum class X : T { NAME = literal }` or `constexpr T NAME = literal`
* so the client sync can read it (#346) — never only a comment, a bare literal or a format string.
* - `FaultSource` — the interface a fault-producing object implements so a node-level aggregator
* can roll it up. A `FaultSource` is *one piece* of node status, not the whole story.
*
Expand All @@ -34,6 +39,40 @@
#define NODE_STATUS_MSG_NAME "_NODE_STATUS"
#define NODE_STATUS_END_MSG_NAME "_NODE_STATUS_END"

/**
* @brief Node ID of the Gateway board's PanelBridge MCU — the CAN master.
*
* It sends HB_0 / HEALTH_0, and its own row leads every `_NODE_STATUS` roster reply (#344). CAN
* nodes take 1–63 (Firmware/NODE_IDS.md).
*/
constexpr uint8_t GATEWAY_NODE_ID = 0;

/// `_NODE_STATUS` row length in bytes, sent as twice as many hex chars (proto v3: 14 → 28 hex).
constexpr uint8_t NODE_STATUS_ROW_BYTES = 14;

/**
* @brief `_NODE_STATUS` row field offsets, in bytes of the decoded row (hex index = 2 × offset).
*
* Each field is its numeric value as fixed-width uppercase hex, most-significant byte first, so
* the u16 fields read big-endian here although they are little-endian on CAN. A field runs to the
* next offset; the last runs to NODE_STATUS_ROW_BYTES. The first six come from the node's HB_n
* (plus `present`), the last five from its HEALTH_n. (Not `NODE_ID`: every node build defines
* that as a macro.)
*/
enum class NodeStatusField : uint8_t {
ID = 0, // u8: node ID; GATEWAY_NODE_ID = the bridge's own row, 1-63 = CAN nodes
PRESENT = 1, // u8: 01 alive, 00 removed
HB_FLAGS = 2, // u8: HeartbeatFlag bits
UPTIME = 3, // u16: seconds since boot, wraps at 65535
RX_COUNT = 5, // u16: node-owned accepted RX count, wraps at 65535
ESR = 7, // u16: TEC at HB_ESR_TEC_SHIFT, REC at HB_ESR_REC_SHIFT
DIE_TEMP_C = 9, // i8: whole degC, DIE_TEMP_UNAVAILABLE = not seen
HEALTH_FLAGS = 10, // u8: NodeHealthFlag bits
FAULT_MASK = 11, // u8: reserved, 00
FAULT_ID = 12, // u8: NodeFaultCode
NODE_KIND = 13, // u8: NodeKind
};

// ── _NODE_TELEM DCS-BIOS host-reporting contract (#216) ───────────────────────
// PanelBridge forwards every TELEM_n CAN frame to the host unread, as one DCS-BIOS message per
// frame: `_NODE_TELEM <nodeId(2 hex)><8 frame bytes as 16 hex, wire order>` (18 hex chars). Pushed
Expand All @@ -43,12 +82,41 @@
#define NODE_TELEM_PROTO_VERSION 1
#define NODE_TELEM_MSG_NAME "_NODE_TELEM"

/// `_NODE_TELEM` row length in bytes, sent as twice as many hex chars (9 → 18 hex).
constexpr uint8_t NODE_TELEM_ROW_BYTES = 9;

/** @brief `_NODE_TELEM` row field offsets, in bytes of the decoded row (hex index = 2 × offset). */
enum class NodeTelemField : uint8_t {
ID = 0, // u8: sending node ID (from the CAN ID)
FRAME = 1, // 8 bytes: the TELEM_n frame unread, wire order (TelemPage at FRAME + 0)
};

/** @brief HB_n `flags` bits — CAN controller error state (maps to HeartbeatPayload.flags). */
enum class HeartbeatFlag : uint8_t {
BOFF = 0x01, // CAN controller is bus-off
EPVF = 0x02, // error passive: TEC or REC >= 128
};

/// HB_n `esr` field: bit position of the transmit error counter (TEC, low byte).
constexpr uint8_t HB_ESR_TEC_SHIFT = 0;
/// HB_n `esr` field: bit position of the receive error counter (REC, high byte).
constexpr uint8_t HB_ESR_REC_SHIFT = 8;

/** @brief HEALTH_n `flags` bits — node-level health conditions (maps to NodeHealthPayload.flags). */
enum class NodeHealthFlag : uint8_t {
OVERHEAT = 0x01, // die temp >= NODE_OVERHEAT_C (opt-in, #213)
DEGRADED = 0x02, // a FaultSource reports faultCode() != NodeFaultCode::NONE (#163)
};

/**
* @brief HEALTH_n `dieTempC` meaning "unavailable / not yet seen" (`80` on the wire).
*
* Sent when the die sensor can't be read, and cached by PanelBridge for a node whose first
* HEALTH_n hasn't arrived. Written as a literal so the client sync can read it; equal to INT8_MIN.
*/
constexpr int8_t DIE_TEMP_UNAVAILABLE = -128;
static_assert(DIE_TEMP_UNAVAILABLE == INT8_MIN, "DIE_TEMP_UNAVAILABLE is the int8 minimum");

/**
* @brief HEALTH_n `faultId` dictionary (#163) — cross-node fault codes.
*
Expand Down Expand Up @@ -90,11 +158,8 @@ enum class NodeKind : uint8_t {
* @brief TELEM_n page ids (#216) — byte 0 of TelemPayload, naming the layout of bytes 1-7.
*
* Global and append-only: a page id means the same layout whichever node sends it, so the client
* decodes by page, not by node type. 0x00 is invalid. Layouts (little-endian):
* PDU_RAIL_12V / PDU_RAIL_5V: [1] PduRailFlag bits · [2-3] vAvgMv u16 · [4-5] vMinMv u16 ·
* [6-7] iAvgMa int16
* PDU_STATUS: [1] PduStatusFlag bits · [2-3] ntcTempDC int16 (0.1 °C,
* INT16_MIN = unavailable) · [4-5] i12PeakMa u16 · [6-7] i5PeakMa u16
* decodes by page, not by node type. 0x00 is invalid. Each page's layout is its field-offset enum
* below (PDU_RAIL_* → PduRailField, PDU_STATUS → PduStatusField); multi-byte fields little-endian.
*/
enum class TelemPage : uint8_t {
INVALID = 0x00, // never sent
Expand All @@ -121,6 +186,38 @@ enum class PduStatusFlag : uint8_t {
NTC_FAULT = 0x10, // NTC channel open or shorted
};

/**
* @brief PDU_RAIL_12V / PDU_RAIL_5V field offsets, in bytes of the 8-byte TELEM_n frame.
*
* Byte 0 is the TelemPage; a field runs to the next offset, the last to byte 7. Little-endian.
*/
enum class PduRailField : uint8_t {
FLAGS = 1, // u8: PduRailFlag bits
V_AVG_MV = 2, // u16: window-average rail voltage, mV
V_MIN_MV = 4, // u16: window-minimum rail voltage, mV
I_AVG_MA = 6, // int16: window-average rail current, mA (signed: zero offset stays visible)
};

/**
* @brief PDU_STATUS field offsets, in bytes of the 8-byte TELEM_n frame.
*
* Byte 0 is the TelemPage; a field runs to the next offset, the last to byte 7. Little-endian.
*/
enum class PduStatusField : uint8_t {
FLAGS = 1, // u8: PduStatusFlag bits
NTC_TEMP_DC = 2, // int16: board temperature, 0.1 degC (NTC_TEMP_UNAVAILABLE = no reading)
I12_PEAK_MA = 4, // u16: 12 V rail peak current since the last page, mA
I5_PEAK_MA = 6, // u16: 5 V rail peak current since the last page, mA
};

/**
* @brief PDU_STATUS `ntcTempDC` meaning "unavailable" — no reading yet, or the NTC is open/shorted.
*
* `0x8000` on the wire. Written as a literal so the client sync can read it; equal to INT16_MIN.
*/
constexpr int16_t NTC_TEMP_UNAVAILABLE = -32768;
static_assert(NTC_TEMP_UNAVAILABLE == INT16_MIN, "NTC_TEMP_UNAVAILABLE is the int16 minimum");

namespace OpenSkyhawk {

/**
Expand Down
Loading
Loading