Collision:__vector_1 in WInterrupts.c, USBAVR, v13

I have a problem: I've designed a serial-to-USB-HID shield for old serial gaming devices, which works well enough that I made several and sent them to people. Unfortunately in the interim some folks downloaded the Arduino environment 0013, which changed something in the way interrupts were handled and coded, so that while 0012 users have no trouble, 0013 users (which include some who had their boards silently upgraded to the 328 and can't use 0012) get this:

C:\(...)\WInterrupts.c.o: In function `__vector_1`:

C:\bin\arduino-0013\hardware\cores\arduino/WInterrupts.c:81: multiple definition of `__vector_1`

hardware\libraries\SpaceOrb\usbdrvasm.o:(.text+0x032): first defined here

The problem seems to be that the USBAVR code has an "ORG INT0_vect" line in it which binds to _VECTOR(1) which is defined as __vector_1, and there's a name collision. Looking at WInterrupts.c, it looks like the Arduino environment binds a couple of functions to INT0_vect and INT1_vect which do nothing unless an interrupt handler is set (if I understand this right). I can get the code to compile and run by going into WInterrupts.c and commenting out the following:

/* SIGNAL(INT0_vect) { */
/*   if(intFunc[EXTERNAL_INT_0]) */
/*     intFunc[EXTERNAL_INT_0](); */
/* } */

...but I'm not crazy about modifying a "system library". Is there any better way to do this? My code doesn't set up any of its own interrupts--that's all done in the usbavr code, and I'm a little anxious about modifying assembly.

The usbavr code does have (in usbdrvasm.S) :

#   ifndef USB_INTR_VECTOR
        ORG     INT0_vect
#   else /* USB_INTR_VECTOR */
        ORG     USB_INTR_VECTOR
#       undef   USB_INTR_VECTOR
#   endif /* USB_INTR_VECTOR */
#   define  USB_INTR_VECTOR usbInterruptHandler

...which implies that you could change USB_INTR_VECTOR somehow, but it's not obvious what the right thing to do is.

Is there a clean fix? Really frustrating my users who bought hardware and can't get it to work. I'm asking them to modify WInterrupts.c for the moment, but I'm not crazy about that.

Oh, that's a pain. I hadn't got as far as discovering it had been broken for my Arduino AVRUSB stuff too. :frowning:

This seems to be the revision where the break occurs.

There is AVRUSB wiki documentation for using Other interrupts than INT0 for USB but I don't think that's what we want to look at.

A couple of thoughts:- What confuses me a bit is that the change doesn't really change the existing behaviour which makes me wonder why it worked previously. i.e. as I understand it, the name of the interrupt vector changed but the actual vector itself didn't.

  • The use of SIGNAL() is actually (somewhat) deprecated in favour of ISR anyway.
  • AFAICT there's no reason why the current shouldn't work because there isn't actually a clash of actual interrupt routines by default. To me this suggests the core code should be modified in such a way that it only defines the routine if it doesn't already exist. Or maybe that's inconsistent and we need to work out how to attach the the AVRUSB interrupt using the standard Arduino approach.
    I suspect we should probably move this discussion to the developer list.

--Phil.

P.S. Given the number of issues with third party libraries with each release I think we really need to look at developing a testing infrastructure that includes the libraries. Even if it's only compilation testing and not functionality testing.

Update: Hmmm, as I think of it further I guess maybe this really has to be handled specifically for AVRUSB rather than expecting a change in core. But if we do this then the AVRUSB stuff may not even work anymore due to the overhead of the function call in the Arduino implementation. And we can't change the interrupt vector at runtime because the table is apparently in flash.

I still end up wondering how this actually worked before if there really was a clash of interrupt service routines.

as I think of it further I guess maybe this really has to be handled specifically for AVRUSB

Yeah, not sure. It's not trivial; I don't think it's cosmic, but it's not trivial and it'd be nice to get it right rather than just hacking away (although the hack works for my end users, which solves my immediate problem). I understand the approach of "modify the library, not the core", but on the other hand if the core means that no add-on library can define and use interrupt vectors without linking through the Arduino library, I'm not crazy about that either. There must be a middle ground, even if it's a "#define USE_EXTERNAL_INTVECTORS" or something cheesy like that.

if we do this then the AVRUSB stuff may not even work anymore due to the overhead of the function call in the Arduino implementation.

I'd be surprised if it was THAT sensitive, but it might be. I'd like the option to turn off the Arduino's code at compile-time, though--thus the #define idea. But I bet it would work using the Arduino infrastructure.