fix: resolve 3 cross-layer bugs (status_words truncation, mode readback, buffer overread)
Bug 1 (FPGA): status_words[0] was 37 bits (8+3+2+5+3+16), silently
truncated to 32. Restructured to {0xFF, mode[1:0], stream[2:0],
3'b000, threshold[15:0]} = 32 bits exactly. Fixed in both
usb_data_interface_ft2232h.v and usb_data_interface.v.
Bug 2 (Python): radar_mode extracted at bit 21 but was actually at
bit 24 after truncation — always returned 0. Updated shift/mask in
parse_status_packet() to match new layout (mode>>22, stream>>19).
Bug 3 (STM32): parseFromUSB() minimum size check was 74 bytes but
9 doubles + uint32 + markers = 82 bytes. Buffer overread on last
fields when 74-81 bytes passed.
All 166 tests pass (29 cross-layer, 92 GUI, 20 MCU, 25 FPGA).
This commit is contained in:
@@ -7,8 +7,8 @@ RadarSettings::RadarSettings() {
|
|||||||
|
|
||||||
void RadarSettings::resetToDefaults() {
|
void RadarSettings::resetToDefaults() {
|
||||||
system_frequency = 10.0e9; // 10 GHz
|
system_frequency = 10.0e9; // 10 GHz
|
||||||
chirp_duration_1 = 30.0e-6; // 30 µs
|
chirp_duration_1 = 30.0e-6; // 30 �s
|
||||||
chirp_duration_2 = 0.5e-6; // 0.5 µs
|
chirp_duration_2 = 0.5e-6; // 0.5 �s
|
||||||
chirps_per_position = 32;
|
chirps_per_position = 32;
|
||||||
freq_min = 10.0e6; // 10 MHz
|
freq_min = 10.0e6; // 10 MHz
|
||||||
freq_max = 30.0e6; // 30 MHz
|
freq_max = 30.0e6; // 30 MHz
|
||||||
@@ -21,8 +21,8 @@ void RadarSettings::resetToDefaults() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
bool RadarSettings::parseFromUSB(const uint8_t* data, uint32_t length) {
|
bool RadarSettings::parseFromUSB(const uint8_t* data, uint32_t length) {
|
||||||
// Minimum packet size: "SET" + 8 doubles + 1 uint32_t + "END" = 3 + 8*8 + 4 + 3 = 74 bytes
|
// Minimum packet size: "SET" + 9 doubles + 1 uint32_t + "END" = 3 + 9*8 + 4 + 3 = 82 bytes
|
||||||
if (data == nullptr || length < 74) {
|
if (data == nullptr || length < 82) {
|
||||||
settings_valid = false;
|
settings_valid = false;
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -258,10 +258,9 @@ always @(posedge ft601_clk_in or negedge ft601_reset_n) begin
|
|||||||
// Gap 2: Capture status snapshot when request arrives in ft601 domain
|
// Gap 2: Capture status snapshot when request arrives in ft601 domain
|
||||||
if (status_req_ft601) begin
|
if (status_req_ft601) begin
|
||||||
// Pack register values into 5x 32-bit status words
|
// Pack register values into 5x 32-bit status words
|
||||||
// Word 0: {0xFF, mode[1:0], stream_ctrl[2:0], cfar_threshold[15:0]}
|
// Word 0: {0xFF[31:24], mode[23:22], stream[21:19], 3'b000[18:16], threshold[15:0]}
|
||||||
status_words[0] <= {8'hFF, 3'b000, status_radar_mode,
|
status_words[0] <= {8'hFF, status_radar_mode, status_stream_ctrl,
|
||||||
5'b00000, status_stream_ctrl,
|
3'b000, status_cfar_threshold};
|
||||||
status_cfar_threshold};
|
|
||||||
// Word 1: {long_chirp_cycles[15:0], long_listen_cycles[15:0]}
|
// Word 1: {long_chirp_cycles[15:0], long_listen_cycles[15:0]}
|
||||||
status_words[1] <= {status_long_chirp, status_long_listen};
|
status_words[1] <= {status_long_chirp, status_long_listen};
|
||||||
// Word 2: {guard_cycles[15:0], short_chirp_cycles[15:0]}
|
// Word 2: {guard_cycles[15:0], short_chirp_cycles[15:0]}
|
||||||
|
|||||||
@@ -275,9 +275,9 @@ always @(posedge ft_clk or negedge ft_reset_n) begin
|
|||||||
|
|
||||||
// Status snapshot on request
|
// Status snapshot on request
|
||||||
if (status_req_ft) begin
|
if (status_req_ft) begin
|
||||||
status_words[0] <= {8'hFF, 3'b000, status_radar_mode,
|
// Word 0: {0xFF[31:24], mode[23:22], stream[21:19], 3'b000[18:16], threshold[15:0]}
|
||||||
5'b00000, status_stream_ctrl,
|
status_words[0] <= {8'hFF, status_radar_mode, status_stream_ctrl,
|
||||||
status_cfar_threshold};
|
3'b000, status_cfar_threshold};
|
||||||
status_words[1] <= {status_long_chirp, status_long_listen};
|
status_words[1] <= {status_long_chirp, status_long_listen};
|
||||||
status_words[2] <= {status_guard, status_short_chirp};
|
status_words[2] <= {status_guard, status_short_chirp};
|
||||||
status_words[3] <= {status_short_listen, 10'd0, status_chirps_per_elev};
|
status_words[3] <= {status_short_listen, 10'd0, status_chirps_per_elev};
|
||||||
|
|||||||
@@ -219,10 +219,10 @@ class RadarProtocol:
|
|||||||
return None
|
return None
|
||||||
|
|
||||||
sr = StatusResponse()
|
sr = StatusResponse()
|
||||||
# Word 0: {0xFF, 3'b0, mode[1:0], 5'b0, stream[2:0], threshold[15:0]}
|
# Word 0: {0xFF[31:24], mode[23:22], stream[21:19], 3'b000[18:16], threshold[15:0]}
|
||||||
sr.cfar_threshold = words[0] & 0xFFFF
|
sr.cfar_threshold = words[0] & 0xFFFF
|
||||||
sr.stream_ctrl = (words[0] >> 16) & 0x07
|
sr.stream_ctrl = (words[0] >> 19) & 0x07
|
||||||
sr.radar_mode = (words[0] >> 21) & 0x03
|
sr.radar_mode = (words[0] >> 22) & 0x03
|
||||||
# Word 1: {long_chirp[31:16], long_listen[15:0]}
|
# Word 1: {long_chirp[31:16], long_listen[15:0]}
|
||||||
sr.long_listen = words[1] & 0xFFFF
|
sr.long_listen = words[1] & 0xFFFF
|
||||||
sr.long_chirp = (words[1] >> 16) & 0xFFFF
|
sr.long_chirp = (words[1] >> 16) & 0xFFFF
|
||||||
|
|||||||
@@ -130,8 +130,8 @@ class TestRadarProtocol(unittest.TestCase):
|
|||||||
pkt = bytearray()
|
pkt = bytearray()
|
||||||
pkt.append(STATUS_HEADER_BYTE)
|
pkt.append(STATUS_HEADER_BYTE)
|
||||||
|
|
||||||
# Word 0: {0xFF, 3'b0, mode[1:0], 5'b0, stream[2:0], threshold[15:0]}
|
# Word 0: {0xFF[31:24], mode[23:22], stream[21:19], 3'b000[18:16], threshold[15:0]}
|
||||||
w0 = (0xFF << 24) | ((mode & 0x03) << 21) | ((stream & 0x07) << 16) | (threshold & 0xFFFF)
|
w0 = (0xFF << 24) | ((mode & 0x03) << 22) | ((stream & 0x07) << 19) | (threshold & 0xFFFF)
|
||||||
pkt += struct.pack(">I", w0)
|
pkt += struct.pack(">I", w0)
|
||||||
|
|
||||||
# Word 1: {long_chirp, long_listen}
|
# Word 1: {long_chirp, long_listen}
|
||||||
|
|||||||
@@ -225,12 +225,8 @@ class TestTier1BitWidths:
|
|||||||
|
|
||||||
|
|
||||||
class TestTier1StatusWordTruncation:
|
class TestTier1StatusWordTruncation:
|
||||||
"""Catch the status_words[0] 37->32 bit truncation bug."""
|
"""Verify each status_words[] concatenation is exactly 32 bits."""
|
||||||
|
|
||||||
@pytest.mark.xfail(
|
|
||||||
reason="BUG: status_words[0] is 37 bits, truncated to 32 (FT2232H)",
|
|
||||||
strict=True,
|
|
||||||
)
|
|
||||||
def test_status_words_concat_widths_ft2232h(self):
|
def test_status_words_concat_widths_ft2232h(self):
|
||||||
"""Each status_words[] concat must be EXACTLY 32 bits."""
|
"""Each status_words[] concat must be EXACTLY 32 bits."""
|
||||||
port_widths = cp.get_usb_interface_port_widths(
|
port_widths = cp.get_usb_interface_port_widths(
|
||||||
@@ -250,10 +246,6 @@ class TestTier1StatusWordTruncation:
|
|||||||
f"Fragments: {result.fragments}"
|
f"Fragments: {result.fragments}"
|
||||||
)
|
)
|
||||||
|
|
||||||
@pytest.mark.xfail(
|
|
||||||
reason="BUG: status_words[0] is 37 bits, truncated to 32 (FT601)",
|
|
||||||
strict=True,
|
|
||||||
)
|
|
||||||
def test_status_words_concat_widths_ft601(self):
|
def test_status_words_concat_widths_ft601(self):
|
||||||
"""Same check for the FT601 interface (same bug expected)."""
|
"""Same check for the FT601 interface (same bug expected)."""
|
||||||
ft601_path = cp.FPGA_DIR / "usb_data_interface.v"
|
ft601_path = cp.FPGA_DIR / "usb_data_interface.v"
|
||||||
@@ -277,39 +269,24 @@ class TestTier1StatusWordTruncation:
|
|||||||
class TestTier1StatusFieldPositions:
|
class TestTier1StatusFieldPositions:
|
||||||
"""Verify Python status parser bit positions match Verilog layout."""
|
"""Verify Python status parser bit positions match Verilog layout."""
|
||||||
|
|
||||||
@pytest.mark.xfail(
|
|
||||||
reason="BUG: Python reads radar_mode at bit 21, actual is bit 24",
|
|
||||||
strict=True,
|
|
||||||
)
|
|
||||||
def test_python_status_mode_position(self):
|
def test_python_status_mode_position(self):
|
||||||
"""
|
"""
|
||||||
The known status_words[0] bug: Python reads mode at bits [22:21]
|
Verify Python reads radar_mode at the correct bit position matching
|
||||||
but after 37→32 truncation, mode is at [25:24]. This test should
|
the Verilog status_words[0] layout:
|
||||||
FAIL, proving the bug exists.
|
{0xFF[31:24], mode[23:22], stream[21:19], 3'b000[18:16], threshold[15:0]}
|
||||||
"""
|
"""
|
||||||
# Get what Python thinks
|
# Get what Python thinks
|
||||||
py_fields = cp.parse_python_status_fields()
|
py_fields = cp.parse_python_status_fields()
|
||||||
mode_field = next((f for f in py_fields if f.name == "radar_mode"), None)
|
mode_field = next((f for f in py_fields if f.name == "radar_mode"), None)
|
||||||
assert mode_field is not None, "radar_mode not found in parse_status_packet"
|
assert mode_field is not None, "radar_mode not found in parse_status_packet"
|
||||||
|
|
||||||
# The Verilog INTENDED layout (from the code comment) says:
|
# Ground truth: mode is at bits [23:22], so LSB = 22
|
||||||
# {0xFF, 3'b000, mode[1:0], 5'b00000, stream[2:0], threshold[15:0]}
|
expected_shift = 22
|
||||||
# But after 37→32 truncation, the actual bits are:
|
|
||||||
# [31:29]=111, [28:26]=000, [25:24]=mode, [23:19]=00000, [18:16]=stream, [15:0]=threshold
|
|
||||||
# Python extracts at shift=21, which is bits [22:21] — WRONG position.
|
|
||||||
|
|
||||||
# Ground truth: after truncation, mode is at [25:24]
|
|
||||||
expected_shift = 24
|
|
||||||
actual_shift = mode_field.lsb
|
actual_shift = mode_field.lsb
|
||||||
|
|
||||||
# This assertion documents the bug. If someone fixes status_words[0]
|
|
||||||
# to be exactly 32 bits, the intended layout becomes:
|
|
||||||
# {0xFF, mode[1:0], stream[2:0], threshold[15:0]} = 8+2+3+16 = 29 bits → pad 3 bits
|
|
||||||
# The Python shift would need updating too.
|
|
||||||
assert actual_shift == expected_shift, (
|
assert actual_shift == expected_shift, (
|
||||||
f"KNOWN BUG: Python reads radar_mode at bit {actual_shift} "
|
f"Python reads radar_mode at bit {actual_shift} "
|
||||||
f"but after status_words[0] truncation, mode is at bit {expected_shift}. "
|
f"but Verilog status_words[0] has mode at bit {expected_shift}."
|
||||||
f"Both Verilog AND Python need fixing."
|
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -442,10 +419,6 @@ class TestTier1STM32SettingsPacket:
|
|||||||
assert f.size == esize, f"{f.name}: size {f.size} != {esize}"
|
assert f.size == esize, f"{f.name}: size {f.size} != {esize}"
|
||||||
assert f.c_type == etype, f"{f.name}: type {f.c_type} != {etype}"
|
assert f.c_type == etype, f"{f.name}: type {f.c_type} != {etype}"
|
||||||
|
|
||||||
@pytest.mark.xfail(
|
|
||||||
reason="BUG: RadarSettings.cpp min check is 74, should be 82",
|
|
||||||
strict=True,
|
|
||||||
)
|
|
||||||
def test_minimum_packet_size(self):
|
def test_minimum_packet_size(self):
|
||||||
"""
|
"""
|
||||||
RadarSettings.cpp says minimum is 74 bytes but actual payload is:
|
RadarSettings.cpp says minimum is 74 bytes but actual payload is:
|
||||||
@@ -593,10 +566,6 @@ class TestTier2VerilogCosim:
|
|||||||
f"detection: {parsed['detection']} != 1"
|
f"detection: {parsed['detection']} != 1"
|
||||||
)
|
)
|
||||||
|
|
||||||
@pytest.mark.xfail(
|
|
||||||
reason="BUG: radar_mode reads 0 due to truncation + wrong bit pos",
|
|
||||||
strict=True,
|
|
||||||
)
|
|
||||||
def test_status_packet_python_round_trip(self, tb_results):
|
def test_status_packet_python_round_trip(self, tb_results):
|
||||||
"""
|
"""
|
||||||
Take the 26 bytes captured by the Verilog TB, run Python's
|
Take the 26 bytes captured by the Verilog TB, run Python's
|
||||||
@@ -649,19 +618,16 @@ class TestTier2VerilogCosim:
|
|||||||
assert sr.self_test_detail == 0xA5, f"self_test_detail: 0x{sr.self_test_detail:02X}"
|
assert sr.self_test_detail == 0xA5, f"self_test_detail: 0x{sr.self_test_detail:02X}"
|
||||||
assert sr.self_test_busy == 1, f"self_test_busy: {sr.self_test_busy}"
|
assert sr.self_test_busy == 1, f"self_test_busy: {sr.self_test_busy}"
|
||||||
|
|
||||||
# Word 0: This tests the truncation bug.
|
# Word 0: stream_ctrl should be 5 (3'b101)
|
||||||
# stream_ctrl should be 5 (3'b101)
|
|
||||||
assert sr.stream_ctrl == 5, (
|
assert sr.stream_ctrl == 5, (
|
||||||
f"stream_ctrl: {sr.stream_ctrl} != 5. "
|
f"stream_ctrl: {sr.stream_ctrl} != 5. "
|
||||||
f"Check status_words[0] bit positions."
|
f"Check status_words[0] bit positions."
|
||||||
)
|
)
|
||||||
|
|
||||||
# radar_mode should be 3 (2'b11) — THIS WILL FAIL due to
|
# radar_mode should be 3 (2'b11)
|
||||||
# the known 37→32 truncation bug + Python wrong shift.
|
|
||||||
assert sr.radar_mode == 3, (
|
assert sr.radar_mode == 3, (
|
||||||
f"KNOWN BUG: radar_mode={sr.radar_mode} != 3. "
|
f"radar_mode={sr.radar_mode} != 3. "
|
||||||
f"status_words[0] is 37 bits (truncated to 32) and "
|
f"Check status_words[0] bit positions."
|
||||||
f"Python reads mode at wrong bit position."
|
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user