doppler: gate S_IDLE→S_ACCUMULATE on frame_start_pulse (AUDIT-S3)

Pre-fix S_IDLE had two independent if-branches: one for frame_start_pulse
(resets pointers) and one for data_valid (transitions to S_ACCUMULATE).
A data_valid arriving before frame_start_pulse would advance the FSM with
whatever pointers happened to be live, and the BRAM write block would write
the sample into mem_write_addr = (write_chirp_index*RANGE_BINS) + 0.

In current operation the race is benign — end-of-S_ACCUMULATE always zeros
write_chirp_index/write_range_bin (line 287-288) and the MF pipeline latency
(~165 µs) is millions of cycles longer than the frame_start CDC latency
(~50 ns), so frame_start always arrives first. But the FSM relies on an
undocumented system-level invariant; a future code path that leaves
pointers stale on entry to S_IDLE would silently corrupt the first sample.

Fix: add a `frame_armed` register set when frame_start_pulse arrives in
S_IDLE, cleared on transition to S_ACCUMULATE. Both the FSM transition and
the BRAM write block gate on `(frame_start_pulse || frame_armed)`. The OR
admits the same-cycle case where both arrive together (write to addr 0
still resolves correctly because both blocks use the same gate).

Verification: tb_doppler_frame_start_gate 21/21 PASS, quick regression
32/32 PASS (was 31/31; +1 new test, 0 regressions). tb_doppler_realdata
(full FFT pipeline) still passes — gate transparent to normal operation.

9_Firmware/9_2_FPGA/doppler_processor.v

@@ -134,6 +134,17 @@ reg frame_buffer_full;
 reg [9:0] chirps_received;
 reg [1:0] chirp_state;
 
+// AUDIT-S3 fix: arm-on-frame-start gating. Set when frame_start_pulse arrives
+// in S_IDLE; cleared when the FSM transitions to S_ACCUMULATE. Prevents stale
+// data_valid from prior MF pipeline residue from advancing S_IDLE → S_ACCUMULATE
+// before the new frame is officially started, which would write the first
+// sample(s) into addr 0 of the previous frame's buffer if write_chirp_index
+// happened to be non-zero. The pointer-reset invariant (line 287-288 always
+// zeros pointers at end of S_ACCUMULATE) makes this race benign in current
+// operation, but the gate makes the FSM robust against future code paths
+// that might leave pointers stale on entry to S_IDLE.
+reg frame_armed;
+
 // Sub-frame tracking
 reg current_sub_frame;   // 0=processing long, 1=processing short
 
@@ -246,6 +257,7 @@ always @(posedge clk or negedge reset_n) begin
         range_bin <= 0;
         sub_frame <= 0;
         current_sub_frame <= 0;
+        frame_armed <= 0;
     end else begin
         doppler_valid <= 0;
         fft_input_valid <= 0;
@@ -262,14 +274,24 @@ always @(posedge clk or negedge reset_n) begin
                     write_range_bin <= 0;
                     frame_buffer_full <= 0;
                     chirps_received <= 0;
+                    frame_armed <= 1;     // AUDIT-S3: arm on frame_start_pulse
                 end
-                
+
-                if (data_valid && !frame_buffer_full) begin
+                // AUDIT-S3 fix: only transition to S_ACCUMULATE when armed,
+                // i.e., when this frame has been officially started by a
+                // frame_start_pulse. Pre-fix code accepted any data_valid in
+                // S_IDLE and could race with a missing/late frame_start_pulse.
+                // (frame_start_pulse || frame_armed) admits the same-cycle case
+                // where both pulse and data_valid arrive together — write to
+                // addr 0 still resolves correctly because the BRAM write block
+                // uses the same gate.
+                if ((frame_start_pulse || frame_armed) && data_valid && !frame_buffer_full) begin
                     state <= S_ACCUMULATE;
                     write_range_bin <= 1;
+                    frame_armed <= 0;     // disarm; S_ACCUMULATE handles its own pointers
                 end
             end
-            
+
             S_ACCUMULATE: begin
                 if (data_valid) begin
                     if (write_range_bin < RANGE_BINS - 1) begin
@@ -393,14 +415,19 @@ always @(posedge clk) begin
         
         case (state)
             S_IDLE: begin
