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

WIP: Fix macOS bootloader reset (ESPTOOL-396) #718

Closed
wants to merge 3 commits into from
Closed

WIP: Fix macOS bootloader reset (ESPTOOL-396) #718

wants to merge 3 commits into from

Conversation

256dpi
Copy link

@256dpi 256dpi commented Jan 26, 2022

Description of change

This change adds back the delay present in esptool prior to v.3.2. It has been removed in the following commit: 2d72f56

This change fixes the following bug(s):

At least for me it fixed the problems mentioned in my comment: #712 (comment)

I have tested this change with the following hardware & software combinations:

  • MacBook Pro M1-Max
  • Mac OS 12.1
  • ESP32-D0WDQ6 (revision 1)

I have run the esptool.py automated integration tests with this change and the above hardware. The results were:

Not run. Not familiar with that.

@256dpi
Copy link
Author

256dpi commented Jan 26, 2022

Actually, and extra delay of 0.1s is enough for my board to trigger the bootloader. The 1.2s is just the value used by esptool in pre v3.2 releases.

@github-actions github-actions bot changed the title Reintroduce Missing Delay Reintroduce Missing Delay (ESPTOOL-396) Jan 26, 2022
@radimkarnis
Copy link
Collaborator

Hello @256dpi,

thank you for contributing! I will answer here, but I've read your original comment.

The removal of esp32r0_delay should affect only revision 0 ESP32 chips. As there are not many of these left in circulation, we've decided to remove this workaround to speed up the connection process for all the other ESP chips (users can reset rev0's into bootloader manually).

I am using a very similar setup (MacBook Pro M1 2020, Mac OS 12.1, ESP32-D0WDQ6 (revision 1)) and the reset works. Can you tell me more about your setup? Are you using a USB hub or connecting directly?

I will debug the original issue #712 probably next week. If we get more reports like yours, we could reasonably increase the delay (adding 1.2 seconds is unnecessary) as a part of the solution.

@256dpi
Copy link
Author

256dpi commented Jan 27, 2022

I got the same result with a USB-A to USB-C adapter and a USB-C 3.0 hub with USB-A ports.

However, I just took a look at the schematic (its an old design) and realized that it is missing the capacitor from EN to GND. So maybe, it has nothing do do with USB but the delay actually just helps to overcome the issues arising from the missing capacitor.

@MMI
Copy link

MMI commented Jan 27, 2022

The removal of esp32r0_delay should affect only revision 0 ESP32 chips. As there are not many of these left in circulation, we've decided to remove this workaround to speed up the connection process for all the other ESP chips (users can reset rev0's into bootloader manually).

The assumption that people are only using dev boards with reset buttons is naive. I have prototype commercial devices built around rev 0 modules that don't have handy reset buttons. But esptool works for me so this is not a complaint, just a comment 😄

@radimkarnis
Copy link
Collaborator

The assumption that people are only using dev boards with reset buttons is naive

@MMI, I agree with you and to be honest, I never assumed that. By "manually" I didn't mean just pressing buttons, but pulling IO0 down (bridging with GND) if necessary.

ESP32 revision 0 chips have been discontinued and marked as not recommended for new designs for a long time. So I don't think anyone is developing commercial devices with rev 0 chips. AFAIK, it is mostly still possible to reset rev0 to bootloader with current esptool, it doesn't work only under specific circumstances - for these, you can use an older version of esptool or bridge IO0 and GND manually.

I know this isn't ideal, but removing this workaround simplifies and speeds up the reset sequence for all the other chips (8266's, 32-S,C,H series). That is the reasoning behind this change 😊


and realized that it is missing the capacitor from EN to GND

@256dpi, this is most likely the reason, thank you for investigating! Adding a capacitor between the EN pin and GND (in the 1uF-10uF range) is necessary for the reset circuitry to work reliably (see more info in esptool docs).

It would be great if you could try adding it and confirm the suspicion!

@MMI
Copy link

MMI commented Jan 27, 2022

I know this isn't ideal, but removing this workaround simplifies and speeds up the reset sequence for all the other chips (8266's, 32-S,C,H series). That is the reasoning behind this change

Hey thanks for the insight. Trust me, I know that it is a thankless job trying to keep all the people and things happy and working.

@MMI
Copy link

MMI commented Jan 27, 2022

Oh and if you have the connections... maybe you could take on this documentation suggestion.

I had been reading the "Auto program" table on the schematic incorrectly. As a software person, I see a table like that, I recognize it as a truth table... Like if DTR and RTS are 1 and 1 then both EN0 and IO0 will be high.

My ah-ha moment came when I realized that it's not a truth table but rather a series of steps -- each row being a step -- and that if I take DTR and RTS through all four of the steps, then I would achieve bootloader nirvana™

So my suggestion is to make the table in the schematic more clearly suggest that it's meant to be a series of state transitions.

@radimkarnis
Copy link
Collaborator

radimkarnis commented Jan 28, 2022

@MMI thanks for your recommendation, but you were right the first time - this really is a truth table 😅 !

