MEGA External Interrupt question

Hi,
Here is a basic Mega External Interrupt INT0 program.
A 10 Hz square wave signal is applied at pin D2 (INT0).
For unknown (to me) reason it does not work as expected.
Can anyone help and make it work?

volatile uint8_t toggle;
void setup() {
cli(); //disable interrupts
EICRA = 0x01; //Interrupt INT0 on change
EIMSK = 0x01; //Enable INT0
sei(); //Enable interrupts,

Serial.begin(115200);
toggle = 0;
}

void loop()
{}

ISR(INT0_vect){
while (toggle < 10)
{
Serial.print(“toggle=”); Serial.println(toggle);
toggle++;
}
}

Serial print doesn't belongin an interrupt routine.

A while loop with 10 Serial prints definitely doesn't belong.

Serial is driven by interrupts that are turned off during your ISR.

What were you expecting to happen?

MorganS:
What were you expecting to happen?

And how is it different than what actually happened?

Gentlemen,
The code I placed is a test, a simplified snippet of a much larger program, and should work independently of any other code.
I realize and know that printing, looping or time commands should normally not be inside the ISR.
The fact is that if I replace the low-level registers commands with the interrupt lib command "attachInterrupt(digitalPinToInterrupt(2), isr, CHANGE);" and change the ISR name from ISR(INT0_vect) to isr, it works perfectly.
My question was actually 'what is wrong with the low-level command set ".
The output should be (and is when using the digitalPinToInterrupt command):
toggle=1
toggle=2
toggle=3
toggle=4
toggle=5
toggle=6
toggle=7
toggle=8
toggle=9

samtal:
The fact is that if I replace the low-level registers commands with the interrupt lib command "attachInterrupt(digitalPinToInterrupt(2), isr, CHANGE);" and change the ISR name from ISR(INT0_vect) to isr, it works perfectly. My question was actually 'what is wrong with the low-level command set ".

How, exactly, was one supposed to determine that was your problem by reading your original post?

One was not supposed to.
One was just supposed to answer the original question as asked, which none has so far done.

Well if you’re thinking that you are going to give us the minimal amount of information and expect us to solve your problem then you’re going to end up on your own very quickly. Nobody is here to work through puzzles. If you want help then try to make it as easy as possible to help you by providing as much information as you can.

You also might consider reading the "How to use this forum" thread and following the rules for posting code. Thumbing your nose at our forum etiquette isn't going to make you any friends either.

samtal:
One was just supposed to answer the original question as asked, which none has so far done.

What was the question?

As I wrote, I posted a code that works perfectly when using the high-level command "attachInterrupt(digitalPinToInterrupt(2), isr, CHANGE);"

For timing reasons and testing, I changed the high-level command into low-level commands (registers manipulation), as I have done in the past, but for some reason, it does not work that way.
The question was:
"For unknown (to me) reason it does not work as expected.
Can anyone help and make it work?"

I suppose everyone was willing to help, but some answers did not relate to the question or were wrong.

"Thumbing your nose at our forum"? OUR form? whos form is that?

Ok. Thumbing your nose at the forum rules then. Either way, you'd probably be getting better help if you weren't ignoring them. You seem to have completely missed the point of what I said and decided to focus on the least important word in the sentence. If you're really just more interested in being abrasive than finding your solution then I'm out. I'll let you see how that goes for you.

Change INT0_vect to INT4_vect.

The interrupt number given to attachInterrupt() and INTn do not match on Mega.

    pin             INT[i]n[/i]    interrupt number for attachInterrupt()
                
    3       PE5     INT5    1
    2       PE4     INT4    0
    18 TX1  PD3     INT3    5
    19 RX1  PD2     INT2    4
    20 SDA  PD1     INT1    3
    21 SCL  PD0     INT0    2

    (nc)    PE6     INT6    -     not connected in 
    (nc)    PE7     INT7    -       Arduino Mega

oqibidipo:
Change INT0_vect to INT4_vect.

And use different registers/bits.

volatile uint8_t toggle;

void setup() {
  Serial.begin(250000);
  pinMode(2, OUTPUT);

  EIMSK &= ~0x10;   //disable INT4
  EICRB  &= ~3;
  EICRB |= 1;       //Interrupt INT4 on change
  EIFR = 0x10;      //clear pending INT4
  EIMSK |= 0x10;    //Enable INT4
 
  showRegs(F("regs"));
}

void loop() {
  if (toggle) {
    toggle = 0;
    Serial.println(F("IRQ"));
  }
  static unsigned long lastToggle;
  if (millis() - lastToggle >= 1000) {
    lastToggle += 1000;
    digitalWrite(2, !digitalRead(2));
  }
}

ISR(INT4_vect) {
  toggle++;
}


void showRegs(const __FlashStringHelper* when) {
  Serial.println(when);
  Serial.print(F("EICRA = 0x"));
  Serial.print(EICRA, HEX);
  Serial.print(F(", EICRB = 0x"));
  Serial.print(EICRB, HEX);
  Serial.print(F(", EIMSK = 0x"));
  Serial.println(EIMSK, HEX);
}

