bugAVR Downloader/UploaDEr - Bugs: bug #34339, back to back avrdude commands fail...

 
 

You are not allowed to post comments on this tracker with your current authentification level.

bug #34339: back to back avrdude commands fail on dragon_isp on Ubuntu 10.10

Submitted by:  None
Submitted on:  Tue 20 Sep 2011 07:49:30 AM UTC  
 
Category: NoneSeverity: 3 - Normal
Priority: 5 - NormalItem Group: None
Status: NonePrivacy: Public
Assigned to: Joerg Wunsch <joerg_wunsch>Originator Name: Bill Perry
Originator Email: -unavailable-Open/Closed: Open

(Jump to the original submission Jump to the original submission)

Thu 08 Dec 2011 07:20:50 AM UTC, comment #10:

One more thing that just occurred to me:

Of course, adding anything like that does have side-effects for
other users. The obvious side-effect is that they have to wait much
longer until they are being told their device is not present at all,
either since the e.g. mistyped the programmer name, or they requested
a particular serial number that is not present in the system. A
three-second "dead time" can be annoying in that situation. Keep in
mind that specifying a serial number that is guaranteed to not exist
(like, -P usb:x) together with one -v option is a common way of
finding out all the serial numbers of the requested devices present on
the system. This (ab)use would also suffer from the delay.

A less obvious (but much more annoying) side-effect here is that it
will announce all the devices that are present in the system 30 times.
Thus, your 6-line hack will need to get more sophisticated to cope
with this, and to ensure each device that has been found will be
announced at most once.

Likewise, nobody can really tell three seconds is enough for
everybody.

I'm not sure I like the "goto" command there, an outer (do … while)
loop to count down the timeout would IMHO be more appropriate.

For those reasons, I'll probably add another commandline option to
activate the hack (like, -x usb_wait_time=3). Obviously, this
requires Arduino users to change their Makefiles then in order to
benefit from the hack, which poses the question why they could not
change their Makefiles for a single avrdude command in the first place

Also, your patch lacks documentation, something that's easily being
skipped by people submitting patches, and then left to the
maintainer(s). All this together (with testing etc.) makes it more a
5-hour job than the 5-minute one you think it were.

I hope now you see why I didn't quickly and blindly apply it the way
you submitted it.

Joerg Wunsch <joerg_wunsch>
Project AdministratorIn charge of this item.
Thu 08 Dec 2011 06:47:50 AM UTC, comment #9:

> I am not understanding the objection to this patch or
> at least using this methodology.


Well, it's a hack, and developers don't love hacks. Actually,
this is the "workaround", while a clean solution would be
anything not requiring this (like, issuing only one
avrdude command straight from the beginning, and have it
do everything).

That doesn't mean the idea is completely rejected.

Joerg Wunsch <joerg_wunsch>
Project AdministratorIn charge of this item.
Thu 08 Dec 2011 06:43:45 AM UTC, comment #8:

Btw., of course, there would be a much easier fix to solve
the Arduino problem: issue the bootloader and application
flash requests in a single avrdude command.

Advantages of this approach:

. works with any avrdude release already right now
. much faster than the avrdude with the hack applied, as
there's no need to wait for the re-enumeration process

Joerg Wunsch <joerg_wunsch>
Project AdministratorIn charge of this item.
Thu 08 Dec 2011 06:33:26 AM UTC, comment #7:

Regardless of why a USB reset is being done,
a USB reset is being done or a device disconnect is happening. Either way, the USB device is
having to re-enumerate which can cause usbdev_open() to fail.
avrdude can easily be updated
to acccount for this so that the usbdev_open()
routine "just works" so that external programs
and scripts using avrdude will no longer have to include work
arounds for this condition.

> The exact time required to see the device again is highly
> OS-dependant (it's the time the OS needs to walk through all
> of the enumeration process). It can be much less than the
> 3+ seconds you are claiming.


Maybe there was some misunderstanding what I meant
by "workarounds". What I intended to mean was that
if people are experiencing issues with the existing avrdude
that they can do either of the two mentioned work a rounds to
their external programs or scripts that call avrdude to be up and going again without having to update their
avrdude code.