Some OS and/or drivers may activate RTS and or DTR automatically when opening the serial port, pulling them low together and holding the ESP in reset. If RTS is wired directly to EN then RTS/CTS “hardware flow control” needs to be disabled in the serial program to avoid this. Additional circuitry is implemented in order to avoid this problem - if both RTS and DTR are asserted together, this doesn’t reset the chip. The schematic shows this specific circuit with two transistors and its truth table.

(Taken from esptool docs Automatic Bootloader page). The purpose of this table is to show that asserting DTR and RTS both at the same time doesn't hold the chip in reset.

(btw, I think we should call download mode the bootloader nirvana™ from now on!)

@256dpi 256dpi changed the title Reintroduce Missing Delay (ESPTOOL-396) WIP: Fix macOS bootloader reset (ESPTOOL-396) Feb 4, 2022
@256dpi
Copy link
Author

256dpi commented Feb 4, 2022

Thanks to the working C testing script by @MMI in #712 I had a hunch and implemented a custom ioctl call to set both DTR and RTS at the same time. Now the bootloader reset works properly with my setup of boards and cables.

Obviously, for a proper release this would need to be gated, as it is not compatible with other platforms (win32).

@256dpi
Copy link
Author

256dpi commented Feb 4, 2022

PS: It seems the initially added extra delay is not need anymore. So I removed it.

@MMI
Copy link

MMI commented Feb 4, 2022

This is sweet -- I was thinking of doing something like this but got swamped with other things. Thank you thank you thank you!

@radimkarnis
Copy link
Collaborator

This is an interesting approach, thank you both for investigating and exploring this option!

For now, we would rather stick with the API provided by pySerial, as this has been successfully used on thousands of occasions and works on all platforms. On the other hand, this will surely come in handy for debugging if I manage to reproduce the reset glitch!

@radimkarnis
Copy link
Collaborator

I will close this now due to the above-mentioned reasons. Feel free to reopen if you believe this should be re-evaluated!

@256dpi 256dpi deleted the delay-fix branch April 6, 2022 20:50
@sbytnar
Copy link

sbytnar commented Sep 9, 2022

@256dpi Thank you for this patch! This makes esptool (flash programming & fuse read/write) go from "impossible to enter bootloader" to "100% reliable" on my development machine with a production PCBA. My dev environment is the latest VirtualBox 6.1.x on Windows 10 with a Ubuntu 20.04LTS guest, using espprog as a shared USB OHCI device.

@silviosan
Copy link

@256dpi Thank you for your patch! I am using an Mac mini M1 and with the changes you made programming the ESP32-Board works now like a charm. Thanks again!

@radimkarnis
Copy link
Collaborator

radimkarnis commented Dec 16, 2022

Hello @256dpi,
there was a lot of positive feedback for this fix and I've also been able to verify that it helps in my environment (but only on some devkits, this seems HW-dependent). We would like to include this as a part of efforts to make the reset sequence more robust.

In order for this to be merged, there are some things to consider:

  1. This works only on posix systems. This has to be safeguarded to not run on windows.
  2. This should not be the primary reset algorithm. That should stay the same and this should kick in on subsequent connection attempts (second or third attempt / or try alternating and run this every second attempt). This is to keep maximal compatibility. We have verified that this helps in some cases, but could also make things worse for the other ones.

Thanks again for coming up with this! Would you want to finish this PR or have you moved to other things? If so, are you ok with me implementing this?

@256dpi
Copy link
Author

256dpi commented Dec 16, 2022

@radimkarnis I'm happy to hear this is considered to be included in esptool! We applied the patch to our internal stack in February and haven't had any issues since regarding connectivity. Since we use Macs only, I haven't bothered to make it OS dependent. Please feel free to add this behaviour, re-implement it or do whatever you feel is best for esptool. The only thing we would be interested in is a CLI flag that allows us to select this method right away. That would speedup the connection process for boards we already know this method is required.

@MMI
Copy link

MMI commented Dec 16, 2022

I too am happy that there is movement on this. I suggest, however, that it would be more correct to push for this API (or some variation) to be pushed upstream into pyserial (in the naive hope that someone in that project has the necessary expertise to do the correct thing on the windows backend). Maybe that's a longer term thing.

@radimkarnis
Copy link
Collaborator

@MMI this API is taken directly from pySerial, this is how their implementation works. The only difference is that this way you can set both DTR and RTS at the same time, pySerial only allows you to do that in two steps.

@MMI
Copy link

MMI commented Dec 16, 2022

Yes, I know. What I'm suggesting is that pyserial itself should be fixed/changed to add the desired function.

Of course, it may fall on us to provide an example implementation ;)

@radimkarnis
Copy link
Collaborator

@256dpi thanks again. Your approach has been merged to master. It's selected as a primary method on MacOS and Linux. We will be adding more options to configure the reset sequence with a config file soon.

Feel free to test and leave feedback!

ahmedesabry added a commit to sg-wireless/sg-sdk that referenced this pull request Aug 1, 2024
ahmedesabry added a commit to sg-wireless/sg-sdk that referenced this pull request Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants