Confusion on implementing button interrupts on Pi Pico written in C

Question

I'm currently a novice programmer learning C, and I'm having trouble understanding implementation of using the interrupts and callback function.
I've written a small program one with interrupts and one without. Both (are supposed to) do the same thing, change which led is fired (of 5).

The non-interrupt one works, not as I'd like, but works. The interrupt one hangs the pico immediately. The below code is an amalgam of the two, in desperate attempts.

#include <stdio.h>
#include "pico.h"
#include "hardware/gpio.h"

#define button 9
#define buttontwo 14
#define gate_pin1 15
#define gate_pin2 10
#define gate_pin3 11
#define gate_pin4 12
#define gate_pin5 13
int place_marker = 3;

void place_check_func(placer) {

  if (placer < 0) {
    printf("Place marker was less than zero, reset.\n");
    sleep_ms(50);
    place_marker == 0;

  } else if (placer > 5) {
    printf("Placer is too high.\n");
    sleep_ms(50);
    place_marker -= 1;
  }
}

void button_callback(uint gpio, uint32_t events) {

  while (1) {

    //  I also tried if(gpio == 9) in this function.

    //  The gpio_get method worked in a different function, per while cycle, as a non interrupt.

    if (gpio_get(button) == 0x000) {
      sleep_ms(10);
      if (gpio_get(button) == 0x000) {

        printf("Button one pressed.\n");

        place_marker++;
      }

    } else if (gpio_get(buttontwo) == 0x000) {
      sleep_ms(10);
      if (gpio_get(buttontwo) == 0x000) {
        printf("Button Two Pressed.\n");

        place_marker--;
      }
    } else {

      printf("Place is %d.\n", place_marker);
      printf("Interrupt occured at pin %d, with event %d\n", gpio, events);

    }

  }
}

void led_fire(place) {

  if (place == 1) {

    for (int aa = 0; aa < 20; aa++) {

      gpio_put(gate_pin1, 1);
      sleep_ms(128);
      gpio_put(gate_pin1, 0);

    }

  } else if (place == 2) {

    for (int ab = 0; ab < 20; ab++) {

      gpio_put(gate_pin2, 1);
      sleep_ms(64);
      gpio_put(gate_pin2, 0);

    }

  } else if (place == 3) {

    for (int ac = 0; ac < 20; ac++) {

      gpio_put(gate_pin3, 1);
      sleep_ms(32);
      gpio_put(gate_pin3, 0);

    }

  } else if (place == 4) {

    for (int ad = 0; ad < 20; ad++) {
      gpio_put(gate_pin4, 1);
      sleep_ms(16);
      gpio_put(gate_pin4, 0);

    }

  } else if (place == 5) {

    for (int ae = 0; ae < 20; ae++) {

      gpio_put(gate_pin5, 1);
      sleep_ms(8);
      gpio_put(gate_pin5, 0);

    }

  } else {

    for (int b = 0; b < 10; b++) {

      printf("Placer hasn't yet incremented.\n");

      sleep_ms(10);
      gpio_put(gate_pin1, 1);
      sleep_ms(10);
      gpio_put(gate_pin1, 0);
      sleep_ms(10);
      gpio_put(gate_pin2, 1);
      sleep_ms(10);
      gpio_put(gate_pin2, 0);
      sleep_ms(10);
      gpio_put(gate_pin3, 1);
      sleep_ms(10);
      gpio_put(gate_pin3, 0);
      sleep_ms(10);
      gpio_put(gate_pin4, 1);
      sleep_ms(10);
      gpio_put(gate_pin4, 0);
      sleep_ms(10);
      gpio_put(gate_pin5, 1);
      sleep_ms(10);
      gpio_put(gate_pin5, 0);
      sleep_ms(10);

    }

  }
}

int main() {
  stdio_init_all();

  gpio_init(button);
  gpio_set_dir(button, GPIO_IN);
  gpio_pull_up(button);

  gpio_init(buttontwo);
  gpio_set_dir(buttontwo, GPIO_IN);
  gpio_pull_up(buttontwo);

  gpio_init(gate_pin1);
  gpio_set_dir(gate_pin1, GPIO_OUT);
  gpio_init(gate_pin2);
  gpio_set_dir(gate_pin2, GPIO_OUT);
  gpio_init(gate_pin3);
  gpio_set_dir(gate_pin3, GPIO_OUT);
  gpio_init(gate_pin4);
  gpio_set_dir(gate_pin4, GPIO_OUT);
  gpio_init(gate_pin5);
  gpio_set_dir(gate_pin5, GPIO_OUT);

  gpio_set_irq_enabled_with_callback(button, 0x04, 1, & button_callback);
  gpio_set_irq_enabled_with_callback(buttontwo, 0x04, 1, & button_callback);

  while (1) {

    place_check_func(place_marker);
    printf("Loop running.  Awaiting button press.\n");
    sleep_ms(500);
    led_fire(place_marker);

    sleep_ms(500);
    tight_loop_contents();

  }

}

There are lots of things I could do to clean up this code, but I'm just interested in why the gpio_get method works, and my interrupt doesn't. **I edited some of the code in this text panel, so I may have missed a bracket end somewhere.

I'm still learning, and if the answer requires pointers/referencing | de-reference, I'm not quite comfortable with them yet (hence why no array code).

Please indent your code. It's common to use 4 (or 2) spaces at each level — Support Ukraine, Jun 20 '23 at 05:06
Without proper indentation it's hard to read your code but it seems that `button_callback` is an endless `while` loop. And shouldn't it use the `gpio` parameter instead of hard code values? — Support Ukraine, Jun 20 '23 at 05:15
Sorry about the indentation, this was my first post, I didn't understand the layout for the actual code input in the text panel. I removed the while loop, and it works perfectly now. Still don't fully understand why.. but practice will teach me hopefully! I'll try with the gpio parameters as well. — Alphonse Reitz, Jun 20 '23 at 05:37
Please indent your code (now that you understood the concept) by [edit]ing. — Yunnosch, Jun 20 '23 at 05:39

score 2 · Answer 1 · answered Jun 20 '23 at 05:56

You are getting an infinite loop in void button_callback(). The main issue I see is in the while loop inside that function. It's hanging because when the interrupt occurs it's called but it never returns. while(1) creates an infinite loop that wont let the main loop continue (it is waiting on the function.) I would scrape the while loop and have the callback set a flag or modify a value. What little I know about interrupt service subroutines is that the functions should be as short and straightforward as possible, with a simple conditional and a flag. Something like this inside button_callback:

if(gpio == button){
   buttonPressed =1;
} else if(gpio == buttontwo){
   buttonTwoPressed = 1;
}

And that's all it needs. Similarly for your other functions with loops in them. The ISR is supposed to be a short bit of code that reacts to an event, and flips a flag or value, that the main loop then sees. I hope that helps you make some progress. Good luck.

score 0 · Answer 2 · edited Jun 20 '23 at 05:42

Based on recommendation I removed the while loop in the callback.

void button_callback(uint gpio, uint32_t events)
{
    printf("Interrupt occured at pin %d, with event %d\n", gpio, events);
    if(gpio_get(button) == 0x000)
    {
        printf("Button one pressed.\n");

        place_marker++;
        printf("Place is %d.\n", place_marker);
    
    } else if(gpio_get(buttontwo) == 0x000)
    {
        printf("Button Two Pressed.\n");
        
        place_marker--;
        printf("Place is %d.\n", place_marker);

    } else
    {
    }
}

Confusion on implementing button interrupts on Pi Pico written in C

2 Answers2