Add parameters for more control over general purpose timer CR1.

[dhebbeker]

For compatibility reasons with legacy code I need more control over the TIMx->CR1 register. Thus I added more parameters. Default values render the previous behavior.

I am not sure, if this is the best / correct solution?

[salkinium]

I am not sure, if this is the best / correct solution?

I… guess? 🤷‍♂
The timers have so many use-cases, it's difficult to make one API that serves them all.
Backwards compatibility is great, but adding all those bools isn't ideal in C++, due to a lack of named arguments.

I think @rleh and @se-bi have more stake in the Timer APIs than me, what do you think?

[rleh]

At @roboterclubaachen it is common to write the registers directly for complex timer configurations.

[rleh]

See this #275 (comment):

Implementing a generic API for timers is probably not possible and in my opinion the wrong attempt of abstraction. On STM32 every timer and every channel sometimes has different features/options and behaves slightly different.
If you look at more manufacturers, it gets even worse in terms of compatibility/interoperability.

I could imagine a platform independent API which provides a PWM signal with adjustable frequency and duty cycle at one pin.

Depending on how much abstraction we consider suitable, the user should not care about the hardware.
For example, he wants to generate PWM with a certain frequency and duty cycle. Then I imagine the interface like this:

modm::Pwm<Gpio> pwm{100_kHz /* frequency */};
pwm.setDutyCycle(50_pct);

Or the user wants to run a DC or BLDC/PMSM motor:

librobots2::motor::Bldc<Timer, Pins, ...> ...

Or measure a pulse width. And so on...

For more complex timer configurations I would suggest to directly access the registers.

Since it is easily possible and well documented (in STs datasheets) to access the registers I don't think we need an pass-through wrapper.

[dhebbeker]

I agree that the interface should be clear and unambiguous.

The code documentation has this warning:

The Timer has much more possibilities than presented by this interface (e.g. Input Capture, Trigger for other Timers, DMA). It might be expanded in the future.

I am not persuaded that directly accessing registers is the way I want to use it¹ as it kind of breaks the programming paradigm.

¹: The usage, and possibly alternative solution, being outside of modm.

[se-bi]

At @roboterclubaachen it is common to write the registers directly for complex timer configurations.

For more complex timer configurations I would suggest to directly access the registers.

True, we did that for a case with AdvancedTimer and the I-want-it-to-work-now situation...

(But i would suggest to bring that stuff also into modm, I remember it as quite error prune to get what we wanted out of the documentation into the registers, better for the future to use the ready-to-go solution...)

I am not persuaded that directly accessing registers is the way I want to use it¹ as it kind of breaks the programming paradigm.

I would agree here, since also the General Purpose Timer (I hope the "basic" one is meant *) is the same or very similar for stm32 devices or at least some families*?

* Correct if i am wrong...

Regarding:

I am not sure, if this is the best / correct solution?

I… guess? 🤷‍♂️
The timers have so many use-cases, it's difficult to make one API that serves them all.
Backwards compatibility is great, but adding all those bools isn't ideal in C++, due to a lack of named arguments.

Bools..:man_shrugging:
Backwards compatibility 👍 what against that 😉

[rleh]

Don't get me wrong: I don't mind merging this pull request at all.

[rleh]

I am not persuaded that directly accessing registers is the way I want to use it¹ as it kind of breaks the programming paradigm.

That's right.
Maybe we should rename the current API to 'TimerHal' (as in Uart) when implementing the application-driven APIs.

[salkinium]

Maybe we should rename the current API to 'TimerHal' (as in Uart) when implementing the application-driven APIs.

Yes!

Don't get me wrong: I don't mind merging this pull request at all.

Ok, I will rebase and merge these changes.

[dhebbeker]

Ok, I will rebase and merge these changes.

@salkinium: Is it a general policy to merge ff-only for this repository, or why the rebase? I could attempt to comply with that in future.

[salkinium]

Yes, otherwise the history is full of merge commits which creates a lot of noise.