diff --git a/9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/adf4382.c b/9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/adf4382.c index 668ae78..4e13254 100644 --- a/9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/adf4382.c +++ b/9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/adf4382.c @@ -2106,15 +2106,21 @@ error_dev: * @brief Free resources allocated for ADF4382 * @param dev - The device structure. * @return - 0 in case of success or negative error code. + * + * F-5.4: upstream ADI driver had `if (ret) no_os_free(dev); return 0;` — + * leaked dev on success and lost the SPI-remove error. Now: NULL guard, + * unconditional free of dev struct, and propagate the actual return code. */ int adf4382_remove(struct adf4382_dev *dev) { int ret; - ret = no_os_spi_remove(dev->spi_desc); - if (ret) - no_os_free(dev); + if (!dev) + return -EINVAL; - return 0; + ret = no_os_spi_remove(dev->spi_desc); + no_os_free(dev); + + return ret; } diff --git a/9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/adf4382a_manager.c b/9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/adf4382a_manager.c index 0ca2026..45afc16 100644 --- a/9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/adf4382a_manager.c +++ b/9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/adf4382a_manager.c @@ -8,17 +8,16 @@ // External SPI handle extern SPI_HandleTypeDef hspi4; -// External timer for DELADJ PWM (configured in CubeMX, period = DELADJ_MAX_DUTY_CYCLE) -// TX DELADJ uses TIM3_CH2, RX DELADJ uses TIM3_CH3 -extern TIM_HandleTypeDef htim3; +// F-5.1: DELADJ is hardware-binary on AERIS-10. PG7 (RX) / PG13 (TX) on +// STM32F746ZGT7 have no TIM3 alternate function (Port G has FMC/ETH/USART6 +// AFs only) and the FreqSynth-board DELADJ net has only a 200 kOhm pulldown +// (R22, R35) — no series-R + shunt-C low-pass filter. There is no path for +// PWM-to-DC. Earlier "Bug #5" + "B15" PWM scaffolding has been reverted. // Static function prototypes static void set_chip_enable(uint16_t ce_pin, bool state); static void set_deladj_pin(uint8_t device, bool state); static void set_delstr_pin(uint8_t device, bool state); -static void start_deladj_pwm(uint8_t device, uint16_t duty_cycle); -static void stop_deladj_pwm(uint8_t device); -static uint16_t phase_ps_to_duty_cycle(uint16_t phase_ps); int ADF4382A_Manager_Init(ADF4382A_Manager *manager, SyncMethod method) { @@ -31,14 +30,30 @@ int ADF4382A_Manager_Init(ADF4382A_Manager *manager, SyncMethod method) static stm32_spi_extra spi_rx_extra; DIAG_SECTION("ADF4382A LO MANAGER INIT"); - DIAG("LO", "Init called with sync_method=%d (%s)", - method, (method == SYNC_METHOD_TIMED) ? "TIMED" : "EZSYNC"); + /* F-5.10: SYNC_METHOD_TIMED is the only valid value after C-14a deleted + * SYNC_METHOD_EZSYNC. The old ternary printed a stale "EZSYNC" string. */ + DIAG("LO", "Init called with sync_method=TIMED (%d)", method); if (!manager) { DIAG_ERR("LO", "Init called with NULL manager pointer"); return ADF4382A_MANAGER_ERROR_INVALID; } - + + /* F-5.3: free any prior tx_dev/rx_dev before re-allocating. The struct + * persists across re-init (recovery path in main.cpp on TX/RX unlock + * calls Manager_Init again), and the previous adf4382_dev allocations + * would otherwise leak. Mirrors F-4.5 fix for AD9523. */ + if (manager->tx_dev != NULL) { + DIAG("LO", "Releasing prior tx_dev before re-init"); + adf4382_remove(manager->tx_dev); + manager->tx_dev = NULL; + } + if (manager->rx_dev != NULL) { + DIAG("LO", "Releasing prior rx_dev before re-init"); + adf4382_remove(manager->rx_dev); + manager->rx_dev = NULL; + } + // Initialize manager structure manager->tx_dev = NULL; manager->rx_dev = NULL; @@ -84,12 +99,20 @@ int ADF4382A_Manager_Init(ADF4382A_Manager *manager, SyncMethod method) TX_CS_Pin, RX_CS_Pin, (unsigned long)ADF4382A_SPI_SPEED_HZ, (const void*)manager->spi_tx_param.platform_ops); - // Configure TX parameters (10.5 GHz) + /* F-5.2: split ref_div per device. + * TX target 10.5 GHz / PFD = 35 (integer mode). PFD = 300/1 = 300 MHz, + * under the 625 MHz integer-mode spec. ref_div = 1. + * RX target 10.38 GHz / PFD = 34.6 (fractional mode). The ADF4382 + * datasheet caps fractional-mode PFD at 250 MHz + * (ADF4382_PFD_FREQ_FRAC_MAX); 300 MHz exceeds spec. Use ref_div=2 + * so PFD = 300/2 = 150 MHz, in spec. + */ + // Configure TX parameters (10.5 GHz, integer mode) memset(&tx_param, 0, sizeof(tx_param)); tx_param.spi_3wire_en = 0; tx_param.cmos_3v3 = 1; tx_param.ref_freq_hz = REF_FREQ_HZ; - tx_param.ref_div = 1; + tx_param.ref_div = 1; // PFD = 300 MHz (integer mode) tx_param.ref_doubler_en = false; tx_param.freq = TX_FREQ_HZ; tx_param.id = ID_ADF4382A; @@ -97,13 +120,13 @@ int ADF4382A_Manager_Init(ADF4382A_Manager *manager, SyncMethod method) tx_param.bleed_word = 1000; tx_param.ld_count = 0x07; tx_param.spi_init = &manager->spi_tx_param; - - // Configure RX parameters (10.38 GHz) + + // Configure RX parameters (10.38 GHz, fractional mode) memset(&rx_param, 0, sizeof(rx_param)); rx_param.spi_3wire_en = 0; rx_param.cmos_3v3 = 1; rx_param.ref_freq_hz = REF_FREQ_HZ; - rx_param.ref_div = 1; + rx_param.ref_div = 2; // PFD = 150 MHz (fractional-mode in-spec) rx_param.ref_doubler_en = false; rx_param.freq = RX_FREQ_HZ; rx_param.id = ID_ADF4382A; @@ -111,13 +134,17 @@ int ADF4382A_Manager_Init(ADF4382A_Manager *manager, SyncMethod method) rx_param.bleed_word = 1200; rx_param.ld_count = 0x07; rx_param.spi_init = &manager->spi_rx_param; - - DIAG("LO", "TX target: %llu Hz, ref=%llu Hz, cp_i=%d, bleed=%d", + + DIAG("LO", "TX target: %llu Hz, ref=%llu Hz, ref_div=%u -> PFD=%llu Hz, cp_i=%d", (unsigned long long)TX_FREQ_HZ, (unsigned long long)REF_FREQ_HZ, - tx_param.cp_i, tx_param.bleed_word); - DIAG("LO", "RX target: %llu Hz, ref=%llu Hz, cp_i=%d, bleed=%d", + tx_param.ref_div, + (unsigned long long)REF_FREQ_HZ / tx_param.ref_div, + tx_param.cp_i); + DIAG("LO", "RX target: %llu Hz, ref=%llu Hz, ref_div=%u -> PFD=%llu Hz, cp_i=%d", (unsigned long long)RX_FREQ_HZ, (unsigned long long)REF_FREQ_HZ, - rx_param.cp_i, rx_param.bleed_word); + rx_param.ref_div, + (unsigned long long)REF_FREQ_HZ / rx_param.ref_div, + rx_param.cp_i); // Enable chips DIAG("LO", "Asserting CE pins (TX + RX)..."); @@ -398,10 +425,13 @@ int ADF4382A_CheckLockStatus(ADF4382A_Manager *manager, bool *tx_locked, bool *r rx_gpio_locked ? "HIGH" : "LOW"); } - // Use both register and GPIO status - *tx_locked = tx_reg_locked && tx_gpio_locked; - *rx_locked = rx_reg_locked && rx_gpio_locked; - + /* F-5.8: register reg[0x58] LOCKED bit is authoritative for lock state. + * GPIO disagreement is logged via DIAG_WARN above but does not flip the + * result — a mis-routed GPIO LKDET (or wrong MUXOUT) would otherwise + * trigger false-unlock recovery loops via main.cpp's error dispatch. */ + *tx_locked = tx_reg_locked; + *rx_locked = rx_reg_locked; + return ADF4382A_MANAGER_OK; } @@ -470,30 +500,24 @@ int ADF4382A_SetPhaseShift(ADF4382A_Manager *manager, uint16_t tx_phase_ps, uint return ADF4382A_MANAGER_ERROR_NOT_INIT; } - // Clamp phase shift values tx_phase_ps = (tx_phase_ps > PHASE_SHIFT_MAX_PS) ? PHASE_SHIFT_MAX_PS : tx_phase_ps; rx_phase_ps = (rx_phase_ps > PHASE_SHIFT_MAX_PS) ? PHASE_SHIFT_MAX_PS : rx_phase_ps; - DIAG("LO", "SetPhaseShift: TX=%d ps, RX=%d ps", tx_phase_ps, rx_phase_ps); + DIAG("LO", "SetPhaseShift: TX=%d ps, RX=%d ps (binary DELADJ — any nonzero -> HIGH)", + tx_phase_ps, rx_phase_ps); - // Convert phase shift to duty cycle and apply if (tx_phase_ps != manager->tx_phase_shift_ps) { - uint16_t duty_cycle = phase_ps_to_duty_cycle(tx_phase_ps); - DIAG("LO", "TX phase: %d ps -> duty_cycle=%d/%d (TIM3 CH2 PWM)", - tx_phase_ps, duty_cycle, DELADJ_MAX_DUTY_CYCLE); - ADF4382A_SetFinePhaseShift(manager, 0, duty_cycle); // 0 = TX device + ADF4382A_SetFinePhaseShift(manager, 0, + tx_phase_ps != 0 ? DELADJ_MAX_DUTY_CYCLE : 0); manager->tx_phase_shift_ps = tx_phase_ps; } if (rx_phase_ps != manager->rx_phase_shift_ps) { - uint16_t duty_cycle = phase_ps_to_duty_cycle(rx_phase_ps); - DIAG("LO", "RX phase: %d ps -> duty_cycle=%d/%d (TIM3 CH3 PWM)", - rx_phase_ps, duty_cycle, DELADJ_MAX_DUTY_CYCLE); - ADF4382A_SetFinePhaseShift(manager, 1, duty_cycle); // 1 = RX device + ADF4382A_SetFinePhaseShift(manager, 1, + rx_phase_ps != 0 ? DELADJ_MAX_DUTY_CYCLE : 0); manager->rx_phase_shift_ps = rx_phase_ps; } - printf("Phase shift set: TX=%d ps, RX=%d ps\n", tx_phase_ps, rx_phase_ps); return ADF4382A_MANAGER_OK; } @@ -515,31 +539,18 @@ int ADF4382A_SetFinePhaseShift(ADF4382A_Manager *manager, uint8_t device, uint16 return ADF4382A_MANAGER_ERROR_NOT_INIT; } - // Clamp duty cycle duty_cycle = (duty_cycle > DELADJ_MAX_DUTY_CYCLE) ? DELADJ_MAX_DUTY_CYCLE : duty_cycle; - if (duty_cycle == 0) { - // Fully OFF: stop PWM, drive pin LOW - stop_deladj_pwm(device); - set_deladj_pin(device, false); - DIAG("LO", "Dev%d DELADJ=LOW (duty=0, PWM stopped)", device); - } else if (duty_cycle >= DELADJ_MAX_DUTY_CYCLE) { - // Fully ON: stop PWM, drive pin HIGH - stop_deladj_pwm(device); - set_deladj_pin(device, true); - DIAG("LO", "Dev%d DELADJ=HIGH (duty=max, PWM stopped)", device); - } else { - // Intermediate: use TIM3 PWM output - // The PWM output is low-pass filtered externally to produce a DC - // voltage proportional to the duty cycle for the ADF4382 DELADJ input. - start_deladj_pwm(device, duty_cycle); - DIAG("LO", "Dev%d DELADJ PWM started: duty=%d/%d (%.1f%%)", - device, duty_cycle, DELADJ_MAX_DUTY_CYCLE, - (float)duty_cycle * 100.0f / DELADJ_MAX_DUTY_CYCLE); - } + /* Hardware-binary DELADJ (see header comment at top of file). + * duty == 0 -> DELADJ LOW + * duty != 0 (any) -> DELADJ HIGH + */ + bool deladj_high = (duty_cycle != 0); + set_deladj_pin(device, deladj_high); - printf("Device %d DELADJ duty cycle set to %d/%d\n", - device, duty_cycle, DELADJ_MAX_DUTY_CYCLE); + DIAG("LO", "Dev%d DELADJ=%s (binary; requested duty=%d/%d)", + device, deladj_high ? "HIGH" : "LOW", + duty_cycle, DELADJ_MAX_DUTY_CYCLE); return ADF4382A_MANAGER_OK; } @@ -588,25 +599,3 @@ static void set_delstr_pin(uint8_t device, bool state) } } -static void start_deladj_pwm(uint8_t device, uint16_t duty_cycle) -{ - // TX DELADJ → TIM3_CH2, RX DELADJ → TIM3_CH3 - // Timer period (ARR) is configured to DELADJ_MAX_DUTY_CYCLE in CubeMX. - uint32_t channel = (device == 0) ? TIM_CHANNEL_2 : TIM_CHANNEL_3; - __HAL_TIM_SET_COMPARE(&htim3, channel, (uint32_t)duty_cycle); - HAL_TIM_PWM_Start(&htim3, channel); -} - -static void stop_deladj_pwm(uint8_t device) -{ - uint32_t channel = (device == 0) ? TIM_CHANNEL_2 : TIM_CHANNEL_3; - HAL_TIM_PWM_Stop(&htim3, channel); -} - -static uint16_t phase_ps_to_duty_cycle(uint16_t phase_ps) -{ - // Convert phase shift in picoseconds to DELADJ duty cycle - // This is a linear mapping - adjust based on your specific requirements - uint32_t duty = (uint32_t)phase_ps * DELADJ_MAX_DUTY_CYCLE / PHASE_SHIFT_MAX_PS; - return (uint16_t)duty; -} 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 c1353a3..c1c6490 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 @@ -116,7 +116,6 @@ SPI_HandleTypeDef hspi1; SPI_HandleTypeDef hspi4; TIM_HandleTypeDef htim1; -TIM_HandleTypeDef htim3; // B15 fix: DELADJ PWM timer (CH2=TX, CH3=RX) UART_HandleTypeDef huart5; UART_HandleTypeDef huart3; @@ -286,7 +285,6 @@ void PeriphCommonClock_Config(void); static void MPU_Config(void); static void MX_GPIO_Init(void); static void MX_TIM1_Init(void); -static void MX_TIM3_Init(void); // B15 fix: DELADJ PWM timer init static void MX_I2C1_Init(void); static void MX_I2C2_Init(void); static void MX_I2C3_Init(void); @@ -1681,7 +1679,6 @@ int main(void) /* Initialize all configured peripherals */ MX_GPIO_Init(); MX_TIM1_Init(); - MX_TIM3_Init(); // B15 fix: init DELADJ PWM timer before LO manager uses it MX_I2C1_Init(); MX_I2C2_Init(); MX_I2C3_Init(); @@ -2964,64 +2961,6 @@ static void MX_TIM1_Init(void) } -/** - * @brief TIM3 Initialization Function — DELADJ PWM for ADF4382A phase shift - * CH2 = TX DELADJ, CH3 = RX DELADJ - * Period (ARR) = 999 → 1000 counts matching DELADJ_MAX_DUTY_CYCLE - * Prescaler = 71 → 1 MHz tick @ 72 MHz timer clock (APB1=36 MHz, but - * RCC_TIMPRES_ACTIVATED forces TIMxCLK=HCLK) → 1 kHz PWM frequency - * @param None - * @retval None - */ -static void MX_TIM3_Init(void) -{ - /* B15 fix: provide htim3 definition so adf4382a_manager.c extern resolves */ - - TIM_ClockConfigTypeDef sClockSourceConfig = {0}; - TIM_MasterConfigTypeDef sMasterConfig = {0}; - TIM_OC_InitTypeDef sConfigOC = {0}; - - htim3.Instance = TIM3; - htim3.Init.Prescaler = 71; // 72 MHz / (71+1) = 1 MHz tick - htim3.Init.CounterMode = TIM_COUNTERMODE_UP; - htim3.Init.Period = 999; // ARR = 999 → 1000 counts = DELADJ_MAX_DUTY_CYCLE - htim3.Init.ClockDivision = TIM_CLOCKDIVISION_DIV1; - htim3.Init.AutoReloadPreload = TIM_AUTORELOAD_PRELOAD_DISABLE; - if (HAL_TIM_Base_Init(&htim3) != HAL_OK) - { - Error_Handler(); - } - sClockSourceConfig.ClockSource = TIM_CLOCKSOURCE_INTERNAL; - if (HAL_TIM_ConfigClockSource(&htim3, &sClockSourceConfig) != HAL_OK) - { - Error_Handler(); - } - if (HAL_TIM_PWM_Init(&htim3) != HAL_OK) - { - Error_Handler(); - } - sMasterConfig.MasterOutputTrigger = TIM_TRGO_RESET; - sMasterConfig.MasterSlaveMode = TIM_MASTERSLAVEMODE_DISABLE; - if (HAL_TIMEx_MasterConfigSynchronization(&htim3, &sMasterConfig) != HAL_OK) - { - Error_Handler(); - } - sConfigOC.OCMode = TIM_OCMODE_PWM1; - sConfigOC.Pulse = 0; // Start with 0% duty cycle - sConfigOC.OCPolarity = TIM_OCPOLARITY_HIGH; - sConfigOC.OCFastMode = TIM_OCFAST_DISABLE; - // Configure CH2 (TX DELADJ) - if (HAL_TIM_PWM_ConfigChannel(&htim3, &sConfigOC, TIM_CHANNEL_2) != HAL_OK) - { - Error_Handler(); - } - // Configure CH3 (RX DELADJ) - if (HAL_TIM_PWM_ConfigChannel(&htim3, &sConfigOC, TIM_CHANNEL_3) != HAL_OK) - { - Error_Handler(); - } -} - /** * @brief UART5 Initialization Function * @param None diff --git a/9_Firmware/9_1_Microcontroller/tests/Makefile b/9_Firmware/9_1_Microcontroller/tests/Makefile index 92c4980..9bd21c4 100644 --- a/9_Firmware/9_1_Microcontroller/tests/Makefile +++ b/9_Firmware/9_1_Microcontroller/tests/Makefile @@ -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 diff --git a/9_Firmware/9_1_Microcontroller/tests/test_bug15_htim3_dangling_extern.c b/9_Firmware/9_1_Microcontroller/tests/test_bug15_htim3_dangling_extern.c deleted file mode 100644 index af66b21..0000000 --- a/9_Firmware/9_1_Microcontroller/tests/test_bug15_htim3_dangling_extern.c +++ /dev/null @@ -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 -#include - -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; -} diff --git a/9_Firmware/9_1_Microcontroller/tests/test_bug5_fine_phase_gpio_only.c b/9_Firmware/9_1_Microcontroller/tests/test_bug5_fine_phase_gpio_only.c index f702a49..f1ecb86 100644 --- a/9_Firmware/9_1_Microcontroller/tests/test_bug5_fine_phase_gpio_only.c +++ b/9_Firmware/9_1_Microcontroller/tests/test_bug5_fine_phase_gpio_only.c @@ -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 @@ -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; }