[RyanS4]

Summary

Adds debug LED patterns for hard faults, stack overflows, and other faults.

Testing

Verified all 8 terminal fault blink patterns on a STM32G474RET6 board (MotorBoard.elf flashed via onboard STLINK-V3MODS, observing LED3 DEBUG wired directly to PC1) with 200 ms ON/OFF pulse intervals and a 1500 ms pause between pattern repetitions:

• 1 blink (LedFault_HardFault): Forced an unaligned multi-word store (stmia) before enabling SCB->SHCSR fault handlers (USGFAULTENA = 0), escalating to HardFault_Handler and confirming a repeating 1-blink pattern.
• 2 blinks (LedFault_StackOverflow): Overwrote a FreeRTOS task's stack sentinel buffer inside app_main() and yielded (vTaskDelay(1)), triggering vApplicationStackOverflowHook and confirming a repeating 2-blink pattern.
• 3 blinks (LedFault_MallocFailed): Called pvPortMalloc(SIZE_MAX) after startup_init(), triggering vApplicationMallocFailedHook and confirming a repeating 3-blink pattern.
• 4 blinks (LedFault_MemManage): Executed a function pointer in Execute-Never (XN) system space (((void (*)(void))0xE0000001)()), triggering an instruction access violation (IACCVIOL) in MemManage_Handler and confirming a repeating 4-blink pattern.
• 5 blinks (LedFault_BusFault): Read from unmapped memory (*(volatile uint32_t *)0xFFF00000) with BUSFAULTENA enabled, triggering BusFault_Handler and confirming a repeating 5-blink pattern.
• 6 blinks (LedFault_UsageFault): Enabled SCB_CCR_UNALIGN_TRP_Msk and executed an unaligned 32-bit write (*(volatile uint32_t *)0x20000001 = 0xDEADBEEF), triggering UsageFault_Handler and confirming a repeating 6-blink pattern.
• 7 blinks (LedFault_NMI): Set SCB->ICSR = SCB_ICSR_NMIPENDSET_Msk to pend a Non-Maskable Interrupt, triggering NMI_Handler and confirming a repeating 7-blink pattern.
• 9 blinks (LedFault_RTOSAssert): Triggered configASSERT(0), routing into LedFault_halt(LedFault_RTOSAssert) and confirming a repeating 9-blink pattern.

Related Issue

Related to #204

Notes

[claude]

This is outside #204's scope: it removes #include "fatfs_sd.h" and adds a weak SD_TimerProc stub. If Telemetry's H7 build relied on that header for the declaration, the SD_TimerProc(); call at line 199 now depends on this local stub, and the stub shadows the header's prototype. The G4 file already has the weak stub (stm32g4xx_it.c:27), but it keeps the include. Revert the include removal unless the build requires it, and say why in the PR.

[claude]

Each handler now calls the noreturn LedFault_halt, so the while (1) below it is dead code, and the stock "NMI_Handler 1" while (1) after the NMI call is dead too. A reader will wonder which one runs. The PR body also says "LED wiring needs a bench check" and leaves Testing empty, so none of this has run on hardware.

[ethanmathias]

@RyanS4 Structure is good, the three callers into one LedFault_halt() is right. Needs to be cut down a bit though.

1. Hardcode PC1 like I said above. Every board already sets it as DEBUG_OUT, so you don't need FAULT_LED_1/FAULT_LED_2 added to all five pindef.h files. The FAULT_LED_2 pins you picked (PA5, PB0, PE0) are Nucleo user LEDs, not anything on the car. Once pindef.h is out of LedFault.cpp you can also revert the cmake change.

2. Remove the SysTick and the HAL_GetTick override. Use a simple loop instead this is a very simple case.

static void spin_ms(uint32_t ms) {
    uint32_t loops = (SystemCoreClock / 4000U) * ms;
    while (loops-- != 0U) { __NOP(); }
}

3. Use HAL_GPIO_Init instead of writing MODER/OSPEEDR/PUPDR that should be safe.

4. Make the blink count the fault code. HardFault is 1 blink, StackOverflow is 2, and so on.

