Compare commits

..

2 Commits

Author SHA1 Message Date
Jason 5b84af68f6 fix(mcu): add FAULT_ACK command to clear system_emergency_state via USB (closes #83)
The volatile fix in the companion PR (#118) makes the safe-mode blink loop
escapable in principle, but no firmware path existed to actually clear
system_emergency_state at runtime — hardware reset was the only exit, which
fires the IWDG and re-energises the PA rails that Emergency_Stop() cut.

This change adds a FAULT_ACK command (opcode 0x40): the host sends an exact
4-byte CDC packet [0x40, 0x00, 0x00, 0x00]; USBHandler detects it regardless
of USB state and sets fault_ack_received; the blink loop checks the flag each
250 ms iteration and clears system_emergency_state, allowing a controlled
operator-acknowledged recovery without triggering a watchdog reset.

Detection is guarded to exact 4-byte packets only. Scanning larger packets
for the subsequence would false-trigger on the IEEE 754 big-endian encoding
of 2.0 (0x4000000000000000), which starts with the same 4 bytes and can
appear in normal settings doubles.

FAULT_ACK is excluded from the FPGA opcode enum to preserve the
Python/Verilog bidirectional contract test; contract_parser.py reads the
new MCU_ONLY_OPCODES frozenset in radar_protocol.py to filter it.

7 new test vectors in test_gap3_fault_ack_clears_emergency.c cover:
detection, loop exit, loop hold without ack, settings false-positive
immunity, truncated packet, wrong opcode, and multi-iteration sequence.

Reported-by: shaun0927 (Junghwan) <https://github.com/shaun0927>
2026-04-21 03:57:55 +05:45
Jason e979363730 fix(mcu): volatile emergency state + AGC holdoff zero-guard (closes #83)
Bug 1 (main.cpp:630): system_emergency_state lacked volatile. Under -O1+
the compiler is permitted to hoist the read outside the blink loop, making
while (system_emergency_state) unconditionally infinite. Once entered, the
only escape was the 4 s IWDG timeout — which resets the MCU and
re-energizes the PA rails that Emergency_Stop() explicitly cut. Marking the
variable volatile forces a memory read on every iteration so an external
clear (ISR, USB command, manual reset) can break the loop correctly.

Bug 2 (ADAR1000_AGC.cpp:59): holdoff_frames is a public uint8_t; if a
caller sets it to 0, the condition holdoff_counter >= holdoff_frames is
always true (any uint8_t >= 0), causing the AGC outer loop to increase gain
on every non-saturated frame with no holdoff delay. With alternating
sat/no-sat frames this produces a ±step oscillation that prevents the
receiver from settling. Fix: clamp holdoff_frames to a minimum of 1 in the
constructor, preserving all existing test assertions (none use 0; default
remains 4).

Reported-by: shaun0927 (Junghwan) <https://github.com/shaun0927>
2026-04-21 03:35:48 +05:45
8 changed files with 174 additions and 4 deletions
@@ -24,6 +24,7 @@ ADAR1000_AGC::ADAR1000_AGC()
, saturation_event_count(0)
{
memset(cal_offset, 0, sizeof(cal_offset));
if (holdoff_frames == 0) holdoff_frames = 1;
}
// ---------------------------------------------------------------------------
@@ -13,6 +13,7 @@ void USBHandler::reset() {
start_flag_received = false;
buffer_index = 0;
current_settings.resetToDefaults();
fault_ack_received = false;
}
void USBHandler::processUSBData(const uint8_t* data, uint32_t length) {
@@ -23,6 +24,18 @@ void USBHandler::processUSBData(const uint8_t* data, uint32_t length) {
DIAG("USB", "processUSBData: %lu bytes, state=%d", (unsigned long)length, (int)current_state);
// FAULT_ACK: host sends exactly 4 bytes [0x40, 0x00, 0x00, 0x00].
// Requires exact 4-byte packet length: settings packets are always
// >= 82 bytes, so a lone 4-byte payload is unambiguous. Scanning
// inside larger packets would false-trigger on the IEEE 754
// encoding of 2.0 (0x4000000000000000) embedded in settings doubles.
static const uint8_t FAULT_ACK_SEQ[4] = {0x40, 0x00, 0x00, 0x00};
if (length == 4 && memcmp(data, FAULT_ACK_SEQ, 4) == 0) {
fault_ack_received = true;
DIAG("USB", "FAULT_ACK received");
return;
}
switch (current_state) {
case USBState::WAITING_FOR_START:
processStartFlag(data, length);
@@ -29,6 +29,11 @@ public:
// Reset USB handler
void reset();
// Fault-acknowledgement: host sends FAULT_ACK (0x40) to clear
// system_emergency_state and exit the safe-mode blink loop.
bool isFaultAckReceived() const { return fault_ack_received; }
void clearFaultAck() { fault_ack_received = false; }
private:
RadarSettings current_settings;
USBState current_state;
@@ -38,6 +43,7 @@ private:
static constexpr uint32_t MAX_BUFFER_SIZE = 256;
uint8_t usb_buffer[MAX_BUFFER_SIZE];
uint32_t buffer_index;
bool fault_ack_received;
void processStartFlag(const uint8_t* data, uint32_t length);
void processSettingsData(const uint8_t* data, uint32_t length);
@@ -627,7 +627,7 @@ typedef enum {
static SystemError_t last_error = ERROR_NONE;
static uint32_t error_count = 0;
static bool system_emergency_state = false;
static volatile bool system_emergency_state = false;
// Error handler function
SystemError_t checkSystemHealth(void) {
@@ -2054,6 +2054,10 @@ int main(void)
HAL_GPIO_TogglePin(LED_3_GPIO_Port, LED_3_Pin);
HAL_GPIO_TogglePin(LED_4_GPIO_Port, LED_4_Pin);
HAL_Delay(250);
if (usbHandler.isFaultAckReceived()) {
system_emergency_state = false;
usbHandler.clearFaultAck();
}
}
DIAG("SYS", "Exited safe mode blink loop -- system_emergency_state cleared");
}
@@ -70,7 +70,8 @@ TESTS_STANDALONE := test_bug12_pa_cal_loop_inverted \
test_gap3_idq_periodic_reread \
test_gap3_emergency_state_ordering \
test_gap3_overtemp_emergency_stop \
test_gap3_health_watchdog_cold_start
test_gap3_health_watchdog_cold_start \
test_gap3_fault_ack_clears_emergency
# Tests that need platform_noos_stm32.o + mocks
TESTS_WITH_PLATFORM := test_bug11_platform_spi_transmit_only
@@ -178,6 +179,9 @@ test_gap3_overtemp_emergency_stop: test_gap3_overtemp_emergency_stop.c
test_gap3_health_watchdog_cold_start: test_gap3_health_watchdog_cold_start.c
$(CC) $(CFLAGS) $< -o $@
test_gap3_fault_ack_clears_emergency: test_gap3_fault_ack_clears_emergency.c
$(CC) $(CFLAGS) $< -o $@
# Tests that need platform_noos_stm32.o + mocks
$(TESTS_WITH_PLATFORM): %: %.c $(MOCK_OBJS) $(PLATFORM_OBJ)
$(CC) $(CFLAGS) $(INCLUDES) $< $(MOCK_OBJS) $(PLATFORM_OBJ) -o $@
@@ -0,0 +1,121 @@
/*******************************************************************************
* test_gap3_fault_ack_clears_emergency.c
*
* Verifies the FAULT_ACK clear path for system_emergency_state:
* - USBHandler detects exactly [0x40, 0x00, 0x00, 0x00] in a 4-byte packet
* - Detection is false-positive-free: larger packets (settings data) carrying
* the same bytes as a subsequence must NOT trigger the ack
* - Main-loop blink logic clears system_emergency_state on receipt
*
* Logic extracted from USBHandler.cpp + main.cpp to mirror the actual code
* paths without requiring HAL headers.
******************************************************************************/
#include <assert.h>
#include <stdio.h>
#include <stdbool.h>
#include <string.h>
#include <stdint.h>
/* ── Simulated USBHandler state ─────────────────────────────────────────── */
static bool fault_ack_received = false;
static volatile bool system_emergency_state = false;
static const uint8_t FAULT_ACK_SEQ[4] = {0x40, 0x00, 0x00, 0x00};
/* Mirrors USBHandler::processUSBData() detection logic */
static void sim_processUSBData(const uint8_t *data, uint32_t length)
{
if (data == NULL || length == 0) return;
if (length == 4 && memcmp(data, FAULT_ACK_SEQ, 4) == 0) {
fault_ack_received = true;
return;
}
/* (normal state machine omitted — not under test here) */
}
/* Mirrors one iteration of the blink loop in main.cpp */
static void sim_blink_iteration(void)
{
/* HAL_GPIO_TogglePin + HAL_Delay omitted */
if (fault_ack_received) {
system_emergency_state = false;
fault_ack_received = false;
}
}
int main(void)
{
printf("=== Gap-3 FAULT_ACK clears system_emergency_state ===\n");
/* Test 1: exact 4-byte FAULT_ACK packet sets the flag */
printf(" Test 1: exact FAULT_ACK packet detected... ");
fault_ack_received = false;
const uint8_t ack_pkt[4] = {0x40, 0x00, 0x00, 0x00};
sim_processUSBData(ack_pkt, 4);
assert(fault_ack_received == true);
printf("PASS\n");
/* Test 2: flag cleared and system_emergency_state exits blink loop */
printf(" Test 2: blink loop exits on FAULT_ACK... ");
system_emergency_state = true;
fault_ack_received = true;
sim_blink_iteration();
assert(system_emergency_state == false);
assert(fault_ack_received == false);
printf("PASS\n");
/* Test 3: blink loop does NOT exit without ack */
printf(" Test 3: blink loop holds without ack... ");
system_emergency_state = true;
fault_ack_received = false;
sim_blink_iteration();
assert(system_emergency_state == true);
printf("PASS\n");
/* Test 4: settings-sized packet carrying [0x40,0x00,0x00,0x00] as first
* 4 bytes does NOT trigger ack (IEEE 754 double 2.0 = 0x4000000000000000) */
printf(" Test 4: settings packet with 2.0 double does not false-trigger... ");
fault_ack_received = false;
uint8_t settings_pkt[82];
memset(settings_pkt, 0, sizeof(settings_pkt));
/* First 4 bytes look like FAULT_ACK but packet length is 82 */
settings_pkt[0] = 0x40; settings_pkt[1] = 0x00;
settings_pkt[2] = 0x00; settings_pkt[3] = 0x00;
sim_processUSBData(settings_pkt, sizeof(settings_pkt));
assert(fault_ack_received == false);
printf("PASS\n");
/* Test 5: 3-byte packet (truncated) does not trigger */
printf(" Test 5: truncated 3-byte packet ignored... ");
fault_ack_received = false;
const uint8_t short_pkt[3] = {0x40, 0x00, 0x00};
sim_processUSBData(short_pkt, 3);
assert(fault_ack_received == false);
printf("PASS\n");
/* Test 6: wrong opcode byte in 4-byte packet does not trigger */
printf(" Test 6: wrong opcode (0x28 AGC_ENABLE) not detected as FAULT_ACK... ");
fault_ack_received = false;
const uint8_t agc_pkt[4] = {0x28, 0x00, 0x00, 0x01};
sim_processUSBData(agc_pkt, 4);
assert(fault_ack_received == false);
printf("PASS\n");
/* Test 7: multiple blink iterations — loop stays active until ack */
printf(" Test 7: loop stays active across multiple iterations until ack... ");
system_emergency_state = true;
fault_ack_received = false;
sim_blink_iteration();
assert(system_emergency_state == true);
sim_blink_iteration();
assert(system_emergency_state == true);
/* Now ack arrives */
sim_processUSBData(ack_pkt, 4);
assert(fault_ack_received == true);
sim_blink_iteration();
assert(system_emergency_state == false);
printf("PASS\n");
printf("\n=== Gap-3 FAULT_ACK: ALL 7 TESTS PASSED ===\n\n");
return 0;
}
+9
View File
@@ -103,6 +103,15 @@ class Opcode(IntEnum):
STATUS_REQUEST = 0xFF
# MCU-only commands — NOT dispatched to the FPGA opcode switch.
# These values have no corresponding case in radar_system_top.v.
# Listed here so the GUI can build and send them via build_command().
# contract_parser.py filters MCU_ONLY_OPCODES out of the Python/Verilog
# bidirectional check.
FAULT_ACK = 0x40 # Exact 4-byte CDC packet; clears system_emergency_state
MCU_ONLY_OPCODES: frozenset[int] = frozenset({0x40})
# ============================================================================
# Data Structures
# ============================================================================
@@ -108,12 +108,23 @@ class ConcatWidth:
def parse_python_opcodes(filepath: Path | None = None) -> dict[int, OpcodeEntry]:
"""Parse the Opcode enum from radar_protocol.py.
Returns {opcode_value: OpcodeEntry}.
Returns {opcode_value: OpcodeEntry}, excluding MCU_ONLY_OPCODES.
MCU-only opcodes have no FPGA case statement and must not appear in
the bidirectional Python/Verilog contract check.
"""
if filepath is None:
filepath = GUI_DIR / "radar_protocol.py"
text = filepath.read_text()
# Extract MCU_ONLY_OPCODES set so we can exclude those values below.
mcu_only: set[int] = set()
m_set = re.search(r'MCU_ONLY_OPCODES[^=]*=\s*frozenset\(\{([^}]*)\}\)', text)
if m_set:
for tok in m_set.group(1).split(','):
tok = tok.strip()
if tok.startswith(('0x', '0X')):
mcu_only.add(int(tok, 16))
# Find the Opcode class body
match = re.search(r'class Opcode\b.*?(?=\nclass |\Z)', text, re.DOTALL)
if not match:
@@ -123,6 +134,7 @@ def parse_python_opcodes(filepath: Path | None = None) -> dict[int, OpcodeEntry]
for m in re.finditer(r'(\w+)\s*=\s*(0x[0-9a-fA-F]+)', match.group()):
name = m.group(1)
value = int(m.group(2), 16)
if value not in mcu_only:
opcodes[value] = OpcodeEntry(name=name, value=value)
return opcodes