Custom timer

[usbalbin]

This is a (very rough draft of) flexible timer driver for all STM32 timers. Tries to give quite unhindered access to
most timer features while offering some type level protection for incorrect configuration.

Example usage

let out_pin = PwmPin::new(out_pin, crate::gpio::OutputType::PushPull);

let mut tim = CustomPwmBuilder::new(tim)
    //.frequency(Hertz(123))
    .prescaler_and_period(0, 1337)
    .ch1_input(
        trigger_pin,
        FilterValue::FDTS_DIV32_N8,
        InputCaptureMode::BothEdges,
        InputTISelection::Normal,
        1,
    )
    .trigger_from_ch1(TriggerMode::TriggerMode, TriggerSource::Filtered)
    .ch2(out_pin, OutputCompareMode::PwmMode2, 800)
    .ch3_input(
        capture_pin,
        FilterValue::FCK_INT_N2,
        InputCaptureMode::Rising,
        InputTISelection::Normal,
        0,
    )
    .one_pulse_mode()
    .finalize();

tim.set_compare_value(150, super::Channel::Ch2);
tim.waveform_up(dma, super::Channel::Ch1, &[100, 400, 800, 1100, 1200])
    .await;
let _capture = tim.wait_for_configured_edge(super::Channel::Ch3).await;

While the example above is just made up. I could see it useful for something like a triac controlled soft start with some current limit or other event synchronized lo the period and a zero detect input which triggers the timer on both edges.

[usbalbin]

I can not quite wrap my head around how afio A thing is supposed to work. Without specifying the type here, I get an error about it not knowing what type it should be.

[xoviat]

You need to drop the pins in the new constructor, so that the pin types do not propagate into the PwmBuilder. The builder nor the timer should store the pins, which will solve the type issues.

[xoviat]

Any method that accepts a pin should have a type parameter (ch4_input<T>) so that the pin type is not propagated into the struct.

[usbalbin]

I have done that now IIUC. But I still get the inference errors

[usbalbin]

