Merge pull request #136 from joyshmitz/fix/develop-ci-green-restore

fix(ci): restore develop CI green — ruff T20 exemption + ADAR1000 setter regex extension

8_Utils/Python/Gen_Triangular.py

@@ -41,7 +41,7 @@ plt.xlim(0, (fmax+fmax/10))
 plt.ylim(0, 20)
 
 plt.subplot(122)
-plt.plot(2*n, y)
+plt.plot(t, x)
 plt.xlabel('Time (s)')
 plt.ylabel('Amplitude')
 plt.tight_layout()

8_Utils/Python/LUT.py

@@ -21,4 +21,4 @@ y_scaled = np.round(y * 127.5).astype(int)  # Scale to 8-bit range (0-255)
 
 # Print values in Verilog-friendly format
 for _i in range(n):
-    pass
+    print(f"8'd{y_scaled[_i]},")

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

@@ -124,7 +124,10 @@ bool ADAR1000Manager::powerUpSystem() {
 
     // Start in RX mode
     DIAG("BF", "Setting initial mode to RX");
-    switchToRXMode();
+    if (!switchToRXMode()) {
+        DIAG_ERR("BF", "Initial switchToRXMode() FAILED -- leaving in last-known mode");
+        return false;
+    }
 
     DIAG_ELAPSED("BF", "powerUpSystem() total", t0);
     const uint8_t success[] = "System Power-Up Sequence Completed Successfully.\r\n";
@@ -135,7 +138,11 @@ bool ADAR1000Manager::powerUpSystem() {
 bool ADAR1000Manager::powerDownSystem() {
     DIAG_SECTION("BF POWER-DOWN SEQUENCE");
     DIAG("BF", "Switching to RX mode before power-down");
-    switchToRXMode();
+    // Note: even if RX-mode switch fails partially, we still cut the rails
+    // below. Power-down must always proceed -- a stuck PA bias would be
+    // worse than losing the RX-mode telemetry. We capture the status to
+    // return at the end.
+    bool rx_ok = switchToRXMode();
     HAL_Delay(10);
 
     DIAG("BF", "Disabling PA supplies");
@@ -147,95 +154,119 @@ bool ADAR1000Manager::powerDownSystem() {
     DIAG("BF", "Disabling VDD_SW rail");
     HAL_GPIO_WritePin(EN_P_3V3_VDD_SW_GPIO_Port, EN_P_3V3_VDD_SW_Pin, GPIO_PIN_RESET);
 
-    DIAG("BF", "powerDownSystem() complete");
-    return true;
+    DIAG("BF", "powerDownSystem() %s", rx_ok ? "complete" : "complete (RX-mode setup had failures, rails cut anyway)");
+    return rx_ok;
 }
 
 // Mode Switching
-void ADAR1000Manager::switchToTXMode() {
+bool ADAR1000Manager::switchToTXMode() {
     DIAG_SECTION("BF SWITCH TO TX MODE");
+    bool ok = true;
     DIAG("BF", "Step 1: LNA bias OFF");
-    setLNABias(false);
+    ok = setLNABias(false) && ok;
     delayUs(10);
     DIAG("BF", "Step 2: Enable PA supplies");
     enablePASupplies();
     delayUs(100);
     DIAG("BF", "Step 3: PA bias ON");
-    setPABias(true);
+    ok = setPABias(true) && ok;
     delayUs(50);
     DIAG("BF", "Step 4: ADTR1107 -> TX");
-    setADTR1107Control(true);
+    ok = setADTR1107Control(true) && ok;
 
     for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
-        adarWrite(dev, REG_RX_ENABLES, 0x00, BROADCAST_OFF);
-        adarWrite(dev, REG_TX_ENABLES, 0x0F, BROADCAST_OFF);
-        adarSetTxBias(dev, BROADCAST_OFF);
-        devices_[dev]->current_mode = BeamDirection::TX;
-        DIAG("BF", "  dev[%u] TX enables=0x0F, TX bias set", dev);
+        bool dev_ok = true;
+        dev_ok = adarWrite(dev, REG_RX_ENABLES, 0x00, BROADCAST_OFF) && dev_ok;
+        dev_ok = adarWrite(dev, REG_TX_ENABLES, 0x0F, BROADCAST_OFF) && dev_ok;
+        dev_ok = adarSetTxBias(dev, BROADCAST_OFF) && dev_ok;
+        if (dev_ok) {
+            devices_[dev]->current_mode = BeamDirection::TX;
+            DIAG("BF", "  dev[%u] TX enables=0x0F, TX bias set", dev);
+        } else {
+            DIAG_ERR("BF", "  dev[%u] TX setup FAILED -- per-device current_mode unchanged", dev);
+            ok = false;
+        }
     }
-    current_mode_ = BeamDirection::TX;
-    DIAG("BF", "switchToTXMode() complete");
+    if (ok) current_mode_ = BeamDirection::TX;
+    DIAG("BF", "switchToTXMode() %s", ok ? "complete" : "completed WITH FAILURES (mode unchanged)");
+    return ok;
 }
 
-void ADAR1000Manager::switchToRXMode() {
+bool ADAR1000Manager::switchToRXMode() {
     DIAG_SECTION("BF SWITCH TO RX MODE");
+    bool ok = true;
     DIAG("BF", "Step 1: PA bias OFF");
-    setPABias(false);
+    ok = setPABias(false) && ok;
     delayUs(50);
     DIAG("BF", "Step 2: Disable PA supplies");
     disablePASupplies();
     delayUs(10);
     DIAG("BF", "Step 3: ADTR1107 -> RX");
-    setADTR1107Control(false);
+    ok = setADTR1107Control(false) && ok;
     DIAG("BF", "Step 4: Enable LNA supplies");
     enableLNASupplies();
     delayUs(50);
     DIAG("BF", "Step 5: LNA bias ON");
-    setLNABias(true);
+    ok = setLNABias(true) && ok;
     delayUs(50);
 
     for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
-        adarWrite(dev, REG_TX_ENABLES, 0x00, BROADCAST_OFF);
-        adarWrite(dev, REG_RX_ENABLES, 0x0F, BROADCAST_OFF);
-        devices_[dev]->current_mode = BeamDirection::RX;
-        DIAG("BF", "  dev[%u] RX enables=0x0F", dev);
+        bool dev_ok = true;
+        dev_ok = adarWrite(dev, REG_TX_ENABLES, 0x00, BROADCAST_OFF) && dev_ok;
+        dev_ok = adarWrite(dev, REG_RX_ENABLES, 0x0F, BROADCAST_OFF) && dev_ok;
+        if (dev_ok) {
+            devices_[dev]->current_mode = BeamDirection::RX;
+            DIAG("BF", "  dev[%u] RX enables=0x0F", dev);
+        } else {
+            DIAG_ERR("BF", "  dev[%u] RX setup FAILED -- per-device current_mode unchanged", dev);
+            ok = false;
+        }
     }
-    current_mode_ = BeamDirection::RX;
-    DIAG("BF", "switchToRXMode() complete");
+    if (ok) current_mode_ = BeamDirection::RX;
+    DIAG("BF", "switchToRXMode() %s", ok ? "complete" : "completed WITH FAILURES (mode unchanged)");
+    return ok;
 }
 
-void ADAR1000Manager::fastTXMode() {
+bool ADAR1000Manager::fastTXMode() {
     DIAG("BF", "fastTXMode(): ADTR1107 -> TX (no bias sequencing)");
-    setADTR1107Control(true);
+    bool ok = setADTR1107Control(true);
     for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
-        adarWrite(dev, REG_RX_ENABLES, 0x00, BROADCAST_OFF);
-        adarWrite(dev, REG_TX_ENABLES, 0x0F, BROADCAST_OFF);
-        devices_[dev]->current_mode = BeamDirection::TX;
+        bool dev_ok = true;
+        dev_ok = adarWrite(dev, REG_RX_ENABLES, 0x00, BROADCAST_OFF) && dev_ok;
+        dev_ok = adarWrite(dev, REG_TX_ENABLES, 0x0F, BROADCAST_OFF) && dev_ok;
+        if (dev_ok) devices_[dev]->current_mode = BeamDirection::TX;
+        ok = dev_ok && ok;
     }
-    current_mode_ = BeamDirection::TX;
+    if (ok) current_mode_ = BeamDirection::TX;
+    return ok;
 }
 
-void ADAR1000Manager::fastRXMode() {
+bool ADAR1000Manager::fastRXMode() {
     DIAG("BF", "fastRXMode(): ADTR1107 -> RX (no bias sequencing)");
-    setADTR1107Control(false);
+    bool ok = setADTR1107Control(false);
     for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
-        adarWrite(dev, REG_TX_ENABLES, 0x00, BROADCAST_OFF);
-        adarWrite(dev, REG_RX_ENABLES, 0x0F, BROADCAST_OFF);
-        devices_[dev]->current_mode = BeamDirection::RX;
+        bool dev_ok = true;
+        dev_ok = adarWrite(dev, REG_TX_ENABLES, 0x00, BROADCAST_OFF) && dev_ok;
+        dev_ok = adarWrite(dev, REG_RX_ENABLES, 0x0F, BROADCAST_OFF) && dev_ok;
+        if (dev_ok) devices_[dev]->current_mode = BeamDirection::RX;
+        ok = dev_ok && ok;
     }
-    current_mode_ = BeamDirection::RX;
+    if (ok) current_mode_ = BeamDirection::RX;
+    return ok;
 }
 
-void ADAR1000Manager::pulseTXMode() {
+bool ADAR1000Manager::pulseTXMode() {
     DIAG("BF", "pulseTXMode(): TR switch only");
-    setADTR1107Control(true);
+    bool ok = setADTR1107Control(true);
     last_switch_time_us_ = HAL_GetTick() * 1000;
+    return ok;
 }
 
-void ADAR1000Manager::pulseRXMode() {
+bool ADAR1000Manager::pulseRXMode() {
     DIAG("BF", "pulseRXMode(): TR switch only");
-    setADTR1107Control(false);
+    bool ok = setADTR1107Control(false);
     last_switch_time_us_ = HAL_GetTick() * 1000;
+    return ok;
 }
 
 // Beam Steering
@@ -247,39 +278,36 @@ bool ADAR1000Manager::setBeamAngle(float angle_degrees, BeamDirection direction)
     DIAG("BF", "  phase[0..3] = %u, %u, %u, %u",
          phase_settings[0], phase_settings[1], phase_settings[2], phase_settings[3]);
 
-    if (direction == BeamDirection::TX) {
-        setAllDevicesTXMode();
-    } else {
-        setAllDevicesRXMode();
-    }
+    bool ok = (direction == BeamDirection::TX) ? setAllDevicesTXMode() : setAllDevicesRXMode();
 
     for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
         for (uint8_t ch = 0; ch < 4; ++ch) {
             if (direction == BeamDirection::TX) {
-                adarSetTxPhase(dev, ch + 1, phase_settings[ch], BROADCAST_OFF);
-                adarSetTxVgaGain(dev, ch + 1, kDefaultTxVgaGain, BROADCAST_OFF);
+                ok = adarSetTxPhase(dev, ch + 1, phase_settings[ch], BROADCAST_OFF) && ok;
+                ok = adarSetTxVgaGain(dev, ch + 1, kDefaultTxVgaGain, BROADCAST_OFF) && ok;
             } else {
-                adarSetRxPhase(dev, ch + 1, phase_settings[ch], BROADCAST_OFF);
-                adarSetRxVgaGain(dev, ch + 1, kDefaultRxVgaGain, BROADCAST_OFF);
+                ok = adarSetRxPhase(dev, ch + 1, phase_settings[ch], BROADCAST_OFF) && ok;
+                ok = adarSetRxVgaGain(dev, ch + 1, kDefaultRxVgaGain, BROADCAST_OFF) && ok;
             }
         }
     }
-    return true;
+    return ok;
 }
 
 bool ADAR1000Manager::setCustomBeamPattern(const uint8_t phase_settings[4], const uint8_t gain_settings[4], BeamDirection direction) {
+    bool ok = true;
     for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
         for (uint8_t ch = 0; ch < 4; ++ch) {
             if (direction == BeamDirection::TX) {
-                adarSetTxPhase(dev, ch + 1, phase_settings[ch], BROADCAST_OFF);
-                adarSetTxVgaGain(dev, ch + 1, gain_settings[ch], BROADCAST_OFF);
+                ok = adarSetTxPhase(dev, ch + 1, phase_settings[ch], BROADCAST_OFF) && ok;
+                ok = adarSetTxVgaGain(dev, ch + 1, gain_settings[ch], BROADCAST_OFF) && ok;
             } else {
-                adarSetRxPhase(dev, ch + 1, phase_settings[ch], BROADCAST_OFF);
-                adarSetRxVgaGain(dev, ch + 1, gain_settings[ch], BROADCAST_OFF);
+                ok = adarSetRxPhase(dev, ch + 1, phase_settings[ch], BROADCAST_OFF) && ok;
+                ok = adarSetRxVgaGain(dev, ch + 1, gain_settings[ch], BROADCAST_OFF) && ok;
             }
         }
     }
-    return true;
+    return ok;
 }
 
 // Beam Sweeping
@@ -334,7 +362,17 @@ float ADAR1000Manager::readTemperature(uint8_t deviceIndex) {
         return -273.15f;
     }
 
-    uint8_t temp_raw = adarAdcRead(deviceIndex, BROADCAST_OFF);
+    // Snapshot the timeout counter so we can detect a timeout that occurred
+    // anywhere inside adarAdcRead (start-conv write, polling, output read).
+    // This keeps the float signature while still giving callers an explicit
+    // "this reading is invalid" channel via std::isnan().
+    uint32_t timeouts_before = comm_stats_.adc_timeouts;
+    uint8_t  temp_raw        = adarAdcRead(deviceIndex, BROADCAST_OFF);
+    if (comm_stats_.adc_timeouts > timeouts_before) {
+        DIAG_WARN("BF", "readTemperature(dev[%u]): ADC timeout/comm-fail -- returning NaN", deviceIndex);
+        return std::nanf("");
+    }
+
     float temp_c = (temp_raw * 0.5f) - 50.0f;
     DIAG("BF", "readTemperature(dev[%u]): raw=0x%02X => %.1f C", deviceIndex, temp_raw, (double)temp_c);
     return temp_c;
@@ -346,10 +384,21 @@ bool ADAR1000Manager::verifyDeviceCommunication(uint8_t deviceIndex) {
         return false;
     }
 
-    uint8_t test_value = 0xA5;
-    adarWrite(deviceIndex, REG_SCRATCHPAD, test_value, BROADCAST_OFF);
+    // Distinguish three failure modes that previously all looked the same:
+    //   1. scratchpad write failed at the SPI layer
+    //   2. scratchpad read failed at the SPI layer
+    //   3. value round-tripped but didn't match (real chip mismatch)
+    constexpr uint8_t test_value = 0xA5;
+    if (!adarWrite(deviceIndex, REG_SCRATCHPAD, test_value, BROADCAST_OFF)) {
+        DIAG_ERR("BF", "verifyDeviceComm(dev[%u]): scratchpad WRITE failed", deviceIndex);
+        return false;
+    }
     HAL_Delay(1);
-    uint8_t readback = adarRead(deviceIndex, REG_SCRATCHPAD);
+    uint8_t readback = 0;
+    if (!adarReadChecked(deviceIndex, REG_SCRATCHPAD, &readback)) {
+        DIAG_ERR("BF", "verifyDeviceComm(dev[%u]): scratchpad READ failed", deviceIndex);
+        return false;
+    }
     bool pass = (readback == test_value);
     if (pass) {
         DIAG("BF", "verifyDeviceComm(dev[%u]): scratchpad 0xA5 -> 0x%02X OK", deviceIndex, readback);
@@ -363,8 +412,8 @@ uint8_t ADAR1000Manager::readRegister(uint8_t deviceIndex, uint32_t address) {
     return adarRead(deviceIndex, address);
 }
 
-void ADAR1000Manager::writeRegister(uint8_t deviceIndex, uint32_t address, uint8_t value) {
-    adarWrite(deviceIndex, address, value, BROADCAST_OFF);
+bool ADAR1000Manager::writeRegister(uint8_t deviceIndex, uint32_t address, uint8_t value) {
+    return adarWrite(deviceIndex, address, value, BROADCAST_OFF);
 }
 
 // Configuration
@@ -412,7 +461,10 @@ bool ADAR1000Manager::initializeAllDevices() {
     }
 
     DIAG("BF", "All 4 ADAR1000 devices initialized, setting TX mode");
-    setAllDevicesTXMode();
+    if (!setAllDevicesTXMode()) {
+        DIAG_ERR("BF", "initializeAllDevices: setAllDevicesTXMode() failed");
+        return false;
+    }
     return true;
 }
 
@@ -420,23 +472,38 @@ bool ADAR1000Manager::initializeSingleDevice(uint8_t deviceIndex) {
     if (deviceIndex >= devices_.size()) return false;
 
     DIAG("BF", "  dev[%u] soft reset", deviceIndex);
-    adarSoftReset(deviceIndex);
+    if (!adarSoftReset(deviceIndex)) {
+        DIAG_ERR("BF", "  dev[%u] soft reset FAILED -- aborting init", deviceIndex);
+        return false;
+    }
     HAL_Delay(10);
 
     DIAG("BF", "  dev[%u] write ConfigA (SDO_ACTIVE)", deviceIndex);
-    adarWriteConfigA(deviceIndex, INTERFACE_CONFIG_A_SDO_ACTIVE, BROADCAST_OFF);
+    if (!adarWriteConfigA(deviceIndex, INTERFACE_CONFIG_A_SDO_ACTIVE, BROADCAST_OFF)) {
+        DIAG_ERR("BF", "  dev[%u] ConfigA write FAILED -- aborting init", deviceIndex);
+        return false;
+    }
     DIAG("BF", "  dev[%u] set RAM bypass (bias+beam)", deviceIndex);
-    adarSetRamBypass(deviceIndex, BROADCAST_OFF);
+    if (!adarSetRamBypass(deviceIndex, BROADCAST_OFF)) {
+        DIAG_ERR("BF", "  dev[%u] RAM bypass write FAILED -- aborting init", deviceIndex);
+        return false;
+    }
 
     // Initialize ADC
     DIAG("BF", "  dev[%u] enable ADC (2MHz clk)", deviceIndex);
-    adarWrite(deviceIndex, REG_ADC_CONTROL, ADAR1000_ADC_2MHZ_CLK | ADAR1000_ADC_EN, BROADCAST_OFF);
+    if (!adarWrite(deviceIndex, REG_ADC_CONTROL, ADAR1000_ADC_2MHZ_CLK | ADAR1000_ADC_EN, BROADCAST_OFF)) {
+        DIAG_ERR("BF", "  dev[%u] ADC enable write FAILED -- aborting init", deviceIndex);
+        return false;
+    }
 
-    // Verify communication with scratchpad test
+    // Verify communication with scratchpad test. Previous behavior was to log
+    // a warning and mark the device initialized anyway -- that hid completely
+    // dead chips behind a green init. Now this is a hard failure.
     DIAG("BF", "  dev[%u] verifying SPI communication...", deviceIndex);
-    bool comms_ok = verifyDeviceCommunication(deviceIndex);
-    if (!comms_ok) {
-        DIAG_WARN("BF", "  dev[%u] scratchpad verify FAILED but marking initialized anyway", deviceIndex);
+    if (!verifyDeviceCommunication(deviceIndex)) {
+        DIAG_ERR("BF", "  dev[%u] scratchpad verify FAILED -- NOT marking initialized", deviceIndex);
+        devices_[deviceIndex]->initialized = false;
+        return false;
     }
 
     devices_[deviceIndex]->initialized = true;
@@ -466,15 +533,21 @@ bool ADAR1000Manager::initializeADTR1107Sequence() {
 
     // Step 4: Set CTRL_SW to RX mode initially via GPIO
     DIAG("BF", "Step 4: CTRL_SW -> RX (initial safe mode)");
-    setADTR1107Control(false); // RX mode
+    if (!setADTR1107Control(false)) {
+        DIAG_ERR("BF", "ADTR1107 step 4 FAILED -- aborting power sequence");
+        return false;
+    }
     HAL_Delay(1);
 
     // Step 5: Set VGG_LNA to 0
     DIAG("BF", "Step 5: VGG_LNA bias -> OFF (0x%02X)", kLnaBiasOff);
     uint8_t lna_bias_voltage = kLnaBiasOff;
     for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
-        adarWrite(dev, REG_LNA_BIAS_ON, lna_bias_voltage, BROADCAST_OFF);
-        adarWrite(dev, REG_LNA_BIAS_OFF, kLnaBiasOff, BROADCAST_OFF);
+        if (!adarWrite(dev, REG_LNA_BIAS_ON, lna_bias_voltage, BROADCAST_OFF) ||
+            !adarWrite(dev, REG_LNA_BIAS_OFF, kLnaBiasOff, BROADCAST_OFF)) {
+            DIAG_ERR("BF", "ADTR1107 step 5 dev[%u] LNA bias write FAILED", dev);
+            return false;
+        }
     }
 
     // Step 6: Set VDD_LNA to 0V for TX mode
@@ -489,10 +562,14 @@ bool ADAR1000Manager::initializeADTR1107Sequence() {
     DIAG("BF", "Step 7: VGG_PA -> safe bias 0x%02X (~ -1.75V, PA off)", kPaBiasTxSafe);
     uint8_t safe_pa_bias = kPaBiasTxSafe; // Safe negative voltage (-1.75V) to keep PA off
     for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
-        adarWrite(dev, REG_PA_CH1_BIAS_ON, safe_pa_bias, BROADCAST_OFF);
-        adarWrite(dev, REG_PA_CH2_BIAS_ON, safe_pa_bias, BROADCAST_OFF);
-        adarWrite(dev, REG_PA_CH3_BIAS_ON, safe_pa_bias, BROADCAST_OFF);
-        adarWrite(dev, REG_PA_CH4_BIAS_ON, safe_pa_bias, BROADCAST_OFF);
+        if (!adarWrite(dev, REG_PA_CH1_BIAS_ON, safe_pa_bias, BROADCAST_OFF) ||
+            !adarWrite(dev, REG_PA_CH2_BIAS_ON, safe_pa_bias, BROADCAST_OFF) ||
+            !adarWrite(dev, REG_PA_CH3_BIAS_ON, safe_pa_bias, BROADCAST_OFF) ||
+            !adarWrite(dev, REG_PA_CH4_BIAS_ON, safe_pa_bias, BROADCAST_OFF)) {
+            DIAG_ERR("BF", "ADTR1107 step 7 dev[%u] safe PA bias write FAILED -- aborting before enabling supplies",
+                     dev);
+            return false;
+        }
     }
     HAL_Delay(10);
 
@@ -509,10 +586,13 @@ bool ADAR1000Manager::initializeADTR1107Sequence() {
     DIAG("BF", "Step 9: VGG_PA -> Idq cal bias 0x%02X (~ -0.24V, target 220mA)", kPaBiasIdqCalibration);
     uint8_t Idq_pa_bias = kPaBiasIdqCalibration; // Safe negative voltage (-0.2447V) to keep PA off
     for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
-        adarWrite(dev, REG_PA_CH1_BIAS_ON, Idq_pa_bias, BROADCAST_OFF);
-        adarWrite(dev, REG_PA_CH2_BIAS_ON, Idq_pa_bias, BROADCAST_OFF);
-        adarWrite(dev, REG_PA_CH3_BIAS_ON, Idq_pa_bias, BROADCAST_OFF);
-        adarWrite(dev, REG_PA_CH4_BIAS_ON, Idq_pa_bias, BROADCAST_OFF);
+        if (!adarWrite(dev, REG_PA_CH1_BIAS_ON, Idq_pa_bias, BROADCAST_OFF) ||
+            !adarWrite(dev, REG_PA_CH2_BIAS_ON, Idq_pa_bias, BROADCAST_OFF) ||
+            !adarWrite(dev, REG_PA_CH3_BIAS_ON, Idq_pa_bias, BROADCAST_OFF) ||
+            !adarWrite(dev, REG_PA_CH4_BIAS_ON, Idq_pa_bias, BROADCAST_OFF)) {
+            DIAG_ERR("BF", "ADTR1107 step 9 dev[%u] Idq cal bias write FAILED", dev);
+            return false;
+        }
     }
     HAL_Delay(10);
 
@@ -526,162 +606,175 @@ bool ADAR1000Manager::initializeADTR1107Sequence() {
 
 bool ADAR1000Manager::setAllDevicesTXMode() {
     DIAG("BF", "setAllDevicesTXMode(): ADTR1107 -> TX, then configure ADAR1000s");
-    // Set ADTR1107 to TX mode first
-    setADTR1107Mode(BeamDirection::TX);
-
-    // Then configure ADAR1000 for TX
-    for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
-        // Disable RX first
-        adarWrite(dev, REG_RX_ENABLES, 0x00, BROADCAST_OFF);
-
-        // Enable TX channels and set bias
-        adarWrite(dev, REG_TX_ENABLES, 0x0F, BROADCAST_OFF); // Enable all 4 channels
-        adarSetTxBias(dev, BROADCAST_OFF);
-
-        devices_[dev]->current_mode = BeamDirection::TX;
-        DIAG("BF", "  dev[%u] TX mode set (enables=0x0F, bias applied)", dev);
+    // Set ADTR1107 to TX mode first. If this fails, do NOT advance state --
+    // software was previously claiming TX mode while hardware stayed in RX.
+    if (!setADTR1107Mode(BeamDirection::TX)) {
+        DIAG_ERR("BF", "setAllDevicesTXMode: ADTR1107 TX setup FAILED -- not updating mode flags");
+        return false;
     }
-    current_mode_ = BeamDirection::TX;
-    return true;
+
+    bool ok = true;
+    for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
+        bool dev_ok = true;
+        dev_ok = adarWrite(dev, REG_RX_ENABLES, 0x00, BROADCAST_OFF) && dev_ok;
+        dev_ok = adarWrite(dev, REG_TX_ENABLES, 0x0F, BROADCAST_OFF) && dev_ok;
+        dev_ok = adarSetTxBias(dev, BROADCAST_OFF) && dev_ok;
+
+        if (dev_ok) {
+            devices_[dev]->current_mode = BeamDirection::TX;
+            DIAG("BF", "  dev[%u] TX mode set (enables=0x0F, bias applied)", dev);
+        } else {
+            DIAG_ERR("BF", "  dev[%u] TX mode setup FAILED -- per-device current_mode unchanged", dev);
+            ok = false;
+        }
+    }
+    if (ok) {
+        current_mode_ = BeamDirection::TX;
+    } else {
+        DIAG_ERR("BF", "setAllDevicesTXMode: at least one device failed -- global current_mode unchanged");
+    }
+    return ok;
 }
 
 bool ADAR1000Manager::setAllDevicesRXMode() {
     DIAG("BF", "setAllDevicesRXMode(): ADTR1107 -> RX, then configure ADAR1000s");
-    // Set ADTR1107 to RX mode first
-    setADTR1107Mode(BeamDirection::RX);
-
-    // Then configure ADAR1000 for RX
-    for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
-        // Disable TX first
-        adarWrite(dev, REG_TX_ENABLES, 0x00, BROADCAST_OFF);
-
-        // Enable RX channels
-        adarWrite(dev, REG_RX_ENABLES, 0x0F, BROADCAST_OFF); // Enable all 4 channels
-
-        devices_[dev]->current_mode = BeamDirection::RX;
-        DIAG("BF", "  dev[%u] RX mode set (enables=0x0F)", dev);
+    if (!setADTR1107Mode(BeamDirection::RX)) {
+        DIAG_ERR("BF", "setAllDevicesRXMode: ADTR1107 RX setup FAILED -- not updating mode flags");
+        return false;
     }
-    current_mode_ = BeamDirection::RX;
-    return true;
+
+    bool ok = true;
+    for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
+        bool dev_ok = true;
+        dev_ok = adarWrite(dev, REG_TX_ENABLES, 0x00, BROADCAST_OFF) && dev_ok;
+        dev_ok = adarWrite(dev, REG_RX_ENABLES, 0x0F, BROADCAST_OFF) && dev_ok;
+
+        if (dev_ok) {
+            devices_[dev]->current_mode = BeamDirection::RX;
+            DIAG("BF", "  dev[%u] RX mode set (enables=0x0F)", dev);
+        } else {
+            DIAG_ERR("BF", "  dev[%u] RX mode setup FAILED -- per-device current_mode unchanged", dev);
+            ok = false;
+        }
+    }
+    if (ok) {
+        current_mode_ = BeamDirection::RX;
+    } else {
+        DIAG_ERR("BF", "setAllDevicesRXMode: at least one device failed -- global current_mode unchanged");
+    }
+    return ok;
 }
 
-void ADAR1000Manager::setADTR1107Mode(BeamDirection direction) {
+bool ADAR1000Manager::setADTR1107Mode(BeamDirection direction) {
+    bool ok = true;
     if (direction == BeamDirection::TX) {
         DIAG_SECTION("ADTR1107 -> TX MODE");
-        setADTR1107Control(true); // TX mode
+        ok = setADTR1107Control(true) && ok;
 
-        // Step 1: Disable LNA power first
         DIAG("BF", "  Disable LNA supplies");
         disableLNASupplies();
         HAL_Delay(5);
 
-        // Step 2: Set LNA bias to safe off value
         DIAG("BF", "  LNA bias -> OFF (0x%02X)", kLnaBiasOff);
         for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
-            adarWrite(dev, REG_LNA_BIAS_ON, kLnaBiasOff, BROADCAST_OFF); // Turn off LNA bias
+            ok = adarWrite(dev, REG_LNA_BIAS_ON, kLnaBiasOff, BROADCAST_OFF) && ok;
         }
         HAL_Delay(5);
 
-        // Step 3: Enable PA power
         DIAG("BF", "  Enable PA supplies");
         enablePASupplies();
         HAL_Delay(10);
 
-        // Step 4: Set PA bias to operational value
         DIAG("BF", "  PA bias -> operational (0x%02X)", kPaBiasOperational);
-        uint8_t operational_pa_bias = kPaBiasOperational; // Maximum bias for full power
+        uint8_t operational_pa_bias = kPaBiasOperational;
         for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
-            adarWrite(dev, REG_PA_CH1_BIAS_ON, operational_pa_bias, BROADCAST_OFF);
-            adarWrite(dev, REG_PA_CH2_BIAS_ON, operational_pa_bias, BROADCAST_OFF);
-            adarWrite(dev, REG_PA_CH3_BIAS_ON, operational_pa_bias, BROADCAST_OFF);
-            adarWrite(dev, REG_PA_CH4_BIAS_ON, operational_pa_bias, BROADCAST_OFF);
+            ok = adarWrite(dev, REG_PA_CH1_BIAS_ON, operational_pa_bias, BROADCAST_OFF) && ok;
+            ok = adarWrite(dev, REG_PA_CH2_BIAS_ON, operational_pa_bias, BROADCAST_OFF) && ok;
+            ok = adarWrite(dev, REG_PA_CH3_BIAS_ON, operational_pa_bias, BROADCAST_OFF) && ok;
+            ok = adarWrite(dev, REG_PA_CH4_BIAS_ON, operational_pa_bias, BROADCAST_OFF) && ok;
         }
         HAL_Delay(5);
 
-        // Step 5: Set TR switch to TX mode
         DIAG("BF", "  TR switch -> TX (TR_SOURCE=1, BIAS_EN)");
         for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
-            adarSetBit(dev, REG_SW_CONTROL, 2, BROADCAST_OFF); // TR_SOURCE = 1 (TX)
-            adarSetBit(dev, REG_MISC_ENABLES, 5, BROADCAST_OFF); // BIAS_EN
+            ok = adarSetBit(dev, REG_SW_CONTROL, 2, BROADCAST_OFF) && ok;
+            ok = adarSetBit(dev, REG_MISC_ENABLES, 5, BROADCAST_OFF) && ok;
         }
-        DIAG("BF", "  ADTR1107 TX mode complete");
+        DIAG("BF", "  ADTR1107 TX mode %s", ok ? "complete" : "completed WITH FAILURES");
 
     } else {
-        // RECEIVE MODE: Enable LNA, Disable PA
         DIAG_SECTION("ADTR1107 -> RX MODE");
-        setADTR1107Control(false); // RX mode
+        ok = setADTR1107Control(false) && ok;
 
-        // Step 1: Disable PA power first
         DIAG("BF", "  Disable PA supplies");
         disablePASupplies();
         HAL_Delay(5);
 
-        // Step 2: Set PA bias to safe negative voltage
         DIAG("BF", "  PA bias -> safe (0x%02X)", kPaBiasRxSafe);
         uint8_t safe_pa_bias = kPaBiasRxSafe;
         for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
-            adarWrite(dev, REG_PA_CH1_BIAS_ON, safe_pa_bias, BROADCAST_OFF);
-            adarWrite(dev, REG_PA_CH2_BIAS_ON, safe_pa_bias, BROADCAST_OFF);
-            adarWrite(dev, REG_PA_CH3_BIAS_ON, safe_pa_bias, BROADCAST_OFF);
-            adarWrite(dev, REG_PA_CH4_BIAS_ON, safe_pa_bias, BROADCAST_OFF);
+            ok = adarWrite(dev, REG_PA_CH1_BIAS_ON, safe_pa_bias, BROADCAST_OFF) && ok;
+            ok = adarWrite(dev, REG_PA_CH2_BIAS_ON, safe_pa_bias, BROADCAST_OFF) && ok;
+            ok = adarWrite(dev, REG_PA_CH3_BIAS_ON, safe_pa_bias, BROADCAST_OFF) && ok;
+            ok = adarWrite(dev, REG_PA_CH4_BIAS_ON, safe_pa_bias, BROADCAST_OFF) && ok;
         }
         HAL_Delay(5);
 
-        // Step 3: Enable LNA power
         DIAG("BF", "  Enable LNA supplies");
         enableLNASupplies();
         HAL_Delay(10);
 
-        // Step 4: Set LNA bias to operational value
         DIAG("BF", "  LNA bias -> operational (0x%02X)", kLnaBiasOperational);
         uint8_t operational_lna_bias = kLnaBiasOperational;
         for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
-            adarWrite(dev, REG_LNA_BIAS_ON, operational_lna_bias, BROADCAST_OFF);
+            ok = adarWrite(dev, REG_LNA_BIAS_ON, operational_lna_bias, BROADCAST_OFF) && ok;
         }
         HAL_Delay(5);
 
-        // Step 5: Set TR switch to RX mode
         DIAG("BF", "  TR switch -> RX (TR_SOURCE=0, LNA_BIAS_OUT_EN)");
         for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
-            adarResetBit(dev, REG_SW_CONTROL, 2, BROADCAST_OFF); // TR_SOURCE = 0 (RX)
-            adarSetBit(dev, REG_MISC_ENABLES, 4, BROADCAST_OFF); // LNA_BIAS_OUT_EN
+            ok = adarResetBit(dev, REG_SW_CONTROL, 2, BROADCAST_OFF) && ok;
+            ok = adarSetBit(dev, REG_MISC_ENABLES, 4, BROADCAST_OFF) && ok;
         }
-        DIAG("BF", "  ADTR1107 RX mode complete");
+        DIAG("BF", "  ADTR1107 RX mode %s", ok ? "complete" : "completed WITH FAILURES");
     }
