Critical CDC Bootloader USB HUB bug

[NicoHood]

This issue has nothing to do with issue #42.

The story:
I am developing a 16u2 CDC Bootloader variation and I got a very weird error. I tried with my arduino leonardo now. There is no such an error (with the official bootloader). Then I recompiled the CDC Bootloader from lufa and the bug appeared.

The bug:
Use a USB3 port on your pc (tried with two pcs) and connect a usb 2.0 hub to it(usb 3 wont cause the bug). plug in the arduino with the cdc bootloader and try to upload a sketch via the ide. The verification will fail. Now do the same and connect a second device to the usb port like a mouse. For some reason the uploading now doesnt fail.

The sketch will upload anyways but the verification wont fail. I tried with 2 different pcs and different os. (win7, win8, ubuntu). I tried 4 different usb 2 hubs (usb 3 works). I did not try usb 2 pc host ports because i only have usb3 ports. I tried different drivers for windows (shouldnt matter cause linux has this bug too).

There are also different errors like a "dead" usb port until you plug in/out another device on the hub. As long as you dont do this, the cdc bootloader usb port will be dead, you can replug it as long as you want.

It seems that the old CDC bootloader from the Arduino team (with older lufa) has not this problem. I am now searching for the error. If it is the lufa version or the code of the bootloader.

Fuses used (tested with 16u2, 32u2 and 32u4):

#fuses (HoodLoader2 only, not suitable for DFU)
HoodLoader2atmega16u2.bootloader.low_fuses=0xFF
HoodLoader2atmega16u2.bootloader.high_fuses=0xD8
HoodLoader2atmega16u2.bootloader.extended_fuses=0xCB (0xFC for 32u4)
HoodLoader2atmega16u2.bootloader.unlock_bits=0x3F
HoodLoader2atmega16u2.bootloader.lock_bits=0x2F

To recreate this you only need an Arduino Uno/Mega (16u2) board or a leonardo/(pro)micro(32u4) board and test one of the versions. To upload sketches to the 16u2 you need my hid project and hoodloader2 project on github. It'd be easier to test the 32u4 though.

edit: @dean: any idea where the bug could be located, where i have to start searching?

[NicoHood]

Bug is still in 130303

I cannot compile any earlier version I always get:
BootloaderCDC.c:476:68: error: invalid suffix "x" on integer constant
or the same message with "xUL".
I tried to fix it with some trail & error but couldnt find the bug in the makefile. Used avr-gcc 4.7.2 on raspberry pi.

Compiling the older versions with a newer lufa also throws errors of course.

Edit: could compile the LUFA-111009, modified the makefile:

BOOT_START           = 0x0700

But no Serial port shows up. Same for the Pro Micro source. Leonardo Source still doesnt want to compile. The weird thing also is that the precompiled pro micro source works just fine. I just have a newer compiler i think.

[NicoHood]

I managed to compile the LUFA-111009\Bootloaders\CDC for the 32u4. The sketch is not starting, probably because of the magic key or something else (have not looked into that yet). But it is uploading BUT only with a 2nd device on the USB hub.

It seems that the Arduino Team did something different. Or used another compiler. I used the old WINAVR now (to also use an older compiler). The problem right now is that I am still not able to recompile the official bootloader, due to the UL bug. Will look into that now.

Edit:
The leonardo bootloader also takes longer to flash. Not sure if they included a sketch in the hex file as well.

edit2: now able to recompile leonardo source. :) (had to uncomment the PID and VID in the makefile)

[NicoHood]

It is the timeout. They use a timeout after the bootloader reprogrammed successful. This timeout gives us a bit more time to send the verification back to the host. For some reason there are problems with usb 3 hosts and usb2 hubs connected to it.

So the solution is to add something similar with a timeout.
Luckly it is not the wtachdog reset (they use an application jump which causes several other errors in the arduino program afterwards (usb clock is still running)).
And luckly it is not a compiler problem it seems.

The only thing i am wondering about now is how to add a timeout which works on every MCU. I will add compatibility to my HoodLoader2 bootloader, but this is only specific for the u2 Series. Not sure if this will work on your bootloader as well. Also I have very few flash space left, so I'll probably have to think of something very smart.

I'll stop talking to myself now and work on a solution for my bootloader now. I'll post results here and you can tell me which way you want to go for your API which should work on more than one MCU. If you have any suggestions, let me know.

[NicoHood]

Is this of any importance?
https://github.com/arduino/Arduino/blob/master/hardware/arduino/avr/bootloaders/caterina/Caterina.c#L493

Here is where they fixed the bug:
https://github.com/arduino/Arduino/blob/master/hardware/arduino/avr/bootloaders/caterina/Caterina.c#L489