5. Delete LedFault_fault_signal_blink, the active_high flag and the LED_FAULT_*_MS macros. Also take stm32_hal.h out of the header. FreeRTOSConfig.h includes LedFault.h so that should include everything else.

Below is chat advice for register mapping:

• nothing sets SCB->SHCSR, so MemManage, BusFault and UsageFault all escalate to HardFault right now and codes 4-6 can never work. This happens because by default MemManage, BusFault, and UsageFault are disabled and grouped under HardFault.

SCB->SHCSR |= SCB_SHCSR_MEMFAULTENA_Msk | SCB_SHCSR_BUSFAULTENA_Msk | SCB_SHCSR_USGFAULTENA_Msk;

To test:
You can force a hard fault by writing to 0x00000000 and check you get one blink, then a stack overflow for two.

[claude]

Out of scope for the LED fault work (#204): swapping #include "fatfs_sd.h" for a weak SD_TimerProc stub changes the H7 build. If it's needed to link, say so in the PR. If not, revert it. The G4 file already has the same stub at stm32g4xx_it.c:26-27, so the pattern matches, but it belongs in its own change.

[claude]

This is out of scope for the debug-LED issue (#204). It removes the fatfs_sd.h include and adds a weak SD_TimerProc stub. The change does match the G4 file (stm32g4xx_it.c:25-27), so it may be fine, but it should go in its own PR. Alternatively, say in the PR description why it is needed.

[claude]

SecureFault_Handler is never enabled. startup.cpp sets MEMFAULTENA, BUSFAULTENA and USGFAULTENA but not SCB_SHCSR_SECUREFAULTENA_Msk, so a secure fault will not reach this handler. This is untested (the PR lists no 8-blink test). Either enable it for U5 in startup.cpp, with a comment, or drop the handler and LedFault_SecureFault.

[claude]

Review: 5 inline comments posted. Main points: name the PC1 pin and blink-timing constants and document the /4000 cycle estimate in LedFault.cpp; the H743 SD_TimerProc change is out of scope for #204; SecureFault_Handler on U5 is never enabled in startup.cpp.

[claude]

The LED pin is hardcoded to PC1 for every board, and the comment admits it is "unconfirmed on the other boards". On any board where PC1 is something else (e.g. MotorBoard/Inc/pindef.h:44 defines DEBUG_OUT as NC for the other build), a fault handler will reconfigure that pin as a push-pull output and drive it. The repo defines per-board pins in each board's Inc/pindef.h (MotorBoard/Inc/pindef.h:14). Take the port and pin from a per-board define, or build the fault LED only for boards where it is confirmed. A new member debugging a different board should not have to guess whether PC1 is safe.

[claude]

This doc block is garbled. "Fault code determining the number of blinks per cycle." is a dangling fragment with no @param, and the LedFault_halt_assert block starts mid-sentence ("LedFault_halt(...), as a plain void(void) entry point"). Make them complete sentences, e.g. @param fault Number of blinks per cycle., and start the second with "Calls LedFault_halt(LedFault_RTOSAssert) ...".

[claude]

Out of scope for debug LED faults: this removes #include "fatfs_sd.h" and adds a weak SD_TimerProc stub. It is unrelated to #204. Drop it from this PR, or put it in its own PR with the reason. If fatfs_sd.h is still needed for the SD_TimerProc() call at line 199, the build may also break.

[claude]

PC1 is hardcoded for every board, and the comment admits it's "unconfirmed on the other boards". Issue #204 says to take the LED pins from each board's pindef (DEBUG_OUT) and confirm per board. On a board where PC1 is something else, this fault handler would drive an unrelated pin. Either get the pin from the board's pindef, or confirm PC1 on the H743 and U5A9 boards and say so here.

[claude]

200U and 1500U are the blink timing, but they are bare literals. Name them next to the DEBUG_LED_* defines, e.g. BLINK_INTERVAL_MS and REPEAT_PAUSE_MS. The units are then obvious, and the timing can be changed in one place.

[claude]

Swapping #include "fatfs_sd.h" for a weak SD_TimerProc stub is unrelated to the debug LED issue. It is also a behavior change on H7. If it is needed to make the build work, say why in the PR description. Otherwise, leave it out of this PR.