igrr / esptool-ck

ESP8266 build/flash helper tool by Christian Klippel
GNU General Public License v2.0
366 stars 123 forks source link

DTR Line #63

Closed mribble closed 6 years ago

mribble commented 6 years ago

This is related to this issue, but now that I've tracked down part of that issue to esptool-ck I'm filing a bug here.

I'm programming the esp8266 through a sam3x over native USB using the USB CDC serial driver developed for Arduino Due. I used the 0.4.12 prebuilt windows exe and it fails. If I build my own exe using cygwin it succeeds.

The issue is that the USB CDC driver expects DTR to be active. However this project isn't setting DTR active (DTR is active low so I'm expecting DTR to be a low voltage while transmitting, but it is high with your pre-built exe). You can see some waveforms showing the difference in dtr voltage between the cygwin build and the pre-built exe here: http://glacialwanderer.com/files/2017/dtr_waveform.png

I suspect the issue is in serialport.c. Changing "sDCB.fDtrControl = DTR_CONTROL_DISABLE;" to enable might fix this problem in call cases (my guess is disabled might be handled differently using the cygwin build environment than yours).

However, since I can't reproduce the failure I can't verify it. Can you tell me what build environment you are using to make the prebuilt exes? Once I know that then I should be able to reproduce the issue I'm seeing in the prebuilt exe and verify a fix for it.

Here are some things I don't understand: 1) Why cygwin is showing different behavior than whatever build enviroment you are using to make the exe. 2) Why on the wave for you are that active pulse (remember dtr is active low) with the prebuilt exe. However, that brief pulse won't work for Arduino's cdc usb serial driver since it checks for the active dtr state on every usb write.

Here are the command line parameters I'm using with the prebuilt exe: esptool.exe -vv -cd none -cb 115200 -cp COM5 -ca 0x0 -cf Esp8266.ino.bin

Here are the command line parameters I'm using with the prebuilt exe: ./esptool.exe -vv -cd none -cb 115200 -cp /dev/ttyS4 -ca 0x0 -cf ./Esp8266.ino.bin

igrr commented 6 years ago

The build environment is described in Travis CI configuration file: https://github.com/igrr/esptool-ck/blob/master/.travis.yml. For Windows, I use multiarch/crossbuild docker image which comes with i686-w64-mingw32 cross-compiler.

I'm not sure why you see different behavior when building in Cygwin. When building with Cygwin, POSIX APIs for working with serial port are used, instead of Windows APIs, and Cygwin DLLs handle the conversion from POSIX system calls into Windows ones. The POSIX implementation of serial port handling also sets DTR to inactive after the port is opened. So my assumption was that the behavior should be the same as for Windows builds. Anyway, whether or not DTR is kept low during data transfer largely depends on the reset method. My guess is that you can create a new reset method for your board which keeps DTR low after reset is done. See espcomm_boards.c file for the reset methods.

mribble commented 6 years ago

Thanks. I built the project with minGW and that reproduced the failure.

My prefered change would be to not add a new profile and in serialport_setboaudrate() just change this line:

sDCB.fDtrControl = DTR_CONTROL_DISABLE; to: sDCB.fDtrControl = DTR_CONTROL_ENABLE;

I confirmed that fixes my issue without adding a new config. Part of my long explanation above was trying to justify this as the correct thing to do since other tools seem to be doing this. I have verified putty sets dtr. I have also verified esptool_py sets this.

That said if you disagree or think there is too much risk for something like that breaking other boards then I will make a change like you suggested. If we go this direction I have a few questions: 1) What do you think a good name for this mode would be? Best I can think of is dtrset, but if you would rather use some other name I'm fine with anything. 2) Should I be updating any documentation os part of the initial pull request for this feature or would you rather handle that separately? 3) Would you be able to get this mode added to the Arduino IDE like we discussed for the "none"? And if so do you have a rough estimate on timeline until this shows up in the ESP8266 Arduino core?

igrr commented 6 years ago

I think there is some risk in changing the default DTR level, mostly for some less commonly used boards. For example, i can test the change on NodeMCU and say whether it works, but for some other boards out there i don't have a way to test.

Adding dtrset board type sounds okay to me. If you add it, please update the list of possible board types in Readme file. As for Arduino, would adding this new method into your local boards.txt.local file be an option for you?

mribble commented 6 years ago

I have a new branch with the dtrset change ready to push, but it seems I don't have permission to push to this project. Can you enable that? After I push the branch I'll open a pull request. If you'd prefer some other way for me to submit these changes for review let me know what it is.

As for whether boards.txt.local works for my needs, I'm not sure. I couldn't find documentation or get it working. I tried making a version of that with just the things I wanted changed and as a whole copy of the esp8266 version plus the changes I wanted. Then I tried putting those files both in my sketch folder and in Arduino15\packages\esp8266\hardware\esp8266\2.3.0. But nothing worked. Could you explain how this file works and once I get it working I can say if it meets my needs.

mribble commented 6 years ago

Do you want me to just share a diff of my changes or something else instead of giving me access to push a branch to your project? Let me know how you'd like me to share my fix.

igrr commented 6 years ago

The default github way of submitting PRs, AFAIK, is to fork the project, push the desired changes to the branch of your fork, then submit the PR against upstream. At least, this is how i have received all PRs to my projects so far :)

mribble commented 6 years ago

I posted a pull request about 7 days ago. What kind of timeframe should I expect a review to happen?

igrr commented 6 years ago

Thanks for the reminder, just merged the PR!