FlintyLemming/PLFM_RADAR
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix SPI bugs #9 (NULL platform_ops) and #10 (missing CS toggle), widen chip_select to uint16_t

Browse Source 
Bug #9: Both TX and RX SPI init params had platform_ops = NULL, causing
adf4382_init() -> no_os_spi_init() to fail with -EINVAL. Fixed by setting
platform_ops = &stm32_spi_ops and passing stm32_spi_extra with correct CS
port/pin for each device.

Bug #10: stm32_spi_write_and_read() never toggled chip select. Since TX
and RX ADF4382A share SPI4, every register write hit both PLLs. Rewrote
stm32_spi.c to assert CS LOW before transfer and deassert HIGH after,
using stm32_spi_extra metadata. Backward-compatible: legacy callers
(e.g., AD9523) with cs_port=NULL skip CS management.

Also widened chip_select from uint8_t to uint16_t in no_os_spi.h since
STM32 GPIO_PIN_xx values (e.g., GPIO_PIN_14=0x4000) overflow uint8_t.

10/10 tests pass (8 original + 2 new regression tests).
This commit is contained in:
Jason
2026-03-19 10:00:05 +02:00
parent 397969348e
commit 3b32f67087
11 changed files with 325 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
9_Firmware/9_1_Microcontroller/tests/.gitignore
+2
View File
@@ -11,3 +11,5 @@ test_bug5_fine_phase_gpio_only
test_bug6_timer_variable_collision
test_bug7_gpio_pin_conflict
test_bug8_uart_commented_out
test_bug9_platform_ops_null
test_bug10_spi_cs_not_toggled
9_Firmware/9_1_Microcontroller/tests/Makefile
+12 -4
View File
@@ -1,7 +1,7 @@
################################################################################
# Makefile -- MCU firmware unit test harness for AERIS-10
#
# Builds and runs host-side (macOS) tests for the 8 discovered firmware bugs.
# Builds and runs host-side (macOS) tests for the 10 discovered firmware bugs.
# Uses mock HAL + spy/recording pattern to test real firmware code without
# hardware.
#
@@ -34,7 +34,9 @@ REAL_OBJ := adf4382a_manager.o
TESTS_WITH_REAL := test_bug1_timed_sync_init_ordering \
test_bug3_timed_sync_noop \
test_bug4_phase_shift_before_check \
test_bug5_fine_phase_gpio_only
test_bug5_fine_phase_gpio_only \
test_bug9_platform_ops_null \
test_bug10_spi_cs_not_toggled
# Tests that only need mocks (extracted patterns / static analysis)
TESTS_MOCK_ONLY := test_bug2_ad9523_double_setup \
@@ -44,7 +46,7 @@ TESTS_MOCK_ONLY := test_bug2_ad9523_double_setup \
ALL_TESTS := $(TESTS_WITH_REAL) $(TESTS_MOCK_ONLY)
.PHONY: all build test clean $(addprefix test_,bug1 bug2 bug3 bug4 bug5 bug6 bug7 bug8)
.PHONY: all build test clean $(addprefix test_,bug1 bug2 bug3 bug4 bug5 bug6 bug7 bug8 bug9 bug10)
all: build test
@@ -52,7 +54,7 @@ build: $(ALL_TESTS)
test: build
@echo "==============================================="
@echo " Running all 8 bug tests..."
@echo " Running all 10 bug tests..."
@echo "==============================================="
@pass=0; fail=0; \
for t in $(ALL_TESTS); do \
@@ -125,6 +127,12 @@ test_bug7: test_bug7_gpio_pin_conflict
test_bug8: test_bug8_uart_commented_out
./test_bug8_uart_commented_out
test_bug9: test_bug9_platform_ops_null
./test_bug9_platform_ops_null
test_bug10: test_bug10_spi_cs_not_toggled
./test_bug10_spi_cs_not_toggled
# --- Clean ---
clean:
9_Firmware/9_1_Microcontroller/tests/ad_driver_mock.h
+1 -1
View File
@@ -47,7 +47,7 @@ struct no_os_spi_platform_ops {
struct no_os_spi_init_param {
uint32_t device_id;
uint32_t max_speed_hz;
uint8_t chip_select;
uint16_t chip_select;
enum no_os_spi_mode mode;
enum no_os_spi_bit_order bit_order;
enum no_os_spi_lanes lanes;
9_Firmware/9_1_Microcontroller/tests/shims/stm32_spi.h
+26
View File
@@ -0,0 +1,26 @@
/* shim: stm32_spi.h -- provides stm32_spi_extra type and stm32_spi_ops mock
*
* The real stm32_spi.h includes stm32f7xx_hal.h which we can't use in tests.
* This shim provides the stm32_spi_extra struct and a mock stm32_spi_ops
* extern so that adf4382a_manager.c compiles against test infrastructure.
*/
#ifndef STM32_SPI_H_SHIM
#define STM32_SPI_H_SHIM
#include "stm32_hal_mock.h"
#include "ad_driver_mock.h"
/**
* @struct stm32_spi_extra
* @brief Platform-specific SPI data for STM32 (test mock version).
*/
typedef struct {
SPI_HandleTypeDef *hspi; /**< HAL SPI handle */
GPIO_TypeDef *cs_port; /**< GPIO port for software CS (NULL = no SW CS) */
uint16_t cs_pin; /**< GPIO pin mask for software CS */
} stm32_spi_extra;
/* Mock stm32_spi_ops -- declared in stm32_hal_mock.c */
extern const struct no_os_spi_platform_ops stm32_spi_ops;
#endif /* STM32_SPI_H_SHIM */
9_Firmware/9_1_Microcontroller/tests/stm32_hal_mock.c
+12
View File
@@ -2,6 +2,7 @@
* stm32_hal_mock.c -- Spy/recording implementation of STM32 HAL stubs
******************************************************************************/
#include "stm32_hal_mock.h"
#include "ad_driver_mock.h"
#include <stdio.h>
#include <stdlib.h>
@@ -262,3 +263,14 @@ void mock_tim_set_compare(TIM_HandleTypeDef *htim, uint32_t Channel, uint32_t Co
.extra = htim
});
}
/* ========================= Mock stm32_spi_ops ===================== */
/* Stub SPI platform ops -- real adf4382a_manager.c references &stm32_spi_ops.
* In tests, adf4382_init() is mocked so no_os_spi_init() is never called.
* We provide a non-NULL struct so tests can assert platform_ops != NULL. */
static int mock_spi_init_stub(void) { return 0; }
const struct no_os_spi_platform_ops stm32_spi_ops = {
.init = mock_spi_init_stub,
};
9_Firmware/9_1_Microcontroller/tests/test_bug10_spi_cs_not_toggled.c
+94
View File
@@ -0,0 +1,94 @@
/*******************************************************************************
* test_bug10_spi_cs_not_toggled.c
*
* Bug #10 (FIXED): stm32_spi_write_and_read() never toggled chip select.
* Since TX and RX ADF4382A share SPI4, every register write hit BOTH PLLs
* simultaneously.
*
* Post-fix behavior:
* 1. Manager_Init creates stm32_spi_extra structs with the correct CS
* port (GPIOG) and pin for each device (TX_CS_Pin, RX_CS_Pin).
* 2. spi_param.extra points to a stm32_spi_extra with cs_port != NULL,
* so stm32_spi_write_and_read() will assert/deassert CS around
* each transfer.
*
* Note: The actual SPI CS toggling is in stm32_spi.c at the platform level.
* This test verifies that the manager correctly provisions the CS metadata
* that stm32_spi.c uses.
******************************************************************************/
#include "adf4382a_manager.h"
#include "stm32_spi.h"
#include <assert.h>
#include <stdio.h>
int main(void)
{
ADF4382A_Manager mgr;
int ret;
printf("=== Bug #10 (FIXED): SPI CS not toggled ===\n");
/* ---- Test A: Init succeeds ---- */
spy_reset();
ret = ADF4382A_Manager_Init(&mgr, SYNC_METHOD_TIMED);
printf(" Manager_Init returned: %d (expected 0=OK)\n", ret);
assert(ret == ADF4382A_MANAGER_OK);
printf(" PASS: Init returned OK\n");
/* ---- Test B: TX extra is non-NULL ---- */
printf(" spi_tx_param.extra = %p (expected non-NULL)\n", mgr.spi_tx_param.extra);
assert(mgr.spi_tx_param.extra != NULL);
printf(" PASS: TX extra is non-NULL\n");
/* ---- Test C: TX extra has correct CS port and pin ---- */
stm32_spi_extra *tx_extra = (stm32_spi_extra *)mgr.spi_tx_param.extra;
printf(" TX cs_port = %p (expected GPIOG = %p)\n", (void *)tx_extra->cs_port, (void *)GPIOG);
assert(tx_extra->cs_port == GPIOG);
printf(" PASS: TX cs_port == GPIOG\n");
printf(" TX cs_pin = 0x%04X (expected TX_CS_Pin = 0x%04X)\n", tx_extra->cs_pin, TX_CS_Pin);
assert(tx_extra->cs_pin == TX_CS_Pin); /* GPIO_PIN_14 = 0x4000 */
printf(" PASS: TX cs_pin == TX_CS_Pin (GPIO_PIN_14)\n");
/* ---- Test D: TX extra has correct SPI handle ---- */
printf(" TX hspi = %p (expected &hspi4 = %p)\n", (void *)tx_extra->hspi, (void *)&hspi4);
assert(tx_extra->hspi == &hspi4);
printf(" PASS: TX hspi == &hspi4\n");
/* ---- Test E: RX extra is non-NULL ---- */
printf(" spi_rx_param.extra = %p (expected non-NULL)\n", mgr.spi_rx_param.extra);
assert(mgr.spi_rx_param.extra != NULL);
printf(" PASS: RX extra is non-NULL\n");
/* ---- Test F: RX extra has correct CS port and pin ---- */
stm32_spi_extra *rx_extra = (stm32_spi_extra *)mgr.spi_rx_param.extra;
printf(" RX cs_port = %p (expected GPIOG = %p)\n", (void *)rx_extra->cs_port, (void *)GPIOG);
assert(rx_extra->cs_port == GPIOG);
printf(" PASS: RX cs_port == GPIOG\n");
printf(" RX cs_pin = 0x%04X (expected RX_CS_Pin = 0x%04X)\n", rx_extra->cs_pin, RX_CS_Pin);
assert(rx_extra->cs_pin == RX_CS_Pin); /* GPIO_PIN_10 = 0x0400 */
printf(" PASS: RX cs_pin == RX_CS_Pin (GPIO_PIN_10)\n");
/* ---- Test G: RX extra has correct SPI handle ---- */
printf(" RX hspi = %p (expected &hspi4 = %p)\n", (void *)rx_extra->hspi, (void *)&hspi4);
assert(rx_extra->hspi == &hspi4);
printf(" PASS: RX hspi == &hspi4\n");
/* ---- Test H: TX and RX extras are DIFFERENT instances ---- */
printf(" TX extra = %p, RX extra = %p (expected different)\n",
(void *)tx_extra, (void *)rx_extra);
assert(tx_extra != rx_extra);
printf(" PASS: TX and RX have separate stm32_spi_extra instances\n");
/* ---- Test I: TX and RX have DIFFERENT CS pins ---- */
assert(tx_extra->cs_pin != rx_extra->cs_pin);
printf(" PASS: TX and RX CS pins differ (individual addressing)\n");
/* Cleanup */
ADF4382A_Manager_Deinit(&mgr);
printf("=== Bug #10 (FIXED): ALL TESTS PASSED ===\n\n");
return 0;
}
9_Firmware/9_1_Microcontroller/tests/test_bug9_platform_ops_null.c
+58
View File
@@ -0,0 +1,58 @@
/*******************************************************************************
* test_bug9_platform_ops_null.c
*
* Bug #9 (FIXED): Both TX and RX SPI init params had platform_ops = NULL.
* adf4382_init() calls no_os_spi_init() which checks
* if (!param->platform_ops) return -EINVAL;
* so SPI init always silently failed.
*
* Post-fix behavior:
* 1. Manager_Init sets platform_ops = &stm32_spi_ops for both TX and RX.
* 2. platform_ops is non-NULL and points to the STM32 SPI platform ops.
******************************************************************************/
#include "adf4382a_manager.h"
#include "stm32_spi.h"
#include <assert.h>
#include <stdio.h>
int main(void)
{
ADF4382A_Manager mgr;
int ret;
printf("=== Bug #9 (FIXED): platform_ops was NULL ===\n");
/* ---- Test A: Init succeeds ---- */
spy_reset();
ret = ADF4382A_Manager_Init(&mgr, SYNC_METHOD_TIMED);
printf(" Manager_Init returned: %d (expected 0=OK)\n", ret);
assert(ret == ADF4382A_MANAGER_OK);
printf(" PASS: Init returned OK\n");
/* ---- Test B: TX platform_ops is non-NULL ---- */
printf(" spi_tx_param.platform_ops = %p (expected non-NULL)\n",
(void *)mgr.spi_tx_param.platform_ops);
assert(mgr.spi_tx_param.platform_ops != NULL);
printf(" PASS: TX platform_ops is non-NULL\n");
/* ---- Test C: RX platform_ops is non-NULL ---- */
printf(" spi_rx_param.platform_ops = %p (expected non-NULL)\n",
(void *)mgr.spi_rx_param.platform_ops);
assert(mgr.spi_rx_param.platform_ops != NULL);
printf(" PASS: RX platform_ops is non-NULL\n");
/* ---- Test D: Both point to stm32_spi_ops ---- */
printf(" &stm32_spi_ops = %p\n", (void *)&stm32_spi_ops);
assert(mgr.spi_tx_param.platform_ops == &stm32_spi_ops);
printf(" PASS: TX platform_ops == &stm32_spi_ops\n");
assert(mgr.spi_rx_param.platform_ops == &stm32_spi_ops);
printf(" PASS: RX platform_ops == &stm32_spi_ops\n");
/* Cleanup */
ADF4382A_Manager_Deinit(&mgr);
printf("=== Bug #9 (FIXED): ALL TESTS PASSED ===\n\n");
return 0;
}