+    return ok;
 }
 
-void ADAR1000Manager::setADTR1107Control(bool tx_mode) {
+bool ADAR1000Manager::setADTR1107Control(bool tx_mode) {
     DIAG("BF", "setADTR1107Control(%s): setting TR switch on all %u devices, settling %lu us",
          tx_mode ? "TX" : "RX", (unsigned)devices_.size(), (unsigned long)switch_settling_time_us_);
+    bool ok = true;
     for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
-        setTRSwitchPosition(dev, tx_mode);
+        ok = setTRSwitchPosition(dev, tx_mode) && ok;
     }
     delayUs(switch_settling_time_us_);
+    return ok;
 }
 
-void ADAR1000Manager::setTRSwitchPosition(uint8_t deviceIndex, bool tx_mode) {
+bool ADAR1000Manager::setTRSwitchPosition(uint8_t deviceIndex, bool tx_mode) {
     if (tx_mode) {
         // TX mode: Set TR_SOURCE = 1
-        adarSetBit(deviceIndex, REG_SW_CONTROL, 2, BROADCAST_OFF);
-    } else {
-        // RX mode: Set TR_SOURCE = 0
-        adarResetBit(deviceIndex, REG_SW_CONTROL, 2, BROADCAST_OFF);
+        return adarSetBit(deviceIndex, REG_SW_CONTROL, 2, BROADCAST_OFF);
     }
+    // RX mode: Set TR_SOURCE = 0
+    return adarResetBit(deviceIndex, REG_SW_CONTROL, 2, BROADCAST_OFF);
 }
 
 // Add the new public method
 bool ADAR1000Manager::setCustomBeamPattern16(const uint8_t phase_pattern[16], BeamDirection direction) {
+    bool ok = true;
     for (uint8_t dev = 0; dev < 4; ++dev) {
         for (uint8_t ch = 0; ch < 4; ++ch) {
             uint8_t phase = phase_pattern[dev * 4 + ch];
             if (direction == BeamDirection::TX) {
-                adarSetTxPhase(dev, ch + 1, phase, BROADCAST_OFF);
+                ok = adarSetTxPhase(dev, ch + 1, phase, BROADCAST_OFF) && ok;
             } else {
-                adarSetRxPhase(dev, ch + 1, phase, BROADCAST_OFF);
+                ok = adarSetRxPhase(dev, ch + 1, phase, BROADCAST_OFF) && ok;
             }
         }
     }
-    return true;
+    return ok;
 }
 
 void ADAR1000Manager::enablePASupplies() {
@@ -708,25 +801,29 @@ void ADAR1000Manager::disableLNASupplies() {
     HAL_GPIO_WritePin(EN_P_3V3_ADTR_GPIO_Port, EN_P_3V3_ADTR_Pin, GPIO_PIN_RESET);
 }
 
-void ADAR1000Manager::setPABias(bool enable) {
+bool ADAR1000Manager::setPABias(bool enable) {
     uint8_t pa_bias = enable ? kPaBiasOperational : kPaBiasRxSafe; // Operational vs safe bias
     DIAG("BF", "setPABias(%s): bias=0x%02X", enable ? "ON" : "OFF", pa_bias);
 
+    bool ok = true;
     for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
-        adarWrite(dev, REG_PA_CH1_BIAS_ON, pa_bias, BROADCAST_OFF);
-        adarWrite(dev, REG_PA_CH2_BIAS_ON, pa_bias, BROADCAST_OFF);
-        adarWrite(dev, REG_PA_CH3_BIAS_ON, pa_bias, BROADCAST_OFF);
-        adarWrite(dev, REG_PA_CH4_BIAS_ON, pa_bias, BROADCAST_OFF);
+        ok = adarWrite(dev, REG_PA_CH1_BIAS_ON, pa_bias, BROADCAST_OFF) && ok;
+        ok = adarWrite(dev, REG_PA_CH2_BIAS_ON, pa_bias, BROADCAST_OFF) && ok;
+        ok = adarWrite(dev, REG_PA_CH3_BIAS_ON, pa_bias, BROADCAST_OFF) && ok;
+        ok = adarWrite(dev, REG_PA_CH4_BIAS_ON, pa_bias, BROADCAST_OFF) && ok;
     }
+    return ok;
 }
 
-void ADAR1000Manager::setLNABias(bool enable) {
+bool ADAR1000Manager::setLNABias(bool enable) {
     uint8_t lna_bias = enable ? kLnaBiasOperational : kLnaBiasOff; // Operational vs off
     DIAG("BF", "setLNABias(%s): bias=0x%02X", enable ? "ON" : "OFF", lna_bias);
 
+    bool ok = true;
     for (uint8_t dev = 0; dev < devices_.size(); ++dev) {
-        adarWrite(dev, REG_LNA_BIAS_ON, lna_bias, BROADCAST_OFF);
+        ok = adarWrite(dev, REG_LNA_BIAS_ON, lna_bias, BROADCAST_OFF) && ok;
     }
+    return ok;
 }
 
 void ADAR1000Manager::delayUs(uint32_t microseconds) {
@@ -771,7 +868,11 @@ bool ADAR1000Manager::performSystemCalibration() {
 // LOW-LEVEL SPI COMMUNICATION METHODS
 // ============================================================================
 
-uint32_t ADAR1000Manager::spiTransfer(uint8_t* txData, uint8_t* rxData, uint32_t size) {
+void ADAR1000Manager::resetCommStats() {
+    comm_stats_ = {0, 0, 0, 0, 0, 0xFF};
+}
+
+bool ADAR1000Manager::spiTransfer(uint8_t* txData, uint8_t* rxData, uint32_t size) {
     HAL_StatusTypeDef status;
 
     if (rxData) {
@@ -782,9 +883,9 @@ uint32_t ADAR1000Manager::spiTransfer(uint8_t* txData, uint8_t* rxData, uint32_t
 
     if (status != HAL_OK) {
         DIAG_ERR("BF", "SPI1 transfer FAILED: HAL status=%d, size=%lu", (int)status, (unsigned long)size);
+        return false;
     }
-
-    return (status == HAL_OK) ? size : 0;
+    return true;
 }
 
 void ADAR1000Manager::setChipSelect(uint8_t deviceIndex, bool state) {
@@ -794,7 +895,14 @@ void ADAR1000Manager::setChipSelect(uint8_t deviceIndex, bool state) {
                       state ? GPIO_PIN_RESET : GPIO_PIN_SET);
 }
 
-void ADAR1000Manager::adarWrite(uint8_t deviceIndex, uint32_t mem_addr, uint8_t data, uint8_t broadcast) {
+bool ADAR1000Manager::adarWrite(uint8_t deviceIndex, uint32_t mem_addr, uint8_t data, uint8_t broadcast) {
+    if (deviceIndex >= devices_.size()) {
+        comm_stats_.writes_fail++;
+        comm_stats_.last_fail_dev = deviceIndex;
+        DIAG_ERR("BF", "adarWrite(dev[%u]): index out of range", deviceIndex);
+        return false;
+    }
+
     uint8_t instruction[3];
 
     if (broadcast) {
@@ -808,16 +916,38 @@ void ADAR1000Manager::adarWrite(uint8_t deviceIndex, uint32_t mem_addr, uint8_t
     instruction[2] = data;
 
     setChipSelect(deviceIndex, true);
-    spiTransfer(instruction, nullptr, sizeof(instruction));
+    bool ok = spiTransfer(instruction, nullptr, sizeof(instruction));
     setChipSelect(deviceIndex, false);
+
+    if (ok) {
+        comm_stats_.writes_ok++;
+    } else {
+        comm_stats_.writes_fail++;
+        comm_stats_.last_fail_dev = deviceIndex;
+    }
+    return ok;
 }
 
-uint8_t ADAR1000Manager::adarRead(uint8_t deviceIndex, uint32_t mem_addr) {
-    uint8_t instruction[3] = {0};
-    uint8_t rx_buffer[3] = {0};
+bool ADAR1000Manager::adarReadChecked(uint8_t deviceIndex, uint32_t mem_addr, uint8_t* out) {
+    if (out == nullptr) return false;
+    *out = 0;
 
-    // Set SDO active
-    adarWrite(deviceIndex, REG_INTERFACE_CONFIG_A, INTERFACE_CONFIG_A_SDO_ACTIVE, 0);
+    if (deviceIndex >= devices_.size()) {
+        comm_stats_.reads_fail++;
+        comm_stats_.last_fail_dev = deviceIndex;
+        DIAG_ERR("BF", "adarRead(dev[%u]): index out of range", deviceIndex);
+        return false;
+    }
+
+    uint8_t instruction[3] = {0};
+    uint8_t rx_buffer[3]   = {0};
+
+    // Set SDO active. Failure here means we cannot trust the readback that follows.
+    if (!adarWrite(deviceIndex, REG_INTERFACE_CONFIG_A, INTERFACE_CONFIG_A_SDO_ACTIVE, 0)) {
+        comm_stats_.reads_fail++;
+        comm_stats_.last_fail_dev = deviceIndex;
+        return false;
+    }
 
     instruction[0] = 0x80 | ((devices_[deviceIndex]->dev_addr & 0x03) << 5);
     instruction[0] |= ((0xff00 & mem_addr) >> 8);
@@ -825,28 +955,57 @@ uint8_t ADAR1000Manager::adarRead(uint8_t deviceIndex, uint32_t mem_addr) {
     instruction[2] = 0x00;
 
     setChipSelect(deviceIndex, true);
-    spiTransfer(instruction, rx_buffer, sizeof(instruction));
+    bool ok = spiTransfer(instruction, rx_buffer, sizeof(instruction));
     setChipSelect(deviceIndex, false);
 
-    // Set SDO Inactive
-    adarWrite(deviceIndex, REG_INTERFACE_CONFIG_A, 0, 0);
+    // Best-effort: clear SDO active even if the read above failed. Don't let a
+    // failure on the trailing write override the read failure status.
+    bool sdo_off_ok = adarWrite(deviceIndex, REG_INTERFACE_CONFIG_A, 0, 0);
+    (void)sdo_off_ok;  // already counted in writes_*; don't double-count as a read failure.
 
-    return rx_buffer[2];
+    if (!ok) {
+        comm_stats_.reads_fail++;
+        comm_stats_.last_fail_dev = deviceIndex;
+        return false;
+    }
+
+    *out = rx_buffer[2];
+    comm_stats_.reads_ok++;
+    return true;
 }
 
-void ADAR1000Manager::adarSetBit(uint8_t deviceIndex, uint32_t mem_addr, uint8_t bit, uint8_t broadcast) {
-    uint8_t temp = adarRead(deviceIndex, mem_addr);
+uint8_t ADAR1000Manager::adarRead(uint8_t deviceIndex, uint32_t mem_addr) {
+    uint8_t value = 0;
+    (void)adarReadChecked(deviceIndex, mem_addr, &value);
+    return value;
+}
+
+bool ADAR1000Manager::adarSetBit(uint8_t deviceIndex, uint32_t mem_addr, uint8_t bit, uint8_t broadcast) {
+    uint8_t temp = 0;
+    // Critical: read-modify-write must NOT proceed on a failed read, otherwise we
+    // would write back (0 | mask) and clobber every other bit in the register.
+    if (!adarReadChecked(deviceIndex, mem_addr, &temp)) {
+        DIAG_ERR("BF", "adarSetBit(dev[%u], 0x%03lX, bit %u): read failed -- skipping write to avoid corruption",
+                 deviceIndex, (unsigned long)mem_addr, bit);
+        return false;
+    }
     uint8_t data = temp | (1 << bit);
-    adarWrite(deviceIndex, mem_addr, data, broadcast);
+    return adarWrite(deviceIndex, mem_addr, data, broadcast);
 }
 
-void ADAR1000Manager::adarResetBit(uint8_t deviceIndex, uint32_t mem_addr, uint8_t bit, uint8_t broadcast) {
-    uint8_t temp = adarRead(deviceIndex, mem_addr);
+bool ADAR1000Manager::adarResetBit(uint8_t deviceIndex, uint32_t mem_addr, uint8_t bit, uint8_t broadcast) {
+    uint8_t temp = 0;
+    if (!adarReadChecked(deviceIndex, mem_addr, &temp)) {
+        DIAG_ERR("BF", "adarResetBit(dev[%u], 0x%03lX, bit %u): read failed -- skipping write to avoid corruption",
+                 deviceIndex, (unsigned long)mem_addr, bit);
+        return false;
+    }
     uint8_t data = temp & ~(1 << bit);
-    adarWrite(deviceIndex, mem_addr, data, broadcast);
+    return adarWrite(deviceIndex, mem_addr, data, broadcast);
 }
 
-void ADAR1000Manager::adarSoftReset(uint8_t deviceIndex) {
+bool ADAR1000Manager::adarSoftReset(uint8_t deviceIndex) {
+    if (deviceIndex >= devices_.size()) return false;
     DIAG("BF", "adarSoftReset(dev[%u]): addr=0x%02X", deviceIndex, devices_[deviceIndex]->dev_addr);
     uint8_t instruction[3];
     instruction[0] = ((devices_[deviceIndex]->dev_addr & 0x03) << 5);
@@ -854,77 +1013,133 @@ void ADAR1000Manager::adarSoftReset(uint8_t deviceIndex) {
     instruction[2] = 0x81;
 
     setChipSelect(deviceIndex, true);
-    spiTransfer(instruction, nullptr, sizeof(instruction));
+    bool ok = spiTransfer(instruction, nullptr, sizeof(instruction));
     setChipSelect(deviceIndex, false);
+
+    if (ok) {
+        comm_stats_.writes_ok++;
+    } else {
+        comm_stats_.writes_fail++;
+        comm_stats_.last_fail_dev = deviceIndex;
+    }
+    return ok;
 }
 
-void ADAR1000Manager::adarWriteConfigA(uint8_t deviceIndex, uint8_t flags, uint8_t broadcast) {
-    adarWrite(deviceIndex, REG_INTERFACE_CONFIG_A, flags, broadcast);
+bool ADAR1000Manager::adarWriteConfigA(uint8_t deviceIndex, uint8_t flags, uint8_t broadcast) {
+    return adarWrite(deviceIndex, REG_INTERFACE_CONFIG_A, flags, broadcast);
 }
 
-void ADAR1000Manager::adarSetRamBypass(uint8_t deviceIndex, uint8_t broadcast) {
+bool ADAR1000Manager::adarSetRamBypass(uint8_t deviceIndex, uint8_t broadcast) {
     uint8_t data = (MEM_CTRL_BIAS_RAM_BYPASS | MEM_CTRL_BEAM_RAM_BYPASS);
-    adarWrite(deviceIndex, REG_MEM_CTL, data, broadcast);
+    return adarWrite(deviceIndex, REG_MEM_CTL, data, broadcast);
 }
 
-void ADAR1000Manager::adarSetRxPhase(uint8_t deviceIndex, uint8_t channel, uint8_t phase, uint8_t broadcast) {
+bool 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 false;
+    }
     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;
-    uint32_t mem_addr_q = REG_CH1_RX_PHS_Q + (channel & 0x03) * 2;
+    // Subtract 1 to convert 1-based channel to 0-based register offset
+    // 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);
-    adarWrite(deviceIndex, REG_LOAD_WORKING, 0x1, broadcast);
+    bool ok = adarWrite(deviceIndex, mem_addr_i, i_val, broadcast);
+    ok = adarWrite(deviceIndex, mem_addr_q, q_val, broadcast) && ok;
+    ok = adarWrite(deviceIndex, REG_LOAD_WORKING, 0x1, broadcast) && ok;
+    return ok;
 }
 
-void ADAR1000Manager::adarSetTxPhase(uint8_t deviceIndex, uint8_t channel, uint8_t phase, uint8_t broadcast) {
+bool 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 false;
+    }
     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_q = REG_CH1_TX_PHS_Q + (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 - 1) & 0x03) * 2;
 
-    adarWrite(deviceIndex, mem_addr_i, i_val, broadcast);
-    adarWrite(deviceIndex, mem_addr_q, q_val, broadcast);
-    adarWrite(deviceIndex, REG_LOAD_WORKING, 0x1, broadcast);
+    bool ok = adarWrite(deviceIndex, mem_addr_i, i_val, broadcast);
+    ok = adarWrite(deviceIndex, mem_addr_q, q_val, broadcast) && ok;
+    ok = adarWrite(deviceIndex, REG_LOAD_WORKING, 0x1, broadcast) && ok;
+    return ok;
 }
 
-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);
-    adarWrite(deviceIndex, mem_addr, gain, broadcast);
-    adarWrite(deviceIndex, REG_LOAD_WORKING, 0x1, broadcast);
+bool ADAR1000Manager::adarSetRxVgaGain(uint8_t deviceIndex, uint8_t channel, uint8_t gain, uint8_t broadcast) {
+    // 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 false;
+    }
+    uint32_t mem_addr = REG_CH1_RX_GAIN + ((channel - 1) & 0x03);
+    bool ok = adarWrite(deviceIndex, mem_addr, gain, broadcast);
+    ok = adarWrite(deviceIndex, REG_LOAD_WORKING, 0x1, broadcast) && ok;
+    return ok;
 }
 
-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);
-    adarWrite(deviceIndex, mem_addr, gain, broadcast);
-    adarWrite(deviceIndex, REG_LOAD_WORKING, LD_WRK_REGS_LDTX_OVERRIDE, broadcast);
+bool ADAR1000Manager::adarSetTxVgaGain(uint8_t deviceIndex, uint8_t channel, uint8_t gain, uint8_t broadcast) {
+    // 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 false;
+    }
+    uint32_t mem_addr = REG_CH1_TX_GAIN + ((channel - 1) & 0x03);
+    bool ok = adarWrite(deviceIndex, mem_addr, gain, broadcast);
+    ok = adarWrite(deviceIndex, REG_LOAD_WORKING, LD_WRK_REGS_LDTX_OVERRIDE, broadcast) && ok;
+    return ok;
 }
 
-void ADAR1000Manager::adarSetTxBias(uint8_t deviceIndex, uint8_t broadcast) {
-    adarWrite(deviceIndex, REG_BIAS_CURRENT_TX, kTxBiasCurrent, broadcast);
-    adarWrite(deviceIndex, REG_BIAS_CURRENT_TX_DRV, kTxDriverBiasCurrent, broadcast);
-    adarWrite(deviceIndex, REG_LOAD_WORKING, 0x2, broadcast);
+bool ADAR1000Manager::adarSetTxBias(uint8_t deviceIndex, uint8_t broadcast) {
+    bool ok = adarWrite(deviceIndex, REG_BIAS_CURRENT_TX, kTxBiasCurrent, broadcast);
+    ok = adarWrite(deviceIndex, REG_BIAS_CURRENT_TX_DRV, kTxDriverBiasCurrent, broadcast) && ok;
+    ok = adarWrite(deviceIndex, REG_LOAD_WORKING, 0x2, broadcast) && ok;
+    return ok;
 }
 
 uint8_t ADAR1000Manager::adarAdcRead(uint8_t deviceIndex, uint8_t broadcast) {
-    adarWrite(deviceIndex, REG_ADC_CONTROL, ADAR1000_ADC_ST_CONV, broadcast);
+    if (!adarWrite(deviceIndex, REG_ADC_CONTROL, ADAR1000_ADC_ST_CONV, broadcast)) {
+        DIAG_ERR("BF", "adarAdcRead(dev[%u]): ADC start-conversion write failed", deviceIndex);
+        comm_stats_.adc_timeouts++;  // treat as a "no-result" event for caller observability
+        return 0;
+    }
 
-    // Wait for conversion -- WARNING: no timeout, can hang if ADC never completes
     uint32_t t0 = HAL_GetTick();
     uint32_t polls = 0;
-    while (!(adarRead(deviceIndex, REG_ADC_CONTROL) & 0x01)) {
+    uint8_t  ctrl  = 0;
+    while (true) {
+        if (!adarReadChecked(deviceIndex, REG_ADC_CONTROL, &ctrl)) {
+            DIAG_ERR("BF", "adarAdcRead(dev[%u]): ADC poll read failed", deviceIndex);
+            comm_stats_.adc_timeouts++;
+            return 0;
+        }
+        if (ctrl & 0x01) break;
         polls++;
         if (HAL_GetTick() - t0 > 100) {
             DIAG_ERR("BF", "adarAdcRead(dev[%u]): ADC conversion TIMEOUT after %lu ms, %lu polls",
                      deviceIndex, (unsigned long)(HAL_GetTick() - t0), (unsigned long)polls);
+            comm_stats_.adc_timeouts++;
             return 0;
         }
     }
     DIAG("BF", "adarAdcRead(dev[%u]): conversion done in %lu ms (%lu polls)",
          deviceIndex, (unsigned long)(HAL_GetTick() - t0), (unsigned long)polls);
 
-    return adarRead(deviceIndex, REG_ADC_OUT);
+    uint8_t out = 0;
+    if (!adarReadChecked(deviceIndex, REG_ADC_OUT, &out)) {
+        DIAG_ERR("BF", "adarAdcRead(dev[%u]): ADC output read failed", deviceIndex);
+        comm_stats_.adc_timeouts++;
+        return 0;
+    }
+    return out;
 }

9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/ADAR1000_Manager.h

@@ -36,6 +36,19 @@ public:
         }
     };
 
+    // Communication health counters. Incremented by checked SPI helpers; queryable
+    // for telemetry/observability without changing the boolean public contract.
+    // PR2 may promote a richer OpStatus enum; today the policy is bool returns
+    // for control flow + this struct for trends.
+    struct CommStats {
+        uint32_t writes_ok;
+        uint32_t writes_fail;
+        uint32_t reads_ok;
+        uint32_t reads_fail;
+        uint32_t adc_timeouts;
+        uint8_t  last_fail_dev;   // device index of most recent failure (0xFF if none)
+    };
+
     ADAR1000Manager();
     ~ADAR1000Manager();
 
@@ -46,12 +59,12 @@ public:
     bool performSystemCalibration();
 
     // Mode Switching
-    void switchToTXMode();
-    void switchToRXMode();
-    void fastTXMode();
-    void fastRXMode();
-    void pulseTXMode();
-    void pulseRXMode();
+    bool switchToTXMode();
+    bool switchToRXMode();
+    bool fastTXMode();
+    bool fastRXMode();
+    bool pulseTXMode();
+    bool pulseRXMode();
 
     // Beam Steering
     bool setBeamAngle(float angle_degrees, BeamDirection direction);
@@ -68,14 +81,20 @@ public:
     // Device Control
     bool setAllDevicesTXMode();
     bool setAllDevicesRXMode();
-    void setADTR1107Mode(BeamDirection direction);
-    void setADTR1107Control(bool tx_mode);
+    bool setADTR1107Mode(BeamDirection direction);
+    bool setADTR1107Control(bool tx_mode);
 
     // Monitoring and Diagnostics
+    // readTemperature returns NaN when the on-chip ADC times out, so callers
+    // can distinguish a hung chip from a real cold reading via std::isnan().
     float readTemperature(uint8_t deviceIndex);
     bool verifyDeviceCommunication(uint8_t deviceIndex);
     uint8_t readRegister(uint8_t deviceIndex, uint32_t address);
-    void writeRegister(uint8_t deviceIndex, uint32_t address, uint8_t value);
+    bool writeRegister(uint8_t deviceIndex, uint32_t address, uint8_t value);
+
+    // Communication health observability
+    const CommStats& getCommStats() const { return comm_stats_; }
+    void resetCommStats();
 
     // Configuration
     void setSwitchSettlingTime(uint32_t us);
@@ -109,6 +128,9 @@ public:
     std::vector<std::unique_ptr<ADAR1000Device>> devices_;
     BeamDirection current_mode_ = BeamDirection::RX;
 
+    // Comm health counters (zeroed in resetCommStats()).
+    CommStats comm_stats_ = {0, 0, 0, 0, 0, 0xFF};
+
     // Beam Sweeping
     std::vector<BeamConfig> tx_beam_sequence_;
     std::vector<BeamConfig> rx_beam_sequence_;
@@ -142,32 +164,42 @@ public:
     void delayUs(uint32_t microseconds);
 
     // Power Management
+    // PA/LNA supply rails are pure GPIO toggles -- those stay void.
+    // setPABias/setLNABias issue per-device SPI writes, so they propagate.
     void enablePASupplies();
     void disablePASupplies();
     void enableLNASupplies();
     void disableLNASupplies();
-    void setPABias(bool enable);
-    void setLNABias(bool enable);
+    bool setPABias(bool enable);
+    bool setLNABias(bool enable);
 
     // SPI Communication
+    // setChipSelect is a pure GPIO toggle (no failure mode in HAL_GPIO_WritePin).
+    // Everything else returns true on success, false on SPI failure or invalid index.
     void setChipSelect(uint8_t deviceIndex, bool state);
-    uint32_t spiTransfer(uint8_t* txData, uint8_t* rxData, uint32_t size);
-    void adarWrite(uint8_t deviceIndex, uint32_t mem_addr, uint8_t data, uint8_t broadcast);
+    bool spiTransfer(uint8_t* txData, uint8_t* rxData, uint32_t size);
+    bool adarWrite(uint8_t deviceIndex, uint32_t mem_addr, uint8_t data, uint8_t broadcast);
+    // adarRead returns the register byte; on SPI failure it returns 0 and the
+    // failure is reflected in comm_stats_.reads_fail (callers wanting an
+    // explicit ok/fail signal should use adarReadChecked below).
     uint8_t adarRead(uint8_t deviceIndex, uint32_t mem_addr);
-    void adarSetBit(uint8_t deviceIndex, uint32_t mem_addr, uint8_t bit, uint8_t broadcast);
-    void adarResetBit(uint8_t deviceIndex, uint32_t mem_addr, uint8_t bit, uint8_t broadcast);
-    void adarSoftReset(uint8_t deviceIndex);
-    void adarWriteConfigA(uint8_t deviceIndex, uint8_t flags, uint8_t broadcast);
-    void adarSetRamBypass(uint8_t deviceIndex, uint8_t broadcast);
+    bool adarReadChecked(uint8_t deviceIndex, uint32_t mem_addr, uint8_t* out);
+    bool adarSetBit(uint8_t deviceIndex, uint32_t mem_addr, uint8_t bit, uint8_t broadcast);
+    bool adarResetBit(uint8_t deviceIndex, uint32_t mem_addr, uint8_t bit, uint8_t broadcast);
+    bool adarSoftReset(uint8_t deviceIndex);
+    bool adarWriteConfigA(uint8_t deviceIndex, uint8_t flags, uint8_t broadcast);
+    bool adarSetRamBypass(uint8_t deviceIndex, uint8_t broadcast);
 
     // Channel Configuration
-    void adarSetRxPhase(uint8_t deviceIndex, uint8_t channel, uint8_t phase, uint8_t broadcast);
-    void adarSetTxPhase(uint8_t deviceIndex, uint8_t channel, uint8_t phase, uint8_t broadcast);
-    void adarSetRxVgaGain(uint8_t deviceIndex, uint8_t channel, uint8_t gain, uint8_t broadcast);
-    void adarSetTxVgaGain(uint8_t deviceIndex, uint8_t channel, uint8_t gain, uint8_t broadcast);
-    void adarSetTxBias(uint8_t deviceIndex, uint8_t broadcast);
+    bool adarSetRxPhase(uint8_t deviceIndex, uint8_t channel, uint8_t phase, uint8_t broadcast);
+    bool adarSetTxPhase(uint8_t deviceIndex, uint8_t channel, uint8_t phase, uint8_t broadcast);
+    bool adarSetRxVgaGain(uint8_t deviceIndex, uint8_t channel, uint8_t gain, uint8_t broadcast);
+    bool adarSetTxVgaGain(uint8_t deviceIndex, uint8_t channel, uint8_t gain, uint8_t broadcast);
+    bool adarSetTxBias(uint8_t deviceIndex, uint8_t broadcast);
+    // adarAdcRead returns 0 on timeout AND increments comm_stats_.adc_timeouts.
+    // readTemperature() detects the timeout via that counter delta.
     uint8_t adarAdcRead(uint8_t deviceIndex, uint8_t broadcast);
-    void setTRSwitchPosition(uint8_t deviceIndex, bool tx_mode);
+    bool setTRSwitchPosition(uint8_t deviceIndex, bool tx_mode);
 
 private:
 

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

@@ -388,7 +388,12 @@ void systemPowerUpSequence() {
 
     // Step 4: Set to safe TX mode
     DIAG("PWR", "Step 4: setAllDevicesTXMode()");
-    adarManager.setAllDevicesTXMode();
+    if (!adarManager.setAllDevicesTXMode()) {
+        DIAG_ERR("PWR", "setAllDevicesTXMode() FAILED -- calling Error_Handler()");
+        uint8_t err[] = "ERROR: ADAR1000 TX-mode setup failed!\r\n";
+        HAL_UART_Transmit(&huart3, err, sizeof(err)-1, 1000);
+        Error_Handler();
+    }
     DIAG("PWR", "Step 4 OK: All devices set to TX mode");
 
     uint8_t success[] = "Power Up Sequence Completed Successfully\r\n";
@@ -401,9 +406,15 @@ void systemPowerDownSequence() {
     uint8_t msg[] = "Starting Power Down Sequence...\r\n";
     HAL_UART_Transmit(&huart3, msg, sizeof(msg)-1, 1000);
 
-    // Step 1: Set all devices to RX mode (safest state)
+    // Step 1: Set all devices to RX mode (safest state). Failure here is logged
+    // but NOT fatal -- power-down must always proceed to cut the rails below.
+    // Leaving a stuck PA bias would be more dangerous than losing RX-mode telemetry.
     DIAG("PWR", "Step 1: setAllDevicesRXMode()");
-    adarManager.setAllDevicesRXMode();
+    if (!adarManager.setAllDevicesRXMode()) {
+        DIAG_ERR("PWR", "setAllDevicesRXMode() FAILED during power-down -- continuing to cut rails");
+        uint8_t warn[] = "WARNING: RX-mode setup failed during power-down, cutting rails anyway\r\n";
+        HAL_UART_Transmit(&huart3, warn, sizeof(warn)-1, 1000);
+    }
     HAL_Delay(10);
 
     // Step 2: Disable PA power supplies
@@ -480,14 +491,15 @@ void initializeBeamMatrices() {
 void executeChirpSequence(int num_chirps, float T1, float PRI1, float T2, float PRI2) {
     // NOTE: No per-chirp DIAG — this is a us/ns timing-critical path.
     // Only log entry params for post-mortem analysis.
+    
     DIAG("SYS", "executeChirpSequence: num_chirps=%d T1=%.2f PRI1=%.2f T2=%.2f PRI2=%.2f",
          num_chirps, T1, PRI1, T2, PRI2);
     // First chirp sequence (microsecond timing)
     for(int i = 0; i < num_chirps; i++) {
         HAL_GPIO_TogglePin(GPIOD, GPIO_PIN_8); // New chirp signal to FPGA
-        adarManager.pulseTXMode();
+        (void)adarManager.pulseTXMode();
         delay_us((uint32_t)T1);
-        adarManager.pulseRXMode();
+        (void)adarManager.pulseRXMode();
         delay_us((uint32_t)(PRI1 - T1));
     }
 
@@ -496,9 +508,9 @@ void executeChirpSequence(int num_chirps, float T1, float PRI1, float T2, float
     // Second chirp sequence (nanosecond timing)
     for(int i = 0; i < num_chirps; i++) {
     	HAL_GPIO_TogglePin(GPIOD, GPIO_PIN_8); // New chirp signal to FPGA
-        adarManager.pulseTXMode();
+        (void)adarManager.pulseTXMode();
         delay_ns((uint32_t)(T2 * 1000));
-        adarManager.pulseRXMode();
+        (void)adarManager.pulseRXMode();
         delay_ns((uint32_t)((PRI2 - T2) * 1000));
 
     }
@@ -627,7 +639,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) {
@@ -693,6 +705,14 @@ SystemError_t checkSystemHealth(void) {
         }
 
         float temp = adarManager.readTemperature(i);
+        // NaN signals an ADC timeout / comm failure inside the manager. Previously
+        // a hung ADC returned 0, which mapped to -50 C and looked healthy. Map
+        // it to the comm-error bucket so attemptErrorRecovery re-inits the chip.
+        if (isnan(temp)) {
+            current_error = ERROR_ADAR1000_COMM;
+            DIAG_ERR("BF", "Health check: ADAR1000 #%d temperature read returned NaN (ADC timeout)", i);
+            return current_error;
+        }
         if (temp > 85.0f) {
             current_error = ERROR_ADAR1000_TEMP;
             DIAG_ERR("BF", "Health check: ADAR1000 #%d OVERTEMP %.1fC > 85C", i, temp);
@@ -782,11 +802,20 @@ void attemptErrorRecovery(SystemError_t error) {
             break;
 
         case ERROR_ADAR1000_COMM:
-            // Reset ADAR1000 communication
+            // Reset ADAR1000 communication. Previously this discarded the bool
+            // return, so a re-init that failed silently looked like recovery
+            // succeeded -- the next health check would loop right back here.
             DIAG("BF", "Recovery: Re-initializing all ADAR1000 devices");
-            adarManager.initializeAllDevices();
-            HAL_Delay(50);
-            DIAG("BF", "Recovery: ADAR1000 re-init complete");
+            if (!adarManager.initializeAllDevices()) {
+                DIAG_ERR("BF", "Recovery FAILED: ADAR1000 re-init still failing -- escalating to emergency state");
+                uint8_t err[] = "ERROR: ADAR1000 recovery failed, entering emergency state\r\n";
+                HAL_UART_Transmit(&huart3, err, sizeof(err)-1, 1000);
+                system_emergency_state = true;
+                error_count++;
+            } else {
+                HAL_Delay(50);
+                DIAG("BF", "Recovery: ADAR1000 re-init complete");
+            }
             break;
 
         case ERROR_IMU_COMM:
@@ -2054,6 +2083,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
@@ -78,10 +79,17 @@ TESTS_WITH_PLATFORM := test_bug11_platform_spi_transmit_only
 # C++ tests (AGC outer loop)
 TESTS_WITH_CXX := test_agc_outer_loop
 
+# ADAR1000 error/status propagation tests -- link real ADAR1000_Manager.o + mocks
+TESTS_ADAR_STATUS := test_adar_init_aborts_on_scratchpad_mismatch \
+                     test_adar_spi_write_failure_propagates \
+                     test_adar_adc_timeout_returns_nan \
+                     test_adar_mode_switch_does_not_lie \
+                     test_adar_comm_stats_increment
+
 # GPS driver tests (need mocks + GPS source + -lm)
 TESTS_GPS := test_um982_gps
 
-ALL_TESTS := $(TESTS_WITH_REAL) $(TESTS_MOCK_ONLY) $(TESTS_STANDALONE) $(TESTS_WITH_PLATFORM) $(TESTS_WITH_CXX) $(TESTS_GPS)
+ALL_TESTS := $(TESTS_WITH_REAL) $(TESTS_MOCK_ONLY) $(TESTS_STANDALONE) $(TESTS_WITH_PLATFORM) $(TESTS_WITH_CXX) $(TESTS_ADAR_STATUS) $(TESTS_GPS)
 
 .PHONY: all build test clean \
         $(addprefix test_,bug1 bug2 bug3 bug4 bug5 bug6 bug7 bug8 bug9 bug10 bug11 bug12 bug13 bug14 bug15) \
@@ -178,6 +186,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 $@
@@ -200,6 +211,16 @@ test_agc_outer_loop: test_agc_outer_loop.cpp $(CXX_OBJS) $(MOCK_OBJS)
 test_agc: test_agc_outer_loop
 	./test_agc_outer_loop
 
+# --- ADAR1000 status-propagation test rules ---
+# Each test links the real ADAR1000_Manager.cpp (compiled as ADAR1000_Manager.o)
+# against the HAL mock so failure injection drives the production code paths.
+$(TESTS_ADAR_STATUS): %: %.cpp ADAR1000_Manager.o $(MOCK_OBJS)
+	$(CXX) $(CXXFLAGS) $(INCLUDES) $< ADAR1000_Manager.o $(MOCK_OBJS) -o $@
+
+.PHONY: test_adar_status
+test_adar_status: $(TESTS_ADAR_STATUS)
+	@for t in $(TESTS_ADAR_STATUS); do echo "--- $$t ---"; ./$$t || exit 1; done
+
 # --- GPS driver rules ---
 
 $(GPS_OBJ): $(GPS_SRC)

9_Firmware/9_1_Microcontroller/tests/stm32_hal_mock.c

@@ -63,6 +63,12 @@ static struct {
     GPIO_PinState val;
 } gpio_read_table[GPIO_READ_TABLE_SIZE];
 
+/* SPI failure-injection state */
+static int               mock_spi_fail_remaining = 0;
+static HAL_StatusTypeDef mock_spi_fail_status    = HAL_OK;
+static uint8_t           mock_spi_rx_byte        = 0;
+static uint32_t          mock_tick_auto_advance  = 0;
+
 void spy_reset(void)
 {
     spy_count = 0;
@@ -73,6 +79,26 @@ void spy_reset(void)
     memset(mock_uart_rx, 0, sizeof(mock_uart_rx));
     mock_uart_tx_len = 0;
     memset(mock_uart_tx_buf, 0, sizeof(mock_uart_tx_buf));
+    mock_spi_fail_remaining = 0;
+    mock_spi_fail_status    = HAL_OK;
+    mock_spi_rx_byte        = 0;
+    mock_tick_auto_advance  = 0;
+}
+
+void mock_spi_queue_failure(int call_count, HAL_StatusTypeDef status)
+{
+    mock_spi_fail_remaining = call_count;
+    mock_spi_fail_status    = status;
+}
+
+void mock_spi_set_rx_byte(uint8_t value)
+{
+    mock_spi_rx_byte = value;
+}
+
+void mock_set_tick_auto_advance(uint32_t delta)
+{
+    mock_tick_auto_advance = delta;
 }
 
 const SpyRecord *spy_get(int index)
@@ -177,14 +203,16 @@ void HAL_GPIO_TogglePin(GPIO_TypeDef *GPIOx, uint16_t GPIO_Pin)
 
 uint32_t HAL_GetTick(void)
 {
+    uint32_t result = mock_tick;
     spy_push((SpyRecord){
         .type  = SPY_HAL_GET_TICK,
         .port  = NULL,
         .pin   = 0,
-        .value = mock_tick,
+        .value = result,
         .extra = NULL
     });
-    return mock_tick;
+    mock_tick += mock_tick_auto_advance;
+    return result;
 }
 
 void HAL_Delay(uint32_t Delay)
@@ -369,6 +397,7 @@ void mock_tim_set_compare(TIM_HandleTypeDef *htim, uint32_t Channel, uint32_t Co
 
 HAL_StatusTypeDef HAL_SPI_TransmitReceive(SPI_HandleTypeDef *hspi, uint8_t *pTxData, uint8_t *pRxData, uint16_t Size, uint32_t Timeout)
 {
+    (void)pTxData;
     spy_push((SpyRecord){
         .type  = SPY_SPI_TRANSMIT_RECEIVE,
         .port  = NULL,
@@ -376,11 +405,19 @@ HAL_StatusTypeDef HAL_SPI_TransmitReceive(SPI_HandleTypeDef *hspi, uint8_t *pTxD
         .value = Timeout,
         .extra = hspi
     });
+    if (mock_spi_fail_remaining > 0) {
+        mock_spi_fail_remaining--;
+        return mock_spi_fail_status;
+    }
+    if (pRxData && Size > 0) {
+        pRxData[Size - 1] = mock_spi_rx_byte;
+    }
     return HAL_OK;
 }
 
 HAL_StatusTypeDef HAL_SPI_Transmit(SPI_HandleTypeDef *hspi, uint8_t *pData, uint16_t Size, uint32_t Timeout)
 {
+    (void)pData;
     spy_push((SpyRecord){
         .type  = SPY_SPI_TRANSMIT,
         .port  = NULL,
@@ -388,6 +425,10 @@ HAL_StatusTypeDef HAL_SPI_Transmit(SPI_HandleTypeDef *hspi, uint8_t *pData, uint
         .value = Timeout,
         .extra = hspi
     });
+    if (mock_spi_fail_remaining > 0) {
+        mock_spi_fail_remaining--;
+        return mock_spi_fail_status;
+    }
     return HAL_OK;
 }
 

9_Firmware/9_1_Microcontroller/tests/stm32_hal_mock.h

@@ -176,6 +176,11 @@ void mock_set_tick(uint32_t tick);
 /* Advance the mock tick by `delta` ms */
 void mock_advance_tick(uint32_t delta);
 
+/* Each subsequent HAL_GetTick() call advances the mock tick by `delta` ms after
+ * returning the current value. Use to drive timeout loops without wall-clock
+ * waits. Set to 0 (default) for stable tick. */
+void mock_set_tick_auto_advance(uint32_t delta);
+
 /* ========================= Mock GPIO read returns ================= */
 
 /* Set the value HAL_GPIO_ReadPin will return for a specific port/pin */
@@ -212,6 +217,15 @@ void mock_uart_tx_clear(void);
 HAL_StatusTypeDef HAL_SPI_TransmitReceive(SPI_HandleTypeDef *hspi, uint8_t *pTxData, uint8_t *pRxData, uint16_t Size, uint32_t Timeout);
 HAL_StatusTypeDef HAL_SPI_Transmit(SPI_HandleTypeDef *hspi, uint8_t *pData, uint16_t Size, uint32_t Timeout);
 
+/* Queue the next N SPI calls (Transmit and TransmitReceive) to return `status`
+ * instead of HAL_OK. Decremented on each call. Use for failure-propagation tests. */
+void mock_spi_queue_failure(int call_count, HAL_StatusTypeDef status);
+
+/* Set the byte that HAL_SPI_TransmitReceive will write at pRxData[Size-1] for
+ * subsequent calls. ADAR1000 reads land at index 2 of a 3-byte transfer, so
+ * this lets tests inject scratchpad / register readback values. */
+void mock_spi_set_rx_byte(uint8_t value);
+
 /* ========================= no_os compat layer ===================== */
 
 void no_os_udelay(uint32_t usecs);

9_Firmware/9_1_Microcontroller/tests/test_adar_adc_timeout_returns_nan.cpp

@@ -0,0 +1,115 @@
+// test_adar_adc_timeout_returns_nan.cpp
+//
+// readTemperature() previously returned -50.0 C silently when the on-chip
+// ADC never completed a conversion (raw=0 timeout sentinel mapped through
+// (raw * 0.5) - 50). A hung chip looked like a cold radar.
+//
+// New: on ADC timeout or any comm failure inside adarAdcRead(),
+// readTemperature returns NaN, and comm_stats_.adc_timeouts increments.
+// checkSystemHealth() in main.cpp uses isnan() to route NaN to the comm-error
+// bucket (which triggers attemptErrorRecovery).
+
+#include <cassert>
+#include <cmath>
+#include <cstdio>
+
+#include "stm32_hal_mock.h"
+#include "ADAR1000_Manager.h"
+
+uint8_t GUI_start_flag_received = 0;
+uint8_t USB_Buffer[64] = {0};
+extern "C" void Error_Handler(void) {}
+
+static int tests_passed = 0;
+static int tests_total  = 0;
+
+#define RUN_TEST(fn)                                                           \
+    do {                                                                        \
+        tests_total++;                                                          \
+        printf("  [%2d] %-65s ", tests_total, #fn);                            \
+        fn();                                                                   \
+        tests_passed++;                                                         \
+        printf("PASS\n");                                                       \
+    } while (0)
+
+// Helper: bring all 4 devices through init successfully so readTemperature()
+// gets past its "not initialized" guard.
+static void init_devices_clean(ADAR1000Manager& mgr)
+{
+    mock_spi_set_rx_byte(0xA5);
+    bool ok = mgr.initializeAllDevices();
+    assert(ok);
+}
+
+// Default mock returns 0x00 to every read after init. Since bit 0 of the ADC
+// status register never goes high, the polling loop in adarAdcRead is supposed
+// to time out after 100 ms. Auto-advancing the tick on every HAL_GetTick()
+// drives that timer forward without wall-clock waits.
+static void test_polling_timeout_returns_nan()
+{
+    spy_reset();
+    ADAR1000Manager mgr;
+    init_devices_clean(mgr);
+
+    // Switch the rx-byte back to 0x00 so the ADC status bit stays low forever
+    // and the polling loop runs to its 100 ms watchdog.
+    mock_spi_set_rx_byte(0x00);
+    mock_set_tick_auto_advance(150);  // each HAL_GetTick() advances 150 ms
+
+    mgr.resetCommStats();
+    float t = mgr.readTemperature(0);
+
+    assert(std::isnan(t));
+    const auto& stats = mgr.getCommStats();
+    assert(stats.adc_timeouts >= 1);
+}
+
+// SPI failure during adarAdcRead's start-conversion write must also count as
+// a timeout (caller has no valid ADC reading) and produce NaN.
+static void test_start_conv_spi_failure_returns_nan()
+{
+    spy_reset();
+    ADAR1000Manager mgr;
+    init_devices_clean(mgr);
+
+    mgr.resetCommStats();
+    // Fail the very next SPI call -- the start-conversion write inside
+    // adarAdcRead. Following polling reads will also fail, but the function
+    // bails on the start-conv write first.
+    mock_spi_queue_failure(100, HAL_ERROR);
+
+    float t = mgr.readTemperature(0);
+    assert(std::isnan(t));
+
+    const auto& stats = mgr.getCommStats();
+    assert(stats.adc_timeouts >= 1);
+}
+
+// Healthy path: ADC bit 0 is high on the first poll (rx_byte = 0xA5 after init),
+// so adarAdcRead exits the loop immediately, and readTemperature returns a
+// finite number. This guards against false-positive NaN from the new code path.
+static void test_healthy_adc_returns_finite_temp()
+{
+    spy_reset();
+    ADAR1000Manager mgr;
+    init_devices_clean(mgr);
+
+    mgr.resetCommStats();
+    float t = mgr.readTemperature(0);
+
+    assert(!std::isnan(t));
+    const auto& stats = mgr.getCommStats();
+    assert(stats.adc_timeouts == 0);
+}
+
+int main()
+{
+    printf("=== ADAR1000 ADC timeout -> NaN propagation tests ===\n");
+
+    RUN_TEST(test_polling_timeout_returns_nan);
+    RUN_TEST(test_start_conv_spi_failure_returns_nan);
+    RUN_TEST(test_healthy_adc_returns_finite_temp);
+
+    printf("=== Results: %d/%d passed ===\n", tests_passed, tests_total);
+    return (tests_passed == tests_total) ? 0 : 1;
+}

9_Firmware/9_1_Microcontroller/tests/test_adar_comm_stats_increment.cpp

@@ -0,0 +1,191 @@
+// test_adar_comm_stats_increment.cpp
+//
+// Documents and locks the CommStats observability surface added in this PR.
+// Without this test, a future "simplification" could quietly remove the
+// counters and nothing else would catch it.
+//
+// Contract:
+//   - Every successful adarWrite increments writes_ok.
+//   - Every failed   adarWrite increments writes_fail and updates last_fail_dev.
+//   - Every successful adarRead  increments reads_ok.
+//   - Every failed   adarRead  increments reads_fail  and updates last_fail_dev.
+//   - resetCommStats() zeroes all counters and resets last_fail_dev to 0xFF.
+//   - PR2 may promote a richer OpStatus enum return type; this struct is the
+//     forward-compatible observability hook either way.
+
+#include <cassert>
+#include <cstdio>
+
+#include "stm32_hal_mock.h"
+#include "ADAR1000_Manager.h"
+
+uint8_t GUI_start_flag_received = 0;
+uint8_t USB_Buffer[64] = {0};
+extern "C" void Error_Handler(void) {}
+
+static int tests_passed = 0;
+static int tests_total  = 0;
+
+#define RUN_TEST(fn)                                                           \
+    do {                                                                        \
+        tests_total++;                                                          \
+        printf("  [%2d] %-65s ", tests_total, #fn);                            \
+        fn();                                                                   \
+        tests_passed++;                                                         \
+        printf("PASS\n");                                                       \
+    } while (0)
+
+static void test_default_stats_are_zero()
+{
+    spy_reset();
+    ADAR1000Manager mgr;
+
+    const auto& s = mgr.getCommStats();
+    assert(s.writes_ok    == 0);
+    assert(s.writes_fail  == 0);
+    assert(s.reads_ok     == 0);
+    assert(s.reads_fail   == 0);
+    assert(s.adc_timeouts == 0);
+    assert(s.last_fail_dev == 0xFF);
+}
+
+static void test_successful_write_increments_writes_ok()
+{
+    spy_reset();
+    ADAR1000Manager mgr;
+    mgr.resetCommStats();
+
+    bool ok = mgr.writeRegister(0, 0x010, 0x42);
+    assert(ok);
+
+    const auto& s = mgr.getCommStats();
+    assert(s.writes_ok   == 1);
+    assert(s.writes_fail == 0);
+    assert(s.last_fail_dev == 0xFF);
+}
+
+static void test_failed_write_increments_writes_fail_and_records_dev()
+{
+    spy_reset();
+    ADAR1000Manager mgr;
+    mgr.resetCommStats();
+
+    mock_spi_queue_failure(1, HAL_ERROR);
+    bool ok = mgr.writeRegister(2, 0x010, 0x42);  // dev[2]
+    assert(ok == false);
+
+    const auto& s = mgr.getCommStats();
+    assert(s.writes_ok    == 0);
+    assert(s.writes_fail  == 1);
+    assert(s.last_fail_dev == 2);
+}
+
+static void test_successful_read_increments_reads_ok()
+{
+    spy_reset();
+    mock_spi_set_rx_byte(0xA5);
+    ADAR1000Manager mgr;
+    mgr.resetCommStats();
+
+    uint8_t value = 0;
+    bool ok = mgr.adarReadChecked(1, 0x010, &value);
+    assert(ok);
+    assert(value == 0xA5);
+
+    const auto& s = mgr.getCommStats();
+    assert(s.reads_ok   == 1);
+    assert(s.reads_fail == 0);
+}
+
+static void test_failed_read_increments_reads_fail_and_records_dev()
+{
+    spy_reset();
+    ADAR1000Manager mgr;
+    mgr.resetCommStats();
+
+    // adarReadChecked sequence:
+    //   1. adarWrite(SDO active)        -- HAL_SPI_Transmit
+    //   2. HAL_SPI_TransmitReceive (the actual read)   <-- we want this to fail
+    //   3. adarWrite(SDO inactive)
+    // Letting calls 1+2 fail (queue 2 failures) covers the case where SDO-active
+    // also fails -- adarReadChecked aborts after #1 and bumps reads_fail.
+    mock_spi_queue_failure(2, HAL_ERROR);
+
+    uint8_t value = 0xCC;  // pre-poison to confirm out param gets cleared
+    bool ok = mgr.adarReadChecked(3, 0x010, &value);
+    assert(ok == false);
+    assert(value == 0);  // adarReadChecked must zero the out param on failure
+
+    const auto& s = mgr.getCommStats();
+    assert(s.reads_fail >= 1);
+    assert(s.last_fail_dev == 3);
+}
+
+// Reset must clear everything to defaults, including last_fail_dev back to 0xFF.
+static void test_reset_clears_all_counters()
+{
+    spy_reset();
+    ADAR1000Manager mgr;
+
+    // Generate some non-zero state in every field.
+    mgr.writeRegister(0, 0x010, 0x42);            // writes_ok++
+    mock_spi_queue_failure(1, HAL_ERROR);
+    mgr.writeRegister(1, 0x010, 0x42);            // writes_fail++, last_fail_dev=1
+    mock_spi_set_rx_byte(0xA5);
+    uint8_t v = 0;
+    mgr.adarReadChecked(0, 0x010, &v);            // reads_ok++
+
+    {
+        const auto& s = mgr.getCommStats();
+        assert(s.writes_ok     >  0);
+        assert(s.writes_fail   >  0);
+        assert(s.reads_ok      >  0);
+        assert(s.last_fail_dev != 0xFF);
+    }
+
+    mgr.resetCommStats();
+
+    const auto& s = mgr.getCommStats();
+    assert(s.writes_ok     == 0);
+    assert(s.writes_fail   == 0);
+    assert(s.reads_ok      == 0);
+    assert(s.reads_fail    == 0);
+    assert(s.adc_timeouts  == 0);
+    assert(s.last_fail_dev == 0xFF);
+}
+
+// Out-of-range device index counts as a write/read failure (not a silent skip).
+// This is the kind of bug that would otherwise hide behind a "device 4 ignored"
+// log line and never escalate to the caller.
+static void test_out_of_range_device_index_counts_as_failure()
+{
+    spy_reset();
+    ADAR1000Manager mgr;
+    mgr.resetCommStats();
+
+    bool wok = mgr.writeRegister(99, 0x010, 0x42);
+    assert(wok == false);
+    assert(mgr.getCommStats().writes_fail == 1);
+    assert(mgr.getCommStats().last_fail_dev == 99);
+
+    uint8_t v = 0;
+    bool rok = mgr.adarReadChecked(99, 0x010, &v);
+    assert(rok == false);
+    assert(mgr.getCommStats().reads_fail == 1);
+}
+
+int main()
+{
+    printf("=== ADAR1000 CommStats observability tests ===\n");
+
+    RUN_TEST(test_default_stats_are_zero);
+    RUN_TEST(test_successful_write_increments_writes_ok);
+    RUN_TEST(test_failed_write_increments_writes_fail_and_records_dev);
+    RUN_TEST(test_successful_read_increments_reads_ok);
+    RUN_TEST(test_failed_read_increments_reads_fail_and_records_dev);
+    RUN_TEST(test_reset_clears_all_counters);
+    RUN_TEST(test_out_of_range_device_index_counts_as_failure);
+
+    printf("=== Results: %d/%d passed ===\n", tests_passed, tests_total);
+    return (tests_passed == tests_total) ? 0 : 1;
+}

9_Firmware/9_1_Microcontroller/tests/test_adar_init_aborts_on_scratchpad_mismatch.cpp

@@ -0,0 +1,104 @@
+// test_adar_init_aborts_on_scratchpad_mismatch.cpp
+//
+// previously initializeSingleDevice() logged a
+// warning when the scratchpad readback didn't match 0xA5 but still set
+// devices_[i]->initialized = true and returned true. That meant
+// initializeAllDevices() would happily report success with four dead chips.
+//
+// New: any scratchpad mismatch aborts init. The device stays
+// uninitialized, the function returns false, and downstream calls (e.g.
+// readTemperature) return the "not initialized" sentinel -273.15.
+
+#include <cassert>
+#include <cmath>
+#include <cstdio>
+
+#include "stm32_hal_mock.h"
+#include "ADAR1000_Manager.h"
+
+uint8_t GUI_start_flag_received = 0;
+uint8_t USB_Buffer[64] = {0};
+extern "C" void Error_Handler(void) {}
+
+static int tests_passed = 0;
+static int tests_total  = 0;
+
+#define RUN_TEST(fn)                                                           \
+    do {                                                                        \
+        tests_total++;                                                          \
+        printf("  [%2d] %-65s ", tests_total, #fn);                            \
+        fn();                                                                   \
+        tests_passed++;                                                         \
+        printf("PASS\n");                                                       \
+    } while (0)
+
+// With default mock state, HAL_SPI_TransmitReceive writes 0x00 into rx_buffer[2].
+// The scratchpad write programs 0xA5; the readback gets 0x00; mismatch -> abort.
+static void test_scratchpad_mismatch_aborts_init()
+{
+    spy_reset();
+    ADAR1000Manager mgr;
+
+    bool ok = mgr.initializeAllDevices();
+
+    assert(ok == false);  // would have been true under the old bug
+
+    // Downstream proof: readTemperature must report "not initialized" sentinel,
+    // not a temperature value, because the device is not marked initialized.
+    float t = mgr.readTemperature(0);
+    assert(t == -273.15f);
+}
+
+// When scratchpad readback matches, init succeeds. mock_spi_set_rx_byte(0xA5)
+// makes every read return 0xA5, satisfying the verify step.
+static void test_scratchpad_match_lets_init_succeed()
+{
+    spy_reset();
+    mock_spi_set_rx_byte(0xA5);
+    ADAR1000Manager mgr;
+
+    bool ok = mgr.initializeAllDevices();
+
+    assert(ok == true);
+
+    // Now temperature read should produce a real number (not -273.15 sentinel).
+    // adarAdcRead loops on bit 0 of REG_ADC_CONTROL; with rx_byte=0xA5 (bit 0 = 1)
+    // the loop exits immediately, then REG_ADC_OUT also reads 0xA5.
+    float t = mgr.readTemperature(0);
+    assert(!std::isnan(t));
+    assert(t != -273.15f);
+}
+
+// Init aborts on the first device that fails. Stats reflect partial progress
+// (some writes_ok before the abort) which is the trend signal callers will
+// query via getCommStats().
+static void test_init_failure_recorded_in_stats()
+{
+    spy_reset();
+    ADAR1000Manager mgr;
+    mgr.resetCommStats();
+
+    bool ok = mgr.initializeAllDevices();
+    assert(ok == false);
+
+    const auto& stats = mgr.getCommStats();
+    // Several writes happened before scratchpad verify failed: soft reset,
+    // configA, RAM bypass, ADC enable, scratchpad-write itself, and the
+    // SDO-active toggles inside the scratchpad-read.
+    assert(stats.writes_ok > 0);
+    // The scratchpad readback succeeded at the SPI layer (HAL_OK) but produced
+    // a wrong value -- that's not a read failure, it's a verify mismatch. So
+    // reads_fail can stay 0 in the pure-mismatch case.
+}
+
+int main()
+{
+    printf("=== ADAR1000 init scratchpad-mismatch propagation tests ===\n");
+
+    RUN_TEST(test_scratchpad_mismatch_aborts_init);
+    RUN_TEST(test_scratchpad_match_lets_init_succeed);
+    RUN_TEST(test_init_failure_recorded_in_stats);
+
+    printf("=== Results: %d/%d passed ===\n", tests_passed, tests_total);
+    return (tests_passed == tests_total) ? 0 : 1;
+}

9_Firmware/9_1_Microcontroller/tests/test_adar_mode_switch_does_not_lie.cpp

@@ -0,0 +1,113 @@
+// test_adar_mode_switch_does_not_lie.cpp
+//
+// setAllDevicesTXMode / setAllDevicesRXMode previously updated current_mode_
+// (both global and per-device) before issuing the SPI writes that actually
+// reconfigure the chip, then returned true unconditionally. A SPI failure
+// during the mode switch left software believing it was in TX mode while the
+// hardware was still in RX (or vice-versa) -- a real safety hazard given
+// that PA biasing is mode-dependent.
+//
+// New: current_mode_ only updates if every underlying write
+// succeeded; otherwise the function returns false and the mode flag is left
+// at its last-known-good value.
+
+#include <cassert>
+#include <cstdio>
+
+#include "stm32_hal_mock.h"
+#include "ADAR1000_Manager.h"
+
+uint8_t GUI_start_flag_received = 0;
+uint8_t USB_Buffer[64] = {0};
+extern "C" void Error_Handler(void) {}
+
+static int tests_passed = 0;
+static int tests_total  = 0;
+
+#define RUN_TEST(fn)                                                           \
+    do {                                                                        \
+        tests_total++;                                                          \
+        printf("  [%2d] %-65s ", tests_total, #fn);                            \
+        fn();                                                                   \
+        tests_passed++;                                                         \
+        printf("PASS\n");                                                       \
+    } while (0)
+
+static void init_devices_clean(ADAR1000Manager& mgr)
+{
+    mock_spi_set_rx_byte(0xA5);
+    bool ok = mgr.initializeAllDevices();
+    assert(ok);
+}
+
+// After a clean init, the manager is in TX mode (initializeAllDevices ends
+// with setAllDevicesTXMode). Force RX mode under sustained SPI failure: must
+// return false AND must not flip current_mode_ to RX.
+static void test_failed_rx_switch_does_not_update_mode()
+{
+    spy_reset();
+    ADAR1000Manager mgr;
+    init_devices_clean(mgr);
+    assert(mgr.getCurrentMode() == ADAR1000Manager::BeamDirection::TX);
+
+    mgr.resetCommStats();
+    mock_spi_queue_failure(10000, HAL_ERROR);
+
+    bool ok = mgr.setAllDevicesRXMode();
+    assert(ok == false);
+
+    // Mode flag must NOT have moved -- this is the dangerous lie we are fixing.
+    assert(mgr.getCurrentMode() == ADAR1000Manager::BeamDirection::TX);
+}
+
+// Symmetric case: cleanly transition to RX, then fail TX setup. Mode flag
+// must stay RX.
+static void test_failed_tx_switch_does_not_update_mode()
+{
+    spy_reset();
+    ADAR1000Manager mgr;
+    init_devices_clean(mgr);
+
+    mock_spi_set_rx_byte(0xA5);
+    bool rx_ok = mgr.setAllDevicesRXMode();
+    assert(rx_ok);
+    assert(mgr.getCurrentMode() == ADAR1000Manager::BeamDirection::RX);
+
+    mgr.resetCommStats();
+    mock_spi_queue_failure(10000, HAL_ERROR);
+
+    bool ok = mgr.setAllDevicesTXMode();
+    assert(ok == false);
+    assert(mgr.getCurrentMode() == ADAR1000Manager::BeamDirection::RX);
+}
+
+// Healthy round-trip: mode flag tracks the call that was made. Guards against
+// the new code path accidentally refusing to advance the mode on success.
+static void test_clean_mode_switches_update_flag()
+{
+    spy_reset();
+    ADAR1000Manager mgr;
+    init_devices_clean(mgr);
+    assert(mgr.getCurrentMode() == ADAR1000Manager::BeamDirection::TX);
+
+    mock_spi_set_rx_byte(0xA5);
+    bool to_rx = mgr.setAllDevicesRXMode();
+    assert(to_rx);
+    assert(mgr.getCurrentMode() == ADAR1000Manager::BeamDirection::RX);
+
+    bool to_tx = mgr.setAllDevicesTXMode();
+    assert(to_tx);
+    assert(mgr.getCurrentMode() == ADAR1000Manager::BeamDirection::TX);
+}
+
+int main()
+{
+    printf("=== ADAR1000 mode-switch honesty tests ===\n");
+
+    RUN_TEST(test_failed_rx_switch_does_not_update_mode);
+    RUN_TEST(test_failed_tx_switch_does_not_update_mode);
+    RUN_TEST(test_clean_mode_switches_update_flag);
+
+    printf("=== Results: %d/%d passed ===\n", tests_passed, tests_total);
+    return (tests_passed == tests_total) ? 0 : 1;
+}

9_Firmware/9_1_Microcontroller/tests/test_adar_spi_write_failure_propagates.cpp

@@ -0,0 +1,128 @@
+// test_adar_spi_write_failure_propagates.cpp
+//
+// When HAL_SPI_Transmit / HAL_SPI_TransmitReceive returns HAL_ERROR, every
+// caller above must see the failure rather than silently continuing on.
+// Previously adarWrite() returned void and dropped the SPI status on the
+// floor, so a dead bus produced four "successful" inits.
+
+#include <cassert>
+#include <cmath>
+#include <cstdio>
+
+#include "stm32_hal_mock.h"
+#include "ADAR1000_Manager.h"
+
+uint8_t GUI_start_flag_received = 0;
+uint8_t USB_Buffer[64] = {0};
+extern "C" void Error_Handler(void) {}
+
+static int tests_passed = 0;
+static int tests_total  = 0;
+
+#define RUN_TEST(fn)                                                           \
+    do {                                                                        \
+        tests_total++;                                                          \
+        printf("  [%2d] %-65s ", tests_total, #fn);                            \
+        fn();                                                                   \
+        tests_passed++;                                                         \
+        printf("PASS\n");                                                       \
+    } while (0)
+
+// First SPI transmit fails (the soft-reset write inside initializeSingleDevice).
+// Init must abort immediately and report false.
+static void test_first_spi_failure_aborts_init()
+{
+    spy_reset();
+    mock_spi_queue_failure(1, HAL_ERROR);
+    ADAR1000Manager mgr;
+
+    bool ok = mgr.initializeAllDevices();
+    assert(ok == false);
+
+    const auto& stats = mgr.getCommStats();
+    assert(stats.writes_fail >= 1);
+    assert(stats.last_fail_dev == 0);  // failure was on dev[0]
+}
+
+// Sustained SPI failure (every call) must not produce a green init even if
+// individual writes early in the sequence have already counted as failures
+// without aborting -- the function must still return false at the end.
+static void test_sustained_spi_failure_aborts_init()
+{
+    spy_reset();
+    mock_spi_queue_failure(10000, HAL_ERROR);
+    ADAR1000Manager mgr;
+
+    bool ok = mgr.initializeAllDevices();
+    assert(ok == false);
+
+    const auto& stats = mgr.getCommStats();
+    assert(stats.writes_ok == 0);
+    assert(stats.writes_fail >= 1);
+}
+
+// adarSetBit must NOT proceed with the write when the read-modify-write read
+// fails -- otherwise it would clobber every other bit in the register by
+// writing back (0 | mask) over a register whose actual contents are unknown.
+// We use setTRSwitchPosition (which calls adarSetBit) and inject a failure
+// on the read leg.
+static void test_set_bit_skips_write_on_read_failure()
+{
+    spy_reset();
+    ADAR1000Manager mgr;
+    mgr.resetCommStats();
+
+    // adarSetBit calls adarReadChecked first, which itself does:
+    //   1. adarWrite(SDO active)        -- HAL_SPI_Transmit
+    //   2. HAL_SPI_TransmitReceive (the actual read)
+    //   3. adarWrite(SDO inactive)      -- HAL_SPI_Transmit
+    // We want the actual read (call #2) to fail. Queue failure starting at
+    // call 2 by letting call 1 succeed first via a one-shot prime, then
+    // queueing the failure.
+    //
+    // Simpler approach: queue a failure of 100 calls, then call setTRSwitchPosition
+    // and assert reads_fail >= 1 and writes_fail >= 1 (the SDO-active write
+    // also fails). The key invariant: even though the read-modify-write was
+    // attempted, the function returned false.
+    mock_spi_queue_failure(100, HAL_ERROR);
+
+    bool ok = mgr.setTRSwitchPosition(0, true);
+    assert(ok == false);
+
+    const auto& stats = mgr.getCommStats();
+    assert(stats.reads_fail >= 1 || stats.writes_fail >= 1);
+}
+
+// Mode-switch must not fall through to another write block on the device that
+// failed -- and the per-device current_mode must not be updated.
+static void test_mode_switch_failure_propagates()
+{
+    spy_reset();
+    mock_spi_set_rx_byte(0xA5);  // make scratchpad verify succeed first
+    ADAR1000Manager mgr;
+    bool init_ok = mgr.initializeAllDevices();
+    assert(init_ok);
+
+    // Now inject sustained SPI failure for the mode switch.
+    mgr.resetCommStats();
+    mock_spi_queue_failure(10000, HAL_ERROR);
+
+    bool ok = mgr.setAllDevicesRXMode();
+    assert(ok == false);
+
+    const auto& stats = mgr.getCommStats();
+    assert(stats.writes_fail >= 1);
+}
+
+int main()
+{
+    printf("=== ADAR1000 SPI write-failure propagation tests ===\n");
+
+    RUN_TEST(test_first_spi_failure_aborts_init);
+    RUN_TEST(test_sustained_spi_failure_aborts_init);
+    RUN_TEST(test_set_bit_skips_write_on_read_failure);
+    RUN_TEST(test_mode_switch_failure_propagates);
+
+    printf("=== Results: %d/%d passed ===\n", tests_passed, tests_total);
+    return (tests_passed == tests_total) ? 0 : 1;
+}

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.
-// Reset is already synchronous to clk via reset synchronizer in parent module.
+// Sync reset via reset_h (registered, max_fanout=50) — eliminates the shared
+// 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
-    if (!reset_n_400m) begin
+// Sync reset via reset_400m (replicated, max_fanout=50). Was async on
+// 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
-    if (!reset_n_400m || reset_monitors) begin
+always @(posedge clk_400m) 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
-    if (!reset_n_400m)
+always @(posedge clk_400m) begin
+    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
-    if (!reset_n_400m) begin
+always @(posedge clk_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
-    if (!reset_n_400m) begin
+always @(posedge clk_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
-    if (!reset_n_400m) begin
+always @(posedge clk_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
-    if (!reset_n_400m) begin
+always @(posedge clk_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
-    if (!reset_n_400m) begin
+always @(posedge clk_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
-    if (!reset_n_400m) begin
+always @(posedge clk_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
-    if (!reset_n_400m) begin
+always @(posedge clk_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
-    if (!reset_n_400m) begin
+always @(posedge clk_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
-    if (!reset_n_400m) begin
+always @(posedge clk_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
-    if (!reset_n) begin
+always @(posedge clk_400m) 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
-    .RSTC(!reset_n),             // Reset C register (tuning word) on !reset_n
+    .RSTP(reset_h),              // Reset P register (phase accumulator) — registered, max_fanout=50
+    .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
-    if (!reset_n) begin
+always @(posedge clk_400m) 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
-    if (!reset_n) begin
+always @(posedge clk_400m) 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
-    if (!reset_n) begin
+always @(posedge clk_400m) 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
-    if (!reset_n) begin
+always @(posedge clk_400m) 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
-    if (!reset_n) begin
+always @(posedge clk_400m) 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
-    if (!reset_n) begin
+always @(posedge clk_400m) 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}) -
-                                           $signed({1'b0, agc_attack}));
+                    agc_gain <= clamp_gain($signed({agc_gain[3], agc_gain[3], agc_gain}) -
+                                           $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}) +
-                                               $signed({1'b0, agc_decay}));
+                        agc_gain <= clamp_gain($signed({agc_gain[3], agc_gain[3], agc_gain}) +
+                                               $signed({2'b00, agc_decay}));
                     end else begin
                         holdoff_counter <= holdoff_counter - 4'd1;
                     end

9_Firmware/9_2_FPGA/tb/cosim/compare.py

@@ -38,10 +38,11 @@ from fpga_model import SignalChain
 
 # Thresholds for pass/fail
 # These are generous because of LFSR dithering and CDC latency jitter
-MAX_RMS_ERROR_LSB = 50.0     # Max RMS error in 18-bit LSBs
-MIN_CORRELATION = 0.90        # Min Pearson correlation coefficient
-MAX_LATENCY_DRIFT = 15        # Max latency offset between RTL and model (samples)
-MAX_COUNT_DIFF = 20           # Max output count difference (LFSR dithering affects CIC timing)
+MAX_RMS_ERROR_LSB = 50.0       # Max RMS error in 18-bit LSBs
+MAX_ABS_ERROR_LSB = 200.0     # Max absolute error in 18-bit LSBs (4x RMS threshold)
+MIN_CORRELATION = 0.90         # Min Pearson correlation coefficient
+MAX_LATENCY_DRIFT = 15         # Max latency offset between RTL and model (samples)
+MAX_COUNT_DIFF = 20            # Max output count difference (LFSR dithering affects CIC timing)
 
 # Scenarios
 SCENARIOS = {
@@ -314,8 +315,8 @@ def compare_scenario(scenario_name):
     # ---- Error metrics (after alignment) ----
     rms_i = compute_rms_error(aligned_rtl_i, aligned_py_i)
     rms_q = compute_rms_error(aligned_rtl_q, aligned_py_q)
-    compute_max_abs_error(aligned_rtl_i, aligned_py_i)
-    compute_max_abs_error(aligned_rtl_q, aligned_py_q)
+    max_abs_i = compute_max_abs_error(aligned_rtl_i, aligned_py_i)
+    max_abs_q = compute_max_abs_error(aligned_rtl_q, aligned_py_q)
     corr_i_aligned = compute_correlation(aligned_rtl_i, aligned_py_i)
     corr_q_aligned = compute_correlation(aligned_rtl_q, aligned_py_q)
 
@@ -398,6 +399,15 @@ def compare_scenario(scenario_name):
     results.append(('Latency offset', lag_ok,
                      f"|{best_lag}| <= {MAX_LATENCY_DRIFT}"))
 
+    # Check 7: Max absolute error
+    # Catches outlier spikes (pipeline corruption, saturation, CDC glitches)
+    # that RMS and correlation metrics alone can miss.
+    max_abs_err = max(max_abs_i, max_abs_q)
+    max_abs_threshold = cfg.get('max_abs', MAX_ABS_ERROR_LSB)
+    max_abs_ok = max_abs_err <= max_abs_threshold
+    results.append(('Max absolute error', max_abs_ok,
+                     f"max(I={max_abs_i}, Q={max_abs_q}) <= {max_abs_threshold:.0f}"))
+
     # ---- Report ----
     all_pass = True
     for _name, ok, _detail in results:

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/smoke_test.py

@@ -148,11 +148,11 @@ class SmokeTest:
                 if ptype == "status":
                     status = RadarProtocol.parse_status_packet(raw[start:end])
                     if status is not None:
-                        # Self-test results encoded in status fields
-                        # (This is a simplification — in production, the FPGA
-                        #  would have a dedicated self-test result packet type)
-                        result_flags = status.cfar_threshold & 0x1F
-                        result_detail = (status.cfar_threshold >> 8) & 0xFF
+                        # Self-test results live in dedicated status fields
+                        # (word 5: self_test_flags[4:0], self_test_detail[7:0]).
+                        # See radar_protocol.py:257 and test_GUI_V65_Tk.py:198.
+                        result_flags = status.self_test_flags
+                        result_detail = status.self_test_detail
                         return (result_flags, result_detail)
 
             time.sleep(0.1)

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
-        and velocity_resolution (should be calibrated to actual waveform).
+        Bin-to-physical conversion uses self._waveform (WaveformConfig) to keep
+        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
-        v_res = self._settings.velocity_resolution
+        r_res = self._waveform.range_resolution_m
+        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
-            velocity_ms = float(dbin - 16) * v_res
+            # Doppler: centre bin = 0 m/s; positive bins = approaching.
+            # 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
 
@@ -387,6 +389,68 @@ class TestTier1StatusFieldPositions:
             f"but Verilog status_words[0] has mode at bit {expected_shift}."
         )
 
+    @pytest.mark.parametrize(
+        "usb_variant",
+        ["usb_data_interface_ft2232h.v", "usb_data_interface.v"],
+    )
+    def test_status_field_positions_match_verilog_concat_layout(self, usb_variant):
+        """
+        Independent static check: every Python field in parse_status_packet()
+        must sit at the (word_idx, lsb, width) where Verilog places it inside
+        status_words[0..5]. Walks each Verilog concat MSB->LSB and compares to
+        what the Python parser extracts. Exercised across both USB variants
+        because both are live post PR #89.
+        """
+        rtl_path = cp.FPGA_DIR / usb_variant
+        if not rtl_path.exists():
+            pytest.skip(f"{usb_variant} not present")
+
+        concats = cp.parse_verilog_status_word_concats(rtl_path)
+        port_widths = cp.get_usb_interface_port_widths(rtl_path)
+
+        # Build verilog_layout: signal_name -> (word_idx, lsb, width)
+        verilog_layout: dict[str, tuple[int, int, int]] = {}
+        literal_re = re.compile(r"^\d+'[bdhoBDHO]")
+        for word_idx, concat_expr in concats.items():
+            result = cp.count_concat_bits(concat_expr, port_widths)
+            # Skip malformed words; total-width assertion belongs to
+            # TestTier1StatusWordTruncation, not this test.
+            if result.total_bits != 32:
+                continue
+            running_lsb = result.total_bits  # MSB-first walk
+            for name_or_literal, width in result.fragments:
+                running_lsb -= width
+                if literal_re.match(name_or_literal):
+                    continue
+                verilog_layout[name_or_literal] = (word_idx, running_lsb, width)
+
+        py_fields = cp.parse_python_status_fields()
+
+        mismatches: list[str] = []
+        for f in py_fields:
+            v_name = f"status_{f.name}"
+            v_pos = verilog_layout.get(v_name)
+            if v_pos is None:
+                mismatches.append(
+                    f"  {f.name}: py=(word={f.word_index}, lsb={f.lsb}, "
+                    f"width={f.width})  v=<no '{v_name}' fragment in Verilog>"
+                )
+                continue
+            v_word, v_lsb, v_width = v_pos
+            if (v_word, v_lsb, v_width) != (f.word_index, f.lsb, f.width):
+                mismatches.append(
+                    f"  {f.name}: py=(word={f.word_index}, lsb={f.lsb}, "
+                    f"width={f.width})  v=(word={v_word}, lsb={v_lsb}, "
+                    f"width={v_width})"
+                )
+
+        if mismatches:
+            pytest.fail(
+                f"Status field layout drift between Python parse_status_packet() "
+                f"and Verilog status_words[] in {usb_variant}:\n"
+                + "\n".join(mismatches)
+            )
+
 
 class TestTier1PacketConstants:
     """Verify packet header/footer/size constants match across layers."""
@@ -625,6 +689,433 @@ 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`` or ``bool``
+    ``ADAR1000Manager::<setter>`` and return a list of (setter,
+    base_register, offset_expr_c, stride) for every ``REG_CHn_XXX +
+    <expr>`` memory-address assignment.
+
+    The setters originally returned ``void``. After the SPI/ADC error-
+    propagation refactor they return ``bool`` so callers can observe
+    write/read failures. The body form (``REG_CHn_XXX + <expr>``,
+    ``(channel - 1) & 0x03`` mask, ``* 2`` stride for phase I/Q) is
+    unchanged.
+    """
+    sites = []
+    for setter in setter_names:
+        m = re.search(
+            rf"(?:void|bool)\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).
+
+    Two terminating forms are accepted:
+      * ``setter(args);``                                — standalone call.
+      * ``ok = setter(args) && ok;`` (one or more chains) — error-propagation
+        idiom introduced by the SPI/ADC refactor where setters return
+        ``bool`` and callers AND the result into a running success flag.
+    """
+    out = []
+    call_re = re.compile(rf"\b{setter}\s*\(([^;]*?)\)(?:\s*&&\s*\w+)*\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."""
 

pyproject.toml

@@ -49,3 +49,6 @@ select = [
 "test_*.py" = ["ARG", "T20", "ERA"]
 # Re-export modules: unused imports are intentional
 "v7/hardware.py" = ["F401"]
+# 8_Utils/Python/LUT.py is a single-purpose Verilog LUT generator whose only
+# output is `print(f"8'd{val},")` per the file's docstring intent.
+"8_Utils/Python/LUT.py" = ["T20"]