Merge pull request #119 from NawfalMotii79/fix/mcu-fault-ack-emergency-clear

fix(mcu): FAULT_ACK USB command clears system_emergency_state (closes #83)

9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/ADAR1000_AGC.cpp

@@ -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;
 }
 
 // ---------------------------------------------------------------------------

9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/ADAR1000_Manager.cpp

@@ -868,11 +868,22 @@ void ADAR1000Manager::adarSetRamBypass(uint8_t deviceIndex, uint8_t broadcast) {
 }
 
 void ADAR1000Manager::adarSetRxPhase(uint8_t deviceIndex, uint8_t channel, uint8_t phase, uint8_t broadcast) {
+    // channel is 1-based (CH1..CH4) per API contract documented in
+    // ADAR1000_AGC.cpp and matching ADI datasheet terminology.
+    // Reject out-of-range early so a stale 0-based caller does not
+    // silently wrap to ((0-1) & 0x03) == 3 and write to CH4.
+    // See issue #90.
+    if (channel < 1 || channel > 4) {
+        DIAG("BF", "adarSetRxPhase: channel %u out of range [1..4], ignored", channel);
+        return;
+    }
     uint8_t i_val = VM_I[phase % 128];
     uint8_t q_val = VM_Q[phase % 128];
 
-    uint32_t mem_addr_i = REG_CH1_RX_PHS_I + (channel & 0x03) * 2;
+    // Subtract 1 to convert 1-based channel to 0-based register offset
-    uint32_t mem_addr_q = REG_CH1_RX_PHS_Q + (channel & 0x03) * 2;
+    // before masking. See issue #90.
+    uint32_t mem_addr_i = REG_CH1_RX_PHS_I + ((channel - 1) & 0x03) * 2;
+    uint32_t mem_addr_q = REG_CH1_RX_PHS_Q + ((channel - 1) & 0x03) * 2;
 
     adarWrite(deviceIndex, mem_addr_i, i_val, broadcast);
     adarWrite(deviceIndex, mem_addr_q, q_val, broadcast);
@@ -880,11 +891,16 @@ void ADAR1000Manager::adarSetRxPhase(uint8_t deviceIndex, uint8_t channel, uint8
 }
 
 void ADAR1000Manager::adarSetTxPhase(uint8_t deviceIndex, uint8_t channel, uint8_t phase, uint8_t broadcast) {
+    // channel is 1-based (CH1..CH4). See issue #90.
+    if (channel < 1 || channel > 4) {
+        DIAG("BF", "adarSetTxPhase: channel %u out of range [1..4], ignored", channel);
+        return;
+    }
     uint8_t i_val = VM_I[phase % 128];
     uint8_t q_val = VM_Q[phase % 128];
 
-    uint32_t mem_addr_i = REG_CH1_TX_PHS_I + (channel & 0x03) * 2;
+    uint32_t mem_addr_i = REG_CH1_TX_PHS_I + ((channel - 1) & 0x03) * 2;
-    uint32_t mem_addr_q = REG_CH1_TX_PHS_Q + (channel & 0x03) * 2;
+    uint32_t mem_addr_q = REG_CH1_TX_PHS_Q + ((channel - 1) & 0x03) * 2;
 
     adarWrite(deviceIndex, mem_addr_i, i_val, broadcast);
     adarWrite(deviceIndex, mem_addr_q, q_val, broadcast);
@@ -892,13 +908,23 @@ void ADAR1000Manager::adarSetTxPhase(uint8_t deviceIndex, uint8_t channel, uint8
 }
 
 void ADAR1000Manager::adarSetRxVgaGain(uint8_t deviceIndex, uint8_t channel, uint8_t gain, uint8_t broadcast) {
-    uint32_t mem_addr = REG_CH1_RX_GAIN + (channel & 0x03);
+    // channel is 1-based (CH1..CH4). See issue #90.
+    if (channel < 1 || channel > 4) {
+        DIAG("BF", "adarSetRxVgaGain: channel %u out of range [1..4], ignored", channel);
+        return;
+    }
+    uint32_t mem_addr = REG_CH1_RX_GAIN + ((channel - 1) & 0x03);
     adarWrite(deviceIndex, mem_addr, gain, broadcast);
     adarWrite(deviceIndex, REG_LOAD_WORKING, 0x1, broadcast);
 }
 
 void ADAR1000Manager::adarSetTxVgaGain(uint8_t deviceIndex, uint8_t channel, uint8_t gain, uint8_t broadcast) {
-    uint32_t mem_addr = REG_CH1_TX_GAIN + (channel & 0x03);
+    // channel is 1-based (CH1..CH4). See issue #90.
+    if (channel < 1 || channel > 4) {
+        DIAG("BF", "adarSetTxVgaGain: channel %u out of range [1..4], ignored", channel);
+        return;
+    }
+    uint32_t mem_addr = REG_CH1_TX_GAIN + ((channel - 1) & 0x03);
     adarWrite(deviceIndex, mem_addr, gain, broadcast);
     adarWrite(deviceIndex, REG_LOAD_WORKING, LD_WRK_REGS_LDTX_OVERRIDE, broadcast);
 }

9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/USBHandler.cpp

@@ -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);

9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/USBHandler.h

@@ -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);

9_Firmware/9_1_Microcontroller/9_1_3_C_Cpp_Code/main.cpp

@@ -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");
 	    }

9_Firmware/9_1_Microcontroller/tests/Makefile

@@ -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 $@

9_Firmware/9_1_Microcontroller/tests/test_gap3_fault_ack_clears_emergency.c

@@ -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_Firmware/9_2_FPGA/cic_decimator_4x_enhanced.v

@@ -32,11 +32,50 @@ localparam COMB_WIDTH = 28;
 // adjacent DSP48E1 tiles — zero fabric delay, guaranteed to meet 400+ MHz
 // on 7-series regardless of speed grade.
 //
-// Active-high reset derived from reset_n (inverted).
+// Active-high reset derived from reset_n (inverted and REGISTERED).
 // CEP (clock enable for P register) gated by data_valid.
-// ============================================================================
+//
-
+// ----------------------------------------------------------------------------
-wire reset_h = ~reset_n;  // active-high reset for DSP48E1 RSTP
+// RESET FAN-OUT INVARIANT (Build N+1 fix for WNS=-0.626ns at 400 MHz):
+// ----------------------------------------------------------------------------
+// Previously this was a combinational wire (`wire reset_h = ~reset_n`). Vivado
+// collapsed all per-module inversions across the DDC hierarchy into a SINGLE
+// shared LUT1, whose output fanned out to 702 loads (DSP48E1 RSTP/RSTB/RSTC
+// plus FDRE R pins of all comb-stage DSP48E1s inferred via use_dsp="yes").
+// Route delay alone on that net was 2.019–2.268 ns — nearly one full 2.5 ns
+// period. Timing failed by 626 ps on the 400 MHz domain.
+//
+// Fix: convert reset_h to a REGISTERED signal with (* max_fanout = 50 *).
+// Vivado treats max_fanout on a REG (not a wire) as authoritative and
+// replicates the register into N copies, each placed near its ≈50 loads.
+// Invariants preserved:
+//   I1 (correctness):       reset_h is still active-high, equals ~reset_n
+//                           after one clk edge; CIC reset is a RECEIVER-side
+//                           synchronizer anyway (driven by reset_n_400m which
+//                           is already sync'd in the parent DDC), so adding
+//                           one more clk cycle of latency is safe.
+//   I2 (glitch-free):       Registered output => inherently glitch-free,
+//                           feeding DSP48E1 RST pins (which are synchronous
+//                           to CLK, so they capture on the same edge anyway).
+//   I3 (power-up safety):   reset_h is NOT async-reset itself. On power-up,
+//                           FDRE INIT=0 starts reset_h LOW. First clk edge
+//                           samples ~reset_n which is LOW on power-up (the
+//                           parent DDC holds reset_n_400m low until the 2-
+//                           stage synchronizer releases), so reset_h goes
+//                           HIGH on cycle 1 and all DSPs see reset during
+//                           the following cycles. System is held in reset
+//                           for enough cycles that any initial register
+//                           state garbage is overwritten. ✅
+//   I4 (reset de-assertion):reset_h goes LOW one cycle AFTER reset_n_400m
+//                           goes HIGH. Downstream DSPs come out of reset on
+//                           the next clk edge after that. Total latency
+//                           from system reset release to first valid sample:
+//                           2 (sync chain) + 1 (reset_h reg) + 1 (first
+//                           DSP output) = 4 cycles at 400 MHz = 10 ns.
+//                           Negligible vs system reset assertion duration.
+// ----------------------------------------------------------------------------
+(* max_fanout = 50 *) reg reset_h = 1'b1;  // INIT=1'b1: registers start in reset state on power-up
+always @(posedge clk) reset_h <= ~reset_n;
 
 // Sign-extended input for integrator_0 C port (48-bit)
 wire [ACC_WIDTH-1:0] data_in_c = {{(ACC_WIDTH-18){data_in[17]}}, data_in};
@@ -699,10 +738,11 @@ initial begin
 end
 
 // Decimation control + monitoring (integrators are now DSP48E1 instances)
-// Sync reset: enables FDRE inference for better timing at 400 MHz.
+// Sync reset via reset_h (registered, max_fanout=50) — eliminates the shared
-// Reset is already synchronous to clk via reset synchronizer in parent module.
+// LUT1 inverter that previously fanned out to all fabric FDRE R pins plus
+// DSP48E1 RST pins (702 loads total). See "RESET FAN-OUT INVARIANT" at top.
 always @(posedge clk) begin
-    if (!reset_n) begin
+    if (reset_h) begin
         integrator_sampled <= 0;
         decimation_counter <= 0;
         data_valid_delayed <= 0;
@@ -755,9 +795,9 @@ always @(posedge clk) begin
 end
 
 // Pipeline the valid signal for comb section
-// Sync reset: matches decimation control block reset style.
+// Sync reset via reset_h — same replicated-register source as DSP48E1 RSTs.
 always @(posedge clk) begin
-    if (!reset_n) begin
+    if (reset_h) begin
         data_valid_comb <= 0;
         data_valid_comb_pipe <= 0;
         data_valid_comb_0_out <= 0;
@@ -792,7 +832,7 @@ end
 //   - Each stage: comb[i] = comb[i-1] - comb_delay[i][last]
 
 always @(posedge clk) begin
-    if (!reset_n) begin
+    if (reset_h) begin
         for (i = 0; i < STAGES; i = i + 1) begin
             comb[i] <= 0;
             for (j = 0; j < COMB_DELAY; j = j + 1) begin

9_Firmware/9_2_FPGA/ddc_400m.v

@@ -53,46 +53,6 @@ reg [2:0] saturation_count;
 reg overflow_detected;
 reg [7:0] error_counter;
 
