SpenceKonde / ATTinyCore

Arduino core for ATtiny 1634, 828, x313, x4, x41, x5, x61, x7 and x8
Other
1.57k stars 306 forks source link

attachInterrupt() on ATTinyx5 defunct? #410

Closed 5chufti closed 4 years ago

5chufti commented 4 years ago

Hi, I have various issues with attachInterrupt(). Given the simple example of setting a pinchange interrupt on PB1

void fISR() {
}

void setup() {
  attachInterrupt(PCINT1,fISR,CHANGE);
}

void loop() {
}

and looking at the .lst I see that the ISR is placed on the INT0 vector, not the PCINT0 vector as expected.

00000000 <__vectors>:
__vectors():
   0:   0e c0       rjmp    .+28        ; 0x1e <__ctors_end>
   2:   1f c0           rjmp    .+62        ; 0x42 <__vector_1>
   4:   1c c0           rjmp    .+56        ; 0x3e <__bad_interrupt>

If I go for a LOW on INT0

void fISR() {
}

void setup() {
  attachInterrupt(INT0,fISR,LOW);
}

void loop() {
}

I get exactly the same but it is still not working because the attachInterrupt does not generate relevant register settings. Only changing attachInterrupt(INT0,fISR,LOW); to attachInterrupt(0,fISR,LOW); does work. But this is a bit odd ... as INT0 is defined as 0?

Is it just me overlooking some essentials (noob alert!) or somesthing serious?

rgds, schufti

p.s.: if you need a "real life" example don't hesitate to ask... p.p.s.: configuring the registers manually and using ISR(PCINT0) instead of attachInterrupt() does work.

SpenceKonde commented 4 years ago

You are using incorrect syntax; per Arduino reference, the correct syntax for attachInterrupt() is:

attachInterrupt(digitalPinToInterrupt(pin), ISR, mode)

or you can use the interrupt number.

INT0 is NOT defined as 0 - the definition of INT0 comes from the part-specific io*.h file supplied by the manufacturer. For many parts, it does happen to be 0, which is why your code works on some devices, despite being incorrect. The ATtiny85 is not one of those parts.

PCINTn defines ALSO are supplied by these io*.h files... they are defined... to the number of the pin within the port so that so you can enable and disable PCINTn with things like PCMSK0|=1<<PCINT1;

As described in the official Arduino reference for attachInterrupt(), it is ONLY for INTn type interrupts, not for PCINTs. To use PCINTs, either use a third party library (not recommended - any implementation that supports this in a generic way pulls a lot of bloat into the ISR that runs calls your "ISR") or enable them by manually manipulating the appropriate registers (this is the recommended approach, as you don't need to make provisions for having PCINTs on multiple pins within a port unless you're actually doing that, so the ISR can be smaller, faster, and more efficient).

It may be that on some parts, PCINT1 just so happens to work (though I would be extremely surprised, and would suspect that under the hood it's not doing what you think).

https://www.arduino.cc/reference/en/language/functions/external-interrupts/attachinterrupt/

5chufti commented 4 years ago

Ok, thank you. So my assumption of "noob alert" was correct ;) Thank you for the clarification. P.S.: just for info to "searchers/finders": if pin-change-interrupts are needed one seems better off doing the activation manually and bind the function via ISR(int_vector)

SpenceKonde commented 4 years ago

Quite frankly, I'd do that for INTn interrupts too... but at least for that the overhead isn't too bad. I can't say I'm terribly impressed with the Arduino team's prowess at either embedded software architecture nor hardware design - their main achievement has been lowering the barrier to entry around an IDE for embedded development, and getting incredibly widespread uptake... Who would have thought that kids would be successful having their first introduction to programming with embedded C?! But Arduino works great in schools, while all the more "friendly" attempts have petered out pretty quick...

On the megaAVR 0-series, tinyAVR 0/1-series, and DA-series parts, I would not use attachInterrupt() at all either; it works on all pins there, but the overhead is pretty ugly, because it's basically PCINT there, only with the added feature that you can specify rising/falling/change/level like INTn interrupts on classics, but they're in banks, so the ISR ends up having to check for 8 possible sources and 8 possible functions to call, when one wants an ISR to be as small and fast as possible...