Yes I agree that the re-enumaration can be less than 3
seconds. In fact on my linux machine with only one other
USB device it is just barely under 2 seconds for the AVR dragon to show up in libusb.
The mentioned 3 seconds is meant to be a very conservative
value that is especially useful for applications
or scripts that need to run on different platforms or in different environments.

It was a value that people could use that would "just work",
until avrdude is eventually updated to solve the issue.

===

The patch I provided allows avrdude to attach to the device as soon as the device is ready. There is no blind wait, it simply polls for a given amount of time by looking for the device every 1/10 of second
until it gives up after some amount of time.
It adds no additional time to usbdev_open() beyond what the
device needs. If the device is ready when usvdev_open() is called, ZERO additional time is added and it works exactly like it does today.

I am not understanding the objection to this patch or
at least using this methodology.

====================================
I've reported the issue, very clearly outlined the problem,
and even offered a simple 6 line patch to solve the problem.

There is not much else I can do.

Bill Perry <bperrybap>
Thu 08 Dec 2011 05:58:41 AM UTC, comment #6:

> This is an issue in avrdude (usb_libusb module) caused by avrdude
> resetting the USB when it closes certain USB devices.


No, this is wrong. It primarily happens since these devices
disconnect from and reconnect to the bus when saying good-bye
to them, and as such, it also happens on FreeBSD.

The additional reset by AVRDUDE is supposed to be required on
at least some Linux systems for a correct function, for
unknown reasons. (FreeBSD initially didn't have a user-
visible host-initiated reset in their USB stack at all,
and never experienced issues beyond those caused by the
devices disconnecting themeselves.)

The exact time required to see the device again is highly
OS-dependant (it's the time the OS needs to walk through all
of the enumeration process). It can be much less than the
3+ seconds you are claiming.

Joerg Wunsch <joerg_wunsch>
Project AdministratorIn charge of this item.
Thu 08 Dec 2011 12:34:17 AM UTC, comment #5:

One final comment just to provide the complete information
and background about this issue all in one place/post.

SUMMARY:
=============================================================

This is an issue in avrdude (usb_libusb module) caused by avrdude
resetting the USB when it closes certain USB devices.
It happened to show up when using the Arduino IDE or their makefiles
when attempting to burn a bootloader image
due to the way they use avrdude when burning a bootloader using the avr dragon.

The essential issue is that back to back avrdude commands can fail on certain USB devices.

OSs affected:
- All except BSD.

USB Devices affected:
Any device that calls usbdev_open()/usbdev_close() which currently is:
- JTAG ICE mkII
- AVR Dragon
- AVRISP mkII
- any device that uses USB on stk500v2 or stk600

USB Devices not affected:
- USBtiny
- USBASP

Workarounds:
- Delay 3+ seconds between back to back avrdude commands
- modify avrdude commands to collapse multiple commands into a single avrdude command.

======================================================
Additional detailed background

Whenever you do back to back avrdude commands, the second
avrdude command can fail on certain USB devices because the previous avrdude command resets the USB
and that requires re-enumerating the USB device.
If the second avrdude command runs before the USB device has fully enumerated,
when avrdude looks for the USB device, it will appear to not be there.

avrdude does an explicit usb reset on all OS's other than BSD
when it closes the USB connection.
(The comment in usbdev_close() in usb_libusb.c mentions the AVRISP mkII)
Given that avrdude is resetting the USB and creating this enumeration race condition,
avrdude should also properly handle dealing with the ramifications of
doing that USB bus reset.

The solution is quite simple: Poll for the USB device over a period of time. (a few seconds)
instead of just looking for the USB device just once.

avrdude is currently not taking advantage of libusb capabilities that it could
use to ensure that a USB device that is on the USB it resets during close is fully enumerated by attempting retries when looking for
the USB device at open.
As a result, when you do back to back avrdude commands on certain USB devices the second command will
fail if your USB device is not quickly re-enumerated.

This problem shows up on linux as well as Windows XP and probably other OSs as well.
It is very easy to replicate using the Dragon on either OS.

It is a trivial fix to avrdude to poll up to a few seconds when looking
for the USB device rather than just look once.
(It is only 6 lines of code)

