[patch #7559] 'arduino' programmer type DTR transition fix

[avrs-admin]

Mon 20 Jun 2011 06:19:17 PM UTC

It came to my attention that I could only upload every 2nd try to my bootloaded AVR chip (or 'Arduino' if you like). The first pass would fail and I would abort avrdude (the chip was not reset). With the old code this left the DTR line in a usable state for the next run, which then succeeded reproducibly.

This patch reverses the sequence of setting/clearing DTR/RTS. In depth explanation including a small schematic are in the comment in the .diff file.

file #23541: arduino.c.patch file #23660: rev-logic.patch file #23662: g3215.png file #23663: 0003-Added-serial_toggle_dtr_rtsjusttesting.patch file #23669: test_dtr.c

This issue was migrated from https://savannah.nongnu.org/patch/?7559

[avrs-admin]

Robert Spitzenpfeil Mon 20 Jun 2011 06:24:55 PM UTC

I have tested this with some of my projects, which are loaded with the 'optiboot' bootloader. So far it works 100%.

If if matters, I use an FTDI based USB/TTL-serial adapter.

[avrs-admin]

Bernard Johnson Sat 16 Jul 2011 09:36:03 AM UTC

Actually, I think you got better results by accident.  Isn't the logic in ser_posix.c ser_set_dtr_rts backwards?

Can you try the attached patch instead of your patch?

This fixed the problem with my Uno needing a manual reset to upload code under Linux.

(file #23660)

[avrs-admin]

Robert Spitzenpfeil Sat 16 Jul 2011 12:28:59 PM UTC

In the piece of code you've referred to it seems to be 'inverse logic'. That is if it gets a '1' it clears some bits. Yes.

As far as what happens on the physical DTR signal of my serial port: no. At least on FTDI chip based adapters.

I've made a few traces with my logic analyzer.

1) unpatched avrdude 5.10 2) my patch 3) your patch 4) both patches

It shows that the unaltered 'ser_posix.c' code actually drives the physical signal LOW, when the ser_set_dtr_rts() function gets a '0' as argument.

I think that the code should represent what happens in the real world and not in some register.

Further description of the attached image:

For each test run the arduino board was disconnected and reconnected to USB. This makes sure the serial port is in the same state.

1)

Avrdude is run for the first time and everything works. The AVR chips actually gets 2 reset pulses, which doesn't seem to matter.

Then avrdude is run 3 times in a row. Each run times out, as it can't get an answer from the bootloader. As you can see in the trace, the reset pulse is created very late. This is caused by the fact that the DTR line is not switched in the right way. Reset pulses can only occur at a HIGH-LOW transition.

The next time avrdude is run and interrupted with CTRL+C. This leaves the DTR line high.

The next and last run of avrdude works again, just like the very first time.

2)

With my proposed patch it works and the DTR line is always left in a usable state for next runs of avrdude. Actually it is independent of the initial state of DTR.

3)

With your patch (inversed DTR register writes), the upload works every time as well, BUT ser_set_dtr_rts() now does the inverse thing of what it is told to do. If it gets a '0' it writes a '1' to DTR and vice versa. I don't like that, as this is confusing. Maybe there should be a comment in 'ser_posix.c' why the register writes are inversed to produce the desired output on the physical signal line.

4)

With both patches applied it still works, but it is basically as confusing as 3).

[avrs-admin]

Robert Spitzenpfeil Sat 16 Jul 2011 03:34:21 PM UTC

Another option would be to introduce

serial_toggle_dtr_rts();

and in 'arduino.c' use:

serial_toggle_dtr_rts(&pgm->fd); usleep(50*1000);

serial_toggle_dtr_rts(&pgm->fd); usleep(50*1000);

That way it would not matter (for the purpose of auto-reset) if the stuff in 'ser_posix.c' is inverted or not - as long as it is consistent! Also the initial state of DTR is irrelevant.

It would either produce HIGH_LOW_HIGH or LOW_HIGH_LOW, both sequences reset the chip.