-// ============================================================================
-// 400 MHz Reset Synchronizer
-//
-// reset_n arrives from the 100 MHz domain (sys_reset_n from radar_system_top).
-// Using it directly as an async reset in the 400 MHz domain causes the reset
-// deassertion edge to violate timing: the 100 MHz flip-flop driving reset_n
-// has its output fanning out to 1156 registers across the FPGA in the 400 MHz
-// domain, requiring 18.243ns of routing (WNS = -18.081ns).
-//
-// Solution: 2-stage async-assert, sync-deassert reset synchronizer in the
-// 400 MHz domain. Reset assertion is immediate (asynchronous — combinatorial
-// path from reset_n to all 400 MHz registers). Reset deassertion is
-// synchronized to clk_400m rising edge, preventing metastability.
-//
-// All 400 MHz submodules (NCO, CIC, mixers, LFSR) use reset_n_400m.
-// All 100 MHz submodules (FIR, output stage) continue using reset_n directly
-// (already synchronized to 100 MHz at radar_system_top level).
-// ============================================================================
-(* ASYNC_REG = "TRUE" *) reg [1:0] reset_sync_400m;
-(* max_fanout = 50 *) wire reset_n_400m = reset_sync_400m[1];
-
-// Active-high reset for DSP48E1 RST ports (avoids LUT1 inverter fan-out)
-(* max_fanout = 50 *) reg reset_400m;
-
-always @(posedge clk_400m or negedge reset_n) begin
-    if (!reset_n) begin
-        reset_sync_400m <= 2'b00;
-        reset_400m      <= 1'b1;
-    end else begin
-        reset_sync_400m <= {reset_sync_400m[0], 1'b1};
-        reset_400m      <= ~reset_sync_400m[1];
-    end
-end
-
-// CDC synchronization for control signals (2-stage synchronizers)
-(* ASYNC_REG = "TRUE" *) reg [1:0] mixers_enable_sync_chain;
-(* ASYNC_REG = "TRUE" *) reg [1:0] force_saturation_sync_chain;
-wire mixers_enable_sync;
-wire force_saturation_sync;
-
 // Debug monitoring signals
 reg [31:0] sample_counter;
 wire signed [17:0] debug_mixed_i_trunc;
@@ -130,8 +90,6 @@ reg baseband_valid_reg;
 wire [7:0] phase_dither_bits;
 reg [31:0] phase_inc_dithered;
 
-
-
 // ============================================================================
 // Debug Signal Assignments
 // ============================================================================
@@ -142,13 +100,66 @@ assign debug_mixed_i_trunc = mixed_i[25:8];
 assign debug_mixed_q_trunc = mixed_q[25:8];
 
 // ============================================================================
-// Clock Domain Crossing for Control Signals (2-stage synchronizers)
+// 400 MHz Reset Synchronizer
+//
+// reset_n arrives from the 100 MHz domain (sys_reset_n from radar_system_top).
+// Using it directly as an async reset in the 400 MHz domain causes the reset
+// deassertion edge to violate timing: the 100 MHz flip-flop driving reset_n
+// has its output fanning out to 1156 registers across the FPGA in the 400 MHz
+// domain, requiring 18.243ns of routing (WNS = -18.081ns).
+//
+// Solution: 2-stage async-assert, sync-deassert reset synchronizer in the
+// 400 MHz domain. Reset assertion is immediate (asynchronous — combinatorial
+// path from reset_n to all 400 MHz registers). Reset deassertion is
+//
+// reset_400m   : ACTIVE-HIGH registered reset with (* max_fanout = 50 *).
+//                This is THE signal fed to every synchronous 400 MHz FDRE
+//                and every DSP48E1 RST pin in this module and its children
+//                (NCO, CIC, LFSR). Vivado replicates the register (~14
+//                copies) so each replica drives ≈50 loads regionally,
+//                eliminating the single-LUT1 / 702-load net that caused
+//                WNS=-0.626 ns in Build N.
+//
+// System-level invariants preserved:
+//   I1  Reset assertion propagates to all 400 MHz regs within ≤3 clk edges
+//       (2 sync + 1 replicated-reg fanout).  At 400 MHz = 7.5 ns << any
+//       system-level reset assertion duration.
+//   I2  Reset de-assertion is always synchronous to clk_400m (via
+//       reset_sync_400m), never glitches.
+//   I3  DSP48E1 RST pins are all fed from Q of a register — glitch-free.
+//   I4  No new CDC introduced: reset_400m is entirely in clk_400m domain.
+//   I5  Power-up: reset_n is asserted externally and mmcm_locked is low;
+//       reset_sync_400m stays 2'b00, reset_400m stays 1'b1, downstream
+//       FDREs stay cleared. Safe.
 // ============================================================================
+(* ASYNC_REG = "TRUE" *) reg [1:0] reset_sync_400m = 2'b00;
+(* max_fanout = 50 *) wire reset_n_400m = reset_sync_400m[1];
+
+// Active-high replicated reset for all synchronous 400 MHz consumers
+(* max_fanout = 50 *) reg reset_400m = 1'b1;
+
+always @(posedge clk_400m or negedge reset_n) begin
+    if (!reset_n) begin
+        reset_sync_400m <= 2'b00;
+        reset_400m      <= 1'b1;
+    end else begin
+        reset_sync_400m <= {reset_sync_400m[0], 1'b1};
+        reset_400m      <= ~reset_sync_400m[1];
+    end
+end
+
+// CDC synchronization for control signals (2-stage synchronizers)
+(* ASYNC_REG = "TRUE" *) reg [1:0] mixers_enable_sync_chain;
+(* ASYNC_REG = "TRUE" *) reg [1:0] force_saturation_sync_chain;
+wire mixers_enable_sync;
+wire force_saturation_sync;
 assign mixers_enable_sync = mixers_enable_sync_chain[1];
 assign force_saturation_sync = force_saturation_sync_chain[1];
 
-always @(posedge clk_400m or negedge reset_n_400m) begin
+// Sync reset via reset_400m (replicated, max_fanout=50). Was async on
-    if (!reset_n_400m) begin
+// reset_n_400m — see "400 MHz RESET DISTRIBUTION" comment above.
+always @(posedge clk_400m) begin
+    if (reset_400m) begin
         mixers_enable_sync_chain <= 2'b00;
         force_saturation_sync_chain <= 2'b00;
     end else begin
@@ -160,8 +171,8 @@ end
 // ============================================================================
 // Sample Counter and Debug Monitoring
 // ============================================================================
-always @(posedge clk_400m or negedge reset_n_400m) begin
+always @(posedge clk_400m) begin
-    if (!reset_n_400m || reset_monitors) begin
+    if (reset_400m || reset_monitors) begin
         sample_counter <= 0;
         error_counter <= 0;
     end else if (adc_data_valid_i && adc_data_valid_q ) begin
@@ -189,8 +200,8 @@ lfsr_dither_enhanced #(
 localparam PHASE_INC_120MHZ = 32'h4CCCCCCD;
 
 // Apply dithering to reduce spurious tones (registered for 400 MHz timing)
-always @(posedge clk_400m or negedge reset_n_400m) begin
+always @(posedge clk_400m) begin
-    if (!reset_n_400m)
+    if (reset_400m)
         phase_inc_dithered <= PHASE_INC_120MHZ;
     else
         phase_inc_dithered <= PHASE_INC_120MHZ + {24'b0, phase_dither_bits};
@@ -229,8 +240,8 @@ assign adc_signed_w = {1'b0, adc_data, {(MIXER_WIDTH-ADC_WIDTH-1){1'b0}}} -
                       {1'b0, {ADC_WIDTH{1'b1}}, {(MIXER_WIDTH-ADC_WIDTH-1){1'b0}}} / 2;
 
 // Valid pipeline: 5-stage shift register (1 NCO pipe + 3 DSP48E1 AREG+MREG+PREG + 1 retiming)
-always @(posedge clk_400m or negedge reset_n_400m) begin
+always @(posedge clk_400m) begin
-    if (!reset_n_400m) begin
+    if (reset_400m) begin
         dsp_valid_pipe <= 5'b00000;
     end else begin
         dsp_valid_pipe <= {dsp_valid_pipe[3:0], (nco_ready && adc_data_valid_i && adc_data_valid_q)};
@@ -246,8 +257,8 @@ reg signed [MIXER_WIDTH+NCO_WIDTH-1:0] mult_i_internal, mult_q_internal;  // Mod
 reg signed [MIXER_WIDTH+NCO_WIDTH-1:0] mult_i_reg, mult_q_reg;            // Models PREG
 
 // Stage 0: NCO pipeline — breaks long NCO→DSP route (matches synthesis fabric registers)
-always @(posedge clk_400m or negedge reset_n_400m) begin
+always @(posedge clk_400m) begin
-    if (!reset_n_400m) begin
+    if (reset_400m) begin
         cos_nco_pipe <= 0;
         sin_nco_pipe <= 0;
     end else begin
@@ -257,8 +268,8 @@ always @(posedge clk_400m or negedge reset_n_400m) begin
 end
 
 // Stage 1: AREG/BREG equivalent (uses pipelined NCO outputs)
-always @(posedge clk_400m or negedge reset_n_400m) begin
+always @(posedge clk_400m) begin
-    if (!reset_n_400m) begin
+    if (reset_400m) begin
         adc_signed_reg <= 0;
         cos_pipe_reg <= 0;
         sin_pipe_reg <= 0;
@@ -270,8 +281,8 @@ always @(posedge clk_400m or negedge reset_n_400m) begin
 end
 
 // Stage 2: MREG equivalent
-always @(posedge clk_400m or negedge reset_n_400m) begin
+always @(posedge clk_400m) begin
-    if (!reset_n_400m) begin
+    if (reset_400m) begin
         mult_i_internal <= 0;
         mult_q_internal <= 0;
     end else begin
@@ -281,8 +292,8 @@ always @(posedge clk_400m or negedge reset_n_400m) begin
 end
 
 // Stage 3: PREG equivalent
-always @(posedge clk_400m or negedge reset_n_400m) begin
+always @(posedge clk_400m) begin
-    if (!reset_n_400m) begin
+    if (reset_400m) begin
         mult_i_reg <= 0;
         mult_q_reg <= 0;
     end else begin
@@ -292,8 +303,8 @@ always @(posedge clk_400m or negedge reset_n_400m) begin
 end
 
 // Stage 4: Post-DSP retiming register (matches synthesis path)
-always @(posedge clk_400m or negedge reset_n_400m) begin
+always @(posedge clk_400m) begin
-    if (!reset_n_400m) begin
+    if (reset_400m) begin
         mult_i_retimed <= 0;
         mult_q_retimed <= 0;
     end else begin
@@ -311,8 +322,8 @@ wire [47:0] dsp_p_i, dsp_p_q;
 // (1.505ns routing observed in Build 26). These fabric registers are placed
 // near the DSP by the placer, splitting the route into two shorter segments.
 // DONT_TOUCH on the reg declaration (above) prevents absorption/retiming.
-always @(posedge clk_400m or negedge reset_n_400m) begin
+always @(posedge clk_400m) begin
-    if (!reset_n_400m) begin
+    if (reset_400m) begin
         cos_nco_pipe <= 0;
         sin_nco_pipe <= 0;
     end else begin
@@ -329,11 +340,10 @@ DSP48E1 #(
     .USE_DPORT("FALSE"),
     .USE_MULT("MULTIPLY"),
     .USE_SIMD("ONE48"),
-    // Pipeline register attributes — all enabled for max timing
     .AREG(1),
     .BREG(1),
     .MREG(1),
-    .PREG(1),           // P register enabled — absorbs CLK→P delay for timing closure
+    .PREG(1),
     .ADREG(0),
     .ACASCREG(1),
     .BCASCREG(1),
@@ -344,7 +354,6 @@ DSP48E1 #(
     .DREG(0),
     .INMODEREG(0),
     .OPMODEREG(0),
-    // Pattern detector (unused)
     .AUTORESET_PATDET("NO_RESET"),
     .MASK(48'h3fffffffffff),
     .PATTERN(48'h000000000000),
@@ -496,8 +505,8 @@ wire signed [MIXER_WIDTH+NCO_WIDTH-1:0] mult_q_reg = dsp_p_q[MIXER_WIDTH+NCO_WID
 // Stage 4: Post-DSP retiming register — breaks DSP48E1 CLK→P to fabric path
 // Without this, the DSP output prop delay (1.866ns) + routing (0.515ns) exceeds
 // the 2.500ns clock period at slow process corner
-always @(posedge clk_400m or negedge reset_n_400m) begin
+always @(posedge clk_400m) begin
-    if (!reset_n_400m) begin
+    if (reset_400m) begin
         mult_i_retimed <= 0;
         mult_q_retimed <= 0;
     end else begin
@@ -513,8 +522,8 @@ end
 // force_saturation mux is intentionally AFTER the DSP48E1 output to avoid
 // polluting the critical input path with extra logic
 // ============================================================================
-always @(posedge clk_400m or negedge reset_n_400m) begin
+always @(posedge clk_400m) begin
-    if (!reset_n_400m) begin
+    if (reset_400m) begin
         mixed_i <= 0;
         mixed_q <= 0;
         mixed_valid <= 0;
@@ -759,8 +768,17 @@ generate
     end
 endgenerate
 
-always @(posedge clk or negedge reset_n) begin
+// ============================================================================
-    if (!reset_n) begin
+// RESET FAN-OUT INVARIANT: registered active-high reset with max_fanout=50.
+// See cic_decimator_4x_enhanced.v for full reasoning. reset_n here is driven
+// by the parent DDC's reset_n_400m (already synchronized to clk_400m), so
+// sync reset on the LFSR is safe. INIT=1'b1 holds LFSR in reset on power-up.
+// ============================================================================
+(* max_fanout = 50 *) reg reset_h = 1'b1;
+always @(posedge clk) reset_h <= ~reset_n;
+
+always @(posedge clk) begin
+    if (reset_h) begin
         lfsr_reg <= {DITHER_WIDTH{1'b1}};  // Non-zero initial state
         cycle_counter <= 0;
         lock_detected <= 0;

9_Firmware/9_2_FPGA/nco_400m_enhanced.v

@@ -59,6 +59,25 @@ reg [1:0] quadrant_reg2;                 // Pass-through for Stage 5 MUX
 // Valid pipeline: tracks 6-stage latency
 reg [5:0] valid_pipe;
 
+// ============================================================================
+// RESET FAN-OUT INVARIANT (Build N+1 fix for WNS=-0.626ns at 400 MHz):
+// ============================================================================
+// reset_h is an ACTIVE-HIGH, REGISTERED copy of ~reset_n with (* max_fanout=50 *).
+// Vivado replicates this register (14+ copies) so each copy drives ≈50 loads
+// regionally, avoiding the single-LUT1 / 702-load net that caused timing
+// failure in Build N. It feeds:
+//   - DSP48E1 RSTP/RSTC on the phase-accumulator DSP (below)
+//   - All pipeline-stage fabric FDREs (synchronous reset)
+// Invariants (see cic_decimator_4x_enhanced.v for full reasoning):
+//   I1 correctness:      reset_h == ~reset_n one cycle later
+//   I2 glitch-free:      registered output
+//   I3 power-up safe:    INIT=1'b1 holds all downstream in reset until first
+//                        valid clock edge; reset_n is low on power-up anyway
+//   I4 de-assert lat.:   +1 cycle vs. direct async; negligible at 400 MHz
+// ============================================================================
+(* max_fanout = 50 *) reg reset_h = 1'b1;
+always @(posedge clk_400m) reset_h <= ~reset_n;
+
 // Use only the top 8 bits for LUT addressing (256-entry LUT equivalent)
 wire [7:0] lut_address = phase_with_offset[31:24];
 
@@ -135,8 +154,8 @@ wire [15:0] cos_abs_w = sin_lut[63 - lut_index_pipe_cos];
 // Stage 2: phase_with_offset adds phase offset
 reg [31:0] phase_accumulator;
 
-always @(posedge clk_400m or negedge reset_n) begin
+always @(posedge clk_400m) begin
-    if (!reset_n) begin
+    if (reset_h) begin
         phase_accumulator <= 32'h00000000;
         phase_accum_reg   <= 32'h00000000;
         phase_with_offset <= 32'h00000000;
@@ -190,8 +209,8 @@ DSP48E1 #(
     .RSTA(1'b0),
     .RSTB(1'b0),
     .RSTM(1'b0),
-    .RSTP(!reset_n),             // Reset P register (phase accumulator) on !reset_n
+    .RSTP(reset_h),              // Reset P register (phase accumulator) — registered, max_fanout=50
-    .RSTC(!reset_n),             // Reset C register (tuning word) on !reset_n
+    .RSTC(reset_h),              // Reset C register (tuning word)       — registered, max_fanout=50
     .RSTALLCARRYIN(1'b0),
     .RSTALUMODE(1'b0),
     .RSTCTRL(1'b0),
@@ -245,8 +264,8 @@ DSP48E1 #(
 // Stage 1: Capture DSP48E1 P output into fabric register
 // Stage 2: Add phase offset to captured value
 // Split into two registered stages to break DSP48E1.P→CARRY4 critical path
-always @(posedge clk_400m or negedge reset_n) begin
+always @(posedge clk_400m) begin
-    if (!reset_n) begin
+    if (reset_h) begin
         phase_accum_reg   <= 32'h00000000;
         phase_with_offset <= 32'h00000000;
     end else if (phase_valid) begin
@@ -264,8 +283,8 @@ end
 //           Only 2 registers driven (lut_index_pipe + quadrant_pipe)
 //           Minimal fanout → short routes → easy timing
 // ============================================================================
-always @(posedge clk_400m or negedge reset_n) begin
+always @(posedge clk_400m) begin
-    if (!reset_n) begin
+    if (reset_h) begin
         lut_index_pipe_sin <= 6'b000000;
         lut_index_pipe_cos <= 6'b000000;
         quadrant_pipe      <= 2'b00;
@@ -281,8 +300,8 @@ end
 //           Registered address → combinational LUT6 read → register
 //           Only 1 logic level (LUT6), trivial timing
 // ============================================================================
-always @(posedge clk_400m or negedge reset_n) begin
+always @(posedge clk_400m) begin
-    if (!reset_n) begin
+    if (reset_h) begin
         sin_abs_reg <= 16'h0000;
         cos_abs_reg <= 16'h7FFF;
         quadrant_reg <= 2'b00;
@@ -298,8 +317,8 @@ end
 //          CARRY4 x4 chain has registered inputs — easily fits in 2.5ns
 //          Also pass through abs values and quadrant for Stage 5
 // ============================================================================
-always @(posedge clk_400m or negedge reset_n) begin
+always @(posedge clk_400m) begin
-    if (!reset_n) begin
+    if (reset_h) begin
         sin_neg_reg <= 16'h0000;
         cos_neg_reg <= -16'h7FFF;
         sin_abs_reg2 <= 16'h0000;
@@ -318,8 +337,8 @@ end
 // Stage 5: Quadrant sign application → final sin/cos output
 //          Uses pre-computed negated values from Stage 4 — pure MUX, no arithmetic
 // ============================================================================
-always @(posedge clk_400m or negedge reset_n) begin
+always @(posedge clk_400m) begin
-    if (!reset_n) begin
+    if (reset_h) begin
         sin_out <= 16'h0000;
         cos_out <= 16'h7FFF;
     end else if (valid_pipe[4]) begin
@@ -347,8 +366,8 @@ end
 // ============================================================================
 // Valid pipeline and dds_ready (6-stage latency)
 // ============================================================================
-always @(posedge clk_400m or negedge reset_n) begin
+always @(posedge clk_400m) begin
-    if (!reset_n) begin
+    if (reset_h) begin
         valid_pipe <= 6'b000000;
         dds_ready <= 1'b0;
     end else begin

9_Firmware/9_2_FPGA/rx_gain_control.v

@@ -169,11 +169,11 @@ endfunction
 // =========================================================================
 // Clamp a wider signed value to [-7, +7]
 function signed [3:0] clamp_gain;
-    input signed [4:0] val;  // 5-bit to handle overflow from add
+    input signed [5:0] val;  // 6-bit: covers [-22,+22] (max |gain|+step = 7+15)
     begin
-        if (val > 5'sd7)
+        if (val > 6'sd7)
             clamp_gain = 4'sd7;
-        else if (val < -5'sd7)
+        else if (val < -6'sd7)
             clamp_gain = -4'sd7;
         else
             clamp_gain = val[3:0];
@@ -246,15 +246,15 @@ always @(posedge clk or negedge reset_n) begin
                 // Use inclusive counts/peaks (accounting for simultaneous valid_in)
                 if (wire_frame_sat_incr || frame_sat_count > 8'd0) begin
                     // Clipping detected: reduce gain immediately (attack)
-                    agc_gain <= clamp_gain($signed({agc_gain[3], agc_gain}) -
+                    agc_gain <= clamp_gain($signed({agc_gain[3], agc_gain[3], agc_gain}) -
-                                           $signed({1'b0, agc_attack}));
+                                           $signed({2'b00, agc_attack}));
                     holdoff_counter <= agc_holdoff;  // Reset holdoff
                 end else if ((wire_frame_peak_update ? max_iq[14:7] : frame_peak[14:7])
                              < agc_target) begin
                     // Signal too weak: increase gain after holdoff expires
                     if (holdoff_counter == 4'd0) begin
-                        agc_gain <= clamp_gain($signed({agc_gain[3], agc_gain}) +
+                        agc_gain <= clamp_gain($signed({agc_gain[3], agc_gain[3], agc_gain}) +
-                                               $signed({1'b0, agc_decay}));
+                                               $signed({2'b00, agc_decay}));
                     end else begin
                         holdoff_counter <= holdoff_counter - 4'd1;
                     end

9_Firmware/9_3_GUI/radar_protocol.py

@@ -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
 # ============================================================================

9_Firmware/9_3_GUI/test_v7.py

@@ -586,6 +586,135 @@ class TestSoftwareFPGA(unittest.TestCase):
         self.assertEqual(fpga.agc_holdoff, 0x0F)
 
 
+# ============================================================================
+# Test: live vs replay physical-unit parity — regression guard for unit drift
+#
+# Uses AST parse of workers.py (not inspect.getsource / import) so the test
+# runs in headless CI without PyQt6 — v7.workers imports PyQt6 unconditionally
+# at workers.py:24, and other worker tests here already use skipUnless(
+# _pyqt6_available()). Contract enforcement must not be gated on GUI deps.
+#
+# Asserts on AST nodes (Call / Attribute / BinOp), not source substrings, so
+# false-pass on comments or docstring wording is impossible.
+# ============================================================================
+
+
+class TestLiveReplayPhysicalUnitsParity(unittest.TestCase):
+    """Contract: live path (RadarDataWorker._run_host_dsp) and replay path
+    (ReplayWorker._emit_frame) both derive bin-to-physical conversion from
+    WaveformConfig — same source of truth, identical (range_m, velocity_ms)
+    for identical detections.
+
+    Regression context: before the fix, live path used
+    RadarSettings.velocity_resolution (default 1.0 in models.py:113) while
+    replay used WaveformConfig.velocity_resolution_mps (~5.343). Live GUI
+    therefore under-reported velocity by factor ~5.34x vs replay for
+    identical frames. See test_v7.py:449 for the WaveformConfig pin.
+    """
+
+    @staticmethod
+    def _parse_method(class_name: str, method_name: str):
+        """Return AST FunctionDef for class_name.method_name from workers.py,
+        without importing v7.workers (PyQt6-independent)."""
+        import ast
+        from pathlib import Path
+        path = Path(__file__).parent / "v7" / "workers.py"
+        tree = ast.parse(path.read_text(encoding="utf-8"))
+        for node in tree.body:
+            if isinstance(node, ast.ClassDef) and node.name == class_name:
+                for item in node.body:
+                    if isinstance(item, ast.FunctionDef) and item.name == method_name:
+                        return item
+        raise RuntimeError(f"{class_name}.{method_name} not found in workers.py")
+
+    @staticmethod
+    def _has_attribute_chain(tree, chain):
+        """True if AST tree contains a dotted attribute access matching chain.
+
+        Chain ('self', '_settings', 'range_resolution') matches
+        ``self._settings.range_resolution`` exactly.
+        """
+        import ast
+        for n in ast.walk(tree):
+            if isinstance(n, ast.Attribute):
+                parts = [n.attr]
+                cur = n.value
+                while isinstance(cur, ast.Attribute):
+                    parts.append(cur.attr)
+                    cur = cur.value
+                if isinstance(cur, ast.Name):
+                    parts.append(cur.id)
+                    parts.reverse()
+                    if tuple(parts) == tuple(chain):
+                        return True
+        return False
+
+    @staticmethod
+    def _has_call_to(tree, func_name):
+        """True if AST tree contains a call to a bare name (func_name())."""
+        import ast
+        for n in ast.walk(tree):
+            if (isinstance(n, ast.Call) and isinstance(n.func, ast.Name)
+                    and n.func.id == func_name):
+                return True
+        return False
+
+    @staticmethod
+    def _has_dbin_minus(tree, literal):
+        """True if AST tree contains ``dbin - <literal>`` binary op."""
+        import ast
+        for n in ast.walk(tree):
+            if (isinstance(n, ast.BinOp) and isinstance(n.op, ast.Sub)
+                    and isinstance(n.left, ast.Name) and n.left.id == "dbin"
+                    and isinstance(n.right, ast.Constant)
+                    and n.right.value == literal):
+                return True
+        return False
+
+    def test_live_path_uses_waveform_config(self):
+        """RadarDataWorker.__init__ must instantiate WaveformConfig() into
+        self._waveform; _run_host_dsp must read self._waveform.range_resolution_m
+        / velocity_resolution_mps — not self._settings equivalents."""
+        init = self._parse_method("RadarDataWorker", "__init__")
+        self.assertTrue(self._has_call_to(init, "WaveformConfig"),
+            "RadarDataWorker.__init__ must instantiate WaveformConfig() into self._waveform.")
+        method = self._parse_method("RadarDataWorker", "_run_host_dsp")
+        self.assertTrue(
+            self._has_attribute_chain(method, ("self", "_waveform", "range_resolution_m")),
+            "Live path must read self._waveform.range_resolution_m.")
+        self.assertTrue(
+            self._has_attribute_chain(method, ("self", "_waveform", "velocity_resolution_mps")),
+            "Live path must read self._waveform.velocity_resolution_mps. "
+            "RadarSettings.velocity_resolution default 1.0 caused ~5.34x "
+            "underreport vs replay (test_v7.py:449 pins ~5.343).")
+        self.assertFalse(self._has_attribute_chain(
+            method, ("self", "_settings", "range_resolution")),
+            "Live path still reads stale RadarSettings.range_resolution.")
+        self.assertFalse(self._has_attribute_chain(
+            method, ("self", "_settings", "velocity_resolution")),
+            "Live path still reads stale RadarSettings.velocity_resolution.")
+
+    def test_live_path_doppler_center_not_hardcoded(self):
+        """_run_host_dsp must derive doppler_center from frame shape, not
+        use hardcoded ``dbin - 16`` — mirrors processing.py:520."""
+        method = self._parse_method("RadarDataWorker", "_run_host_dsp")
+        self.assertFalse(self._has_dbin_minus(method, 16),
+            "Hardcoded doppler_center=16 breaks if frame shape changes. "
+            "Use frame.detections.shape[1] // 2 like processing.py:520.")
+
+    def test_replay_path_still_uses_waveform_config(self):
+        """Parity half: replay path (ReplayWorker._emit_frame) must keep
+        reading self._waveform.range_resolution_m / velocity_resolution_mps —
+        guards against someone breaking the replay side of the invariant."""
+        method = self._parse_method("ReplayWorker", "_emit_frame")
+        self.assertTrue(self._has_attribute_chain(
+            method, ("self", "_waveform", "range_resolution_m")),
+            "Replay path lost WaveformConfig range source of truth.")
+        self.assertTrue(self._has_attribute_chain(
+            method, ("self", "_waveform", "velocity_resolution_mps")),
+            "Replay path lost WaveformConfig velocity source of truth.")
+
+
 class TestSoftwareFPGASignalChain(unittest.TestCase):
     """SoftwareFPGA.process_chirps with real co-sim data."""
 

9_Firmware/9_3_GUI/v7/workers.py

@@ -23,7 +23,7 @@ import numpy as np
 
 from PyQt6.QtCore import QThread, QObject, QTimer, pyqtSignal
 
-from .models import RadarTarget, GPSData, RadarSettings
+from .models import RadarTarget, GPSData, RadarSettings, WaveformConfig
 from .hardware import (
     RadarAcquisition,
     RadarFrame,
@@ -84,6 +84,7 @@ class RadarDataWorker(QThread):
         self._recorder = recorder
         self._gps = gps_data_ref
         self._settings = settings or RadarSettings()
+        self._waveform = WaveformConfig()
         self._running = False
 
         # Frame queue for production RadarAcquisition → this thread
@@ -97,6 +98,9 @@ class RadarDataWorker(QThread):
         self._byte_count = 0
         self._error_count = 0
 
+    def set_waveform(self, wf: "WaveformConfig") -> None:
+        self._waveform = wf
+
     def stop(self):
         self._running = False
         if self._acquisition:
@@ -169,8 +173,8 @@ class RadarDataWorker(QThread):
         The FPGA already does: FFT, MTI, CFAR, DC notch.
         Host-side DSP adds: clustering, tracking, geo-coordinate mapping.
 
-        Bin-to-physical conversion uses RadarSettings.range_resolution
+        Bin-to-physical conversion uses self._waveform (WaveformConfig) to keep
-        and velocity_resolution (should be calibrated to actual waveform).
+        live and replay units aligned. Override via set_waveform() if needed.
         """
         targets: list[RadarTarget] = []
 
@@ -180,8 +184,11 @@ class RadarDataWorker(QThread):
 
         # Extract detections from FPGA CFAR flags
         det_indices = np.argwhere(frame.detections > 0)
-        r_res = self._settings.range_resolution
+        r_res = self._waveform.range_resolution_m
-        v_res = self._settings.velocity_resolution
+        v_res = self._waveform.velocity_resolution_mps
+        n_doppler = (frame.detections.shape[1] if frame.detections.ndim == 2
+                     else self._waveform.n_doppler_bins)
+        doppler_center = n_doppler // 2
 
         for idx in det_indices:
             rbin, dbin = idx
@@ -190,8 +197,9 @@ class RadarDataWorker(QThread):
 
             # Convert bin indices to physical units
             range_m = float(rbin) * r_res
-            # Doppler: centre bin (16) = 0 m/s; positive bins = approaching
+            # Doppler: centre bin = 0 m/s; positive bins = approaching.
-            velocity_ms = float(dbin - 16) * v_res
+            # Derived from frame shape — mirrors processing.py:520.
+            velocity_ms = float(dbin - doppler_center) * v_res
 
             # Apply pitch correction if GPS data available
             raw_elev = 0.0  # FPGA doesn't send elevation per-detection

9_Firmware/tests/cross_layer/contract_parser.py

@@ -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,7 +134,8 @@ 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)
-        opcodes[value] = OpcodeEntry(name=name, value=value)
+        if value not in mcu_only:
+            opcodes[value] = OpcodeEntry(name=name, value=value)
     return opcodes
 
 

9_Firmware/tests/cross_layer/test_cross_layer_contract.py

@@ -26,12 +26,14 @@ layers agree (because both could be wrong).
 
 from __future__ import annotations
 
+import ast
 import os
 import re
 import struct
 import subprocess
 import tempfile
 from pathlib import Path
+from typing import ClassVar
 
 import pytest
 
@@ -625,6 +627,420 @@ class TestTier1AgcCrossLayerInvariant:
         )
 
 
+# ===================================================================
+# ADAR1000 channel→register round-trip invariant (issue #90)
+# ===================================================================
+#
+# Ground-truth invariant crossing three system layers:
+#   Chip (datasheet) -> Driver (MCU helpers) -> Application (callers).
+#
+# For every logical element ch in {0,1,2,3} (hardware channels CH1..CH4),
+# the round-trip
+#     caller_expr(ch) --> helper_offset(channel) * stride --> base + off
+# must land on the physical register REG_CH{ch+1}_* defined in the ADI
+# ADAR1000 register map parsed from ADAR1000_Manager.h.
+#
+# Catches:
+#   * #90 channel rotation regardless of which side is fixed (caller OR helper).
+#   * Wrong stride (e.g. phase written with stride 1 instead of 2).
+#   * Bad mask (e.g. `channel & 0x07`, `channel & 0x01`).
+#   * Wrong base register in a helper.
+#   * New setter added with mismatched convention.
+#   * Caller moved to a file the test no longer scans (fails loudly).
+#
+# Cannot be defeated by:
+#   * Renaming/refactoring helper layout: the setter coverage test
+#     (`test_helper_sites_exist_for_all_setters`) catches missing parse.
+#   * Changing 0x03 to 3 or adding a named constant: the offset is
+#     evaluated symbolically via AST, not matched by regex.
+
+
+def _parse_adar_register_map(header_text):
+    """Extract `#define REG_CHn_(RX|TX)_(GAIN|PHS_I|PHS_Q)` values."""
+    regs = {}
+    for m in re.finditer(
+        r"^#define\s+(REG_CH[1-4]_(?:RX|TX)_(?:GAIN|PHS_I|PHS_Q))\s+(0x[0-9A-Fa-f]+)",
+        header_text,
+        re.MULTILINE,
+    ):
+        regs[m.group(1)] = int(m.group(2), 16)
+    return regs
+
+
+def _safe_eval_int_expr(expr, **variables):
+    """
+    Evaluate a small integer expression with +, -, *, &, |, ^, ~, <<, >>.
+    Python's & / | / ^ / ~ / << / >> have the same semantics as C for the
+    operand widths we care about here (uint8_t after the mask makes the
+    result fit in 0..3). No floating point, no function calls, no names
+    outside ``variables``.
+
+    SECURITY: ``expr`` MUST come from a trusted source -- specifically,
+    C/C++ source text under version control in this repository (e.g.
+    arguments parsed out of ``main.cpp``/``ADAR1000_AGC.cpp``).  Although
+    the AST whitelist below rejects function calls, attribute access,
+    subscripts, and any name not in ``variables``, ``eval`` is still
+    invoked on the compiled tree.  Do NOT pass user-supplied / network /
+    GUI input here.
+    """
+    tree = ast.parse(expr, mode="eval")
+    allowed = (
+        ast.Expression, ast.BinOp, ast.UnaryOp, ast.Constant,
+        ast.Name, ast.Load,
+        ast.Add, ast.Sub, ast.Mult, ast.Mod, ast.FloorDiv,
+        ast.BitAnd, ast.BitOr, ast.BitXor,
+        ast.USub, ast.UAdd, ast.Invert,
+        ast.LShift, ast.RShift,
+    )
+    for node in ast.walk(tree):
+        if not isinstance(node, allowed):
+            raise ValueError(
+                f"disallowed AST node {type(node).__name__!s} in `{expr}`"
+            )
+    return eval(
+        compile(tree, "<expr>", "eval"),
+        {"__builtins__": {}},
+        variables,
+    )
+
+
+def _extract_adar_helper_sites(manager_cpp, setter_names):
+    """
+    For each setter, locate the body of ``void ADAR1000Manager::<setter>``
+    and return a list of (setter, base_register, offset_expr_c, stride)
+    for every ``REG_CHn_XXX + <expr>`` memory-address assignment.
+    """
+    sites = []
+    for setter in setter_names:
+        m = re.search(
+            rf"void\s+ADAR1000Manager::{setter}\s*\([^)]*\)\s*\{{(.+?)^\}}",
+            manager_cpp,
+            re.MULTILINE | re.DOTALL,
+        )
+        if not m:
+            continue
+        body = m.group(1)
+        for access in re.finditer(
+            r"=\s*(REG_CH[1-4]_(?:RX|TX)_(?:GAIN|PHS_I|PHS_Q))\s*\+\s*([^;]+);",
+            body,
+        ):
+            base = access.group(1)
+            rhs = access.group(2).strip()
+            # Trailing `* <integer>` = stride multiplier (2 for phase I/Q).
+            stride_match = re.match(r"(.+?)\s*\*\s*(\d+)\s*$", rhs)
+            if stride_match:
+                offset_expr = stride_match.group(1).strip()
+                stride = int(stride_match.group(2))
+            else:
+                offset_expr = rhs
+                stride = 1
+            sites.append((setter, base, offset_expr, stride))
+    return sites
+
+
+# Method-definition line pattern: `[qualifier...] <ret-type> <Class>::<setter>(`
+# Covers: plain `void X::f(`, `inline void X::f(`, `static bool X::f(`, etc.
+_DEFN_RE = re.compile(
+    r"^\s*(?:inline\s+|static\s+|virtual\s+|constexpr\s+|explicit\s+)*"
+    r"(?:void|bool|uint\w+|int\w*|auto)\s+\S+::\w+\s*\("
+)
+
+
+def _extract_adar_caller_sites(sources, setter):
+    """
+    Find every call ``<obj>.<setter>(dev, <channel_expr>, ...)`` across
+    ``sources = [(filename, text), ...]``. Returns (filename, line_no,
+    channel_expr) for each. Skips function declarations/definitions.
+
+    Arg list up to matching `)`: restricted to a single line. All existing
+    call sites fit on one line; a future multi-line refactor would drop
+    callers from the scan, which the round-trip test surfaces loudly via
+    `assert callers` (rather than silently missing a site).
+    """
+    out = []
+    call_re = re.compile(rf"\b{setter}\s*\(([^;]*?)\)\s*;")
+    for filename, text in sources:
+        for line_no, line in enumerate(text.splitlines(), start=1):
+            # Skip method definition / declaration lines.
+            if _DEFN_RE.match(line):
+                continue
+            cm = call_re.search(line)
+            if not cm:
+                continue
+            args = _split_top_level_commas(cm.group(1))
+            if len(args) < 2:
+                continue
+            channel_expr = args[1].strip()
+            out.append((filename, line_no, channel_expr))
+    return out
+
+
+def _split_top_level_commas(text):
+    """Split on commas that sit at paren-depth 0 (ignores nested calls)."""
+    parts, depth, cur = [], 0, []
+    for ch in text:
+        if ch == "(":
+            depth += 1
+            cur.append(ch)
+        elif ch == ")":
+            depth -= 1
+            cur.append(ch)
+        elif ch == "," and depth == 0:
+            parts.append("".join(cur))
+            cur = []
+        else:
+            cur.append(ch)
+    if cur:
+        parts.append("".join(cur))
+    return parts
+
+
+class TestTier1Adar1000ChannelRegisterRoundTrip:
+    """
+    Cross-layer round-trip: caller channel expr -> helper offset formula
+    -> physical register address must equal REG_CH{ch+1}_* for every
+    caller and every ch in {0,1,2,3}.
+
+    See module-level block comment above and upstream issue #90.
+    """
+
+    _SETTERS = (
+        "adarSetRxPhase",
+        "adarSetTxPhase",
+        "adarSetRxVgaGain",
+        "adarSetTxVgaGain",
+    )
+
+    # Register base -> stride override. Parsed values of stride are
+    # trusted; this table is the independent ground truth for cross-check.
+    _EXPECTED_STRIDE: ClassVar[dict[str, int]] = {
+        "REG_CH1_RX_GAIN": 1,
+        "REG_CH1_TX_GAIN": 1,
+        "REG_CH1_RX_PHS_I": 2,
+        "REG_CH1_RX_PHS_Q": 2,
+        "REG_CH1_TX_PHS_I": 2,
+        "REG_CH1_TX_PHS_Q": 2,
+    }
+
+    @classmethod
+    def setup_class(cls):
+        cls.header_txt = (cp.MCU_LIB_DIR / "ADAR1000_Manager.h").read_text()
+        cls.manager_txt = (cp.MCU_LIB_DIR / "ADAR1000_Manager.cpp").read_text()
+        cls.reg_map = _parse_adar_register_map(cls.header_txt)
+        cls.helper_sites = _extract_adar_helper_sites(
+            cls.manager_txt, cls._SETTERS,
+        )
+        # Auto-discover every C++ TU under the MCU tree so a new caller
+        # added to e.g. a future ``ADAR1000_Calibration.cpp`` cannot
+        # silently escape the round-trip check (issue #90 reviewer note).
+        # Exclude any path containing a ``tests`` segment so this test
+        # does not parse its own fixtures.  The resulting list is
+        # deterministic (sorted) for reproducible parametrization.
+        scanned = []
+        seen = set()
+        for root in (cp.MCU_LIB_DIR, cp.MCU_CODE_DIR):
+            for path in sorted(root.rglob("*.cpp")):
+                if "tests" in path.parts:
+                    continue
+                if path in seen:
+                    continue
+                seen.add(path)
+                scanned.append((path.name, path.read_text()))
+        cls.sources = scanned
+        # Sanity: the two TUs known to call ADAR1000 setters at the time
+        # of issue #90 must be in scope.  If a future refactor renames or
+        # moves them this assert fires loudly rather than silently
+        # passing an empty round-trip.
+        scanned_names = {n for (n, _) in scanned}
+        for required in ("ADAR1000_AGC.cpp", "main.cpp", "ADAR1000_Manager.cpp"):
+            assert required in scanned_names, (
+                f"Auto-discovery missed `{required}`; check MCU_LIB_DIR / "
+                f"MCU_CODE_DIR roots in contract_parser.py."
+            )
+
+    # ---------- Tier A: chip ground truth ----------------------------
+
+    def test_register_map_gain_stride_is_one_per_channel(self):
+        """Datasheet invariant: RX/TX VGA gain registers are 1 byte apart."""
+        for kind in ("RX_GAIN", "TX_GAIN"):
+            for n in range(1, 4):
+                delta = (
+                    self.reg_map[f"REG_CH{n+1}_{kind}"]
+                    - self.reg_map[f"REG_CH{n}_{kind}"]
+                )
+                assert delta == 1, (
+                    f"ADAR1000 register map invariant broken: "
+                    f"REG_CH{n+1}_{kind} - REG_CH{n}_{kind} = {delta}, "
+                    f"datasheet says 1. Either the header was mis-edited "
+                    f"or ADI released a part with a different map."
+                )
+
+    def test_register_map_phase_stride_is_two_per_channel(self):
+        """Datasheet invariant: phase I/Q pairs occupy 2 bytes per channel."""
+        for kind in ("RX_PHS_I", "RX_PHS_Q", "TX_PHS_I", "TX_PHS_Q"):
+            for n in range(1, 4):
+                delta = (
+                    self.reg_map[f"REG_CH{n+1}_{kind}"]
+                    - self.reg_map[f"REG_CH{n}_{kind}"]
+                )
+                assert delta == 2, (
+                    f"ADAR1000 register map invariant broken: "
+                    f"REG_CH{n+1}_{kind} - REG_CH{n}_{kind} = {delta}, "
+                    f"datasheet says 2."
+                )
+
+    # ---------- Tier B: driver parses cleanly -------------------------
+
+    def test_helper_sites_exist_for_all_setters(self):
+        """Every channel-indexed setter must parse at least one register access."""
+        found = {s for (s, _, _, _) in self.helper_sites}
+        missing = set(self._SETTERS) - found
+        assert not missing, (
+            f"Helper parse failed for: {sorted(missing)}. "
+            f"Either a setter was renamed (update _SETTERS), moved out of "
+            f"ADAR1000_Manager.cpp (extend scan scope), or the register-"
+            f"access form changed beyond `REG_CHn_XXX + <expr>`. "
+            f"DO NOT weaken this test without reviewing issue #90."
+        )
+
+    def test_helper_parsed_stride_matches_datasheet(self):
+        """Parsed helper strides must match the datasheet register spacing."""
+        for setter, base, offset_expr, stride in self.helper_sites:
+            expected = self._EXPECTED_STRIDE.get(base)
+            assert expected is not None, (
+                f"{setter} writes to unrecognised base `{base}`. "
+                f"If ADI added a new channel-indexed register block, "
+                f"extend _EXPECTED_STRIDE with its datasheet stride."
+            )
+            assert stride == expected, (
+                f"{setter} helper uses stride {stride} for `{base}` "
+                f"(`{offset_expr} * {stride}`), datasheet says {expected}. "
+                f"Writes will overlap or skip channels."
+            )
+
+    # ---------- Tier C: round-trip to physical register ---------------
+
+    def test_all_callers_pass_one_based_channel(self):
+        """
+        INVARIANT: every caller's channel argument must, for ch in
+        {0,1,2,3}, evaluate to a 1-based ADI channel index in {1,2,3,4}.
+
+        The bug fixed in #90 was that helpers used ``channel & 0x03``
+        directly, so a caller passing bare ``ch`` (0..3) appeared to
+        work for ch=0..2 and silently aliased ch=3 onto CH4-then-CH1.
+        After the fix, helpers do ``(channel - 1) & 0x03`` and reject
+        ``channel < 1 || channel > 4``.  A future caller written as
+        ``adarSetRxPhase(dev, ch, ...)`` (bare 0-based) or
+        ``adarSetRxPhase(dev, 0, ...)`` (literal 0) would silently be
+        dropped by the bounds-check at runtime; this test catches it at
+        CI time instead.
+
+        The check intentionally lives one tier above the round-trip test
+        so the failure message points the reader at the API contract
+        (1-based per ADI datasheet & ADAR1000_AGC.cpp:76) rather than at
+        a register-arithmetic mismatch.
+        """
+        offenders = []
+        for setter in self._SETTERS:
+            callers = _extract_adar_caller_sites(self.sources, setter)
+            for filename, line_no, ch_expr in callers:
+                for ch in range(4):
+                    try:
+                        channel_val = _safe_eval_int_expr(ch_expr, ch=ch)
+                    except (NameError, KeyError, ValueError) as e:
+                        offenders.append(
+                            f"  - {filename}:{line_no} {setter}("
+                            f"…, `{ch_expr}`, …) -- ch={ch}: "
+                            f"unparseable ({e})"
+                        )
+                        continue
+                    if channel_val not in (1, 2, 3, 4):
+                        offenders.append(
+                            f"  - {filename}:{line_no} {setter}("
+                            f"…, `{ch_expr}`, …) -- ch={ch}: "
+                            f"channel={channel_val}, expected 1..4"
+                        )
+        assert not offenders, (
+            "ADAR1000 1-based channel API contract violated. The fix "
+            "for issue #90 requires every caller to pass channel in "
+            "{1,2,3,4} (CH1..CH4 per ADI datasheet). Bare 0-based ch "
+            "or a literal 0 will be silently dropped by the helper's "
+            "bounds check.  Offenders:\n" + "\n".join(offenders)
+        )
+
+    @pytest.mark.parametrize(
+        "setter",
+        [
+            "adarSetRxPhase",
+            "adarSetTxPhase",
+            "adarSetRxVgaGain",
+            "adarSetTxVgaGain",
+        ],
+    )
+    def test_round_trip_lands_on_intended_physical_channel(self, setter):
+        """
+        INVARIANT: for every caller of ``<setter>`` and every logical ch
+        in {0,1,2,3}, the effective register address equals
+        REG_CH{ch+1}_*. Catches #90 regardless of fix direction.
+        """
+        callers = _extract_adar_caller_sites(self.sources, setter)
+        assert callers, (
+            f"No callers of `{setter}` found. Either the test scope is "
+            f"incomplete (extend `setup_class.sources`) or the symbol was "
+            f"inlined/removed. A blind test is a dangerous test — "
+            f"investigate before weakening."
+        )
+        helpers = [
+            (b, e, s) for (nm, b, e, s) in self.helper_sites if nm == setter
+        ]
+        assert helpers, f"helper body for `{setter}` not parseable"
+
+        errors = []
+        for filename, line_no, ch_expr in callers:
+            for ch in range(4):
+                try:
+                    channel_val = _safe_eval_int_expr(ch_expr, ch=ch)
+                except (NameError, KeyError, ValueError) as e:
+                    pytest.fail(
+                        f"{filename}:{line_no}: caller channel expression "
+                        f"`{ch_expr}` uses symbol outside {{ch}} or a "
+                        f"disallowed operator ({e}). Extend "
+                        f"_safe_eval_int_expr variables or rewrite the "
+                        f"call site with a supported expression."
+                    )
+                for base_sym, offset_expr, stride in helpers:
+                    try:
+                        offset = _safe_eval_int_expr(
+                            offset_expr, channel=channel_val,
+                        )
+                    except (NameError, KeyError, ValueError) as e:
+                        pytest.fail(
+                            f"helper `{setter}` offset expr "
+                            f"`{offset_expr}` uses symbol outside "
+                            f"{{channel}} or a disallowed operator ({e}). "
+                            f"Extend _safe_eval_int_expr variables if new "
+                            f"driver state is introduced."
+                        )
+                    final = self.reg_map[base_sym] + offset * stride
+                    expected_sym = base_sym.replace("CH1", f"CH{ch + 1}")
+                    expected = self.reg_map[expected_sym]
+                    if final != expected:
+                        errors.append(
+                            f"  - {filename}:{line_no} {setter} "
+                            f"caller `{ch_expr}` | ch={ch} -> "
+                            f"channel={channel_val} -> "
+                            f"`{base_sym} + ({offset_expr})"
+                            f"{' * ' + str(stride) if stride != 1 else ''}`"
+                            f" = 0x{final:03X} "
+                            f"(expected {expected_sym} = 0x{expected:03X})"
+                        )
+        assert not errors, (
+            f"ADAR1000 channel round-trip FAILED for {setter} "
+            f"({len(errors)} mismatches) — writes routed to wrong physical "
+            f"channel. This is issue #90.\n" + "\n".join(errors)
+        )
+
+
 class TestTier1DataPacketLayout:
     """Verify data packet byte layout matches between Python and Verilog."""
 

README.md

@@ -7,7 +7,6 @@
 [![Frequency: 10.5GHz](https://img.shields.io/badge/Frequency-10.5GHz-blue)](https://github.com/NawfalMotii79/PLFM_RADAR)
 [![PRs Welcome](https://img.shields.io/badge/PRs-welcome-brightgreen.svg)](https://github.com/NawfalMotii79/PLFM_RADAR/pulls)
 
-![AERIS-10 Radar System](https://raw.githubusercontent.com/NawfalMotii79/PLFM_RADAR/main/8_Utils/3fb1dabf-2c6d-4b5d-b471-48bc461ce914.jpg)
 
 AERIS-10 is an open-source, low-cost 10.5 GHz phased array radar system featuring Pulse Linear Frequency Modulated (LFM) modulation. Available in two versions (3km and 20km range), it's designed for researchers, drone developers, and serious SDR enthusiasts who want to explore and experiment with phased array radar technology.
 
@@ -47,7 +46,7 @@ The AERIS-10 main sub-systems are:
 
 - **Main Board** containing:
   - **DAC** - Generates the RADAR Chirps
-  - **2x Microwave Mixers (LT5552)** - For up-conversion and IF-down-conversion
+  - **2x Microwave Mixers (LTC5552)** - For up-conversion and IF-down-conversion
   - **4x 4-Channel Phase Shifters (ADAR1000)** - For RX and TX chain beamforming
   - **16x Front End Chips (ADTR1107)** - Used for both Low Noise Amplifying (RX) and Power Amplifying (TX) stages
   - **XC7A50T FPGA** - Handles RADAR Signal Processing on the upstream FTG256 board:
@@ -92,7 +91,7 @@ The AERIS-10 main sub-systems are:
 ### Processing Pipeline
 
 1. **Waveform Generation** - DAC creates LFM chirps
-2. **Up/Down Conversion** - LT5552 mixers handle frequency translation
+2. **Up/Down Conversion** - LTC5552 mixers handle frequency translation
 3. **Beam Steering** - ADAR1000 phase shifters control 16 elements
 4. **Signal Processing (FPGA)**:
    - Raw ADC data capture