Interrupt only execute once

Hi, I'm trying to control the speed of some LEDs using two buttons (one to increase and one to decrease) using interrupts but it only triggers the function once.

Here's my diagram and code

const int min = 20;
const int max = 1000;
const int step = 20;
volatile int speed = 500;


void speedIncrease() {
  speed = speed - 20;
  if (speed < min) {
    speed = max;
  }
}

void decrease() {
  speed += step;

  if (speed > max) {
    speed = min;
  }
}

void setup()
{
  Serial.begin(9600);
  for (int i = 8; i >=4; i--) {
    pinMode(i, OUTPUT);
    digitalWrite(i, LOW);
  }
  attachInterrupt(digitalPinToInterrupt(2), speedIncrease, RISING);  
  attachInterrupt(digitalPinToInterrupt(3), decrease, RISING);

}

void loop()
{
    for (int j = 8; j > 3; j--) {
      if (j < 8) 
        digitalWrite(j+1, LOW);
      digitalWrite(j, HIGH);
      delay(speed);
    }
  
   digitalWrite(4, LOW);
  for (int j = 4;j <= 8; j++) {
      if (j >4)
        digitalWrite(j-1,LOW);
      digitalWrite(j, HIGH);
      delay(speed);
    }
   digitalWrite(8, LOW);
  Serial.print("Speed: ");  
  Serial.println(speed);
}

It works in the simulator but not in real life.

how did you wire the buttons in real life and not on the model? best would be to connect in diagonal across the button to avoid connecting to 2 pins that are "the same".

image

using the builtin pull-ups would make it easier (just invert the logic)

(and we usually place the current limiting resistor on the Anode of the LED)

1 Like
volatile int speed = 500;

void speedIncrease() {
  const int min = 20;
  speed  -=  20;
  if (speed < min)speed = min;
}

void decrease() {
  const int max = 1000;
  speed += 20;
  if (speed > max)speed = max;
}

void setup()
{
  Serial.begin(9600);
  for (int i = 4; i < 9; i++)pinMode(i, OUTPUT);
  attachInterrupt(digitalPinToInterrupt(2), speedIncrease, RISING);
  attachInterrupt(digitalPinToInterrupt(3), decrease, RISING);
}

void loop() {
  for (int j = 8; j > 3; j--) {
    digitalWrite(j, HIGH);
    delay(speed);
    digitalWrite(j , LOW);
  }

  for (int j = 4; j <= 8; j++) {
    digitalWrite(j, HIGH);
    delay(speed);
    digitalWrite(j, LOW);
  }

  if ((millis() % 1000) == 0) {
    Serial.print("Speed: ");
    Serial.println(speed);
    delay(1);
  }
}

debounce not implemented

speed could be modified by the ISR whilst the function is being called... a non blocking approach would be better or make a copy of the speed in a critical section and use the copy and only take into account changes at the next iteration.

Do we, and if so, why ?

OK - may be I generalised abusively with "we" and it might be just my experience.

I understand that placing the resistor on the anode side or the cathode side has no impact on the LED’s performance or protection because the resistor’s primary function is to limit current, which is determined by Ohm’s / Kirchhoff’s Laws and the resistor’s position in the circuit doesn’t affect this calculation.

it's just that withthis placement the components are in an easy to interpret order (may be for me, I'm a software guy) with the resistor limiting current "before" it reaches the LED. Feels more natural to me and easier to follow the circuit.

What does it do in real life?
Do any of the LEDs light up?
What does the serial monitor show?
Are you using an Uno R3?

Yes, I'm using UNO R3,
LEDs light up in both real life and in the simulator

It just the speed that's not working (changing) in real life.
In the serial monitor speed changes just once (+-20) and then no matter how many times I press the button it doesn't work

I'll follow the suggestion mentioned above and come back

If the resistor is connected between the LED cathode and GND it is more convenient to monitor / measure the LED current using an oscilloscope.

speed should be unsigned long

Your circuit is fine, no need to change it

but important to implement it in real life in the same way (eg it's easy to turn the buttons 90° and get it wrong)

this doesn't do anything, this part of the code only executes twice and then it never show anything on the serial ouput

probably OP meant a % instead of /

1 Like

I was using this breadboard and here all the vertical lines are not connected. It's connected in block of 3. So my common ground wire was working for the leds but not for the buttons as they were further away. @J-M-L suggestion of using builtin pull-up also made the circuit simpler.

Btw how many pullup-resistors I can use on arduino uno?

Thanks @everyone for the support

If you mean the built in pullups, then there is one for each pin, including the A* pins when used as digital pins

1 Like

What is more natural to experienced engineers (because it's rooted in history) is to connect the LED's anode to VCC (resistor on either side) and the cathode to the I/O pin. This harkens back to the days when logic circuits could sink more current than they could source. So, it was safer to sink current in order to turn ON the LED.

There are no vertical lines on that breadboard, it is at about 45 degrees angle to the camera. So there are are only SE-NW and SW-NE lines. Please explain what you mean.

What is connected in block of what 3 items? Again, please explain what you mean.

I can see exactly what is connected to what on your breadboard simply by looking at it. But I am familiar with breadboards. I want to check that you truly have understood what is connected on your breadboard, to avoid more problems in future

Only if you are a conventional flow guy (+ to -) if you are a modern - to + then what???

I’m very conventional :wink:

1 Like