Two independent root causes landed together in the recent develop merge
burst, leaving CI red on every commit since. Both fixes are narrow parser/
config updates; no code logic changes.
1. Ruff T201 violation in 8_Utils/Python/LUT.py:24
The recent restoration of `print(f"8'd{val},")` (replacing `pass`) made
the LUT generator functional again, but the file is not in pyproject's
per-file-ignores so ruff's T20 rule flags it. Add a narrow per-file-
ignore for T20 specifically scoped to LUT.py.
2. Five tests in TestTier1Adar1000ChannelRegisterRoundTrip
The recent SPI/ADC error-propagation refactor on ADAR1000_Manager.cpp
changed the four setters adarSet{Rx,Tx}{Phase,VgaGain} from `void` to
`bool` returns AND introduced the `ok = setter(args) && ok` error-chain
idiom at internal call sites. The test class's parser regexes were
authored for the pre-refactor convention.
Two regex extensions:
* Body parser: `void\s+...` -> `(?:void|bool)\s+...` so bodies are
found under either return-type convention.
* Caller finder: `\)\s*;` -> `\)(?:\s*&&\s*\w+)*\s*;` so the
error-chain idiom is matched alongside standalone calls.
Body extraction logic, REG_CHn_XXX + <expr> regex, symbolic AST
evaluation, round-trip strict-equality, and stride detection are all
unchanged. The two regex extensions only acknowledge the new C++
conventions introduced by the SPI/ADC refactor.
Verified locally:
- `uv run ruff check .` returns 0 errors.
- `uv run pytest 9_Firmware/tests/cross_layer/test_cross_layer_contract.py`
passes 51 tests + 5 skipped (Tier2VerilogCosim local-only, requires
iverilog which is installed in CI).
- `uv run pytest 9_Firmware/9_3_GUI/test_v7.py` passes 83 tests + 3
skipped (no regression).
smoke_test.py:154-155 extracted self-test flags/detail from
status.cfar_threshold — but that field carries the CFAR detection
threshold (opcode 0x03 readback), not self-test results.
RadarProtocol.parse_status_packet already exposes the correct fields
via status.self_test_flags and status.self_test_detail (radar_protocol.py:257,
word 5: {7'd0, self_test_busy, 8'd0, self_test_detail[7:0], 3'd0,
self_test_flags[4:0]}). test_GUI_V65_Tk.py:198 pins these fields as
the contract.
Result: smoke_test on live hardware would have decoded garbage (CFAR
threshold bits) as per-subsystem PASS/FAIL flags. Fix replaces the
two assignments and refreshes the stale "simplification" comment
that predated the dedicated status fields.
Regression: 137 passed, 3 skipped (test_v7.py + cross_layer). 8
self_test invariant tests in test_GUI_V65_Tk.py still pass.
The volatile fix in the companion PR (#118) makes the safe-mode blink loop
escapable in principle, but no firmware path existed to actually clear
system_emergency_state at runtime — hardware reset was the only exit, which
fires the IWDG and re-energises the PA rails that Emergency_Stop() cut.
This change adds a FAULT_ACK command (opcode 0x40): the host sends an exact
4-byte CDC packet [0x40, 0x00, 0x00, 0x00]; USBHandler detects it regardless
of USB state and sets fault_ack_received; the blink loop checks the flag each
250 ms iteration and clears system_emergency_state, allowing a controlled
operator-acknowledged recovery without triggering a watchdog reset.
Detection is guarded to exact 4-byte packets only. Scanning larger packets
for the subsequence would false-trigger on the IEEE 754 big-endian encoding
of 2.0 (0x4000000000000000), which starts with the same 4 bytes and can
appear in normal settings doubles.
FAULT_ACK is excluded from the FPGA opcode enum to preserve the
Python/Verilog bidirectional contract test; contract_parser.py reads the
new MCU_ONLY_OPCODES frozenset in radar_protocol.py to filter it.
7 new test vectors in test_gap3_fault_ack_clears_emergency.c cover:
detection, loop exit, loop hold without ack, settings false-positive
immunity, truncated packet, wrong opcode, and multi-iteration sequence.
Reported-by: shaun0927 (Junghwan) <https://github.com/shaun0927>
Bug 1 (main.cpp:630): system_emergency_state lacked volatile. Under -O1+
the compiler is permitted to hoist the read outside the blink loop, making
while (system_emergency_state) unconditionally infinite. Once entered, the
only escape was the 4 s IWDG timeout — which resets the MCU and
re-energizes the PA rails that Emergency_Stop() explicitly cut. Marking the
variable volatile forces a memory read on every iteration so an external
clear (ISR, USB command, manual reset) can break the loop correctly.
Bug 2 (ADAR1000_AGC.cpp:59): holdoff_frames is a public uint8_t; if a
caller sets it to 0, the condition holdoff_counter >= holdoff_frames is
always true (any uint8_t >= 0), causing the AGC outer loop to increase gain
on every non-saturated frame with no holdoff delay. With alternating
sat/no-sat frames this produces a ±step oscillation that prevents the
receiver from settling. Fix: clamp holdoff_frames to a minimum of 1 in the
constructor, preserving all existing test assertions (none use 0; default
remains 4).
Reported-by: shaun0927 (Junghwan) <https://github.com/shaun0927>
5-bit signed subtraction in clamp_gain wrapped for agc_attack >= 10 or
agc_decay >= 9 when |agc_gain| + step > 16, inverting gain polarity
instead of clamping — e.g. gain=-7, attack=10 produced +7 (max amplify)
rather than -7 (max attenuate), causing ADC saturation on strong returns.
Widen clamp_gain input to [5:0] and sign-extend both operands to 6 bits
({agc_gain[3],agc_gain[3],agc_gain} and {2'b00,agc_attack/decay}),
covering the full [-22,+22] range before clamping. Default attack/decay
values (1-4) are unaffected; behaviour changes only for values >= 10/9.
- Move WaveformConfig() from per-frame local in _run_host_dsp to
self._waveform in __init__, mirroring ReplayWorker pattern.
- Add set_waveform() to RadarDataWorker for injection symmetry with
ReplayWorker.set_waveform() — live path is now configurable.
- Replace hardcoded fallback 32 with self._waveform.n_doppler_bins.
- Update AST contract tests: WaveformConfig() check moves to __init__
parse; attribute chains updated from ("wf", ...) to
("self", "_waveform", ...) to match renamed accessor.
Use WaveformConfig for live range/velocity conversion in RadarDataWorker
and add headless AST-based regression checks in test_v7.py.
Before: RadarDataWorker._run_host_dsp used RadarSettings.velocity_resolution
(default 1.0 in models.py:113), while ReplayWorker used WaveformConfig
(~5.343 m/s/bin). Live GUI under-reported velocity by factor ~5.34x.
Fix: local WaveformConfig() in _run_host_dsp, mirroring ReplayWorker
pattern. Doppler center derived from frame shape, matching processing.py:520.
Test: TestLiveReplayPhysicalUnitsParity in test_v7.py uses ast.parse on
workers.py (no v7.workers import, headless-CI-safe despite PyQt6 dep)
and asserts AST Call/Attribute/BinOp nodes for both RadarDataWorker
and ReplayWorker paths.
Three conflicts — all resolved in favor of develop, which has a more
refined version of the same work this branch introduced:
- radar_system_top.v: develop's cleaner USB_MODE=1 comment (same value).
- run_regression.sh: develop's ${SYSTEM_RTL[@]} refactor + added
USB_MODE=1 test variants.
- tb/radar_system_tb.v: develop's ifdef USB_MODE_1 to dump the correct
USB instance based on mode.
The 400 MHz reset fan-out fix (nco_400m_enhanced, cic_decimator_4x_enhanced,
ddc_400m) and ADAR1000 channel-indexing fix remain intact on this branch.
Replace direct !reset_n async sense with a registered active-high reset_h
(max_fanout=50) in nco_400m_enhanced, cic_decimator_4x_enhanced, and
ddc_400m. The prior single-LUT1 / 700+ load net was the root cause of
WNS=-0.626 ns in the 400 MHz clock domain on the xc7a50t build. Vivado
replicates the constrained register into ≈14 regional copies, each driving
≤50 loads, closing timing at 2.5 ns.
Change radar_system_top default USB_MODE from 0 (FT601) to 1 (FT2232H).
FT601 remains available for the 200T premium board via explicit parameter
override; the 50T production wrapper already hard-codes USB_MODE=1.
Regression: add usb_data_interface_ft2232h.v to PROD_RTL lint list and
both system-top TB compile commands; fix legacy radar_system_tb hierarchical
probe from gen_ft601.usb_inst to gen_ft2232h.usb_inst.
Golden reference files (rtl_bb_dc.csv, rx_final_doppler_out.csv,
golden_doppler.mem) regenerated to reflect the +1-cycle registered-reset
boundary behaviour; Receiver golden-compare passes 18/18 checks.
All 25 regression tests pass (0 failures, 0 skipped).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Surfaces the three-point AI usage policy Jason articulated in #106 by
placing it in CONTRIBUTING.md between "Code Standards & Tooling" and
"Running the Test Suites", so first-time AI-assisted contributors see
the expectation in the onboarding doc rather than having to discover
it via issue archaeology. Text is the #106 comment verbatim with two
small typo fixes only (use if AI -> use of AI; doesnt -> doesn't); no
structural or stylistic rewriting.
Per Jason's green light to open this PR in
NawfalMotii79/PLFM_RADAR#106 (comment-4273144522).
The four channel-indexed ADAR1000 setters (adarSetRxPhase, adarSetTxPhase,
adarSetRxVgaGain, adarSetTxVgaGain) computed their register offset as
`(channel & 0x03) * stride`, which silently aliased CH4 (channel=4 ->
mask=0) onto CH1 and shifted CH1..CH3 by one. The API contract (1-based
CH1..CH4) is documented in ADAR1000_AGC.cpp:76 and matches the ADI
datasheet; every existing caller already passes `ch + 1`.
Fix: subtract 1 before masking -- `((channel - 1) & 0x03) * stride` --
and reject `channel < 1 || channel > 4` early with a DIAG message so a
future stale 0-based caller fails loudly instead of writing to CH4.
Adds TestTier1Adar1000ChannelRegisterRoundTrip (9 tests) which closes
the loop independently of the driver:
- parses the ADI register map directly from ADAR1000_Manager.h,
- verifies the datasheet stride invariants (gain=1, phase=2),
- auto-discovers every C++ TU under MCU_LIB_DIR / MCU_CODE_DIR so a
new caller cannot silently escape the round-trip check,
- asserts every caller's channel argument evaluates to {1,2,3,4} for
ch in {0,1,2,3} (catches bare 0-based or literal-0 callers at CI
time before the runtime bounds-check would silently drop them),
- round-trips each (caller, ch) through the helper arithmetic and
checks the final address equals REG_CH{ch+1}_*.
Adversarially validated: reverting any one helper, all four helpers,
corrupting the parsed register map, injecting a bare-ch caller, and
auto-discovering a literal-0 caller in a fresh TU each cause the
expected (and only the expected) test to fail.
Stacked on fix/adar1000-vm-tables (PR #107).
The ADAR1000 vector-modulator I/Q lookup tables VM_I[128] and VM_Q[128]
were declared but defined as empty initialiser lists since the first
commit (5fbe97f). Every call to adarSetRxPhase / adarSetTxPhase therefore
wrote (I=0x00, Q=0x00) to registers 0x21/0x23 (Rx) and 0x32/0x34 (Tx)
regardless of the requested phase state, leaving beam steering completely
non-functional in firmware.
This commit:
* Populates VM_I[128] and VM_Q[128] from ADAR1000 datasheet Rev. B
Tables 13-16 (p.34) on a uniform 2.8125 deg grid (360 / 128 states).
Byte format: bits[7:6] reserved 0, bit[5] polarity (1 = positive
lobe), bits[4:0] 5-bit unsigned magnitude - exactly as specified.
* Removes VM_GAIN[128] declaration and (empty) definition. The
ADAR1000 has no separate VM gain register; per-channel VGA gain is
set via CHx_RX_GAIN (0x10-0x13) / CHx_TX_GAIN (0x1C-0x1F) by
adarSetRxVgaGain / adarSetTxVgaGain. VM_GAIN was never populated,
never read anywhere in the firmware, and its presence falsely
suggested a missing scaling step in the signal path.
* Adds 9_Firmware/tests/cross_layer/adar1000_vm_reference.py: an
independently-derived ground-truth module containing the full
datasheet table plus byte-format / uniform-grid / quadrant-symmetry
/ cardinal-point invariant checkers and a tolerant C array parser.
* Adds TestTier2Adar1000VmTableGroundTruth (9 tests) to
test_cross_layer_contract.py, including a tokenising C/C++
comment+string stripper used by the VM_GAIN reintroduction guard,
and an adversarial self-test that corrupts one byte and asserts
the comparison detects it (defends against silent bypass via
future fixture/parser refactors).
Adversarially validated: removing the firmware definitions, flipping
a single byte, or reintroducing VM_GAIN as code each cause the suite
to fail; restoring causes it to pass. VM_GAIN appearing inside string
literals or comments correctly does NOT trip the guard.
Closes the empty-table half of the ADAR1000 phase-control bug class.
The separate channel-rotation issue (#90) will be addressed in a
follow-up PR.
Refs: 7_Components Datasheets and Application notes/ADAR1000.pdf
Rev. B Tables 13-16 p.34
Tier-1 had only one explicit per-field assertion (radar_mode at lsb=22).
The Tier-2 round-trip uses the same Python parser as oracle for status
word decoding, so a coupled Verilog+Python bit-position shift in
status_words[0]/[3]/[4]/[5] passes Tier-2 silently — only [1] and [2]
have an independent raw-byte check in tb_cross_layer_ft2232h.v.
Add an independent static check that walks each Verilog status_words[N]
concatenation MSB->LSB via count_concat_bits, computes (word_idx, lsb,
width) for every named payload fragment, drops literal padding, and
compares against every field parse_status_packet() extracts. Name match
is status_<python_name> (current convention has zero exceptions).
Mismatches accumulate into a single failure message so one run surfaces
all drift at once.
Parametrized over both usb_data_interface_ft2232h.v and
usb_data_interface.v since both are live post PR #89.
Brings in main-only commits that never reached develop:
754d919 Added silk screen and headers description (MainBoard .brd)
0443516 Added thermal vias (RF_PA .brd)
5fbe051 Added ABAC INDUSTRY web site (Project_Description.docx)
12b549d / 5d5e9ff Merge PR #101: README BOM sensor counts fix
No conflicts expected: develop has not touched any of these paths.
The STM32 peripheral list in the README disagreed with the production
BOM (4_7_Production Files/Gerber_Main_Board/RADAR_Main_Board_BOM_csv)
and with the firmware (9_1_Microcontroller/.../main.cpp). Corrections
based on origin/main commit 754d919:
- ADS7830 Idq ADCs: placed on the Main Board (U88 @ 0x48, U89 @ 0x4A),
not on the Power Amplifier Boards. Added the INA241A3 (x50) and 5 mOhm
shunt detail that completes the current-sense chain.
- DAC5578 Vg DACs: placed on the Main Board (U7 @ 0x48, U69 @ 0x49),
not on the Power Amplifier Boards. Noted closed-loop Idq calibration
at boot (main.cpp powerUpSequence).
- Temperature sensors: 1x ADS7830 (U10) with 8 single-ended channels
reading 8 thermistors -- not 8 separate ADS7830 chips. Cooling is a
single GPIO (EN_DIS_COOLING), bang-bang, not PWM.
- GPS: reflect the UM982 driver merged in #79 and its role in
per-detection position tagging beyond map centering.
Counts now match the 3x ADS7830 / 2x DAC5578 / 16x INA241A3 population
in the production BOM.
