From 658752abb7966556e86ac380423d60907c5335ca Mon Sep 17 00:00:00 2001 From: Jason <83615043+JJassonn69@users.noreply.github.com> Date: Fri, 17 Apr 2026 00:04:37 +0545 Subject: [PATCH] fix: propagate FPGA AGC enable to MCU outer loop via DIG_6 GPIO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve cross-layer AGC control mismatch where opcode 0x28 only controlled the FPGA inner-loop AGC but the STM32 outer-loop AGC (ADAR1000_AGC) ran independently with its own enable state. FPGA: Drive gpio_dig6 from host_agc_enable instead of tied low, making the FPGA register the single source of truth for AGC state. MCU: Change ADAR1000_AGC constructor default from enabled(true) to enabled(false) so boot state matches FPGA reset default (AGC off). Read DIG_6 GPIO every frame with 2-frame confirmation debounce to sync outerAgc.enabled — prevents single-sample glitch from causing spurious AGC state transitions. Tests: Update MCU unit tests for new default, add 6 cross-layer contract tests verifying the FPGA-MCU-GUI AGC invariant chain. --- .../9_1_1_C_Cpp_Libraries/ADAR1000_AGC.cpp | 2 +- .../9_1_3_C_Cpp_Code/main.cpp | 21 +++- .../tests/test_agc_outer_loop.cpp | 10 +- .../9_2_FPGA/constraints/xc7a50t_ftg256.xdc | 2 +- 9_Firmware/9_2_FPGA/radar_system_top.v | 8 +- .../cross_layer/test_cross_layer_contract.py | 118 ++++++++++++++++++ 6 files changed, 152 insertions(+), 9 deletions(-) diff --git a/9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/ADAR1000_AGC.cpp b/9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/ADAR1000_AGC.cpp index 0d4d477..068b80b 100644 --- a/9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/ADAR1000_AGC.cpp +++ b/9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/ADAR1000_AGC.cpp @@ -18,7 +18,7 @@ ADAR1000_AGC::ADAR1000_AGC() , min_gain(0) , max_gain(127) , holdoff_frames(4) - , enabled(true) + , enabled(false) , holdoff_counter(0) , last_saturated(false) , saturation_event_count(0) diff --git a/9_Firmware/9_1_Microcontroller/9_1_3_C_Cpp_Code/main.cpp b/9_Firmware/9_1_Microcontroller/9_1_3_C_Cpp_Code/main.cpp index 497378b..45d4923 100644 --- a/9_Firmware/9_1_Microcontroller/9_1_3_C_Cpp_Code/main.cpp +++ b/9_Firmware/9_1_Microcontroller/9_1_3_C_Cpp_Code/main.cpp @@ -2180,9 +2180,24 @@ int main(void) runRadarPulseSequence(); - /* [AGC] Outer-loop AGC: read FPGA saturation flag (DIG_5 / PD13), - * adjust ADAR1000 VGA common gain once per radar frame (~258 ms). - * Only run when AGC is enabled — otherwise leave VGA gains untouched. */ + /* [AGC] Outer-loop AGC: sync enable from FPGA via DIG_6 (PD14), + * then read saturation flag (DIG_5 / PD13) and adjust ADAR1000 VGA + * common gain once per radar frame (~258 ms). + * FPGA register host_agc_enable is the single source of truth — + * DIG_6 propagates it to MCU every frame. + * 2-frame confirmation debounce: only change outerAgc.enabled when + * two consecutive frames read the same DIG_6 value. Prevents a + * single-sample glitch from causing a spurious AGC state transition. + * Added latency: 1 extra frame (~258 ms), acceptable for control plane. */ + { + bool dig6_now = (HAL_GPIO_ReadPin(FPGA_DIG6_GPIO_Port, + FPGA_DIG6_Pin) == GPIO_PIN_SET); + static bool dig6_prev = false; // matches boot default (AGC off) + if (dig6_now == dig6_prev) { + outerAgc.enabled = dig6_now; + } + dig6_prev = dig6_now; + } if (outerAgc.enabled) { bool sat = HAL_GPIO_ReadPin(FPGA_DIG5_SAT_GPIO_Port, FPGA_DIG5_SAT_Pin) == GPIO_PIN_SET; diff --git a/9_Firmware/9_1_Microcontroller/tests/test_agc_outer_loop.cpp b/9_Firmware/9_1_Microcontroller/tests/test_agc_outer_loop.cpp index 9b23d28..8eb5292 100644 --- a/9_Firmware/9_1_Microcontroller/tests/test_agc_outer_loop.cpp +++ b/9_Firmware/9_1_Microcontroller/tests/test_agc_outer_loop.cpp @@ -50,7 +50,7 @@ static void test_defaults() assert(agc.min_gain == 0); assert(agc.max_gain == 127); assert(agc.holdoff_frames == 4); - assert(agc.enabled == true); + assert(agc.enabled == false); // disabled by default — FPGA DIG_6 is source of truth assert(agc.holdoff_counter == 0); assert(agc.last_saturated == false); assert(agc.saturation_event_count == 0); @@ -67,6 +67,7 @@ static void test_defaults() static void test_saturation_reduces_gain() { ADAR1000_AGC agc; + agc.enabled = true; // default is OFF; enable for this test uint8_t initial = agc.agc_base_gain; // 30 agc.update(true); // saturation @@ -82,6 +83,7 @@ static void test_saturation_reduces_gain() static void test_holdoff_prevents_early_gain_up() { ADAR1000_AGC agc; + agc.enabled = true; // default is OFF; enable for this test agc.update(true); // saturate once -> gain = 26 uint8_t after_sat = agc.agc_base_gain; @@ -101,6 +103,7 @@ static void test_holdoff_prevents_early_gain_up() static void test_recovery_after_holdoff() { ADAR1000_AGC agc; + agc.enabled = true; // default is OFF; enable for this test agc.update(true); // saturate -> gain = 26 uint8_t after_sat = agc.agc_base_gain; @@ -119,6 +122,7 @@ static void test_recovery_after_holdoff() static void test_min_gain_clamp() { ADAR1000_AGC agc; + agc.enabled = true; // default is OFF; enable for this test agc.min_gain = 10; agc.agc_base_gain = 12; agc.gain_step_down = 4; @@ -136,6 +140,7 @@ static void test_min_gain_clamp() static void test_max_gain_clamp() { ADAR1000_AGC agc; + agc.enabled = true; // default is OFF; enable for this test agc.max_gain = 32; agc.agc_base_gain = 31; agc.gain_step_up = 2; @@ -226,6 +231,7 @@ static void test_apply_gain_spi() static void test_reset_preserves_config() { ADAR1000_AGC agc; + agc.enabled = true; // default is OFF; enable for this test agc.agc_base_gain = 42; agc.gain_step_down = 8; agc.cal_offset[3] = -5; @@ -255,6 +261,7 @@ static void test_reset_preserves_config() static void test_saturation_counter() { ADAR1000_AGC agc; + agc.enabled = true; // default is OFF; enable for this test for (int i = 0; i < 10; ++i) { agc.update(true); @@ -274,6 +281,7 @@ static void test_saturation_counter() static void test_mixed_sequence() { ADAR1000_AGC agc; + agc.enabled = true; // default is OFF; enable for this test agc.agc_base_gain = 30; agc.gain_step_down = 4; agc.gain_step_up = 1; diff --git a/9_Firmware/9_2_FPGA/constraints/xc7a50t_ftg256.xdc b/9_Firmware/9_2_FPGA/constraints/xc7a50t_ftg256.xdc index c4d61f0..12df629 100644 --- a/9_Firmware/9_2_FPGA/constraints/xc7a50t_ftg256.xdc +++ b/9_Firmware/9_2_FPGA/constraints/xc7a50t_ftg256.xdc @@ -224,7 +224,7 @@ set_property IOSTANDARD LVCMOS33 [get_ports {stm32_mixers_enable}] # DIG_5 = H11, DIG_6 = G12, DIG_7 = H12 — FPGA→STM32 status outputs # DIG_5: AGC saturation flag (PD13 on STM32) -# DIG_6: reserved (PD14) +# DIG_6: AGC enable flag (PD14) — mirrors FPGA host_agc_enable to STM32 # DIG_7: reserved (PD15) set_property PACKAGE_PIN H11 [get_ports {gpio_dig5}] set_property PACKAGE_PIN G12 [get_ports {gpio_dig6}] diff --git a/9_Firmware/9_2_FPGA/radar_system_top.v b/9_Firmware/9_2_FPGA/radar_system_top.v index a133453..8e9373a 100644 --- a/9_Firmware/9_2_FPGA/radar_system_top.v +++ b/9_Firmware/9_2_FPGA/radar_system_top.v @@ -130,7 +130,7 @@ module radar_system_top ( // FPGA→STM32 GPIO outputs (DIG_5..DIG_7 on 50T board) // Used by STM32 outer AGC loop to read saturation state without USB polling. output wire gpio_dig5, // DIG_5 (H11→PD13): AGC saturation flag (1=clipping detected) - output wire gpio_dig6, // DIG_6 (G12→PD14): reserved (tied low) + output wire gpio_dig6, // DIG_6 (G12→PD14): AGC enable flag (mirrors host_agc_enable) output wire gpio_dig7 // DIG_7 (H12→PD15): reserved (tied low) ); @@ -1037,9 +1037,11 @@ assign system_status = status_reg; // ============================================================================ // DIG_5: AGC saturation flag — high when per-frame saturation_count > 0. // STM32 reads PD13 to detect clipping and adjust ADAR1000 VGA gain. -// DIG_6, DIG_7: Reserved (tied low for future use). +// DIG_6: AGC enable flag — mirrors host_agc_enable so STM32 outer-loop AGC +// tracks the FPGA register as single source of truth. +// DIG_7: Reserved (tied low for future use). assign gpio_dig5 = (rx_agc_saturation_count != 8'd0); -assign gpio_dig6 = 1'b0; +assign gpio_dig6 = host_agc_enable; assign gpio_dig7 = 1'b0; // ============================================================================ diff --git a/9_Firmware/tests/cross_layer/test_cross_layer_contract.py b/9_Firmware/tests/cross_layer/test_cross_layer_contract.py index 6d103de..f1acf7b 100644 --- a/9_Firmware/tests/cross_layer/test_cross_layer_contract.py +++ b/9_Firmware/tests/cross_layer/test_cross_layer_contract.py @@ -27,6 +27,7 @@ layers agree (because both could be wrong). from __future__ import annotations import os +import re import struct import subprocess import tempfile @@ -369,6 +370,123 @@ class TestTier1ResetDefaults: ) +class TestTier1AgcCrossLayerInvariant: + """ + Verify AGC enable/disable is consistent across FPGA, MCU, and GUI layers. + + System-level invariant: the FPGA register host_agc_enable is the single + source of truth for AGC state. It propagates to MCU via DIG_6 GPIO and + to GUI via status word 4 bit[11]. At boot, all layers must agree AGC=OFF. + At runtime, the MCU must read DIG_6 every frame to sync its outer-loop AGC. + """ + + def test_fpga_dig6_drives_agc_enable(self): + """FPGA must drive gpio_dig6 from host_agc_enable, NOT tied low.""" + rtl = (cp.FPGA_DIR / "radar_system_top.v").read_text() + # Must find: assign gpio_dig6 = host_agc_enable; + assert re.search( + r'assign\s+gpio_dig6\s*=\s*host_agc_enable\s*;', rtl + ), "gpio_dig6 must be driven by host_agc_enable (not tied low)" + # Must NOT have the old tied-low pattern + assert not re.search( + r"assign\s+gpio_dig6\s*=\s*1'b0\s*;", rtl + ), "gpio_dig6 must NOT be tied low — it carries AGC enable" + + def test_fpga_agc_enable_boot_default_off(self): + """FPGA host_agc_enable must reset to 0 (AGC off at boot).""" + v_defaults = cp.parse_verilog_reset_defaults() + assert "host_agc_enable" in v_defaults, ( + "host_agc_enable not found in reset block" + ) + assert v_defaults["host_agc_enable"] == 0, ( + f"host_agc_enable reset default is {v_defaults['host_agc_enable']}, " + "expected 0 (AGC off at boot)" + ) + + def test_mcu_agc_constructor_default_off(self): + """MCU ADAR1000_AGC constructor must default enabled=false.""" + agc_cpp = (cp.MCU_LIB_DIR / "ADAR1000_AGC.cpp").read_text() + # The constructor initializer list must have enabled(false) + assert re.search( + r'enabled\s*\(\s*false\s*\)', agc_cpp + ), "ADAR1000_AGC constructor must initialize enabled(false)" + assert not re.search( + r'enabled\s*\(\s*true\s*\)', agc_cpp + ), "ADAR1000_AGC constructor must NOT initialize enabled(true)" + + def test_mcu_reads_dig6_before_agc_gate(self): + """MCU main loop must read DIG_6 GPIO to sync outerAgc.enabled.""" + main_cpp = (cp.MCU_CODE_DIR / "main.cpp").read_text() + # DIG_6 must be read via HAL_GPIO_ReadPin + assert re.search( + r'HAL_GPIO_ReadPin\s*\(\s*FPGA_DIG6', main_cpp, + ), "main.cpp must read DIG_6 GPIO via HAL_GPIO_ReadPin" + # outerAgc.enabled must be assigned from the DIG_6 reading + # (may be indirect via debounce variable like dig6_now) + assert re.search( + r'outerAgc\.enabled\s*=', main_cpp, + ), "main.cpp must assign outerAgc.enabled from DIG_6 state" + + def test_boot_invariant_all_layers_agc_off(self): + """ + At boot, all three layers must agree: AGC is OFF. + - FPGA: host_agc_enable resets to 0 -> DIG_6 low + - MCU: ADAR1000_AGC.enabled defaults to false + - GUI: reads status word 4 bit[11] = 0 -> reports MANUAL + """ + # FPGA + v_defaults = cp.parse_verilog_reset_defaults() + assert v_defaults.get("host_agc_enable") == 0 + + # MCU + agc_cpp = (cp.MCU_LIB_DIR / "ADAR1000_AGC.cpp").read_text() + assert re.search(r'enabled\s*\(\s*false\s*\)', agc_cpp) + + # GUI: status word 4 bit[11] is host_agc_enable, which resets to 0. + # Verify the GUI parses bit[11] of status word 4 as the AGC flag. + gui_py = (cp.GUI_DIR / "radar_protocol.py").read_text() + assert re.search( + r'words\[4\].*>>\s*11|status_words\[4\].*>>\s*11', + gui_py, + ), "GUI must parse AGC status from words[4] bit[11]" + + def test_status_word4_agc_bit_matches_dig6_source(self): + """ + Status word 4 bit[11] and DIG_6 must both derive from host_agc_enable. + This guarantees the GUI status display can never lie about MCU AGC state. + """ + rtl = (cp.FPGA_DIR / "radar_system_top.v").read_text() + + # DIG_6 driven by host_agc_enable + assert re.search( + r'assign\s+gpio_dig6\s*=\s*host_agc_enable\s*;', rtl + ) + + # Status word 4 must contain host_agc_enable (may be named + # status_agc_enable at the USB interface port boundary). + # Also verify the top-level wiring connects them. + usb_ft2232h = (cp.FPGA_DIR / "usb_data_interface_ft2232h.v").read_text() + usb_ft601 = (cp.FPGA_DIR / "usb_data_interface.v").read_text() + + # USB interfaces use the port name status_agc_enable + found_in_ft2232h = "status_agc_enable" in usb_ft2232h + found_in_ft601 = "status_agc_enable" in usb_ft601 + + assert found_in_ft2232h or found_in_ft601, ( + "status_agc_enable must appear in at least one USB interface's " + "status word to guarantee GUI status matches DIG_6" + ) + + # Verify top-level wiring: status_agc_enable port is connected + # to host_agc_enable (same signal that drives DIG_6) + assert re.search( + r'\.status_agc_enable\s*\(\s*host_agc_enable\s*\)', rtl + ), ( + "Top-level must wire .status_agc_enable(host_agc_enable) " + "so status word and DIG_6 derive from the same signal" + ) + + class TestTier1DataPacketLayout: """Verify data packet byte layout matches between Python and Verilog."""