I've attached the changes needed for this proposal. But it adds more changes.

The generated waveforms on DTR and reset are the same as with my initial patch (trace #2)

(file #23663)

[avrs-admin]

Robert Spitzenpfeil Sat 16 Jul 2011 05:48:16 PM UTC

Info:

The latest proposed changes, provided in file #23663, assume that the patch given by file #23541 has already been applied.

[avrs-admin]

Bernard Johnson Sat 16 Jul 2011 08:35:44 PM UTC

Actually, if you look at my patch and then look at ser_win32.c, my patch makes the logic work the same on both platforms.  Windows has always been bulletproof (and I can't tell you how much it bothers me that it wasn't on Linux).  Once I made that change, it became just as reliable under Linux for me (Uno).

Now that being said, what I haven't mentioned is that is not the problem I was trying to fix.  I have a board with a FT232 and basically nothing is working under Linux (works just fine in Windows).  My patch at least gets the board reset after attempted programming, which was a step forward.

With your patches, the close doesn't reset the device and start the code.  I think unless you intend to remove serial_set_dtr_rts function, the logic has to be reversed as I did otherwise places like serial_close will still be reversed.

You're right though, the code is just confusing.  For example is_on is unfortunately named because it implies current state rather than desired state.

Btw, multiple times now you have said that your device resets on a DTR transition to LOW: "Reset pulses can only occur at a HIGH-LOW transition".  Unless I misunderstand you (and I do not have a logic analyzer), my Uno goes into reset when DTR is pulled low, and then boots once DTR is released (verifed by LED).  So long as I have DTR/reset pulled low, it does not boot.  Is that what you see with your device?

So here is the end result with your patch:

Uno - works fine Zigduino/FT232 - neither programs (this was also problem with my patch) or boots when closing the device (appears that reset is held LOW)

[avrs-admin]

Bernard Johnson Sat 16 Jul 2011 08:36:17 PM UTC

I said serial_close below but I meant arduino_close.

[avrs-admin]

Robert Spitzenpfeil Sat 16 Jul 2011 09:12:28 PM UTC

Hmmm.

I don't want to turn this into a "which patch is better" discussion ;-)

The patch(es) that provide the best (and identical) results on all platforms should be used. Preferably with code that has the same logic and enough comments to understand it, if there are significant differences.

Now back to the reset business.

Why does your arduino require an additional reset after the bootloader is done? So far every bootloader I've used automatically starts the application after the upload is done. No additional external reset required here.

Regarding holding DTR low. This only keeps the AVR in reset state if DTR is wired directly to the reset pin.

To my knowledge all 'arduino'-type boards have a 100nF capacitor between the DTR line and the chip's reset pin. Therefore it is physically impossible that keeping DTR low holds the chip in reset. DC voltages are blocked by capacitors. That's why I keep repeating the HIGH-->LOW business all the time.

Anyhow. If getting this to work needs a 2nd reset on windows, then it should be inserted.

I will look into the ser_posix.c stuff some more and record traces.

[avrs-admin]

Robert Spitzenpfeil Sat 16 Jul 2011 10:15:03 PM UTC

OK. On linux + FTDI chip, these are the facts. Measured with my logic analyzer on the DTR line.

/ set line to LOW /

ctl |= TIOCM_DTR; ioctl(fdp->ifd, TIOCMSET, &ctl);

/ set line to HIGH /

ctl &= ~TIOCM_DTR; ioctl(fdp->ifd, TIOCMSET, &ctl);

So it seems the code in 'ser_posix.c' is currently correct for the OS/hardware/driver constellation mentioned above.

[avrs-admin]

Robert Spitzenpfeil Sat 16 Jul 2011 10:25:33 PM UTC

Only the comments in 'ser_posix.c' are quite misleading. If clearing a bit in a register actually sets the signal line to high and vice versa. Almost like clearing an interrupt flag in an AVR by writing a 1 to the bit.

Someone else on a linux system should check this as well. Just to rule out that my system is weird.

Nevertheless nothing of this changes the fact that only a HIGH-->LOW transition can cause a reset pulse if there is a capacitor between DTR and the reset pin. All arduinos have that.

Toggling the line twice always creates at least one H->L transition.

[avrs-admin]

Bernard Johnson Sat 16 Jul 2011 11:12:54 PM UTC

"I don't want to turn this into a "which patch is better" discussion"

Nope, not that at all.  I'm just trying to share what I see and what if any success I'm having.

Thanks for verifying with a logic analyzer.

"Nevertheless nothing of this changes the fact that only a HIGH-->LOW transition can cause a reset pulse if there is a capacitor between DTR and the reset pin."

Agreed

"Toggling the line twice always creates at least one H->L transition."

Agreed, and my only concern was really around unintended consequences.  I can't really think of any though.

"Why does your arduino require an additional reset after the bootloader is done? So far every bootloader I've used automatically starts the application after the upload is done. No additional external reset required here."

I do not know.  There are several things I can't adequately explain with this board yet.

"Regarding holding DTR low. This only keeps the AVR in reset state if DTR is wired directly to the reset pin."

Sorry, what I meant was "as if held LOW" - The reason I say so is I can't even reset it with the reset button at this point.  It's literally like someone tied RESET to ground.

[avrs-admin]

Bernard Johnson Sat 16 Jul 2011 11:22:16 PM UTC

"I do not know. There are several things I can't adequately explain with this board yet."

Looks like the supplier just reported a minor hardware defect.  I'll try to make the mod next weekend and let you know if it works better then.

[avrs-admin]

David A. Mellis Sun 17 Jul 2011 06:35:12 PM UTC

Have you tried the latest avrdude from SVN?

This looks like a duplicate of patch #7100 (which is already applied in the repository): http://savannah.nongnu.org/patch/index.php?7100

[avrs-admin]

Robert Spitzenpfeil Sun 17 Jul 2011 08:45:00 PM UTC

Hmmm. Not on my system.

From my point of view it looks like that patch #7100 is just wrong. It makes the auto-reset functional, but that's about it.

When I use that patch and have a look at DTR with my logic analyzer, the signal is inverted. (FTDI adapter).

I've attached some code that you can play with. Even the ioctl() that reads back the status of DTR shows that it is set LOW after having written a '1' to the bit, and set HIGH after having written a '0' to the bit.

Maybe the bug is somewhere deeper in the food chain.

I think that when you call ser_set_dtr_rts(1) the DTR line should actually be set HIGH. With your patch that is not the case on my machine.

(file #23669)

[avrs-admin]

Robert Spitzenpfeil Sun 17 Jul 2011 08:51:12 PM UTC

Forget about what I've said about the ioctl(). The logic in there is reversed as well.

Nevertheless, I doubt that my logic analyzer lies to me. Clear the TIOCM_DTR bit and the line goes HIGH.

[avrs-admin]

David A. Mellis Mon 18 Jul 2011 02:59:16 PM UTC

Have you checked the behavior on Windows?  There, I believe that a call to ser_set_dtr_rts(1) brings the lines low and 0 brings the line high.  So patch #7100 brought the two OS's (Windows and Posix) into alignment: i.e. interpreting a 1 to mean "set the corresponding bit in the control register", which, on both operating systems, sets the physical line low.  But someone should probably verify this, since it's been a while since I tested it and my notes are a bit obscure.

If I'm right, then I believe the current behavior (in SVN) is correct: it's consistent between operating systems, uses the OS interpretation of the meaning of "set the line" (i.e. bringing it low), and correctly resets Arduino boards (which is, after all, the goal).

Maybe we just need to clarify with better comments the behavior of the ser_set_dtr_rts() function (i.e. that a logical 1 means a low level on the physical line and vice-versa)?

[avrs-admin]

Robert Spitzenpfeil Mon 18 Jul 2011 03:43:28 PM UTC

I have abandoned windows almost completely.

The only way I can run it is inside VirtualBox, and I don't think it is wise to use that as reference.

Identical behaviour across platforms is a very good thing!

According to wikipedia, the RS-232 control signals are not inverted.

[...]

When one of these signals is active [DTR...], the voltage on the line will be between +3 to +15 volts. The inactive state for these signals is the opposite voltage condition, between −3 and −15 volts. Examples of control lines include request to send (RTS), clear to send (CTS), data terminal ready (DTR), and data set ready (DSR).

[...]

So I would think that translating that to 0...5V would be just scaling and shifting the voltage interval without inversion.

I would assume that ser_set_dtr_rts(1) would therefore set the line HIGH (active), and ser_set_dtr_rts(0) to LOW.

Why the low-level register writes are inverted on linux I don't know. I'm trying to find out why that is the case. Finding the right kernel mailing list is a tedium.

[avrs-admin]

David A. Mellis Mon 18 Jul 2011 04:57:13 PM UTC

Those are real full-swing (e.g. +-12V) RS-232 voltages.  The voltage levels are inverted for TTL (0 to 5V) serial, so the data signals are 5V for a logic 1 and 0V for a logic 0 (whereas -12V is a logic 1 for the data signals of full-swing serial) and the control signals (apparently) become 0V for logic 1 and 5V for logic 0.

[avrs-admin]

Robert Spitzenpfeil Mon 18 Jul 2011 06:32:55 PM UTC

Hmmm. I have the strong urge to slap someone over the head with a stinking wet fish. But probably they are already dead... this is like hunting a misguided minus-sign on pages after pages of formulae.

As I don't see how avrdude should easily find out if it talks to an old-fashioned serial port or a 5V variant, it doesn't make much sense to talk about setting a signal line to a certain voltage level. Logic levels are all that remain accessible.

This brings me back to 'arduino.c'.

As 'arduino_open()' talks about charging/discharging capacitors and HIGH/LOW levels (and when I hear HIGH, I think about 5V - not 0), there should be clear additional comments about what writing a 'logic 0' or 'logic 1' to DTR actually do on the piece of wire in terms of a real measurable Voltage (not logic states).

[avrs-admin]

Robert Spitzenpfeil Mon 18 Jul 2011 06:40:49 PM UTC

And another one...

Now what happens if someone should connect an AVR chip to a real RS-232 interface ?

As the levels for DTR seem to be inverse there, the transition for resetting the arduino will be wrong as well. (just like before patch #7100 for 5V systems).

Therefore I suggest to use something similar to comment #4 / file #23663. This will work independently of which way around the voltage levels actually are.

[avrs-admin]

David A. Mellis Tue 19 Jul 2011 01:20:07 AM UTC

If someone hooked up an AVR to a real RS-232 interface, the +-12V would probably blow it up.  So I think the code is fine.

You're right, though, that the comment in arduino.c is confusing and, I think, wrong.  It's probably worth submitting / applying a patch for that.

[avrs-admin]

Robert Spitzenpfeil Tue 19 Jul 2011 10:27:10 AM UTC

There are level shifter ICs... as you know very well. And e.g. the MAX232 doesn't translate more than RX/TX.

But maybe we should forget about RS-232. Nobody with a sane mind uses that anymore (industrial stuff excluded for now).

And whoever thought that using a negative voltage for logic 1 is a good thing should not come anywhere near me.

[avrs-admin]

Bernard Johnson Mon 25 Jul 2011 06:07:50 AM UTC

There was a hardware bug in my 2nd board, so after repairing that both boards upload and reset equally well with your proposed toggle patches Robert.

[avrs-admin]

Joerg Wunsch Wed 24 Aug 2011 02:02:49 PM UTC

Can someone summarize for me the action to take?

As I don't have an Arduino around, I'd only like to commit something that makes everybody happy.

[avrs-admin]

David A. Mellis Wed 24 Aug 2011 02:16:06 PM UTC

No changes to the code are needed.  The current (SVN) avrdude correctly resets Arduino boards on upload.

It could be useful to patch the comments to clarify the situation (i.e. that a logical 1 is a physical low level voltage, and 0 is Vcc).  Robert, do you want to submit such a patch?

[avrs-admin]

Robert Spitzenpfeil Wed 24 Aug 2011 02:37:35 PM UTC

Personally I'd like to see something that is completely invariant to whether a real RS232 port/device is connected or a TTL serial adapter, so that it doesn't matter if signal levels are inverted or not. As I had to find out, there doesn't seem to be an easy way for the computer to know which is which.

It should always work. There should be comments that explain what is done and why.

My previously proposed addition of the 'toggle' function would provide just that, BUT a) I'm not capable of providing the necessary patches to 'ser_win32.c' b) it would also require changes to 'ser_posix.c' as well.

So what to do?

If the bootloader timeout is long enough (really don't know about all optiboot versions for sure), 'arduino.c' could be modified as such:

---

// explanatory comments should go here...

serial_set_dtr_rts(&pgm->fd, 0); usleep(501000); serial_set_dtr_rts(&pgm->fd, 1); usleep(501000); serial_set_dtr_rts(&pgm->fd, 0); usleep(501000); serial_set_dtr_rts(&pgm->fd, 1); usleep(501000);

---

This would provide at least one usable (5V/0V) transition for the reset capacitor to be pulled in the right direction, regardless what 'HIGH' actually means.

I could do some reliability testing on homebrew and a Diecimila board using various bootloaders, but only on linux.

[avrs-admin]

David A. Mellis Wed 24 Aug 2011 02:52:58 PM UTC

I appreciate your desire to make this as portable as possible, but I don't think we should worry about true RS232 connections for a few reasons:

First, we don't use RS232 for current Arduino boards.  The old serial boards that did use it don't have auto-reset.

Second, it's not clear how the auto-reset is supposed to be triggered for RS232 boards.  I'd argue that the behavior of existing Arduino boards (i.e resetting on a high->low voltage transition of DTR) sets a software standard (i.e. a 0 to 1 logical transition) for triggering the auto-reset.  So if someone does make an Arduino-like board with true RS232, they should base its auto-reset circuitry on this defacto software reset standard - i.e. they should make their board work with the current code.

Finally, the current code works and I've tested it on multiple operating systems.  Any changes will require retesting on a variety of configurations.  This doesn't seem worth it for hypothetical future boards whose behavior we don't know (and therefore can't test).

[avrs-admin]

Joerg Wunsch Wed 24 Aug 2011 02:57:35 PM UTC

OK, I'm closing it as "won't do" then.  The discussion can be found both, here in the patch tracker as well as the mailinglist archives, so hopefully, someone interested in the details will be able to find it.

[avrs-admin]

Robert Spitzenpfeil Wed 24 Aug 2011 03:28:15 PM UTC

Would it be possible to at least

a) add the comments about the HIGH/LOW business to 'arduino.c' ?

b) create an updated release file (.tar.gz) that includes the latest patches. Always having to drill through the source repository is not funny at all. Patch #7100 is out for quite a while and sadly not part of the 'latest' avrdude-5.10.tar.gz that is available on the download web page.

[avrs-admin]

Joerg Wunsch Wed 24 Aug 2011 03:49:57 PM UTC

a) add the comments about the HIGH/LOW business to 'arduino.c' ?

Please do.  I don't have an idea what I'm supposed to add there ... I reopened the patch tracker item, so you could add it here easily.

b) create an updated release file

I'm about to do that, that's why I'm walking down the bug and patch lists.

[avrs-admin]

Robert Spitzenpfeil Fri 09 Sep 2011 10:10:37 PM UTC

@mellis:

I will cook something up for arduino.c (comments only, but they might be extensive).

[MCUdude]

No changes to the code are needed. The current (SVN) avrdude correctly resets Arduino boards on upload.

I think this can be closed. Auto-reset works on pretty much all boards that communicate over UART and have a capacitor in series between the reset and DTR line.