Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

HardwareTimer: Fix assert failed when using TIMER_OUTPUT_COMPARE#1247

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@ABOSTM
Copy link
Contributor

Summary

HardwareTimer: Fix assert failed when using TIMER_OUTPUT_COMPARE

When assert is activated there may be assert failed, specially
when using Tone or Servo with TIM6 or TIM7.
"assert_param(IS_TIM_CC1_INSTANCE(htim->Instance));"
This is due to the fact that when using timer instances without
output (like TIM6 and TIM7 specially used for Tone and Servo)
in TIMER_OUTPUT_COMPARE mode, the API setMode() requires a channel,
even if it is not used.
This was made like this to simplify the HardwareTimer driver,
and there is no functional issue, but as there is an assert failed
reported when assert is activated, this should be fixed.

TIMER_OUTPUT_COMPARE becomes obsolete, but kept for compatibility
reason.
When only timing configuration is needed, no need to set mode,
just keep the default TIMER_DISABLED.

Validation
Test passed:

  • Servo
  • Tone
  • HardwareTimer non regression test (HardwareTimer_OutputInput_test)

Fixes#1244

ABOSTM added a commit to ABOSTM/STM32Examples that referenced this pull requestDec 1, 2020
After implementation of:stm32duino/Arduino_Core_STM32#1247it is better to use TIMER_DISABLED instead of  TIMER_OUTPUT_COMPAREEven if TIMER_OUTPUT_COMPARE has been kept for compatibilityreason and is still working.Signed-off-by: Alexandre Bourdiol <alexandre.bourdiol@st.com>
ABOSTM added a commit to ABOSTM/STM32Examples that referenced this pull requestDec 1, 2020
After implementation of:stm32duino/Arduino_Core_STM32#1247it is better to use TIMER_DISABLED instead of  TIMER_OUTPUT_COMPAREEven if TIMER_OUTPUT_COMPARE has been kept for compatibilityreason and is still working.Signed-off-by: Alexandre Bourdiol <alexandre.bourdiol@st.com>
@ABOSTMABOSTM requested a review fromfpistmDecember 1, 2020 17:02
When assert is activated there may be assert failed, speciallywhen using Tone or Servo with TIM6 or TIM7."assert_param(IS_TIM_CC1_INSTANCE(htim->Instance));"This is due to the fact that when using timer instances withoutoutput (like TIM6 and TIM7 specially used for Tone and Servo)in TIMER_OUTPUT_COMPARE mode, the API setMode() requires a channel,even if it is not used.This was made like this to simplify the HardwareTimer driver,and there is no functional issue, but as there is an assert failedreported when assert is activated, this should be fixed.TIMER_OUTPUT_COMPARE becomes obsolete, but kept for compatibilityreason.When only timing configuration is needed, no need to set mode,just keep the default TIMER_DISABLED.Fixesstm32duino#1244Signed-off-by: Alexandre Bourdiol <alexandre.bourdiol@st.com>
@ABOSTMABOSTMforce-pushed theHARDWARETIMER_ASSERT_FAILED branch from09b074b to3e313f9CompareDecember 1, 2020 17:06
@ABOSTM
Copy link
ContributorAuthor

@ghent360 fix is there.

@fpistmfpistm added bug 🐛Something isn't working fix 🩹Bug fix labelsDec 2, 2020
@fpistmfpistm added this to the2.0.0 milestoneDec 2, 2020
Copy link
Member

@fpistmfpistm left a comment

Choose a reason for hiding this comment

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

LGTM

@fpistm
Copy link
Member

Due to GH Action issue (resolved now) some of the checks are failed anyway it is ok.
PR could be merged.

@fpistmfpistm merged commit8888918 intostm32duino:masterDec 2, 2020
fpistm pushed a commit to stm32duino/STM32Examples that referenced this pull requestDec 2, 2020
After implementation of:stm32duino/Arduino_Core_STM32#1247it is better to use TIMER_DISABLED instead of  TIMER_OUTPUT_COMPAREEven if TIMER_OUTPUT_COMPARE has been kept for compatibilityreason and is still working.Signed-off-by: Alexandre Bourdiol <alexandre.bourdiol@st.com>
ABOSTM added a commit to ABOSTM/STM32Ethernet that referenced this pull requestDec 3, 2020
With introduction of PRstm32duino/Arduino_Core_STM32#1247Usage of TIMER_OUTPUT_COMPARE becomes obsolete.Note: removing setMode(1, TIMER_OUTPUT_COMPARE)also works before PR #1247.Signed-off-by: Alexandre Bourdiol <alexandre.bourdiol@st.com>
@VitorBoss
Copy link
Contributor

VitorBoss commentedJun 11, 2021
edited
Loading

Sorry to send a message here, I've tried to do it on the forum but couldn't login, I won't open a issue without confirming it first.
This PR has been included in a while but I can't use the API like before, here is an example code:

    Timer1.setOverflow(0xFFFF, TICK_FORMAT);    Timer1.setPrescaleFactor(((Timer1.getTimerClkFreq()/1000000) * TIMER_RESOLUTION)-1);   //4us resolution    Timer1.setMode(2, TIMER_OUTPUT_COMPARE);    Timer1.setMode(3, TIMER_OUTPUT_COMPARE);    Timer1.attachInterrupt(2, boostInterrupt);    Timer1.attachInterrupt(3, vvtInterrupt);    Timer1.resume();

