ArduinoISP on the Leonardo does not work on windows

[PeterVH]

Versions:
Tried this on windows 7 SP1 and on XP SP2 (though on XP SP2, composite device stuff must be stripped to bring up leanardo all together).
The problem does not occur on linux.
Arduino 1.0.2

What steps will reproduce the problem?
From the IDE, perform following steps:

1. Upload the ArduinoISP sample to the leonardo.
2. Select ArduinoISP as programmer
3. In the preferences select verbose output during uploads.
4. Select an arbitrary board type. (no need to actually connect that target to the leonardo, we just want to see whether serial communictation between pc an leonardo works).
5. Perform "burn bootloader".

Expected output: avrdude should send some bytes to the leonardo and the latter should send some bytes back (this should of coarse result in a failed upload because no target is attached)

Actual output: Leonardo sends nothing back, TX led never lights up. (RX led does)

Analysis: It turns out that on windows when avrdude opens the serial port, neither DTR nor RTS are asserted. Under these conditions, the leonardo does not send out data, see CDC.cpp:

size_t Serial_::write(uint8_t c)
{
        /* only try to send bytes if the high-level CDC connection itself 
         is open (not just the pipe) - the OS should set lineState when the port
         is opened and clear lineState when the port is closed.
         bytes sent before the user opens the connection or after
         the connection is closed are lost - just like with a UART. */

        // TODO - ZE - check behavior on different OSes and test what happens if an
        // open connection isn't broken cleanly (cable is yanked out, host dies
        // or locks up, or host virtual serial port hangs) */
        if (_usbLineInfo.lineState > 0) {
                int r = USB_Send(CDC_TX,&c,1);
                if (r > 0) {
                        return r;
                } else {
                        setWriteError();
                        return 0;
                }
        }
        setWriteError();
        return 0;
}

The comment in the code makes sense but unfortunately windows/avrdude do not set RTS/DTR. Note that this does work on classic arduino's: on the duemilanove the ftdi chip apparently just forwards the data received from its uart on the usb. On the uno, the (LUFA) code in the atmega8u4, does not check RTS/DTR, but instead it checks whether the device is in the usb CONFIGURED state and whether the baudrate has been set (the latter is a bit odd, but maybe it is because of this problem). Myself I tried with no checks at all ( if ( 1 /_usbLineInfo.lineState > 0/) ). It works for ArduinoISP, but this may have side effects (hanging other sketches if serial port is not opened?).

A work around has been proposed to use ArduinoISP with the arduino protocol instead of the stk500v1 protocol. This works because the the arduino protocol's autoreset code briefly drops RTS/DTR and sets it back high. After this, the leonardo can send out data. (but this is making use of a side effect)

Not 100% sure what the best solution is. But I think that (unless there is a danger to lock up things), we should not prevent the leonardo from sending out data. The perception of the leonardo would benefit from this. (In the forum it is often heard "I gave up and bought an Uno and it worked out of the box". It is a pity the leonardo is great for ArduinoISP)

See also:
http://petervanhoyweghen.wordpress.com/2012/09/16/arduinoisp-on-the-leonardo/
http://arduino.cc/forum/index.php/topic,108270.msg1013258.html#msg1013258

[NicoHood]

As he describes on his page its now working if you change the reset to pin 10 and add this programmer. Why dont we add this fix to the IDE?

#define RESET SS
into:
#define RESET 10



arduinoispleo.name=Arduino as ISP (Leonardo)
arduinoispleo.communication=serial
arduinoispleo.protocol=arduino
arduinoispleo.speed=19200
arduinoispleo.program.protocol=arduino
arduinoispleo.program.speed=19200
arduinoispleo.program.tool=avrdude
arduinoispleo.program.extra_params=-P{serial.port} -b{program.speed}

[matthijskooijman]

I'm not sure that changing the protocol to arduino will help here, since that will break ArduinoISP for non-32u4 boards AFAIU.

Would it make sense to instead modify the CDC write method to send the data even if DTR is not asserted? If nobody is listening and that causes an error, then the error handling will kick in and the result is the same. If nobody is listening and no error is triggered, that is probably fine as well. If a sender is interested in not sending any data when nobody is listening, they can use the if(Serial) trick that is commonly used (but, as shown by this report, not fully accurate).

[PeterVH]

Would it make sense to instead modify the CDC write method to send the data even if DTR is not asserted?

I am also inclined to do that. But there is a consequence: if nobody is listening, bytes will accumulate in the buffer until it is full. After that the CDC write call will block! So after that bytes are not simply thrown away, the code just blocks.

That could be fixed if the CDC write would return an error if the buffer is full, but then when somebody starts listening it receives the eldest data stored in the buffer... Letting CDC write check on whether there is room in the buffer is a bad idea...

One could leave it to the sketch level to check using dataAvailableForWrite() before calling Serial.send()...

Basically the code in CDC write does the right thing, the problem is on the host side where neither windows nor avrdude assert DTR.

Note that on the Zero they already out commented the check for DTR. So fragmentation already started.

Also worth noting is that in the code for the u2 on an UNO, they do a similar check. Except that they check for the baud rate being set instead of checking for DTR. Theoretically this is wrong but I suspect they do this for the same reason of windows not correctly handling DTR.

I am thinking in the direction of doing something similar as on the u2, but that is of coarse a big change...

[matthijskooijman]

I am also inclined to do that. But there is a consequence: if nobody is listening, bytes will accumulate in the buffer until it is full.

What buffer is this? An file descriptor buffer on the PC? Or do things stick around on the Arduino side? If the latter, that sounds like something you could detect (not just the full buffer, but also if the buffer is being read regularly)?

[PeterVH]

I meant the hardware buffer in the USB controller on the atmega.

In mean time I looked a bit more close in the code. The write call is blocking, but not forever: there is a 250 millisec time budget to send out the complete (max 64 byte) packet.

My suggestion is to replace:

    if (_usbLineInfo.lineState > 0)       {

with:

    if (USBGetConfiguration())

So if nothing on the host is listening and you run this sketch...

   void loop( void )
   {
       delay(1000);

       digitalWrite(LED, HIGH);    
       delay( 100 );
       digitalWrite(LED, LOW);  

       Serial.println("hello");
    }

... it will happily keep on blinking every 1.25 sec or so. However if you replace the println with this:

    for(i=0; i<10; i++)
        Serial.println("hello");

... it will blink about ever 5 sec! (I don't completely understand the timing, but that is not really important right now....)

If this blocking behavior is not desirable, a sketch could use Serial.availableForWrite() before trying to send.

The check 'if USBGetConfiguration()' avoids unnecessarily blocking when the usb cable is not plugged. This is better than simply omitting the check like it is currently done on the zero.

This way of working seems common: the firmwares on the 16u2 on Uno and Mega don't check for DTR either, they only check for the usb device being in the configured state andfor the baud rate being set. This baudrate check only works once, after the usb cable is plugged in, so I don't see the point of it. The teensy 2.0 core also checks on the configuration (and also keeps track of whether there was previously a timeout. I don't fully understand this either... it alternates: block - don't block?).

I am now going to do my ArduinoISP tests on windows, with this fix in place...

[PeterVH]

Btw can I attach a pull request to this issue without having to create a new one?

[matthijskooijman]

Btw can I attach a pull request to this issue without having to create a new one?

No, but you can mention "Closes #1182" in the commit message to link them.

[NicoHood]

Havent tested this, but the idea sounds correct. If this also fixes the windows bug that'd be nice.