With the small patch provided earlier,
if the USB device fully enumerated before the search for it was done, there is no added delay.
If it is not enumerated yet, the code will poll for it up to the timeout period
(patch used 3 seconds) until the device is enumerated.
If the device is not present, it will poll the full timeout period and then error off just like it does now.

The patch seemed like a nice easy solution to the problem since it
uses the existing code to handle everything and is not specific to the AVR Dragon.
It simply adds a level of robustness to the USB device connection code by polling
for the device rather than only looking for it a single time.

Devices like USBtiny and USBASP don't experience this issue because they don't
call the usbdev_open() or usbdev_close() functions in usb_libusb.c
They have their own versions of the same routines that they use, so that
when
the USB connection is closed the USB bus is not reset.
While they don't poll for the USB device in their open routines, because
they don't reset the USB
in their close routine, those devices don't have to re-enumerate and the
problem is avoided.

Bill Perry <bperrybap>
Sun 04 Dec 2011 10:33:17 PM UTC, comment #4:

Any update on this?
Patch was supplied. It is a mere 6 lines.

avrdude just needs to be a little smarter about how it uses libusb so it can retry looking for the desired USB device to account for devices still enumerating during back to back avrdude command executions.

BTW, this avrdude patch has now missed the official Arduino 1.0 release.

It is ironic that without this patch it is not possible to burn a bootloader using the Arduino IDE or Arduino bootloader Makefiles
since the AVR dragon is made by Atmel and Arduino is the most popular software environment for the Atmel AVR.

Granted the Arduino IDE and the Arduino bootloader Makefiles could work around this issue, but it really should be solved in avrdude for any/all USB devices since the Dragon is not necessarily the only USB device that can experience this issue.

Even if the Arduino guys were to work around this issue,
it doesn't solve the problem for non Arduino users of the dragon.

What can I do to help move this along?

--- bill

Bill Perry <bperrybap>
Tue 20 Sep 2011 08:15:18 PM UTC, comment #3:

My previous patch was wrong.
(working with too many svn trees)
Here is the proper patch.

--- bill

