Interrupt ordering for 32u2 and 16u2 MCU

[M-Reimer]

This commit actually places a copy of your existing code for interrupt handling, but doesn't do the "mix up" you have there for "historic reasons".

The "non-mixup" code is used for the ATmega32u2 and ATmega16u2.

I didn't check every possible interrupt as the board, I'm using, doesn't expose every pin, so please have a short look over the code.

[NicoHood]

Please use:

defined(__AVR_AT90USB82__) || defined(__AVR_AT90USB162__) || defined(__AVR_ATmega32U2__) || defined(__AVR_ATmega16U2__) || defined(__AVR_ATmega8U2__)

To cover all chip variants. The rest looks correct to me.

[M-Reimer]

Done.

[NicoHood]

Could someone please merge this patch? It does not even affect the arduinos boards, as they do not use those MCUs.

[matthijskooijman]

The patch itself looks good to me. However, merging this does break compatibility, third-party cores that are already using these MCU types would break AFAICS (i.e. they would need to update their variants pin to interrupt mapping accordingly, and sketches that use hardcoded interrupt numbers also need to be changed). I'm not sure if such cores exist (I suspect @NicoHood maintains one, but other than that?).

Also, what is the compelling reason to change this? I agree that logically numbered interrupts are nicer (and we should have had them from the start on all boards, but that's not going to happen anymore). Is there anything that is actually broken because of this number shuffling?

[M-Reimer]

See also:
NicoHood/HoodLoader2#67 (comment)
One reason for fixing this in the core would be that not even more MCU's get the mixed up interrupt numbers. It would be more "technically correct".
Currently digitalPinToInterrupt(pin) does not work for the Atmega32u2 which is a MCU used in some chinese device I want to create custom (Arduino based) firmware for.
In the Arduino core the interrupts are mixed up but the HoodLoader2 header file expect them to be not mixed up. So currently I hard code interrupt numbers in my sketch.
So for me, as a user of the Arduino framework, it does not matter where it is fixed. Either in the core (not mixing interrupt numbers) or in HoodLoader2 (mixing the order there so it fits with core).

[NicoHood]

Yup, and I'd want that to get fixed in the Arduino code, rather than another workaround that is not obvious. Since I've implemented the 16u2/32u2 code, I'd call that more a bug, than a feature. So please fix it sooner than later.

[M-Reimer]

There at least has to be made some decision. It doesn't help if this Pull Request is open for another 7 months. If this Pull Request has no chance of being applied, then please close it with some explanation.

[matthijskooijman]

@NicoHood, ah, I see that your core currently does not do the interrupt-shuffle, so changing this in Arduino would fix your core rather than break it. Any idea if there are other cores out there for these MCUs?

There at least has to be made some decision. It doesn't help if this Pull Request is open for another 7 months. If this Pull Request has no chance of being applied, then please close it with some explanation.

I'll need to defer to the devs here, @cmaglie, @lxrobotics, @facchinm, any thoughts?

Personally, I would not mind merging this if we can't find any other cores that would be affected (or their maintainers are ok with changing this).