void showHex(byte value) {
  if (value < 16) {
    Serial.write('0');
  }
  Serial.print(value, HEX);
}
regs
EICRA = 0x0, EICRB = 0x1, EIMSK = 0x10
IRQ
IRQ
IRQ
IRQ
IRQ

Thanks Whandall for a clean, correct and professional reply (unlike some others' philosophical replies...)
I corrected the vector and regs and got it immediately to work perfectly.

One important comment:
As I wrote, I moved from the high-level command to the low level due to significant delays.
The low level does not seem to suffer from the delays, which actually means that the high level should not be used in time-critical applications (and mine is not really critical, in the mSec level!).

Another comment: I wonder why in the Arduino references I have found, Arduino Mega pin D2 is INT0.
This table is from the Arduino reference pages: (which pin is the actual INT0?)

arduino reference

and:
https://arduino-info.wikispaces.com/MegaQuickRef

samtal:
One important comment:
As I wrote, I moved from the high-level command to the low level due to significant delays.
The low level does not seem to suffer from the delays, which actually means that the high level should not be used in time-critical applications (and mine is not really critical, in the mSec level!).

I doubt that.

Please construct a small program that prooves the "significant delay".

There is only a very small overhead from the additional indirect call.

I can not see a significant delay.

Measured from rising edge on D2 to rising edge of D3.

Registers and ISR 8.92 µs
BareMetal_Attach1.png

AttachInterrupt 9.42 µs
BareMetal_Bare.png

The test code

volatile uint8_t toggle;

void setup() {
  Serial.begin(250000);
  pinMode(2, OUTPUT);
  pinMode(3, OUTPUT);

//  EIMSK &= ~0x10;   //disable INT4
//  EICRB  &= ~3;
//  EICRB |= 1;       //Interrupt INT4 on change
//  EIFR = 0x10;      //clear pending INT4
//  EIMSK |= 0x10;    //Enable INT4

  attachInterrupt(0, toggleIRQ, CHANGE);
  showRegs(F("regs"));
}

void loop() {
  if (toggle) {
    toggle = 0;
    Serial.println(F("IRQ"));
  }
  static unsigned long lastToggle;
  if (millis() - lastToggle >= 1000) {
    lastToggle += 1000;
    digitalWrite(2, !digitalRead(2));
  }
}

void toggleIRQ() {
//ISR(INT4_vect) {
  digitalWrite(3, HIGH);
  digitalWrite(3, LOW);
  toggle++;
}

void showRegs(const __FlashStringHelper* when) {
  Serial.println(when);
  Serial.print(F("EICRA = 0x"));
  Serial.print(EICRA, HEX);
  Serial.print(F(", EICRB = 0x"));
  Serial.print(EICRB, HEX);
  Serial.print(F(", EIMSK = 0x"));
  Serial.println(EIMSK, HEX);
}

void showHex(byte value) {
  if (value < 16) {
    Serial.write('0');
  }
  Serial.print(value, HEX);
}

There is a small penalty for using the API.

Attach 3776 Bytes Flash, 209 Bytes RAM
Bare   3146 Bytes Flash, 193 Bytes RAM

samtal:
Another comment: I wonder why in the Arduino references I have found, Arduino Mega pin D2 is INT0.
This table is from the Arduino reference pages:

arduino reference

The reference page does say that the numbers don't always match:

Note that in the table below, the interrupt numbers refer to the number to be passed to attachInterrupt(). For historical reasons, this numbering does not always correspond directly to the interrupt numbering on the atmega chip (e.g. int.0 corresponds to INT4 on the Atmega2560 chip).

(which pin is the actual INT0?)

I already told you that:

oqibidipo:

    pin             INTn    interrupt number for attachInterrupt()

...
    21 SCL  PD0    INT0    2
[/quote]

Whandall you are right.
Your test clearly shows approx. 0.5 micros overhead of the app, which for my application is negligible.
I tested with my small test program, and indeed it runs well after changing the INT pin numbers and fixing the registers.
Nevertheless, in my large program (over 2K lines) the difference between the low-level and the attachInterrupt still exists. I will probably have to look elsewhere, but for now, my problem is solved.
(I never run any time-consuming lines inside the ISR, and normally I disable the interrupt as soon as I enter the ISR).
Thank you again for your help and effort. It really helped me out.
I wish the Arduino reference team will correct the interrupt pin numbers.

samtal:
and normally I disable the interrupt as soon as I enter the ISR.

which is an error, because the interrupts are disabled on entering the ISR anyway.
Same holds true for the matching enabling of interrupts at the end of your ISR probably,
which only opens a window for unintended interrupt nesting.

If you have a bigger delay in your fat program, it could be that you disable somewhere interrupts for too long,
like using FastLed on a long string of ws2812b.

samtal:
I wish the Arduino reference team will correct the interrupt pin numbers.

A note that the interrupt numbers used for attachInterrupt are not the numbers Atmel uses would be nice.

P.S. A cheap logic analyzer helps a lot and it is really cheap, €3.92 with free shipping to Germany.