Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for Input Capture timer mode for frequency and pulse duration measurement #830

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

faunel
Copy link

@faunel faunel commented Jan 1, 2025

Added support for the Input Capture timer mode.

Why this is needed:

The Input Capture mode allows measuring signal frequency or pulse duration. This is especially useful for tasks that require precise frequency measurement, such as monitoring fan RPM, working with sensors, or analyzing signals. Additionally, a single timer can use up to four channels, enabling the measurement of multiple signals simultaneously.

src/timer.rs Outdated
Comment on lines 429 to 436
pub trait WithCcCommon: General {
const CC_CH_NUMBER: u8;
const CC_COMP_CH_NUMBER: u8;
fn read_cc_value(channel: u8) -> u32;
fn enable_channel(channel: u8, b: bool);
fn set_channel_polarity(channel: u8, p: Polarity);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see ant sense in additional copy of WithPwmCommon trait.
It is better to just rename WithPwmCommon to something more common.

src/timer.rs Outdated
@@ -411,6 +501,26 @@ macro_rules! split {
};
}

macro_rules! split_cc {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This macro can also be joined with previous as it is implemented for same timers.

@faunel
Copy link
Author

faunel commented Jan 2, 2025

Yes, I agree. That was code duplication.
I refactored it and added comments.

Comment on lines 111 to 116
pub enum CaptureLines<P> {
One(P),
Two(P, P),
Three(P, P, P),
Four(P, P, P, P),
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was done to allow connecting several pins on same timer channel (although as for me I've never used it). Is it allowed in capture mode?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand how this is supposed to work.
I took a lot of code from pwm.rs as a basis. So if it worked there, it should also work in input capture mode.
If you tell me how I can test it, I'll try to do so.

Copy link
Author

@faunel faunel Jan 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is what you mean, then yes, it works:

let (mut tim5, (ch1, ..)) = Timer::new(dp.TIM3, &clocks).capture_hz(48.MHz());
let mut ch1 = ch1.with(gpioa.pa6).with(gpiob.pb4.into_alternate());

But in this case, PB4 will be the active input. That means it will register signals and measure the frequency.
PA6 won't do anything.
In other words, the last pin in the chain is active.
I don't know if this is the correct behavior.

Copy link
Author

@faunel faunel Jan 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested further and got unexpected results.

The pins are as follows:
TIM3 channel1 = PA6 original
TIM3 channel1 = PB4 remap

TIM3 channel2 = PA7 original
TIM3 channel2 = PB5 remap

Using channel 1

In this case, only PB4 is active:

let (mut tim5, (ch1, _ch2, ..)) = Timer::new(dp.TIM3, &clocks).capture_hz(48.kHz());
ch1.with(gpiob.pb4.into_alternate()).with(gpioa.pa6.into_alternate());

If you swap them, the same happens: only PB4 is active:

let (mut tim5, (ch1, _ch2, ..)) = Timer::new(dp.TIM3, &clocks).capture_hz(48.kHz());
let mut ch1 = ch1.with(gpioa.pa6.into_alternate()).with(gpiob.pb4.into_alternate());

If you specify only one pin PA6 for channel 1, it works:

ch1.with(gpioa.pa6.into_alternate());

But when using channel 2, the result is different:

Both pins PA7 and PB5 are active:

let (mut tim5, (_ch1, ch2, ..)) = Timer::new(dp.TIM3, &clocks).capture_hz(48.kHz());
ch2.with(gpioa.pa7.into_alternate()).with(gpiob.pb5.into_alternate());

If you swap them, both pins PA7 and PB5 are still active:

let (mut tim5, (_ch1, ch2, ..)) = Timer::new(dp.TIM3, &clocks).capture_hz(48.kHz());
ch2.with(gpiob.pb5.into_alternate()).with(gpioa.pa7.into_alternate());

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting results.

Both pins PA7 and PB5 are active:

Is this useful or should we prohibit this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I don't see any benefit in this and can't come up with use cases for it.
I would remove this entirely:

pub enum CaptureLines<P> {
    One(P),
    Two(P, P),
    Three(P, P, P),
    Four(P, P, P, P),
}

Usually, it should be used like this:

let timer = Timer::new(dp.TIM4, &clocks);
let (_, (ch1, ch2, ch3, ch4)) = timer.pwm_hz(25.kHz());
let mut ch1 = ch1.with(pwm4_ch1);
let mut ch2 = ch2.with(pwm4_ch2);
let mut ch3 = ch3.with(pwm4_ch3);
let mut ch4 = ch4.with(pwm4_ch4);
ch1.enable();
ch2.enable();
ch3.enable();
ch4.enable();
let tim_4 = (ch1, ch2, ch3, ch4);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about "complementary pins"?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the input capture mode of the timer, complementary inputs do not exist.
In this mode, both CC1P and CC1NP bits in the TIMx_CCER register
are used to configure the input:

  • on the rising edge,
  • on the falling edge,
  • on both.
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants