[Observation] attachInterrupt reconfigures the pin, removing a pull-up and so triggers a falling interrupt

[PaulZC]

Sorry Kyle...

With v2.1.1 of the core, attachInterrupt re-configures the pin, removing a pull-up if one was present. If you're using a FALLING interrupt, then removing the pull-up causes the interrupt to trigger immediately.

Steps to reproduce:
RedBoard Artemis ATP.
Apollo3 v2.1.1.
Don't connect anything to pin 10. Leave it floating.
Run the following example:

void setup() {
  Serial.begin(115200);
  Serial.println(F("Apollo3 Interrupt Pull-Up Test"));

  pinMode(10, INPUT_PULLUP); // Put a pull-up on pin 10

  //Attach a falling interrupt to pin 10
  //This should never trigger thanks to the pull-up on pin 10 right?
  attachInterrupt(10, interruptHandler, FALLING);
}

void loop() {
}

void interruptHandler(void) {
  Serial.println(F("Interrupt!"));
}

If the pull-up on pin 10 remains enabled, the interrupt should never trigger. However, we get not one but two interrupts:

If we add a second call to pinMode(10, INPUT_PULLUP); after the attachInterrupt:

void setup() {
  Serial.begin(115200);
  Serial.println(F("Apollo3 Interrupt Pull-Up Test"));

  pinMode(10, INPUT_PULLUP); // Put a pull-up on pin 10

  //Attach a falling interrupt to pin 10
  //This should never trigger thanks to the pull-up on pin 10 right?
  attachInterrupt(10, interruptHandler, FALLING);

  pinMode(10, INPUT_PULLUP); // Re-enable the pull-up on pin 10
}

void loop() {
}

void interruptHandler(void) {
  Serial.println(F("Interrupt!"));
}

Then we still get the first interrupt - but not the second one:

I've got a work-around for this on OpenLog Artemis, which involves using the ISR to set a flag. I clear the flag after manually re-enabling the pull-up.

With v1.2.1, the first example does not trigger an interrupt:

Cheers,
Paul

[paulvha]

Hi

Had a look at this issue and found the root cause and solution.

Root cause
In short, the clearing and enable of IRQ on a pad is happening too early. It is happening at the end of gpio_irq_init() which is called early during initialization through MBED. Hence when you change the IRQ from NONE to FALL, RISE it will trigger.

Solution
In the file apollo3/2.1.1/cores/mbed-os/targets/TARGET_Ambiq_Micro/TARGET_Apollo3/device/gpio_irq_api.c make the following changes:
At the end of routine gpio_irq_init(), around line 62, comment out / remove

        //Enable GPIO IRQ's in the NVIC
        // gpio_irq_enable(obj);
        //NVIC_SetVector((IRQn_Type)GPIO_IRQn, (uint32_t)am_gpio_isr);
        //NVIC_EnableIRQ((IRQn_Type)GPIO_IRQn);

At the end of routine gpio_irq_set(), around line 140, add after ap3_gpio_enable_interrupts(obj->control->pad, ap3_int_dir);

        //Enable GPIO IRQ's in the NVIC
        gpio_irq_enable(obj);
        NVIC_SetVector((IRQn_Type)GPIO_IRQn, (uint32_t)am_gpio_isr);
        NVIC_EnableIRQ((IRQn_Type)GPIO_IRQn);

ADDITIONAL NOTE 1: The initialization is a bit chaotic. some routines are called multiple times ( e.g. irq_init(), direction setting, mode setting. They do not conflict, but are unnecessary.

ADDITIONAL NOTE 2: There is no need to set the pullup/pulldown in the sketch. It is handled already on 2 places ( InterruptIn.CPP and in Commoninterrupt.cpp)

regards,
Paulvha

[PaulZC]

Thank you Paul - much appreciated!
Very best wishes,
The other Paul

[PaulZC]

Hi Paul (@paulvha ),

Thanks again for all the help you've given us on Artemis and Apollo3 - it is very much appreciated.

Just looking at the interrupt pull-ups again, if you have a 10M resistor lying around, could you please try the following?

Steps to reproduce:
RedBoard Artemis ATP
Link Pin 0 to Pin 1 using a 10M resistor
Run the following sketch:

volatile int interruptCounter = 0;

void setup() {
  Serial.begin(115200);
  Serial.println(F("Apollo3 Interrupt Pull-Up Test"));

  //Pin 0 will generate a 1Hz square wave
  //Use a 10M resistor to link Pin 0 to Pin 1
  pinMode(0, OUTPUT);
  
  //Pin 1 will be our interrupt pin
  pinMode(1, INPUT_PULLUP); // Put a pull-up on Pin 1

  //Attach a falling interrupt to Pin 1
  //If Pin 1 has a pull-up on it, even a WEAK one, we should see no interrupts
  attachInterrupt(1, interruptHandler, FALLING);
}

void loop() {
  digitalWrite(0, !digitalRead(0)); //Toggle Pin 0 at 1Hz
  delay(500);

  Serial.println(interruptCounter);
}

void interruptHandler(void) {
  interruptCounter++;
}

With v1.2.1 of the core, interruptCounter remains zero. No interrupts are seen as the 10M resistor is too high to counteract the pull-up on Pin 1.

With v2.1.1 of the core, I see interrupts:

My thesis is that attachInterrupt reconfigures the pin as an INPUT - removing the pull-up.

If I add a second pinMode(1, INPUT_PULLUP); after the attachInterrupt then interruptCounter stops incrementing. If I then short Pin 0 to Pin 1, just to prove the interrupt is still working, the counts start incrementing again.

Very best wishes,
Paul

[paulvha]

hi Paul

You are right about removing the pull-up.

test

I have tried it on pin 10 and 8, put the 10M between the points and attached a scope.
I started with the sketch as is, but only changed pin 0 to 8 and pin 1 to 10.

On V2.1.1 I get the same result as you. Given the pinmode(10, INPUT_PULLUP) set before this attachInterrupt I would not expect that to happen

I see a start of 500mS the pulse going high and a print of interruptcounter of 0.
Next loop the output on pin 8 is set low, the interrupt triggers and a print of interruptcounter of 1.
Next loop the output on pin 8 is set high and a print of interruptcounter of 1.
Next loop the output on pin 8 is set low, the interrupt triggers and a print of interruptcounter of 2.
Next loop the output on pin 8 is set high and a print of interruptcounter of 2.

If I then add pinMode(10, INPUT_PULLUP); after attachInterrupt() I get the nearly the same output as you, but it fires 1 time every time. (which I will explain later)

Apollo3 Interrupt Pull-Up Test
1
1
1
1
1
1
1

root cause
After debugging a lot I have found the issue. The problem is the already mentioned chaotic initialization happening in different places multiple times. It makes this issue very hard to debug.

When you call AttachInterrupt() it will call indexAttachInterruptParam(). Both are part of Commoninterrupt.cpp of the Sparkfun library. If there is no constructer yet with MBED-OS-interrupt it calls InterruptInParam(pinNameByIndex(index)), which is in the same file.

From there it will call InterruptIn(pin), which is part of MBED-OS. As NO mode is passed in the previous calls we end up with PullDefault, which is NO pullup. As such it is removing the pull-up that was set before! Below is the sequence that is happening

In InterrruptIn.cpp / mbed-os
InterruptIn::InterruptIn(PinName pin) : gpio(),
    gpio_irq(),
    _rise(),
    _fall()
{
    // No lock needed in the constructor
    irq_init(pin);
    gpio_init_in(&gpio, pin);
}

in med_gpio.c /mbed-s : 

void gpio_init_in(gpio_t *gpio, PinName pin)
{
    gpio_init_in_ex(gpio, pin, PullDefault);
}

void gpio_init_in_ex(gpio_t *gpio, PinName pin, PinMode mode)
{
    _gpio_init_in(gpio, pin, mode);
}

static inline void _gpio_init_in(gpio_t *gpio, PinName pin, PinMode mode)
{
    gpio_init(gpio, pin);       // register pad / pin only
    if (pin != NC) {
        gpio_dir(gpio, PIN_INPUT);	  // in gpio_api.c
        gpio_mode(gpio, mode);		  // in gpio_api.c
    }
}

The removing of the pullup on the pin is also the reason it is triggering Interrupt at least once even if we have pinMode(10, INPUT_PULLUP) before in the sketch.

As indicated in Commoninterrupt.cpp :
// Give a default pullup for the pin, since calling InterruptIn with PinMode is impossible.

The solution

Change the routine in indexAttachInterruptParam() in Commoninterrupt.cpp to :

void indexAttachInterruptParam(pin_size_t index, voidFuncPtrParam callback, PinStatus mode, void* param){
    indexDetachInterrupt(index);
    arduino::InterruptInParam* irq = pinIRQByIndex(index);
    if(!irq){
        irq = new arduino::InterruptInParam(pinNameByIndex(index));
    }
    pinIRQByIndex(index) = irq;

    // always reset the pullup first as it is reset by InterruptInParam call
    switch (mode) {
        case FALLING :
            indexPinMode(index, INPUT_PULLUP);
            break;
        case RISING :
            indexPinMode(index, INPUT_PULLDOWN);
            break;
        case CHANGE :
        default:
            indexPinMode(index, INPUT);
            break;
    }

    // now enable the interrupt
    switch (mode) {
        case CHANGE :
            irq->rise(mbed::callback(callback), param);
            irq->fall(mbed::callback(callback), param);
            break;
        case FALLING :
            irq->fall(mbed::callback(callback), param);
            break;
        case RISING :
        default :
            irq->rise(mbed::callback(callback), param);
            break;
    }
}

Next to the earlier change in gpio_irq_api.c mentioned in an earlier post , this needs to change as well.

regards,
Paulvha

[PaulZC]

Thank you Paul - again, very much appreciated,
Very best wishes,
Paul