Edit: any could you also explain my why you use this instead of a simple jump?
https://github.com/abcminiuser/lufa/blob/master/Bootloaders/CDC/BootloaderCDC.c#L135

[NicoHood]

This is my current Test. It turns out that it waits 0,3S till the Bootloader exists (remember Arduino waits 0,5s). I used uint16_t here to get a longer delay. uint8_t only had a few ms delay (but it still worked though).

So I am not sure how long we have to wait after a reprogramming. As I said uint8_t (256 attempts) will be enough to fix it. The problem is that different PCs and OS behave different, so we might need a longer delay. Also I dont thing that waiting 0,3s is that long (remember Arduino use 500ms). However the time might vary on different systems. Meaning it could take forever to practise that many USB calls maybe. Dean, you must know if the time of the task can vary.

So should we use a counter here or a real TIMEout? A counter would ensure that every PC gets the same amount of attempts. A Timeout would ensure the time it waits is always the same. However it needs some more flash to program this and include it inside the timer. Since you are already using the timer both ways would be easy.

What do you say? Timeout or count Timeout?

Edit: and this is my fix in the project (a mix of both)
https://github.com/NicoHood/HoodLoader2/blob/dev/avr/bootloaders/HoodLoader2/HoodLoader2.c#L184-L217

This is another suggestion:

int main(void)
{
    /* Setup hardware required for the bootloader */
    SetupHardware();

    /* Enable global interrupts so that the USB stack can function */
    GlobalInterruptEnable();
    DDRB |= (1 << 1);
    PORTB &= ~(1 << 1);
    while (true)
    {
        CDC_Task();
        USB_USBTask();

        // check Leds (this methode takes less flash than an ISR)
        if (TIFR0 & (1 << TOV0)){
            // reset the timer
            TIFR0 |= (1 << TOV0);

            // Turn off TX LED(s) once the TX pulse period has elapsed
            if (TxLEDPulse && !(--TxLEDPulse))
                LEDs_TurnOffLEDs(LEDMASK_TX);

            // Turn off RX LED(s) once the RX pulse period has elapsed
            if (RxLEDPulse && !(--RxLEDPulse))
                LEDs_TurnOffLEDs(LEDMASK_RX);
        }

        // break if programming has finished
        if (!RunBootloader){
            PORTB |= (1 << 1);
            /* We nearly run out the bootloader timeout clock,
            * leaving just a few hundred milliseconds so the
            * bootloder has time to respond and service any
            * subsequent requests */
            static uint16_t timeout = 0;
            timeout++;
            if (!timeout)
                break;
        }

    }
    PORTB &= ~(1 << 1);

    /* Disconnect from the host - USB interface will be reset later along with the AVR */
    USB_Detach();

    /* Enable the watchdog and force a timeout to reset the AVR */
    // this is the simplest solution since it will clear all the hardware setups
    wdt_enable(WDTO_250MS);

    for (;;);
}

[dean]

@NicoHood - Sorry, don't know where the bug is or where to start searching. :)

[NicoHood]

Hu? I posted a solution! Didnt you see this? You have to wait a bit more after the programming finished to complete the last USB tasks. For some reason this is needed for usb 2 hub with a single device on usb 3 hosts (usb2 host not tested).

All you need to do is add a bit of timeout. But you arent the "real" dean who made lufa or did you change name and face? ;)

[abcminiuser]

Different guy - I checked :-). I've been keeping an eye on this, will look into it on the weekend and make a patch.

[NicoHood]

Okay cool. I dont know if the task only needs to run 1-3 more times or maybe a longer time spawn. I dont know what the Task itself does and how to fix this for all PCs. Since I had other bugs while developing USB devices where my good PC just worked fine and an old laptop crashed.

It would be cool if you could share the information if you have got a fix. Right now I do a mix of 256 attempts and a timeout. Maybe thats just way to much, but I want to ensure also slower pcs work. I cannot test that, because I dont own slow PCs anymore :D It would be nice to find the real cause of this issue.

Also have a look at the PR I opened and also check DFU bootloaders. Never had problems but maybe this could apply here too ;)

edit2: even 100uS is enough. But i'll use 1ms to ensure it works

[NicoHood]

It seems that a very simple _delay_ms(1); between the loop and the USB_Detach() solves the problem. Maybe the device disconnects too fast and the hub/controller throws an error. Not sure if thats enough on other pcs but it works for me. Not sure if you can even reproduce the problem. But this solution is fairly simple and doesnt add much flash.

Edit: I am not sure of the data isnt flushed out till then or simply if the detach causes an USB line state change which is recognized as wrong byte. So i am not sure if the delay lets the device flush the data or if the delay simply idles the line. Maybe you need to add a flush in the detach function? You probably know more about this.

[dean]

No, I'm not haha. Which is why using @dean pings me instead of him.