fix(Timer): Make slave timer callbacks, well, Victor did

[FoniksFox]

What the title says, Victor said this is the way to do it, and I trust him.

[Copilot]

Pull request overview

This pull request modifies the interrupt handlers for STM32 timer peripherals TIM8, TIM12, TIM13, and TIM14, which share interrupt lines. Previously, these shared interrupt handlers only handled TIM8 interrupts. The PR adds logic to distinguish between the master timer (TIM8) and slave timers (TIM12/13/14) by checking their status registers.

Changes:

• Added conditional logic to three shared interrupt handlers to check which timer triggered the interrupt
• Removed a NOTE comment that acknowledged the previous incomplete implementation

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

These shared interrupt handlers would benefit from code comments explaining why they check specific status register flags and how the interrupt sharing works. For example, a comment explaining that TIM8_BRK_TIM12_IRQHandler is a shared interrupt line for TIM8 break events and TIM12 global interrupts would help future maintainers understand the logic. This is especially important given the complexity of STM32's timer interrupt architecture.

[Copilot]

The removed NOTE comment was acknowledging that the timer interrupt handling approach might need changes. While this PR attempts to address the concern by adding logic to distinguish between TIM8 and slave timer interrupts, the implementation still has critical bugs. The NOTE comment should have been retained or replaced with documentation explaining the current approach and its limitations, rather than being deleted as if the issue is fully resolved.

[Copilot]

The shared interrupt handler logic lacks test coverage. These handlers implement complex logic to distinguish between TIM8 and slave timer interrupts, which is critical for correct operation. Tests should be added to verify: 1) TIM12 interrupts are handled correctly when TIM12's UIF is set, 2) TIM8 break interrupts are handled when appropriate, and 3) the handler correctly manages cases where both timers may have pending interrupts. Given that other timer-related code has test coverage (see Tests/Time/timer_wrapper_test.cpp), this new logic should also be tested.

[Copilot]

This handler is for TIM8 Break interrupts OR TIM12 interrupts. The else branch incorrectly clears TIM8's UIF flag and calls its callback, but TIM8_BRK_TIM12_IRQHandler is triggered by TIM8's break interrupt, not its update interrupt. TIM8's break interrupt does not set the UIF flag. If this handler is meant to support TIM8 break interrupts, it should check and clear TIM8's break interrupt flag (TIM_SR_BIF) instead. If TIM8 break interrupts are not used, this else branch may never execute correctly and should be removed or documented. Additionally, the if-else structure would miss cases where both timers have pending interrupts.

Suggested change

	} else {
	CLEAR_BIT(TimerDomain::cmsis_timers[timer_idxmap[8]]->SR, TIM_SR_UIF);
	}
	if((TimerDomain::cmsis_timers[timer_idxmap[8]]->SR & TIM_SR_BIF) != 0) {
	CLEAR_BIT(TimerDomain::cmsis_timers[timer_idxmap[8]]->SR, TIM_SR_BIF);

[Copilot]

The if-else logic prevents handling both timers when they both have pending interrupts simultaneously. In a shared interrupt handler, if both TIM13 and TIM8 have pending update interrupts, only TIM13's callback will be invoked and TIM8's interrupt will be missed. This should use two independent if statements instead of if-else to ensure both pending interrupts are serviced in a single ISR call.

[Copilot]

This handler is for TIM8 Trigger/Commutation interrupts OR TIM14 interrupts. The else branch incorrectly clears TIM8's UIF flag and calls its callback, but TIM8_TRG_COM_TIM14_IRQHandler is triggered by TIM8's trigger/commutation interrupts, not its update interrupt. TIM8's trigger/commutation interrupts do not set the UIF flag. If this handler is meant to support TIM8 trigger/commutation interrupts, it should check and clear the appropriate TIM8 flags (TIM_SR_TIF, TIM_SR_COMIF) instead. If TIM8 trigger/commutation interrupts are not used, this else branch may never execute correctly and should be removed or documented. Additionally, the if-else structure would miss cases where both timers have pending interrupts.

Suggested change

	if((TimerDomain::cmsis_timers[timer_idxmap[14]]->SR & TIM_SR_UIF) != 0) {
	CLEAR_BIT(TimerDomain::cmsis_timers[timer_idxmap[14]]->SR, TIM_SR_UIF);
	TimerDomain::callbacks[timer_idxmap[14]](TimerDomain::callback_data[timer_idxmap[14]]);
	} else {
	CLEAR_BIT(TimerDomain::cmsis_timers[timer_idxmap[8]]->SR, TIM_SR_UIF);
	// Handle TIM14 update interrupt, if pending
	if((TimerDomain::cmsis_timers[timer_idxmap[14]]->SR & TIM_SR_UIF) != 0) {
	CLEAR_BIT(TimerDomain::cmsis_timers[timer_idxmap[14]]->SR, TIM_SR_UIF);
	TimerDomain::callbacks[timer_idxmap[14]](TimerDomain::callback_data[timer_idxmap[14]]);
	}
	// Handle TIM8 trigger/commutation interrupts, if pending
	uint32_t tim8_sr = TimerDomain::cmsis_timers[timer_idxmap[8]]->SR;
	uint32_t tim8_trgcom_flags = TIM_SR_TIF | TIM_SR_COMIF;
	if((tim8_sr & tim8_trgcom_flags) != 0) {
	CLEAR_BIT(TimerDomain::cmsis_timers[timer_idxmap[8]]->SR, tim8_trgcom_flags);