Hardware ISR not running.

Projects Programming
1

I am using a Mega and attaching an interrupt to pin 3.

in Setup I have;

 attachInterrupt(digitalPinToInterrupt(FbrakeSw), brakes, LOW);

On a LOW button push on pin 3 (which is set as an input-pullup) the interrupt triggers and stops the program and releases again when the input is high. But my ISR isnt running correctly (LED just stays on or off). If I run the ISR just as a sketch in the loop it is fine.

Is the ISR sensitive to button debounce? If so how could I manage that?

ISR is;

void brakes() {
  unsigned long BrakemillisNow;
  Fbrakestat = digitalRead (FbrakeSw);
  

  if (Fbrakestat == LOW)  {
    BrakemillisNow = millis();
    if (BrakemillisNow - BrakemillisPrev >= Brakeperiod) {

      // Serial.println ("brakes period");
      BrakemillisPrev = BrakemillisNow;
      if (brakelightstat == LOW) {
        brakelightstat = HIGH;
      } else {
        brakelightstat = LOW;
      }
      digitalWrite (brakeLightout, brakelightstat);
    }

  } else if  (Fbrakestat == LOW  {
    digitalWrite (brakeLightout, HIGH);

  }  else {
    digitalWrite (brakeLightout, LOW);
  }
}

Thanks.....

2

Post your entire sketch. We need to see how all the variables are declared.

3 
unsigned long BrakemillisNow;
  Fbrakestat = digitalRead (FbrakeSw);
  

  if (Fbrakestat == LOW)  {

There is no need for the digitalRead() or the IF because you can only be in the ISR if the pin is LOW

Of course the bigger question is whether an interrupt is needed at all in this case.

...R

4

Robin, the main program is very large and as using brakes is an important task I want it to happen quickly.

Declarations are

const byte FbrakeSw = 3;
const unsigned long BrakemillisPrev = 0;
unsigned long Brakeperiod = 500;
int brakelightstat = LOW;
pinMode (FbrakeSw, INPUT_PULLUP);
attachInterrupt(digitalPinToInterrupt(FbrakeSw), brakes, LOW);

I have tidied up the ISR to read

void brakes() {
 unsigned long BrakemillisNow;
 
   BrakemillisNow = millis();
    if (BrakemillisNow - BrakemillisPrev >= Brakeperiod) {
        BrakemillisPrev = BrakemillisNow;
      if (brakelightstat == LOW) {
        brakelightstat = HIGH;
       }else {
        brakelightstat = LOW;
      }
      digitalWrite (brakeLightout, brakelightstat);
    }

}
5

kpg:
Robin, the main program is very large and as using brakes is an important task I want it to happen quickly.

It's more likely that your program contains inefficient, sluggish code that should be improved.

That is a common statement from beginners that don't understand interrupts, or multitasking. How quickly exactly, do you need it to happen? Microseconds? How far does your vehicle travel in a microsecond? A millisecond?

If you set up an interrupt, usually you have to poll it in loop() anyway because interrupts are disabled when an ISR is running, therefore you can not place many kinds of code there, in order to respond to the interrupt.

Generally, if you are polling an interrupt flag which is the only solution to that problem, the response is not faster than just polling a port.

...and yes, an interrupt configured input requires debouncing and there are special problems with that, which you won't encounter with non-interrupt code.

6

My problem is the whole program includes;
GPS, serial reads fro other arduinos, PIR, RFID, OLED SD card, RTC

I am being very careful using delays etc. but I still have a loop time that just doesnt allow the 500ms brakelight flash to function at 500ms so I hoped a simple interrupt would help and I wanted to understand it more anyway.

7

I believe I can do the debounce with a RC chain rather than software so would just like to understand why my ISR in post 3 isnt running even though it clearly attaches because the loop halts while the interrupt line is low.

8

Try by moving the millis() function from ISR routine into loop() function. To my understanding, the millis() function works on interrupt; whereas, the interrupt logic (global interrupt bit) remains disabled in the ISR routine.

9

The ISR should just set a global flag variable, possibly by storing the value of millis() in that global variable, and exit. Deal with the flag in the main loop.

10

kpg:
My problem is the whole program includes;
GPS, serial reads fro other arduinos, PIR, RFID, OLED SD card, RTC

I am being very careful using delays etc.

No, that is not your problem, because your carefulness with delay() didn't work, did it? I have many sketches that use many devices like that. In none of those, have I ever had any problem with a slow loop() time. It is the result of poor programming design 99.999% of the time. Instead of being defensive about it and hiding your code, please post it so you people can help you solve your real problem. Using interrupts will not help you avoid this stumbling block. If your loop() is slow, you won't be able to poll the ISR fast enough anyway, and you will have achieved nothing.

11

GolamMostafa:
To my understanding, the millis() function works on interrupt;

That is correct but there is nothing to prevent the code in an ISR reading the value of millis(). And, assuming the ISR is short, it will not interfere with the updating of millis().

...R

12

You declare:
const unsigned long BrakemillisPrev = 0;

But in the ISR you have:
BrakemillisPrev = BrakemillisNow;

Now it either is a const or not, if not then you should declare it as volatile.

int brakelightstat = LOW; should be declared as volatile as you change it in the ISR

13

I fixed the declare issue and it was actually a typo in the post rather than the sketch (sorry). I added a serial print to see what BrakemillisNow was doing and with the interrupt pin LOW, it doesnt increment so that explains why the flashing routine doesnt work (the ISR only runs once). Obviously moving the Millis function to the loop doesnt help because the loop is halted by the ISR.

The ISR halts the loop correctly when held low but the ISR only runs once as BrakemillisNow isnt updating.

This is on a Mega.

Declares are now..

unsigned long BrakemillisPrev = 0;
unsigned long BrakemillisNow;
const unsigned long Brakeperiod = 500;
volatile byte brakelightstat = LOW;
const byte FbrakeSw = 3;

setup has the interrupt initialisation in...

attachInterrupt(digitalPinToInterrupt(FbrakeSw), brakes, LOW);

and the code is;

void brakes(void) {
  BrakemillisNow = millis();
  Serial.println (BrakemillisNow);//this doesnt increment with the interrupt pin held low. Why??
 

  if (BrakemillisNow - BrakemillisPrev >= Brakeperiod) {
    BrakemillisPrev = BrakemillisNow;
    if (brakelightstat == LOW) {
      brakelightstat = HIGH;
    } else {
      brakelightstat = LOW;
    }
    digitalWrite (brakeLightout, brakelightstat);
    Serial.println (brakelightstat);
  }

  }
14

BrakemillisNow is being set in the ISR it should therefore be definend as volatile, similarly BrakemilliPrev.

15

Millis isn't incrementing presumably because the interrupt you are using for the button is higher priority than the timer interrupt used to increment millis().

Also using Serial from an interrupt is also a big no no (I appreciate you were only debugging).

Stop abusing interrupts to try and achieve this. It is entirely the wrong solution.

The correct solution (as you've already been told) is to address whatever code is preventing your loop() from executing any faster then 500ms per iteration.

16

countrypaul:
BrakemillisNow is being set in the ISR it should therefore be definend as volatile, similarly BrakemilliPrev.

Only if it's used by mainline code also. Presumably it isn't as the OP has rejected using loop() for anything brake light related.

17

kpg:
The ISR halts the loop correctly

You said that you wanted to understand interrupts. It is never correct for an ISR to halt the loop function. You have actually created an insoluble problem by refusing to do things the normal way. It isn't just, you can't make it work this way. It's, nobody can make it work this way. Not unless you can accept sluggish response times.

18

Thankyou and I do appreciate the loop is the main issue but I wanted to understand why Millis wasn't running properly when used in an ISR...

19

kpg:
Thankyou and I do appreciate the loop is the main issue but I wanted to understand why Millis wasn't running properly when used in an ISR...

Did you get a satisfactory answer to that? Anyway, it's technically interesting to look at something that you don't see very often and observe the possibilities... you can definitely debounce an interrupt driven event using millis() in an ISR. I've seen the code, perhaps I even wrote it... so much water under the bridge. Anyway, that is possible because although the value of millis() doesn't change during the ISR, it does change between calls to the ISR. So the ISR can capture and utilize the value of millis() from previous calls. We know that the ISR will be called regardless of what the main code is doing as long as interrupts are enabled. So the ISR can implement a timed lockout easily, which is the simplest and very effective method of debounce.

It is even possible for such an ISR to do some digital I/O or some other activity that does not require interrupts to work, and react swiftly because in that case the sluggish blocking main code isn't involved. So you could turn on or off some lights for example. The problems come in when you want to do something in the ISR that calls another interrupt handler. In that case, you have to rely on the main code which can only poll the ISR, therefore can not react instantly if it is sluggish. Usually the events that an ISR handles are roughly of an equal importance as other events that the program handles, and so smooth communication is desired between ISR and main program is vital. That is the corner you paint yourself into if you insist on having a slow loop. It leaves you with ISR that can not interact smoothly with the rest of the program.

20

pcbbc:
Only if it's used by mainline code also. Presumably it isn't as the OP has rejected using loop() for anything brake light related.

If that is the case, presumably he could avoid the global variables and just use statics.
Either way unlikley to make much difference to the curent problems.