(file #24003)

Bill Perry <bperrybap>
Tue 20 Sep 2011 08:06:30 PM UTC, comment #2:

There is already other code in avrdude that tries to work around things that are outside the control of avrdude.
In fact the usb_reset() that is already there is one such attempt. There are other things like trying to resync communication with the device. Some of those can be due to the device not being ready or data lost in the OS or device due to buffer overruns which are outside the control of avrdude.

From a user perspective they don't care where the problem is, They just want things to "just work".

I do agree with you that there should not be any added delay for the normal/working path. My philosophy always is that normal paths should be as fast as possible but the handling of errors should be allowed to take extra time especially if there is the possibility to recover from them.

It is also true that the is no way to guarantee that any
delay picked will work 100% of the time.

That said, I think it is worth trying to come up with
a simple retry mechanism.

So I offer a tiny/simple patch that will do retries looking for the device if no device is discovered.
It delays for short period of time (1/10th of a second) and then does retries to wait a maximum of 3 seconds.
(polls for device every 1/10 of a second)

There is no change in behavior or timing when things are connected and working normally.

With Ubuntu 10.10 the device on the first command is found "instantly" and on the second command is found just a little after 1 second.

While there is no way to every guarantee that the maximum time polling time picked will be enough, waiting a few seconds (3, 4, 5 ....) will more than likely work in the vast majority of cases.

--- bill

(file #24002)

Bill Perry <bperrybap>
Tue 20 Sep 2011 09:52:35 AM UTC, comment #1:

This is not an AVRDUDE issue but an operating system one. It
is completely independent of whether there's a usb_reset()
being done; even on FreeBSD, you'll observe the same behaviour.
It's due to the programming tool itself disconnecting from the
bus, and reconnecting, so your OS'es entire enumeration
process has to be passed, before you can use it again.

Adding any kind of delay inside AVRDUDE would simply annoy
those who don't see their shell prompt back again after
programming, for no obvious reason. Also, any delay value
could be too low or too high, and your OS'es delay value can
be completely wrong for the next OS, or even the next version
of your OS. This is nothing but speculation. I'm not sure
whether retrying the USB open will really work well enough
in all situations. I probably wouldn't resist applying a
well-tested patch for that if it proves to work on all
supported operating systems (Linux, Win32, FreeBSD, MacOS X,
maybe Solaris though I'm not sure about the state of USB
support there).

As it's not an AVRDUDE issue, it should be solved outside, in
your case probably in the Arduino IDE itself. It's responsible
for adding a sufficient delay between two calls.

Another option for Arduino would be that they use the AVRDUDE
library rather than the command, within a single AVRDUDE
programming session, so no USB reconnect is needed. This
library is currently not being installed into the target system,
simply because it's not clear whether the library/UI
separation works well enough for a real application, so this
would require a close cooperation of the Arduino and the
AVRDUDE developers.

Joerg Wunsch <joerg_wunsch>
Project AdministratorIn charge of this item.
Tue 20 Sep 2011 07:49:30 AM UTC, original submission:

While I'm seeing this on Ubuntu 10.10 with the AVR dragon I'm guessing that this
os and this device may not be the only one to experience this problem.

If you issue back to back avrdude commands when using dragon_isp the second command will fail unless a delay is done between the commends.

This shows up when applications like the Arduino IDE
use 2 avrdude commands to burn a bootloader. 1 to set the fuses and 1 to burn the actual bootloader. Since they are back to back, the second avrdude fails.

The problem is that the usbdev_close() function in usb_libusb.c is reseting the USB device by calling usb_reset() an all OSs but FreeBSD. This forces the dragon (or any USB device) to have to re-enumerate which takes time. If the next open attempt is done before the enumeration is finished, the dragon will not be seen and you get an error:

avrdude: usbdev_open(): did not find any USB device "usb"

Through experimentation 2 seconds seems to be enough delay
to allow the enumeration to complete at least in my environment.

But in looking at the comment above the usb_reset() call it appears
that usb_reset() was added to resolve an issue with the AVRISP mkII on the second time around (probably also back to back commands).

If the usb_reset() is omitted the dragon_isp will also fail
on back to back commands but in a different way:

avrdude: usb_open(): cannot read serial number "error sending control message: Cannot send after transport endpoint shutdown"
avrdude: usb_open(): cannot read product name "error sending control message: No such device"
avrdude: usbdev_open(): error setting configuration 1: could not set config 1: No such device
avrdude: usbdev_open(): did not find any USB device "usb"

However, again if you delay for 2 seconds the problem goes away.
So at least for the dragon you cannot attempt to talk to the dragon for a few seconds after you terminate a connection to it.

This could be fixed in several ways.
A few possibilities:
- have usbdev_open() do a retry internally a few seconds later if no devices were found. (no changes to any other code)
- add a dumb/blind sleep in usbdev_close() after the usb_reset()/usb_close() to slow down back to back commands.

For now, because I'm lazy, I've patched my version of 5.11.1 to have a blind/dumb sleep(2) after usb_close() in the function usbdev_close()

BTW, according to the libusb API it is not valid to call usb_close() after usb_reset() so for now, I've also taken a short cut
by simply commenting out the usb_reset() which does not
seem to be needed for the Dragon on Ubuntu 10.10 anyway.

--- bill

Anonymous

 

Attached Files
file #24003:  usbdev_open_retry.diff added by bperrybap (718B - text/x-patch)
file #24002:  usbdev_open_retry.diff added by bperrybap (622B - text/x-patch - patch to poll for usb device every 1/10 second for 3 seconds in usbdev_open())

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by bperrybap (Updated the item)
  • -unavailable- added by joerg_wunsch (Posted a comment)
  • -unavailable- added by None (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 06 Oct 2011 09:00:31 AM UTCjoerg_wunschStatusWont Fix=>None
    Tue 20 Sep 2011 08:15:18 PM UTCbperrybapAttached File-=>Added usbdev_open_retry.diff, #24003
    Tue 20 Sep 2011 08:06:30 PM UTCbperrybapAttached File-=>Added usbdev_open_retry.diff, #24002
    Tue 20 Sep 2011 09:52:35 AM UTCjoerg_wunschStatusNone=>Wont Fix
      Assigned toNone=>joerg_wunsch

    Back to the top


    Powered by Savane 3.1-cleanup