Mirrors/NawfalMotii79-PLFM_RADAR
1
0
Fork 0
mirror of https://github.com/NawfalMotii79/PLFM_RADAR.git synced 2026-06-10 07:27:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(mcu): PR-V — ADF4382A Stage-5 audit fixes (F-5.1..F-5.10)

Browse Source 
F-5.1: revert PWM scaffolding to binary DELADJ. Schematic-verified:
  PG7/PG13 on STM32F746ZGT7 have no TIM3 alternate function (Port G AFs are
  FMC/ETH/USART6/SAI2/SDMMC2 — no TIMx routes), and the FreqSynth-board
  DELADJ net has only a 200 kOhm pulldown (R22, R35) — no series-R +
  shunt-C LPF for PWM-to-DC. The 3979693 (Bug #5) + c466021 (B15) PWM
  scaffolding was a false-fix; 5fbe97f's original honest TODO matched the
  actual hardware. Delete htim3, MX_TIM3_Init, start/stop_deladj_pwm,
  phase_ps_to_duty_cycle. Rewrite test_bug5 for binary; delete test_bug15.

F-5.2: split ADF4382A ref_div per device. RX 10.38 GHz / 300 MHz = 34.6 is
  fractional mode, but ADF4382_PFD_FREQ_FRAC_MAX = 250 MHz — driver does
  not reject the out-of-spec config, ldwin_pw silently left at 0. Set
  rx_param.ref_div = 2 -> PFD = 150 MHz, in spec. TX unchanged (integer).

F-5.3: free prior tx_dev/rx_dev in Manager_Init before re-allocating. The
  recovery dispatch on TX/RX unlock calls Manager_Init again; previous
  adf4382_dev allocations were leaking. Mirrors F-4.5 fix for AD9523.

F-5.4: fix upstream adf4382_remove() — only freed dev struct on FAILED SPI
  removal (success path leaked) and always returned 0. Now: NULL guard,
  unconditional free, propagate ret.

F-5.8: lock-detect uses register reg[0x58] LOCKED bit as authoritative.
  GPIO disagreement still logged via DIAG_WARN but no longer flips the
  result — a mis-routed GPIO LKDET would otherwise trigger false-unlock
  recovery loops.

F-5.10: drop stale "EZSYNC" diagnostic string (post-C-14a residue).

Bench-side checks for first power-on:
- Scope PG13 (TX_DELADJ) and PG7 (RX_DELADJ) — both should be HIGH (3.3V)
  after SetPhaseShift(500,500) runs at boot.
- Confirm both ADF4382A LOs lock with PFD=150 MHz on RX (was 300 MHz).
  Lock-time may be slightly longer; phase-noise sidebands shift.
- Confirm no false-unlock storms on the recovery path — the GPIO LKDET
  disagreement DIAG_WARN should no longer flip the lock decision.

Regression: tests/ make test 34/34 PASS (was 35/35 baseline; -1 from
test_bug15 deletion as planned).
This commit is contained in:
Jason
2026-05-05 09:20:06 +05:45
parent e1e5ae464a
commit 00d5d5f220
6 changed files with 148 additions and 305 deletions
Download Patch File Download Diff File Expand all files Collapse all files
9_Firmware/9_1_Microcontroller/tests/Makefile
+2 -6
View File
@@ -51,8 +51,7 @@ TESTS_WITH_REAL := test_bug1_timed_sync_init_ordering \
test_bug4_phase_shift_before_check \
test_bug5_fine_phase_gpio_only \
test_bug9_platform_ops_null \
test_bug10_spi_cs_not_toggled \
test_bug15_htim3_dangling_extern
test_bug10_spi_cs_not_toggled
# Tests that only need mocks (extracted patterns / static analysis)
TESTS_MOCK_ONLY := test_bug2_ad9523_double_setup \
@@ -95,7 +94,7 @@ TESTS_GPS := test_um982_gps
ALL_TESTS := $(TESTS_WITH_REAL) $(TESTS_MOCK_ONLY) $(TESTS_STANDALONE) $(TESTS_WITH_PLATFORM) $(TESTS_WITH_CXX) $(TESTS_GPS)
.PHONY: all build test clean \
$(addprefix test_,bug1 bug2 bug3 bug4 bug5 bug6 bug7 bug8 bug9 bug10 bug11 bug12 bug13 bug14 bug15) \
$(addprefix test_,bug1 bug2 bug3 bug4 bug5 bug6 bug7 bug8 bug9 bug10 bug11 bug12 bug13 bug14) \
test_gap3_estop test_gap3_iwdg test_gap3_temp test_gap3_idq test_gap3_order \
test_gap3_overtemp test_gap3_wdog
@@ -302,9 +301,6 @@ test_bug13: test_bug13_dac2_adc_buffer_mismatch
test_bug14: test_bug14_diag_section_args
./test_bug14_diag_section_args
test_bug15: test_bug15_htim3_dangling_extern
./test_bug15_htim3_dangling_extern
test_gap3_estop: test_gap3_emergency_stop_rails
./test_gap3_emergency_stop_rails
9_Firmware/9_1_Microcontroller/tests/test_bug15_htim3_dangling_extern.c
-89
View File
@@ -1,89 +0,0 @@
/*******************************************************************************
* test_bug15_htim3_dangling_extern.c
*
* Bug #15 (FIXED): adf4382a_manager.c declared `extern TIM_HandleTypeDef htim3`
* but main.cpp had no `TIM_HandleTypeDef htim3` definition and no
* `MX_TIM3_Init()` call. The extern resolved to nothing → linker error or
* zero-initialized BSS → PWM calls operate on unconfigured timer hardware.
*
* Fix:
* - Added `TIM_HandleTypeDef htim3;` definition in main.cpp (line ~117)
* - Added `static void MX_TIM3_Init(void)` prototype + implementation
* - Added `MX_TIM3_Init();` call in peripheral init sequence
* - TIM3 configured for PWM mode: Prescaler=71, Period=999 (=DELADJ_MAX_DUTY_CYCLE),
* CH2 (TX DELADJ) and CH3 (RX DELADJ) in PWM1 mode
*
* Test strategy:
* 1. Verify htim3 is defined (not just extern) in the mock environment
* 2. Verify SetFinePhaseShift works with the timer (reuses test_bug5 pattern)
* 3. Verify PWM start/stop on both channels works without crash
******************************************************************************/
#include "adf4382a_manager.h"
#include <assert.h>
#include <stdio.h>
int main(void)
{
ADF4382A_Manager mgr;
int ret;
printf("=== Bug #15 (FIXED): htim3 defined + TIM3 PWM configured ===\n");
/* Test 1: htim3 exists and has a valid id */
printf(" Test 1: htim3 is defined (id=%u)... ", htim3.id);
assert(htim3.id == 3);
printf("PASS\n");
/* Test 2: Init manager, then use SetFinePhaseShift which exercises htim3 */
spy_reset();
ret = ADF4382A_Manager_Init(&mgr, SYNC_METHOD_TIMED);
assert(ret == ADF4382A_MANAGER_OK);
printf(" Test 2: Manager init OK\n");
/* Test 3: Intermediate duty cycle on TX (CH2) → PWM start + set compare */
spy_reset();
ret = ADF4382A_SetFinePhaseShift(&mgr, 0, 500);
assert(ret == ADF4382A_MANAGER_OK);
int pwm_starts = spy_count_type(SPY_TIM_PWM_START);
int set_compares = spy_count_type(SPY_TIM_SET_COMPARE);
printf(" Test 3: TX duty=500 → PWM_START=%d SET_COMPARE=%d... ",
pwm_starts, set_compares);
assert(pwm_starts == 1);
assert(set_compares == 1);
/* Verify the timer used is htim3 (id=3) */
int idx = spy_find_nth(SPY_TIM_PWM_START, 0);
const SpyRecord *r = spy_get(idx);
assert(r != NULL && r->value == 3); /* htim3.id == 3 */
printf("timer_id=%u (htim3) PASS\n", r->value);
/* Test 4: Intermediate duty cycle on RX (CH3) */
spy_reset();
ret = ADF4382A_SetFinePhaseShift(&mgr, 1, 300);
assert(ret == ADF4382A_MANAGER_OK);
idx = spy_find_nth(SPY_TIM_PWM_START, 0);
r = spy_get(idx);
assert(r != NULL);
printf(" Test 4: RX duty=300 → channel=0x%02X (expected 0x%02X=CH3) timer_id=%u... ",
r->pin, TIM_CHANNEL_3, r->value);
assert(r->pin == TIM_CHANNEL_3);
assert(r->value == 3);
printf("PASS\n");
/* Test 5: duty=0 stops PWM gracefully */
spy_reset();
ret = ADF4382A_SetFinePhaseShift(&mgr, 0, 0);
assert(ret == ADF4382A_MANAGER_OK);
int pwm_stops = spy_count_type(SPY_TIM_PWM_STOP);
printf(" Test 5: duty=0 → PWM_STOP=%d... ", pwm_stops);
assert(pwm_stops == 1);
printf("PASS\n");
ADF4382A_Manager_Deinit(&mgr);
printf("\n=== Bug #15: ALL TESTS PASSED (post-fix) ===\n\n");
return 0;
}
9_Firmware/9_1_Microcontroller/tests/test_bug5_fine_phase_gpio_only.c
+68 -66
View File
@@ -1,19 +1,31 @@
/*******************************************************************************
* test_bug5_fine_phase_gpio_only.c
*
* Bug #5 (FIXED): ADF4382A_SetFinePhaseShift() was a GPIO-only placeholder.
* For intermediate duty_cycle values it just set GPIO HIGH — same as max.
* F-5.1: SetFinePhaseShift / SetPhaseShift use binary GPIO only.
*
* Fix: Intermediate duty cycles now use TIM3 PWM output (CH2 for TX, CH3 for
* RX). The PWM output is low-pass filtered externally to produce a DC voltage
* proportional to the delay. Edge cases (0 and max) still use static GPIO.
* Schematic-verified rationale:
* - MCU is STM32F746ZGT7. PG7 (RX_DELADJ) and PG13 (TX_DELADJ) have no TIM3
* alternate function (Port G AFs are FMC/ETH/USART6/SAI2/SDMMC2 — no
* TIMx routes). Even configuring AF mode would have no valid AF code.
* - FreqSynth board: DELADJ net has only a 200 kOhm pulldown (R22 on TX,
* R35 on RX). No series-R + shunt-C low-pass filter exists on the board,
* so a PWM-to-DC scheme cannot work as built.
*
* Test strategy (post-fix):
* 1. duty=0 → PWM stopped, GPIO LOW (no change).
* 2. duty=MAX → PWM stopped, GPIO HIGH (no change).
* 3. duty=500 (intermediate) → SPY_TIM_SET_COMPARE + SPY_TIM_PWM_START
* recorded, NO static GPIO write for the DELADJ pin.
* 4. Verify compare value matches the duty cycle.
* Prior history:
* - 5fbe97f (2026-03-09 initial upload): GPIO-only with explicit TODO
* "// In a real system, you would generate a PWM signal on DELADJ pin".
* - 3979693 (2026-03-19 "Bug #5 fix"): added HAL_TIM_PWM_Start scaffolding
* calling htim3 (which didn't exist yet) — false-fix.
* - c466021 (2026-03-19 "B15 fix"): added MX_TIM3_Init to define htim3 —
* timer ran internally but the pin's AF mux was never enabled.
* - This commit: revert to binary GPIO matching the schematic's actual
* intent. Delete the PWM scaffolding and htim3 entirely.
*
* Test strategy (binary):
* 1. duty=0 -> 1 GPIO write LOW (no PWM API touched)
* 2. duty=MAX -> 1 GPIO write HIGH (no PWM API touched)
* 3. duty=500 (any nonzero) -> 1 GPIO write HIGH (no PWM API touched)
* 4. RX device (1) -> writes RX_DELADJ_Pin (PG7), not TX_DELADJ_Pin (PG13)
******************************************************************************/
#include "adf4382a_manager.h"
#include <assert.h>
@@ -24,98 +36,88 @@ int main(void)
ADF4382A_Manager mgr;
int ret;
printf("=== Bug #5 (FIXED): SetFinePhaseShift uses TIM PWM ===\n");
printf("=== F-5.1: SetFinePhaseShift uses binary GPIO (no PWM) ===\n");
/* Setup: init manager */
/* Setup */
spy_reset();
ret = ADF4382A_Manager_Init(&mgr, SYNC_METHOD_TIMED);
assert(ret == ADF4382A_MANAGER_OK);
/* ---- Test A: duty_cycle=0 → PWM stopped, GPIO LOW ---- */
/* ---- Test A: duty=0 -> GPIO LOW, no PWM ---- */
spy_reset();
ret = ADF4382A_SetFinePhaseShift(&mgr, 0, 0);
assert(ret == ADF4382A_MANAGER_OK);
int pwm_stop_count = spy_count_type(SPY_TIM_PWM_STOP);
int gpio_writes = spy_count_type(SPY_GPIO_WRITE);
printf(" duty=0: PWM_STOP=%d GPIO_WRITE=%d\n", pwm_stop_count, gpio_writes);
assert(pwm_stop_count == 1); /* stop PWM before driving GPIO */
assert(gpio_writes >= 1); /* at least one GPIO write (LOW) */
int gpio_writes = spy_count_type(SPY_GPIO_WRITE);
int pwm_starts = spy_count_type(SPY_TIM_PWM_START);
int pwm_stops = spy_count_type(SPY_TIM_PWM_STOP);
int set_compares = spy_count_type(SPY_TIM_SET_COMPARE);
printf(" duty=0: GPIO_WRITE=%d PWM_START=%d PWM_STOP=%d SET_COMPARE=%d\n",
gpio_writes, pwm_starts, pwm_stops, set_compares);
assert(gpio_writes == 1);
assert(pwm_starts == 0 && pwm_stops == 0 && set_compares == 0);
/* Verify the GPIO write is LOW */
int idx = spy_find_nth(SPY_GPIO_WRITE, 0);
const SpyRecord *r = spy_get(idx);
assert(r != NULL && r->value == GPIO_PIN_RESET);
printf(" PASS: duty=0 → PWM stopped + GPIO LOW\n");
assert(r->pin == TX_DELADJ_Pin);
printf(" PASS: duty=0 -> GPIO LOW on TX_DELADJ_Pin (PG13), no PWM\n");
/* ---- Test B: duty_cycle=MAX → PWM stopped, GPIO HIGH ---- */
/* ---- Test B: duty=MAX -> GPIO HIGH, no PWM ---- */
spy_reset();
ret = ADF4382A_SetFinePhaseShift(&mgr, 0, DELADJ_MAX_DUTY_CYCLE);
assert(ret == ADF4382A_MANAGER_OK);
pwm_stop_count = spy_count_type(SPY_TIM_PWM_STOP);
gpio_writes = spy_count_type(SPY_GPIO_WRITE);
printf(" duty=MAX(%d): PWM_STOP=%d GPIO_WRITE=%d\n",
DELADJ_MAX_DUTY_CYCLE, pwm_stop_count, gpio_writes);
assert(pwm_stop_count == 1);
assert(gpio_writes >= 1);
gpio_writes = spy_count_type(SPY_GPIO_WRITE);
pwm_starts = spy_count_type(SPY_TIM_PWM_START);
pwm_stops = spy_count_type(SPY_TIM_PWM_STOP);
set_compares = spy_count_type(SPY_TIM_SET_COMPARE);
printf(" duty=MAX(%d): GPIO_WRITE=%d PWM_START=%d PWM_STOP=%d SET_COMPARE=%d\n",
DELADJ_MAX_DUTY_CYCLE, gpio_writes, pwm_starts, pwm_stops, set_compares);
assert(gpio_writes == 1);
assert(pwm_starts == 0 && pwm_stops == 0 && set_compares == 0);
idx = spy_find_nth(SPY_GPIO_WRITE, 0);
r = spy_get(idx);
assert(r != NULL && r->value == GPIO_PIN_SET);
printf(" PASS: duty=MAX → PWM stopped + GPIO HIGH\n");
assert(r->pin == TX_DELADJ_Pin);
printf(" PASS: duty=MAX -> GPIO HIGH on TX_DELADJ_Pin (PG13), no PWM\n");
/* ---- Test C: duty_cycle=500 (intermediate) → TIM PWM ---- */
/* ---- Test C: duty=500 (intermediate) -> GPIO HIGH (binary mapping) ---- */
spy_reset();
ret = ADF4382A_SetFinePhaseShift(&mgr, 0, 500); /* device=0 (TX) */
ret = ADF4382A_SetFinePhaseShift(&mgr, 0, 500);
assert(ret == ADF4382A_MANAGER_OK);
int pwm_start_count = spy_count_type(SPY_TIM_PWM_START);
int set_compare_count = spy_count_type(SPY_TIM_SET_COMPARE);
gpio_writes = spy_count_type(SPY_GPIO_WRITE);
printf(" duty=500: PWM_START=%d SET_COMPARE=%d GPIO_WRITE=%d\n",
pwm_start_count, set_compare_count, gpio_writes);
assert(pwm_start_count == 1);
assert(set_compare_count == 1);
assert(gpio_writes == 0); /* No static GPIO write for intermediate */
gpio_writes = spy_count_type(SPY_GPIO_WRITE);
pwm_starts = spy_count_type(SPY_TIM_PWM_START);
set_compares = spy_count_type(SPY_TIM_SET_COMPARE);
printf(" duty=500 (intermediate): GPIO_WRITE=%d PWM_START=%d SET_COMPARE=%d\n",
gpio_writes, pwm_starts, set_compares);
assert(gpio_writes == 1);
assert(pwm_starts == 0 && set_compares == 0);
/* Verify compare value is 500 */
idx = spy_find_nth(SPY_TIM_SET_COMPARE, 0);
idx = spy_find_nth(SPY_GPIO_WRITE, 0);
r = spy_get(idx);
assert(r != NULL);
printf(" SET_COMPARE value=%u (expected 500)\n", r->value);
assert(r->value == 500);
assert(r != NULL && r->value == GPIO_PIN_SET);
printf(" PASS: duty=500 -> GPIO HIGH (binary: any nonzero -> HIGH)\n");
/* Verify TIM channel is CH2 (TX device = 0 → TIM_CHANNEL_2 = 0x04) */
idx = spy_find_nth(SPY_TIM_PWM_START, 0);
r = spy_get(idx);
assert(r != NULL);
printf(" PWM_START channel=0x%02X (expected 0x%02X = TIM_CHANNEL_2)\n",
r->pin, TIM_CHANNEL_2);
assert(r->pin == TIM_CHANNEL_2);
printf(" PASS: duty=500 → TIM PWM with correct compare value\n");
/* ---- Test D: RX device (1) uses TIM_CHANNEL_3 ---- */
/* ---- Test D: RX device (1) writes RX_DELADJ_Pin, not TX ---- */
spy_reset();
ret = ADF4382A_SetFinePhaseShift(&mgr, 1, 750); /* device=1 (RX) */
ret = ADF4382A_SetFinePhaseShift(&mgr, 1, 750);
assert(ret == ADF4382A_MANAGER_OK);
idx = spy_find_nth(SPY_TIM_PWM_START, 0);
idx = spy_find_nth(SPY_GPIO_WRITE, 0);
r = spy_get(idx);
assert(r != NULL);
printf(" RX PWM_START channel=0x%02X (expected 0x%02X = TIM_CHANNEL_3)\n",
r->pin, TIM_CHANNEL_3);
assert(r->pin == TIM_CHANNEL_3);
idx = spy_find_nth(SPY_TIM_SET_COMPARE, 0);
r = spy_get(idx);
assert(r != NULL && r->value == 750);
printf(" RX SET_COMPARE value=%u (expected 750) OK\n", r->value);
printf(" PASS: RX device uses TIM_CHANNEL_3 with correct compare\n");
printf(" RX duty=750: pin=0x%04X (expected 0x%04X = RX_DELADJ_Pin/PG7), value=%u\n",
r->pin, RX_DELADJ_Pin, r->value);
assert(r->pin == RX_DELADJ_Pin);
assert(r->value == GPIO_PIN_SET);
printf(" PASS: RX device writes PG7 with HIGH (binary)\n");
/* Cleanup */
ADF4382A_Manager_Deinit(&mgr);
printf("\n=== Bug #5: ALL TESTS PASSED (post-fix) ===\n\n");
printf("\n=== F-5.1: ALL TESTS PASSED (binary DELADJ) ===\n\n");
return 0;
}