Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update esptool reset method #6429

Merged
merged 6 commits into from
Nov 6, 2019
Merged

Conversation

f5soh
Copy link
Contributor

@f5soh f5soh commented Aug 19, 2019

Related to #6296

This is my first PR here, so be patient :)

Since 2.5.1 the upload tool moved to esptool.py, reset methods are deprecated and all boards use in fact the default_reset with dtr.

First commit update the reset methods available:

  • none becomes as 'no dtr' in menu,
  • nodemcu, dtrset as 'dtr',
  • ck (fixed) temporary handled like 'none' (not sure about this),
  • new reset method 'no dtr, no_sync' in extra menu

Second commit is a workaround because the flash erase and write_flash is done in two commands and allow the two consecutive commands using 'no dtr, no_sync' without a reset circuit on board.
This workaround will not be needed if esptool.py handle the erase and write in one command.

@earlephilhower
Copy link
Collaborator

For a first PR, you did pretty well! Normally the boards.txt or an astyle problem hits users.

@earlephilhower
Copy link
Collaborator

I think @d-a-v is the expert on this, but he's on vacation for another week, so do please be patient.

@f5soh
Copy link
Contributor Author

f5soh commented Aug 20, 2019

No issue, this PR still need some work and more information about how the ck method can be supported with esptool.py

Will be rebased when ready for merge

@devyte devyte requested a review from d-a-v August 21, 2019 13:12
( '.menu.ResetMethod.nodemcu.upload.resetmethod', 'nodemcu' ),
( '.menu.ResetMethod.dtr', 'dtr' ),
( '.menu.ResetMethod.dtr.upload.resetmethod', '--before default_reset --after hard_reset' ),
( '.menu.ResetMethod.nodtr', 'no dtr' ),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would mention the old names with something like this:

  • dtr (aka nodemcu)
  • no dtr (aka ck)

so users are not lost

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, added changes.

@f5soh f5soh force-pushed the esptool_reset_method branch 2 times, most recently from c785994 to 725344a Compare August 29, 2019 21:45
@earlephilhower
Copy link
Collaborator

@f5soh, you'll need to pull in master to fix the compile problems the CI system is having. For some reason I can't do it for you from the website just now.

@f5soh f5soh force-pushed the esptool_reset_method branch from 725344a to 76d327c Compare August 30, 2019 17:26
@f5soh
Copy link
Contributor Author

f5soh commented Aug 30, 2019

Seems the build.py need update for tests:

'ResetMethod=nodemcu'.format(**vars(args))

Error resolving FQBN: getting build properties for board esp8266com:esp8266:generic: invalid value 'nodemcu' for option 'ResetMethod'

@earlephilhower
Copy link
Collaborator

Ah, yeah. You'll need to fix that in your PR, too. There was also just 2 nights ago an update to the Arduino.cc executable nightly build which changes the FQBN format. See #6461 . Double whammy.

@earlephilhower earlephilhower added this to the 2.6.0 milestone Sep 5, 2019
@d-a-v
Copy link
Collaborator

d-a-v commented Sep 11, 2019

@f5soh Would you mind renaming back the internal fqbn part to what it currently is (nodemcu and not dtr, ck and not no-dtr) - It is OK to leave the displayed name though.
Mainly because of annoyance following such change, like #5572 (comment) (and included links, also the number of references to it show how many people are affected by such change).

@d-a-v d-a-v added the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Sep 11, 2019
@f5soh f5soh force-pushed the esptool_reset_method branch from 32d3387 to fb31335 Compare September 12, 2019 18:34
@f5soh
Copy link
Contributor Author

f5soh commented Sep 12, 2019

I keep all previous resetmethod names and simply translate them to new esptool.py options.
Hope this matches what you are talking about.

I can squash reset method changes in one commit for better clarity.

@d-a-v d-a-v removed the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Sep 26, 2019
@@ -1075,7 +1077,7 @@

