Some pins interfers with timer interrupts?

[Gregwar]

Hi, after investigations I figured out that using some pins seems to interfer with timer interrupts. Here is my example:

#include <wirish/wirish.h>

#define TIMER   1
#define CHANNEL TIMER_CH1

static void timer_interrupt()
{
    digitalWrite(BOARD_LED_PIN, HIGH);
}

__attribute__((constructor)) void premain()
{
    init();
}

int main(void)
{
    pinMode(BOARD_LED_PIN, OUTPUT);

    HardwareTimer timer(TIMER);
    timer.pause();
    timer.setPeriod(50000);
    timer.setMode(CHANNEL, TIMER_OUTPUT_COMPARE);
    timer.setCompare(CHANNEL, 1);
    timer.attachInterrupt(CHANNEL, timer_interrupt);
    timer.refresh();
    timer.resume();

    pinMode(27, OUTPUT);
    digitalWrite(27, LOW);
    digitalWrite(BOARD_LED_PIN, LOW);

    while (1);
}

In the example above, the led will never be turned on if you use CHANNEL=TIMER_CH1, and it will be turned on if you use CHANNEL=TIMER_CH2

It look like that pin 27 is indeed the timer pin 1_CH1, but why setting just it as OUTPUT would interfer with it?

Best regards

[Gregwar]

Initializing the timer after the pinMode seems to be a solution, but I think something should be explained in the documentation about that

[mbolivar]

Ah, OK. Yes, that's a bug. Thanks for the example! It helped me understand what you meant.

The problem is that in pinMode(), if the pin is capable of PWM, we disable the corresponding timer channel when going to OUTPUT mode.

https://github.com/leaflabs/libmaple/blob/master/wirish/stm32f1/wirish_digital.cpp#L82

The reason why is because the timer channel's capture/compare functionality is also what drives PWM, so disabling the channel is meant to also disable PWM on that pin. That does break your use case, though, and honestly, I've never liked that code, so I think we should just remove it (so users would have to disable the timer channels themselves instead). Most programs should be using pins for one thing only, so this shouldn't affect too many people.

Seem reasonable to you?

[Gregwar]

I agree, this is really odd

The problem of this code with my use case is that my pinMode was not defined before, so there is indeed no reason to disable the timer

What you suggest is replacing this code block by something like this?:

    if (PIN_MAP[pin].timer_device != NULL && pwm) {
        /* Enable timer channels if we're switching into PWM */
        timer_set_mode(PIN_MAP[pin].timer_device,
                       PIN_MAP[pin].timer_channel, TIMER_PWM);
    }

So I guess the only side-effect would be letting the timer running when switching from PWM to OUTPUT for instance, right?

[mbolivar]

That's right -- the side effect would be to leave the timer running. The timers are all running from board initialization time, anyway, so that doesn't seem like a problem to me:

https://github.com/leaflabs/libmaple/blob/master/wirish/boards.cpp#L178

[Gregwar]

Hm, indeed, it's better like this then