-                if (data_valid && !frame_buffer_full) begin
+                // AUDIT-S3 fix: gate BRAM write on frame_armed so stale
+                // data_valid arriving before frame_start_pulse cannot
+                // overwrite addr 0 of the buffer. Same gate as the FSM's
+                // S_IDLE → S_ACCUMULATE transition above, so the two blocks
+                // stay coherent.
+                if ((frame_start_pulse || frame_armed) && data_valid && !frame_buffer_full) begin
                     mem_we      <= 1;
                     mem_waddr_r <= mem_write_addr;
                     mem_wdata_i <= range_data[15:0];
                     mem_wdata_q <= range_data[31:16];
                 end
             end
-            
+
             S_ACCUMULATE: begin
                 if (data_valid) begin
                     mem_we      <= 1;

9_Firmware/9_2_FPGA/run_regression.sh

@@ -554,6 +554,10 @@ run_test "FT2232H Frame Drop Counter (AUDIT-C12)" \
     tb/tb_ft2232h_frame_drop.vvp \
     tb/tb_ft2232h_frame_drop.v usb_data_interface_ft2232h.v
 
+run_test "Doppler Frame-Start Gate (AUDIT-S3)" \
+    tb/tb_doppler_frame_start_gate.vvp \
+    tb/tb_doppler_frame_start_gate.v doppler_processor.v xfft_16.v fft_engine.v
+
 echo ""
 
 # ===========================================================================

9_Firmware/9_2_FPGA/tb/tb_doppler_frame_start_gate.v