On v1.9.0 it works flawlessly, on 2.0.0 it don't work at all, I tried this:

    Timer1.setOverflow(0xFFFF, TICK_FORMAT);    Timer1.setPrescaleFactor(((Timer1.getTimerClkFreq()/1000000) * TIMER_RESOLUTION)-1);   //4us resolution    Timer1.setMode(2, TIMER_DISABLED);    Timer1.setMode(3, TIMER_DISABLED);    Timer1.attachInterrupt(2, boostInterrupt);    Timer1.attachInterrupt(3, vvtInterrupt);    Timer1.resume();

It refuses to work with channels, if I remove the channel number I got timer interrupts, what am I missing?
Edit: Tested on Black F407VE and F411CE BlackPill

@ABOSTM
Copy link
ContributorAuthor

Hi@VitorBoss, you are right, the behavior has been changed/improved thanks to this PR:
As you noticed TIMER_OUTPUT_COMPARE becomes obsolete and you changed it to TIMER_DISABLED, great.
But the behavior you described looks normal to me,
TIMER_DISABLED will disable the channel: it is intend to configure timer in timebase mode without PWM output and thus only period is valid, not the channel. So in this case you cannot get channel interrupt, but only period interrupt.

Alternatively you can configure the timer with channel configuration,
for example (depending whether you need PWM output)
Timer1.setMode(channel, TIMER_OUTPUT_COMPARE_PWM1, PA_8); PWM will be output to PA8
or
Timer1.setMode(channel, TIMER_OUTPUT_COMPARE_PWM1); implicitly pin is NC (Not connected)

fpistm reacted with rocket emoji

@VitorBoss
Copy link
Contributor

First, thank you for the reply.

If I understood you right is just a matter of replacing theTIMER_OUTPUT_COMPARE withTIMER_OUTPUT_COMPARE_TOGGLE with no pin as argument right?

Please add this to the documentation, I was trying to make it work for a week looking on the wiki

@ABOSTM
Copy link
ContributorAuthor

UsingTIMER_OUTPUT_COMPARE_TOGGLEorTIMER_OUTPUT_COMPARE_PWM1just impact output wave form (see reference manual TIMx_CCMR1 for details) so if you intend to use timer with no pin, both will be the same.
That said, there something not clear to me in your code:
both callback boostInterrupt() and vvtInterrupt() will be called at the same frequency (period is unique for a given timer whatever the cahnnel). And because you didn't specify the pulse duration, both callback will occurs at the same time.
If this is what you wanted to do, there is no need to use 2 channels, only update interrupt is necessary and in the update callback you call both boostInterrupt() and vvtInterrupt().
Seehttps://github.com/stm32duino/STM32Examples/blob/master/examples/Peripherals/HardwareTimer/Timebase_callback/

Using 2 channel may be interesting only if you wanted to have the same frequency for both callbacks, AND you want a specific delay between both callback. If it is what you wanted to do, it is necessary to specify both pulse duration (the delta between both will determined the delay between both callbacks).
And even, there is another (better) way to do this: use update interrupt callback and only 1 channel to specify the delay between the 2 callbacks.

@VitorBoss
Copy link
Contributor

VitorBoss commentedJun 11, 2021
edited
Loading

The use case here is an engine management system, each channel do 2 independent PWM wave form with independent frequency settings, the period isn't the same.

Thank you for clarify my doubts.

@uzi18
Copy link

@valeros do you have more info about this project?

@ABOSTM
Copy link
ContributorAuthor

@VitorBoss,

with independent frequency settings, the period isn't the same.

1 timer = 1 period, so you need 2 timers

each channel do 2 independent PWM wave form

So totally there are 4 PWM wave form?
2 with a the same frequency F1, and 2 with another frequency F2 ?
If yes, then you need 2 timers, each timer with 2 channels and pin associated to output the wave form.
Mode should probably be TIMER_OUTPUT_COMPARE_PWM1 (or TIMER_OUTPUT_COMPARE_PWM2).
And, if all you need is to output wave form, then you don't need to configure interruption at all.

You can have a look at examplehttps://github.com/stm32duino/STM32Examples/tree/master/examples/Peripherals/HardwareTimer/PWM_FullConfiguration
Look at how"pin" is configured with no need to toggle it in interrupt, PWM wave form for pin is automatically generated by TIM hardware (interrupt is forpin2only notpin).

Also as stated in wiki:https://github.com/stm32duino/wiki/wiki/HardwareTimer-library

The use of this library suppose you have some basic knowledge of STM32 hardware timer architecture. First of all remind that all timers are not equivalent and doesn't support the same features. Please refer to the Reference Manual of your MCU.

So I strongly suggest you to read Reference Manual, chapter relative to TIMx

VitorBoss reacted with heart emoji

@VitorBoss
Copy link
Contributor

Sorry, I won't clear, each channel do 2 software PWM, even with same timer period the code uses interrupts to trigger the PWM period(pwm_max_count = 1000000L / (TIMER_RESOLUTION * configPage.idleFreq * 2);), the code is made to be flexible and compatible with any MCU, when I first start porting the code to be compatible with STM32 I didn't use the TIMx pins, everything is handle inside the interrupts.

The only problem I had with latest release was the compatibility withTIMER_OUTPUT_COMPARE, now everything is working again. Thank you for your support, it is very much appreciated.

I know you are busy, if you have a time check out the projecthttps://github.com/noisymime/speeduino

ABOSTM reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fpistmfpistmfpistm approved these changes

Assignees

No one assigned

Labels

bug 🐛Something isn't workingfix 🩹Bug fix

Projects

None yet

Milestone

2.0.0

Development

Successfully merging this pull request may close these issues.

HardwareTimer: assert failed when assert is activated, specially when using Servo or Tone

4 participants

@ABOSTM@fpistm@VitorBoss@uzi18

[8]ページ先頭

©2009-2025 Movatter.jp