NicoHood / HoodLoader2

16u2 Bootloader to reprogram 16u2 + 328/2560 with Arduino IDE
http://www.nicohood.de
734 stars 186 forks source link

Interrupt handling not properly implemented #67

Closed M-Reimer closed 4 years ago

M-Reimer commented 5 years ago

Currently interrupt handling doesn't seem to be implemented at all. At least not properly.

In the Arduino core libraries, we reach at this line:

https://github.com/arduino/ArduinoCore-avr/blob/master/cores/arduino/WInterrupts.c#L293

This effectively causes the interrupts to be mixed up. "INT1" actually is reported as interrupt "3".

I guess the easiest way to fix this would be if you modify your digitalPinToInterrupt to also be "mixed up" the way the Arduino core libraries expect it.

If I completely ignore "digitalPinToInterrupt" and just "attachInterrupt" to "3", then I have proper interrupts from INT1.

NicoHood commented 5 years ago

WTF that sounds strange. I have never tested the interrupts, so I guess you are right. I think this should not be patched here, but rather in the arduino core. The line you linked, where they swap the interrupts, could you please add another #ifdef atmega16u2 etc so that for those MCUs the issue will be fixed? That sounds like a better solution to me.

M-Reimer commented 5 years ago

Do you think we actually have a chance to get this fixed in the "core" board files? My guess would be, that they only support "their" boards and there never was any "official" Arduino with an 16u2 or 32u2 Atmel controller. Fixing it in your files would be an "easy fix" as it doesn't require upstream review and as most users don't care about the interrupt numbers anyway.

NicoHood commented 5 years ago

They will fix it, they also integrated the 16u2 support. I had to patch some files in the core as well before. If the fix is correct, they will accept it. Give me the link and I will post a comment.

M-Reimer commented 5 years ago

https://github.com/arduino/ArduinoCore-avr/pull/66

I flashed your bootloader to some chinese "USB volume knob" device and for the rotary encoder, I use the interrupts. That's why I found out about this issue in the first place.

NicoHood commented 4 years ago

This can be closed now, correct?

M-Reimer commented 4 years ago

Once they release a new AVR core version. I would keep it open as the problem still exists for the average user.

M-Reimer commented 4 years ago

FYI: https://github.com/M-Reimer/IwitVolumeKnob

The fact that the interrupt now depends on the AVR core version forces me to add some "hack": https://github.com/M-Reimer/IwitVolumeKnob/blob/master/IwitVolumeKnob.cpp#L35-L60

NicoHood commented 4 years ago

Well, you initiated that fix :-D

M-Reimer commented 4 years ago

And you didn't want to mix in your code :-D