$ cargo b --features stm32f107rb,time-driver-any,unstable-pac,exti,rt
[...]
error[E0283]: type annotations needed
    --> src/timer/custom_timer.rs:569:10
     |
 569 |         .ch1_input(
     |          ^^^^^^^^^ cannot infer type for type parameter `A` declared on the method `ch1_input`
     |
note: multiple `impl`s satisfying `PA8: TimerPin<peripherals::TIM1, timer::Ch1, _>` found
    --> src/macros.rs:91:13
     |
  91 | ...   impl crate::$mod::$trait<crate::peripherals::$instance, crate::$mod::$mode, crate::gpio::$type<$val>> for crate::peripherals::$pin {
     |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
    ::: /home/albin/my_projects/embassy/embassy-stm32/target/debug/build/embassy-stm32-184b20372b5872d0/out/_generated.rs:1323:1
     |
1323 | pin_trait_afio_impl ! (crate :: timer :: TimerPin < Ch1 > , TIM1 , PA8 , { mapr , set_tim1_remap , AfioRemap , [0u8 , 1u8] });
     | ----------------------------------------------------------------------------------------------------------------------------- in this macro invocation
note: required by a bound in `CustomPwmBuilder::<'d, T, InternalOutput, CH2, CH3, CH4, ETR, TS>::ch1_input`
    --> src/timer/custom_timer.rs:285:29
     |
 283 |     pub fn ch1_input<#[cfg(afio)] A>(
     |            --------- required by a bound in this associated function
 284 |         self,
 285 |         _pin: if_afio!(impl TimerPin<T, Ch1, A>),
     |                             ^^^^^^^^^^^^^^^^^^^ required by this bound in `CustomPwmBuilder::<'d, T, InternalOutput, CH2, CH3, CH4, ETR, TS>::ch1_input`
     = note: this error originates in the macro `pin_trait_afio_impl` (in Nightly builds, run with -Z macro-backtrace for more info)
[...]

[xoviat]

@usbalbin see https://github.com/Dectron-AB/embassy/pull/1/files

[usbalbin]

Thanks for the explanation!

[Dirbaio]

We already have two timer APIs: low_level, and then the high-level helpers (Qei, PWM, etc). I think we shouldn't add a third one, unless there's a really really good reason.

• What can you do with this API that you can't do with the low_level API?
• Why does it need a new API instead of extending the low_level API?

[usbalbin]

• What can you do with this API that you can't do with the low_level API?

Safe construction with guardrails to prevent lots of configuration errors.

• Why does it need a new API instead of extending the low_level API?

See above. Since low_level has all levers available without any checks we can not really add things to make the bad configs go away.

When trying to implement a triac motor control at work we looked at the options. We need external trigger and one pulse mode to trigger on mains zero. Therefore one_pulse timer made sense. However that does not offer any outputs. For the output simple/complementary PWM would have worked. So we ended up having to use the low_level timer. Maybe this is just a weird use case but we felt it useful to allow as much as possible of what the hardware can do without having to drop down to the pac level.

I personally find that there is quite a huge gap between low_level and the high level types. The high level types, while very convenient when you do exactly what they intend, are very hard to work with once you do something slightly different. At that point you lose all(most) the nice high level features when you have to drop down to the low_level timer.

The custom timer(I am open to name suggestions) sits somewhere in between. It can do(once complete) pretty much everything that the high level things does almost as nicely but with less tailored method names and configuration is slightly more involved.

With that said, I am very open to suggestions for how to do this differently/alternatives :)

---

Perhaps it does not alleviate your concern but what if the high level types where just very thin wrappers around custom timer? Or even just different constructors?

[usbalbin]

Perhaps it does not alleviate but what if the high level types where just very thin wrappers around custom timer? [...]

On second thought, I suppose that is pretty much just making the low_level timer more high level...

[Dirbaio]

Yeah. I think 3 APIs is too much.

I'm open to making the low_level timer easier to use. For example we could add methods for all settings so you never have to drop down to the PAC.

About ruling out invalid configs: the builder solution you're proposing with 7 generic params and more traits is too complex. The design philosophy of Embassy is to avoid art installations of generics and traits. I don't think it's doable without.

[usbalbin]

Thanks for the read :)

You absolutely have a point. I'll think about it

[usbalbin]

I have removed CustomPwm and turned the builder into one that builds a low_level::Timer instead. I have also removed all type arguments except 'd and T. I have removed all but channel1 and not updated the examples for now until we agree more on the way forward.

Is this somewhat closer to something you'd be happy with?

[usbalbin]

Updated the above example(does not compile since not channels are done yet)

let out_pin = PwmPin::new(out_pin, crate::gpio::OutputType::PushPull);

let mut tim = CustomPwmBuilder::new(tim)
    //.frequency(Hertz(123))
    .prescaler_and_period(0, 1337)
    .ch1_input_as_trigger(
        trigger_pin,
        FilterValue::FDTS_DIV32_N8,
        InputCaptureMode::BothEdges,
        InputTISelection::Normal,
        1,
        TriggerMode::TriggerMode, TriggerSource::Filtered
    )
    .ch2(out_pin, OutputCompareMode::PwmMode2, 800)
    .ch3_input(
        capture_pin,
        FilterValue::FCK_INT_N2,
        InputCaptureMode::Rising,
        InputTISelection::Normal,
        0,
    )
    .one_pulse_mode()
    .finalize();

tim.set_compare_value(150, super::Channel::Ch2);
tim.waveform_up(dma, super::Channel::Ch1, &[100, 400, 800, 1100, 1200])
    .await;
//let _capture = tim.wait_for_configured_edge(super::Channel::Ch3).await;  <--- TODO?

I think it would probably make sense to turn the chX_input args into a config struct with sensible defaults