@@ -0,0 +1,275 @@
+`timescale 1ns / 1ps
+
+// ============================================================================
+// tb_doppler_frame_start_gate.v — AUDIT-S3 regression
+// ============================================================================
+// Verifies the post-fix gating in doppler_processor_optimized's S_IDLE state.
+//
+// AUDIT-S3 (pre-fix bug): S_IDLE had two independent if-branches — one for
+// frame_start_pulse (resets pointers) and one for data_valid (transitions to
+// S_ACCUMULATE). A data_valid arriving BEFORE frame_start_pulse would
+// transition the FSM with whatever pointers happened to be set, and the BRAM
+// write block would write into addr (write_chirp_index*RANGE_BINS+0).
+//
+// Post-fix: a `frame_armed` register is set on frame_start_pulse and cleared
+// on transition to S_ACCUMULATE. Both the FSM transition and the BRAM write
+// are gated on `(frame_start_pulse || frame_armed)`. data_valid arriving
+// before frame_start_pulse is silently dropped at the input.
+//
+// Tests:
+//   T1  data_valid alone (no frame_start) → state stays S_IDLE, mem_we=0
+//   T2  frame_start_pulse alone (no data_valid) → state stays S_IDLE,
+//       frame_armed=1, mem_we=0
+//   T3  frame_start, then data_valid 5 cycles later (normal MF latency case)
+//       → state→S_ACCUMULATE on the data_valid cycle, mem_we=1, mem_waddr_r=0
+//   T4  frame_start and data_valid on the SAME cycle
+//       → state→S_ACCUMULATE this cycle, mem_we=1, mem_waddr_r=0
+//   T5  long burst of data_valid in S_IDLE before any frame_start_pulse
+//       → state stays S_IDLE for the entire burst, mem_we never asserts
+// ============================================================================
+
+`include "radar_params.vh"
+
+module tb_doppler_frame_start_gate;
+
+localparam CLK_PERIOD = 10.0;  // 100 MHz
+
+// Tiny config for fast simulation. We only exercise the S_IDLE gating; the
+// rest of the FSM (S_ACCUMULATE, FFT pipeline) is not driven to completion.
+localparam DOPPLER_FFT_SIZE   = 16;   // unchanged
+localparam RANGE_BINS         = 8;
+localparam CHIRPS_PER_FRAME   = 4;
+localparam CHIRPS_PER_SUBFRAME = 2;
+localparam DATA_WIDTH          = 16;
+
+reg clk;
+reg reset_n;
+reg [31:0] range_data;
+reg data_valid;
+reg new_chirp_frame;
+
+wire [31:0] doppler_output;
+wire        doppler_valid;
+wire [4:0]  doppler_bin;
+wire [`RP_RANGE_BIN_WIDTH_MAX-1:0] range_bin;
+wire        sub_frame;
+wire        processing_active;
+wire        frame_complete;
+wire [3:0]  dut_status;
+
+doppler_processor_optimized #(
+    .DOPPLER_FFT_SIZE(DOPPLER_FFT_SIZE),
+    .RANGE_BINS(RANGE_BINS),
+    .CHIRPS_PER_FRAME(CHIRPS_PER_FRAME),
+    .CHIRPS_PER_SUBFRAME(CHIRPS_PER_SUBFRAME),
+    .DATA_WIDTH(DATA_WIDTH)
+) dut (
+    .clk(clk),
+    .reset_n(reset_n),
+    .range_data(range_data),
+    .data_valid(data_valid),
+    .new_chirp_frame(new_chirp_frame),
+    .doppler_output(doppler_output),
+    .doppler_valid(doppler_valid),
+    .doppler_bin(doppler_bin),
+    .range_bin(range_bin),
+    .sub_frame(sub_frame),
+    .processing_active(processing_active),
+    .frame_complete(frame_complete),
+    .status(dut_status)
+);
+
+// ----------------------------------------------------------------------------
+// Hierarchical refs to internal state
+// ----------------------------------------------------------------------------
+wire [2:0] dut_state    = dut.state;
+wire       dut_armed    = dut.frame_armed;
+wire       dut_mem_we   = dut.mem_we;
+wire [`RP_DOPPLER_MEM_ADDR_W-1:0] dut_mem_waddr = dut.mem_waddr_r;
+
+localparam [2:0] S_IDLE       = 3'b000;
+localparam [2:0] S_ACCUMULATE = 3'b001;
+
+// ----------------------------------------------------------------------------
+// Clock
+// ----------------------------------------------------------------------------
+initial clk = 0;
+always #(CLK_PERIOD / 2) clk = ~clk;
+
+// ----------------------------------------------------------------------------
+// Test bookkeeping
+// ----------------------------------------------------------------------------
+integer pass_count = 0;
+integer fail_count = 0;
+integer test_num   = 0;
+
+task check;
+    input cond;
+    input [255:0] label;
+    begin
+        test_num = test_num + 1;
+        if (cond) begin
+            $display("[PASS %0d] %0s", test_num, label);
+            pass_count = pass_count + 1;
+        end else begin
+            $display("[FAIL %0d] %0s  (state=%0d armed=%0b mem_we=%0b waddr=%0d)",
+                     test_num, label, dut_state, dut_armed, dut_mem_we, dut_mem_waddr);
+            fail_count = fail_count + 1;
+        end
+    end
+endtask
+
+task do_reset;
+    begin
+        reset_n         = 0;
+        data_valid      = 0;
+        new_chirp_frame = 0;
+        range_data      = 32'h0;
+        repeat (4) @(posedge clk);
+        #1 reset_n = 1;
+        @(posedge clk); #1;
+    end
+endtask
+
+// ----------------------------------------------------------------------------
+// Stimulus
+// ----------------------------------------------------------------------------
+initial begin
+    $dumpfile("tb_doppler_frame_start_gate.vcd");
+    $dumpvars(0, tb_doppler_frame_start_gate);
+
+    $display("\n=== AUDIT-S3 regression: doppler S_IDLE frame_start gating ===\n");
+
+    // ============================================================
+    // T1: data_valid alone (no frame_start) → no transition, no write
+    // ============================================================
+    do_reset;
+    range_data = 32'hAAAA_BBBB;
+    data_valid = 1;
+    @(posedge clk); #1;
+    check(dut_state == S_IDLE,        "T1.a: state stays S_IDLE under data_valid w/o frame_start");
+    check(dut_armed == 1'b0,          "T1.b: frame_armed remains 0");
+    check(dut_mem_we == 1'b0,         "T1.c: mem_we does NOT fire");
+
+    // Hold for 5 more cycles to confirm no late transition / no write
+    repeat (5) @(posedge clk);
+    #1;
+    check(dut_state == S_IDLE,        "T1.d: state still S_IDLE after 5 more cycles");
+    check(dut_mem_we == 1'b0,         "T1.e: mem_we still 0 after 5 more cycles");
+
+    data_valid = 0;
+    @(posedge clk); #1;
+
+    // ============================================================
+    // T2: frame_start_pulse alone, no data_valid → frame_armed=1, no write
+    // ============================================================
+    do_reset;
+    new_chirp_frame = 1;       // edge-detector inside DUT will fire one-shot
+    @(posedge clk); #1;
+    new_chirp_frame = 0;
+    @(posedge clk); #1;        // frame_start_pulse fires this cycle
+    check(dut_armed == 1'b1,          "T2.a: frame_armed set after frame_start_pulse");
+    check(dut_state == S_IDLE,        "T2.b: state stays S_IDLE without data_valid");
+    check(dut_mem_we == 1'b0,         "T2.c: mem_we does NOT fire (no data_valid)");
+
+    // Wait some cycles — armed should persist
+    repeat (10) @(posedge clk);
+    #1;
+    check(dut_armed == 1'b1,          "T2.d: frame_armed persists across idle cycles");
+    check(dut_state == S_IDLE,        "T2.e: state still S_IDLE");
+
+    // ============================================================
+    // T3: armed (from T2), then data_valid → transition + write to addr 0
+    // ============================================================
+    range_data = 32'h1234_5678;
+    data_valid = 1;
+    @(posedge clk); #1;
+    check(dut_state == S_ACCUMULATE,  "T3.a: state→S_ACCUMULATE on first data_valid after frame_start");
+    check(dut_armed == 1'b0,          "T3.b: frame_armed cleared on transition");
+    check(dut_mem_we == 1'b1,         "T3.c: mem_we asserted on transition");
+    check(dut_mem_waddr == 0,         "T3.d: first sample writes to addr 0");
+
+    data_valid = 0;
+    @(posedge clk); #1;
+
+    // ============================================================
+    // T4: same-cycle frame_start_pulse + data_valid → transition + write
+    // ============================================================
+    do_reset;
+    new_chirp_frame = 1;
+    @(posedge clk); #1;        // 1st cycle of new_chirp_frame=1
+    new_chirp_frame = 0;
+    range_data      = 32'hDEAD_BEEF;
+    data_valid      = 1;
+    // On THIS cycle: new_chirp_frame_d1=1 (latched last cycle), new_chirp_frame=0
+    //   → frame_start_pulse = 0 (XOR is 1 only on rising edge)
+    // We need to time the data_valid to coincide with the rising-edge cycle.
+    // So actually: drive new_chirp_frame=1 simultaneously with data_valid=1
+    // for ONE cycle, starting from a state where new_chirp_frame_d1=0.
+    @(posedge clk); #1;         // give the FSM a cycle to settle (T4 prep)
+    do_reset;                   // reclean
+    range_data      = 32'hDEAD_BEEF;
+    data_valid      = 1;
+    new_chirp_frame = 1;
+    @(posedge clk); #1;          // first cycle: new_chirp_frame_d1 still 0
+                                 // → frame_start_pulse fires THIS cycle.
+                                 // Same cycle as data_valid → transition+write.
+    check(dut_state == S_ACCUMULATE,  "T4.a: same-cycle pulse+data → state→S_ACCUMULATE");
+    check(dut_mem_we == 1'b1,         "T4.b: same-cycle → mem_we fires");
+    check(dut_mem_waddr == 0,         "T4.c: same-cycle → write to addr 0");
+    check(dut_armed == 1'b0,          "T4.d: same-cycle → frame_armed disarmed");
+
+    new_chirp_frame = 0;
+    data_valid      = 0;
+    @(posedge clk); #1;
+
+    // ============================================================
+    // T5: long data_valid burst with no frame_start → no transition, no write
+    // (regression for the audit's specific concern)
+    // ============================================================
+    do_reset;
+    range_data = 32'hCAFE_F00D;
+    data_valid = 1;
+    begin : t5_burst
+        integer i;
+        integer mem_we_count;
+        mem_we_count = 0;
+        for (i = 0; i < 100; i = i + 1) begin
+            @(posedge clk); #1;
+            if (dut_mem_we) mem_we_count = mem_we_count + 1;
+            if (dut_state != S_IDLE) begin
+                $display("[FAIL] T5: state left S_IDLE at cycle %0d (state=%0d)", i, dut_state);
+                fail_count = fail_count + 1;
+                disable t5_burst;
+            end
+        end
+        check(mem_we_count == 0,      "T5.a: 100 cycles of data_valid w/o frame_start → 0 BRAM writes");
+        check(dut_state == S_IDLE,    "T5.b: state still S_IDLE after 100-cycle burst");
+        check(dut_armed == 1'b0,      "T5.c: frame_armed never set");
+    end
+    data_valid = 0;
+    @(posedge clk); #1;
+
+    // ============================================================
+    // Summary
+    // ============================================================
+    $display("\n=== AUDIT-S3 frame-start gate regression ===");
+    $display("  PASSED: %0d / %0d", pass_count, test_num);
+    $display("  FAILED: %0d / %0d", fail_count, test_num);
+    if (fail_count == 0)
+        $display("  ** ALL TESTS PASSED **");
+    else
+        $display("  ** SOME TESTS FAILED **");
+    $display("");
+
+    #20 $finish;
+end
+
+// Timeout safety
+initial begin
+    #500_000;
+    $display("[FAIL] Watchdog timeout — TB hung");
+    $finish;
+end
+
+endmodule