TIM2 SetDutyCycle wrong CCR value computation

[GianluigiForte]

I'm using the PWM example with the Nucleo G431 board configuring the PA5 pin as PWM to drive the LED present on the board.

I think that there is a problem with the SetDutyCycle implementation when is used the TIM2 because for what I understood the $typ in case of TIM2 is 32bit

since

tim_hal! {
TIM1: (tim1, u16, 16, DIR: cms, BDTR: bdtr, set_bit, af1, clear_bit, clear_bit),
TIM2: (tim2, u32, 32, DIR: cms),
TIM3: (tim3, u16, 16, DIR: cms),
TIM4: (tim4, u16, 16, DIR: cms),
}

And so when is called the SetDutyCycle

fn set_duty_cycle(&mut self, duty: u16) -> Result<(), Self::Error> {
// hal trait always has 16 bit resolution
let duty = (duty as $typ) << (8(size_of::<$typ>() - size_of::()));*
let tim = unsafe { &*$TIMX::ptr() };
Ok(tim.$ccrx().write(|w| unsafe { w.ccr().bits(duty.into()) }))
}

The duty value is wrongly shifted by 16bits on the left, creating many troubles.

[usbalbin]

Sorry for the late answer. I am a bit torn here on what the best way to solve this is. When using a 32bit counter with a large period value, lets say 0xFFFF_0000, the current solution should work just fine. Since the duty will then be shifted up to the most significant 16-bit range. However when selecting a smaller period value, say 0x0000_FFFF, then the smallest non zero 0x0001_0000 duty is already larger then the period(so more than 100%).

I am thus not sure removing the shift actually solves the problem since then we will have problems when running with larger period values.

Yet another solution would be to have duty remapped so that duty=u32::MAX is always 100%, duty=u32::MAX/2 being 50% etc. However that would incur a performance penalty and might also be a bit suprising considering how the other timers work.