[timeout, stm32wb, pic32cz] Implement an optional timeout and add it to current drivers#16
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a lightweight timeout abstraction to bound polling loops, integrates it into several STM32WB and PIC32CZ drivers, and wires it into the STM32WB Nucleo board plus core tests.
Changes:
- Add
whal_Timeout+WHAL_TIMEOUT_*macros (with optional compile-out) and a newWHAL_ETIMEOUTerror code. - Extend multiple driver config structs to accept an optional timeout pointer and apply timeouts to busy-wait/polling loops.
- Add core unit tests for timeout behavior and configure an example tick source + timeout instance on the STM32WB55 Nucleo board.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| wolfHAL/wolfHAL.h | Exposes the new timeout API from the umbrella header. |
| wolfHAL/uart/stm32wb_uart.h | Adds optional timeout pointer to STM32WB UART config. |
| wolfHAL/uart/pic32cz_uart.h | Adds optional timeout pointer to PIC32CZ UART config. |
| wolfHAL/timeout.h | Introduces the timeout abstraction and macros. |
| wolfHAL/spi/stm32wb_spi.h | Adds optional timeout pointer to STM32WB SPI config. |
| wolfHAL/rng/stm32wb_rng.h | Adds optional timeout pointer to STM32WB RNG config. |
| wolfHAL/flash/stm32wb_flash.h | Adds optional timeout pointer to STM32WB Flash config. |
| wolfHAL/flash/pic32cz_flash.h | Adds optional timeout pointer to PIC32CZ Flash config. |
| wolfHAL/error.h | Adds WHAL_ETIMEOUT error code. |
| wolfHAL/crypto/stm32wb_aes.h | Adds optional timeout pointer to STM32WB AES config. |
| tests/core/test_timeout.c | Adds unit tests for timeout start/stop/expired and wrap behavior. |
| tests/core/main.c | Registers/runs the new timeout test suite. |
| tests/core/Makefile | Includes test_timeout.c in the core test build. |
| src/uart/stm32wb_uart.c | Applies timeout guards to STM32WB UART send/recv polling. |
| src/uart/pic32cz_uart.c | Adds timeout-bounded sync waits and TX/RX polling with timeout errors. |
| src/spi/stm32wb_spi.c | Adds timeout-bounded polling for TXE/RXNE/BSY in SPI transfers. |
| src/rng/stm32wb_rng.c | Adds timeout-bounded wait for RNG data-ready. |
| src/flash/stm32wb_flash.c | Adds timeout-bounded waits for flash CFGBSY during program/erase. |
| src/flash/pic32cz_flash.c | Adds timeout-bounded mutex/busy waits and propagates timeout errors. |
| src/crypto/stm32wb_aes.c | Adds timeout-bounded polling for AES completion across modes. |
| boards/stm32wb55xx_nucleo/board.h | Updates tick type to uint64_t for the new timeout tick source. |
| boards/stm32wb55xx_nucleo/board.c | Implements Board_GetTick(), defines a default timeout instance, and injects it into device configs. |
Comments suppressed due to low confidence (4)
src/flash/pic32cz_flash.c:363
cfgis read fromflashDevbefore checkingflashDevfor NULL, which can dereference a NULL pointer. Move thecfgassignment after theif (!flashDev)validation (and consider also validatingflashDev->cfg).
whal_Error whal_Pic32czFlash_Erase(whal_Flash *flashDev, size_t addr, size_t dataSz)
{
const whal_Regmap *reg;
whal_Pic32czFlash_Cfg *cfg = flashDev->cfg;
whal_Error err;
size_t pageAddr;
size_t endAddr;
if (!flashDev) {
return WHAL_EINVAL;
}
src/flash/pic32cz_flash.c:256
cfgis read fromflashDevbefore validatingflashDevis non-NULL, which can dereference a NULL pointer when callers passflashDev == NULL. Movewhal_Pic32czFlash_Cfg *cfg = flashDev->cfg;to after theif (!flashDev || !data)check (and similarly in the other APIs in this file).
const whal_Regmap *reg;
whal_Pic32czFlash_Cfg *cfg = flashDev->cfg;
uint8_t *flashAddr = (uint8_t *)addr;
whal_Error err;
size_t i;
if (!flashDev || !data) {
return WHAL_EINVAL;
}
src/flash/pic32cz_flash.c:287
cfgis read fromflashDevbefore theif (!flashDev || !data)guard, which can dereference NULL whenflashDevis NULL. Assigncfgonly after validatingflashDev(andflashDev->cfgif required).
size_t dataSz)
{
const whal_Regmap *reg;
whal_Pic32czFlash_Cfg *cfg = flashDev->cfg;
const uint32_t *src;
whal_Error err;
size_t offset = 0;
if (!flashDev || !data) {
return WHAL_EINVAL;
}
boards/stm32wb55xx_nucleo/board.c:188
Board_WaitMs()still usessize_tlocals (startCount,currentCount) even thoughg_tickis nowuint64_t. This truncates the tick value on 32-bit builds and makes the wrap/overflow handling (g_tickOverflow,SIZE_MAX) inconsistent with the actual counter type. Update the delay logic to useuint64_tconsistently (and drop the overflow flag if 64-bit wrap is no longer a practical concern).
void Board_WaitMs(size_t ms)
{
size_t startCount = g_tick;
g_waiting = 1;
while (1) {
size_t currentCount = g_tick;
if (g_tickOverflow) {
if ((SIZE_MAX - startCount) + currentCount > ms) {
break;
}
} else if (currentCount - startCount > ms) {
break;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0aa7a17 to
0498bb0
Compare
…to current drivers
There was a problem hiding this comment.
Pull request overview
Adds a lightweight, optional timeout abstraction to wolfHAL and integrates it into several polling-heavy drivers to prevent unbounded busy-wait hangs (while preserving a zero-overhead build option via WHAL_CFG_NO_TIMEOUT).
Changes:
- Introduces
whal_TimeoutplusWHAL_TIMEOUT_START/EXPIREDmacros, and addsWHAL_ETIMEOUT. - Adds a reusable register polling helper (
whal_Reg_ReadPoll) and updates STM32WB/PIC32CZ UART/SPI/Flash/RNG/AES code to use it (or equivalent timeout-guarded loops). - Expands docs/tests/CI to cover timeouts and a no-timeout build configuration.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| wolfHAL/wolfHAL.h | Exposes the new timeout API from the umbrella header. |
| wolfHAL/timeout.h | Adds the timeout abstraction and compile-time disable macros. |
| wolfHAL/error.h | Adds WHAL_ETIMEOUT. |
| wolfHAL/regmap.h | Adds whal_Reg_ReadPoll() helper for timeout-bounded polling. |
| wolfHAL/uart/stm32wb_uart.h | Adds optional whal_Timeout* to STM32WB UART config. |
| wolfHAL/uart/pic32cz_uart.h | Adds optional whal_Timeout* to PIC32CZ UART config. |
| wolfHAL/spi/stm32wb_spi.h | Adds optional whal_Timeout* to STM32WB SPI config. |
| wolfHAL/rng/stm32wb_rng.h | Adds optional whal_Timeout* to STM32WB RNG config. |
| wolfHAL/flash/stm32wb_flash.h | Adds optional whal_Timeout* to STM32WB Flash config. |
| wolfHAL/flash/pic32cz_flash.h | Adds optional whal_Timeout* to PIC32CZ Flash config. |
| wolfHAL/crypto/stm32wb_aes.h | Adds optional whal_Timeout* to STM32WB AES config. |
| src/uart/stm32wb_uart.c | Replaces unbounded polling loops with whal_Reg_ReadPoll() using configured timeout. |
| src/uart/pic32cz_uart.c | Replaces sync/flag busy-waits with whal_Reg_ReadPoll() using configured timeout. |
| src/spi/stm32wb_spi.c | Uses whal_Reg_ReadPoll() for TXE/RXNE/BSY waits with optional timeout. |
| src/rng/stm32wb_rng.c | Adds timeout-bounded waiting for RNG DRDY and updates SR reads. |
| src/flash/stm32wb_flash.c | Adds timeout-bounded waits for flash CFGBSY and propagates timeout errors. |
| src/flash/pic32cz_flash.c | Adds timeout-bounded mutex/busy waits and propagates timeout errors. |
| src/crypto/stm32wb_aes.c | Adds a timeout-aware WaitForCCF() helper and uses it throughout AES modes. |
| tests/core/test_timeout.c | Adds unit tests for timeout macros (including wraparound behavior). |
| tests/core/main.c | Registers the timeout test suite when timeouts are enabled. |
| tests/core/Makefile | Builds timeout tests and adjusts CFLAGS handling for CI matrix builds. |
| docs/writing_a_driver.md | Documents timeout usage patterns and integration guidance for drivers. |
| docs/getting_started.md | Updates example usage and documents WHAL_ETIMEOUT. |
| docs/adding_a_board.md | Updates board template to include a global timeout instance. |
| boards/stm32wb55xx_nucleo/board.h | Adjusts tick type and reflects updated board exports. |
| boards/stm32wb55xx_nucleo/board.c | Adds a board whal_Timeout backed by SysTick and wires it into peripherals. |
| .github/workflows/ci.yml | Adds CI matrix to build/test with and without timeouts enabled. |
Comments suppressed due to low confidence (1)
boards/stm32wb55xx_nucleo/board.c:188
Board_WaitMs’s wraparound path still usesSIZE_MAXto compute elapsed time, butg_ticknow wraps atUINT32_MAX. If the tick counter wraps while waiting,(SIZE_MAX - startCount) + currentCountwill vastly overestimate the elapsed time on 64-bit hosts/targets, potentially ending the delay immediately. Compute elapsed time usinguint32_tarithmetic (orUINT32_MAX) to match the tick counter’s width.
void Board_WaitMs(size_t ms)
{
uint32_t startCount = g_tick;
g_waiting = 1;
while (1) {
uint32_t currentCount = g_tick;
if (g_tickOverflow) {
if ((UINT32_MAX - startCount) + currentCount > ms) {
break;
}
} else if (currentCount - startCount > ms) {
break;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.