Skip to content

Unable to set different PWM rates on adjacent pins #192

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

Closed
Topo-png opened this issue Jun 29, 2020 · 8 comments
Closed

Unable to set different PWM rates on adjacent pins #192

Topo-png opened this issue Jun 29, 2020 · 8 comments

Comments

@Topo-png
Copy link

Topo-png commented Jun 29, 2020

When using analogWrite or ap3_pwm_output on pins 12 & 13 I'm unable to set different PWM rates without board freezing.

Using Artemis Redboard ATP and Arduino IDE for packages

This code freezes

void setup()
{

}

void loop()
{

  ap3_pwm_output(12, 0, 100, AM_HAL_CTIMER_HFRC_12MHZ);
  ap3_pwm_output(13, 0, 100, AM_HAL_CTIMER_HFRC_12MHZ);
  delay(1000);
  ap3_pwm_output(12, 50, 100, AM_HAL_CTIMER_HFRC_12MHZ);
  ap3_pwm_output(13, 100, 100, AM_HAL_CTIMER_HFRC_12MHZ);
  delay(500);
}

This code works

void setup()
{

}

void loop()
{

  ap3_pwm_output(12, 0, 100, AM_HAL_CTIMER_HFRC_12MHZ);
  ap3_pwm_output(13, 0, 100, AM_HAL_CTIMER_HFRC_12MHZ);
  delay(1000);
  ap3_pwm_output(12, 100, 100, AM_HAL_CTIMER_HFRC_12MHZ);
  ap3_pwm_output(13, 100, 100, AM_HAL_CTIMER_HFRC_12MHZ);
  delay(500);
}

By changing pins 12 to 32, both pieces of code function as expected.

edit: fixed typo and markdown

@nseidle
Copy link
Member

nseidle commented Jul 2, 2020

I hit this too. I'll have a look.

@nseidle
Copy link
Member

nseidle commented Jul 2, 2020

We're hitting and infinite loop here.

@nseidle
Copy link
Member

nseidle commented Jul 2, 2020

Happens on pins 12/13, 5/6, but not 4/5, 6/7, 11/12

image

Something to do with segments switching...?

@oclyke
Copy link
Contributor

oclyke commented Jul 2, 2020

Thanks for investigating @nseidle - I will keep my eye on this and try to roll in any findings to the v2.0.0 effort

@nseidle
Copy link
Member

nseidle commented Jul 2, 2020

Whew, I've got a gnarly fix:

// if timer is running wait for timer value to roll over (will indicate that at least one pulse has been emitted)
AM_CRITICAL_BEGIN // critical section when reading / writing config registers
    if ((segment == AM_HAL_CTIMER_TIMERA && * ((uint32_t *)CTIMERADDRn(CTIMER, timer, CTRL0)) &(CTIMER_CTRL0_TMRA0EN_Msk)) ||
    (segment == AM_HAL_CTIMER_TIMERB && * ((uint32_t *)CTIMERADDRn(CTIMER, timer, CTRL0)) &(CTIMER_CTRL0_TMRB0EN_Msk))
    )
{
    uint32_t current = 0;
    uint32_t last = 0;
    do
    {
        last = current;
        current = am_hal_ctimer_read(timer, segment);
    } while (current >= last);
}
AM_CRITICAL_END // end critical section

Basically, only enter the while loop if there is a segment that is already enabled that is also the segment that the user is asking us to enable.

This needs lots of testing but it fixes my code for now.

@nseidle
Copy link
Member

nseidle commented Jul 2, 2020

@oclyke - Thanks! I'd like your feedback on the viability of this fix. I'm not entirely confident in the PWM arena but I'm learning as I go.

@nseidle
Copy link
Member

nseidle commented Jul 7, 2020

PR #229 should fix this issue. We'll leave open until merged.

@oclyke
Copy link
Contributor

oclyke commented Sep 9, 2020

Closing since #229 was merged

@oclyke oclyke closed this as completed Sep 9, 2020
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

No branches or pull requests

3 participants