'flash_erase_menu': collections.OrderedDict([
( '.menu.wipe.none', 'Only Sketch' ),
( '.menu.wipe.none.upload.erase_cmd', 'version' ),
( '.menu.wipe.none.upload.erase_cmd', 'flash_id' ),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am concerned about this change.
flash_id asks for a connection to the esp, while version is a no-op which is what we need in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take a board without reset circuit and version as first default command as you want :

  • "Only sketch" works only with the "no dtr" reset method
  • "Flash erase" works only with the "no dtr, no sync" reset method or second programming step will fail.

Do we really want the user change the reset method every time the erasing flash option is changed ?

With flash_id, both "Only sketch" and "Flash erase" works with "no dtr, no sync" reset method.

As described, this is a workaround needed since the esptool do the erasing and programming in two consecutive commands. May need also the "no dtr, no sync" reset method for all "ck" boards.

@sampetrosyan
Copy link

Does anyone know what's wrong here? after I upload the code successfully, below that it says

Leaving...

Hard resetting via RTS pin..

@devyte
Copy link
Collaborator

devyte commented Nov 5, 2019

What is pending here?

@d-a-v
Copy link
Collaborator

d-a-v commented Nov 5, 2019

I'd merge but release is too soon, feedback from users with issues is welcome.
I tested it with nodemcu reset method and nothing seems broken.
If anyone with an esp-01 progammer could also test it, that'd be great.
@mcer12 @denisfou @IllyaMoskvin @ranger81

@d-a-v d-a-v added the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Nov 5, 2019
@d-a-v
Copy link
Collaborator

d-a-v commented Nov 5, 2019

@f5soh
You'd need to update your local master, then on your branch locally remove boards.txt, merge master and regenerate boards.txt

@f5soh f5soh force-pushed the esptool_reset_method branch from 5313300 to fa5c2f0 Compare November 5, 2019 23:05
@Jason2866
Copy link
Contributor

Jason2866 commented Nov 6, 2019

@d-a-v There are more users using NodeMCU and wemos minis than users using ESP-01 devices with special programmer(s). There is no standard ESP-01 programmer. I have two different types... No change in behaviour of this two. One works as before (reset).
The other did not reset the device before this PR nor with this PR

IMHO: Merging this PR will solve more problems, than it maybe! generates

@d-a-v
Copy link
Collaborator

d-a-v commented Nov 6, 2019

@Jason2866 This PR is not mergeable as-is and I don't know why.
@f5soh I made a subPR to see how it goes.

@d-a-v d-a-v merged commit c28838d into esp8266:master Nov 6, 2019
@Erik84750
Copy link

The issue I described in version 2.5.2 does not work when ESP8266 uses auto-reset #6631 is that when using the ESP8266 in conjunction with auto-reset circuitry (using DTR to set the ESP8266 in programming mode, and automatic restoring to normal mode after program download) there needs to be a hard (power-off power-on) reset to run the ESP8266 in normal mode.
Unless I am wrong, versions 2.5.1 and higher do not acknowledge the use of auto-reset circuitry, therefor requiring users to do a hard reset instead of automatically restarting in normal mode.

@JAndrassy
Copy link
Contributor

JAndrassy commented Nov 25, 2019

The issue I described in version 2.5.2 does not work when ESP8266 uses auto-reset #6631 is that when using the ESP8266 in conjunction with auto-reset circuitry (using DTR to set the ESP8266 in programming mode, and automatic restoring to normal mode after program download) there needs to be a hard (power-off power-on) reset to run the ESP8266 in normal mode.
Unless I am wrong, versions 2.5.1 and higher do not acknowledge the use of auto-reset circuitry, therefor requiring users to do a hard reset instead of automatically restarting in normal mode.

and why don't you try the 2.6.1 which has this fix?

@Erik84750
Copy link

Oh I am so sorry, of course I will try it, and asap! I am not sufficiently familiar yet with the workings of Github, my apologies for undue inconvenience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for feedback Waiting on additional info. If it's not received, the issue may be closed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants