Skip to content

scripts: Fix esptool.py path resolution for cross-platform compatibility #88605

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rnicolas
Copy link

This PR ensures the espidf path to esptool.py is correctly resolved and works on both Windows and Linux platforms by:

  • Normalizing the path using os.path.normpath()

  • Replacing backslashes () with forward slashes (/) to avoid issues with subprocess calls and string formatting

  • Adding a safe os.path.exists() check with a clear error message if the tool is missing

These changes improve the reliability of the esp32 runner logic and prevent false negatives when using West-based flashing on Windows.

Copy link

Hello @rnicolas, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

@github-actions github-actions bot added the area: West West utility label Apr 14, 2025
@@ -105,7 +104,10 @@ def do_create(cls, cfg, args):
encrypt=args.esp_encrypt, no_stub=args.esp_no_stub)

def do_run(self, command, **kwargs):
self.require(self.espidf)
if not os.path.exists(self.espidf):
Copy link
Member

Choose a reason for hiding this comment

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

Why? self.require() should do exactly that?

Copy link
Author

Choose a reason for hiding this comment

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

self.require(self.espidf) fails with the following error:

FATAL ERROR: required program C:/Users/user/workspace/zephyrproject/external/modules/hal/espressif/tools/esptool_py/esptool.py not found; install it or add its location to PATH

Even though esptool.py does exist at the specified path, the error occurs because shutil.which()—used internally by self.require()—only resolves executables with extensions like .exe, .bat, .cmd, etc., on Windows. Python scripts (.py) are not included in that search.

An alternative (though not included here) would be to modify core.py and replace:

ret = shutil.which(program, path=path)

with:

ret = os.path.abspath(program)

However, this may have wider implications in other parts of the system and would require more extensive testing. Therefore, this PR opts for the safer workaround without touching core components.

Copy link
Member

Choose a reason for hiding this comment

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

Why not add an optional parameter to require() instead that makes it only check for the existence of the file? then you don't affect the rest but you keep using the standard method.

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure if I follow you correctly, you mean to do something like this?

def require(self, program, check_path_only=False):

    if check_path_only:
        if not os.path.exists(program):
            ret = program
    else:
        ret = shutil.which(program, path=path)

     if ret is None:
            raise MissingProgram(program)
        ...

@pdgendt pdgendt added the platform: ESP32 Espressif ESP32 label Apr 15, 2025
@pdgendt pdgendt requested a review from sylvioalves April 15, 2025 08:07
@pdgendt
Copy link
Collaborator

pdgendt commented Apr 15, 2025

Squash your commits and force push, no fixup commits allowed.

@rnicolas rnicolas requested a review from carlescufi April 16, 2025 16:41
@rnicolas rnicolas changed the title Fix esptool.py path resolution for cross-platform compatibility scripts: Fix esptool.py path resolution for cross-platform compatibility Apr 17, 2025
@sylvioalves
Copy link
Collaborator

sylvioalves commented Apr 19, 2025

@rnicolas Your email does not match the signed-off. Can you update accordingly?
Also, I have tested latest Zephyr in Windows 10 and had no issues at all. Can you provide additional info/description or Issue about it?

@rnicolas
Copy link
Author

rnicolas commented Apr 19, 2025

@sylvioalves I'm out office so I cannot update the email till Tuesday, but yesterday I was wondering if the problem comes from the way of python is installed in MS Windows, in my case, I installed it using winget in a fresh MS Windows 11. How did you install it? Using winget/msstore or downloading the installers from python.org? Also, can you check is your PATHEXT (echo %PATHEXT) if it includes the .py files?

@rnicolas
Copy link
Author

rnicolas commented Apr 22, 2025

@pdgendt , @sylvioalves , @carlescufi It turns out the problem is related to how Python is installed on Microsoft systems. That's why some people don't encounter the issue. If I follow the instructions in the Getting Started Guide, Python gets installed using winget, which is what I initially did. However, that installation misses some extra steps that the official installer from python.org includes.

After reinstalling Python using the installer from python.org, the issues I was facing are now resolved. I recommend updating the guide to remove the winget installation method and instead specify that Python should be installed using the official installer from python.org.

@sylvioalves
Copy link
Collaborator

sylvioalves commented Apr 22, 2025

@pdgendt , @sylvioalves , @carlescufi It turns out the problem is related to how Python is installed on Microsoft systems. That's why some people don't encounter the issue. If I follow the instructions in the Getting Started Guide, Python gets installed using winget, which is what I initially did. However, that installation misses some extra steps that the official installer from python.org includes.

After reinstalling Python using the installer from python.org, the issues I was facing are now resolved. I recommend updating the guide to remove the winget installation method and instead specify that Python should be installed using the official installer from python.org.

Great to see you found out the root cause. I think both actions can take place:

  1. Documentation
  2. This PR change. If this PR fixes the issue when following the guidelines, it is good. +1 for me.

Waiting for @carlescufi feedback.

The esp32.py script had issues when running under Windows due to path
formatting differences (e.g., backslashes vs forward slashes). This
commit adjusts the path resolution logic to work reliably across
platforms.

Signed-off-by: Roger N. Alegret <roger.alegret@aalberts-hfc.com>
@pdgendt pdgendt assigned sylvioalves and unassigned pdgendt May 5, 2025
@Codesane
Copy link

I recently ran into this issue as well. It was quite easy to just remove the self.require check and run the flash command to get past it but I think it makes for quite a bad onboarding experience for people using Espressif's tools.

@sylvioalves
Copy link
Collaborator

I recently ran into this issue as well. It was quite easy to just remove the self.require check and run the flash command to get past it but I think it makes for quite a bad onboarding experience for people using Espressif's tools.

Did this PR fix the issue in your environment?

@Codesane
Copy link

Did this PR fix the issue in your environment?

Yes.

I didn't apply the entire patch at first. This small change was enough to get past the issue:

diff --git a/scripts/west_commands/runners/esp32.py b/scripts/west_commands/runners/esp32.py
index 619c16460e8..009e32d161e 100644
--- a/scripts/west_commands/runners/esp32.py
+++ b/scripts/west_commands/runners/esp32.py
@@ -105,7 +105,7 @@ class Esp32BinaryRunner(ZephyrBinaryRunner):
encrypt=args.esp_encrypt, no_stub=args.esp_no_stub)

     def do_run(self, command, **kwargs):
-        self.require(self.espidf)
+        #self.require(self.espidf)

         # Add Python interpreter
         cmd_flash = [sys.executable, self.espidf, '--chip', 'auto']

For due diligence I also tried applying the entire patch, below are the build logs showing before/after.

Build Logs
(.venv) PS C:\Users\<redacted>\zephyr> git status
On branch main
Your branch is up to date with 'origin/main'.

nothing to commit, working tree clean
(.venv) PS C:\Users\<redacted>\zephyr> west flash
-- west flash: rebuilding
[1/9] Generating include/generated/zephyr/version.h
-- Zephyr version: 4.1.99 (C:/Users/<redacted>/zephyr), build: v4.1.0-4624-g265cfb45a818
[9/9] Linking C executable zephyr\zephyr.elf
Memory region         Used Size  Region Size  %age Used
           FLASH:      132892 B    4194048 B      3.17%
     sram0_0_seg:       49872 B     437512 B     11.40%
     irom0_0_seg:       12852 B         4 MB      0.31%
     drom0_0_seg:        1820 B         4 MB      0.04%
      lp_ram_seg:          0 GB        16 KB      0.00%
        IDT_LIST:          0 GB         8 KB      0.00%
Generating files from C:/Users/<redacted>/zephyr/build/zephyr/zephyr.elf for board: xiao_esp32c6
esptool.py v4.8.1
Creating esp32c6 image...
Image has only RAM segments visible. ROM segments are hidden and SHA256 digest is not appended.
Info: inserting 4 bytes padding between .dram0.data and .loader.data
Merged 10 ELF sections
Successfully created esp32c6 image.
-- west flash: using runner esp32
-- runners.esp32: reset after flashing requested
FATAL ERROR: required program C:/Users/<redacted>/modules/hal/espressif\tools\esptool_py\esptool.py not found;
 install it or add its location to PATH
 
(.venv) PS C:\Users\<redacted>\zephyr> git apply ..\patch.txt

(.venv) PS C:\Users\<redacted>\zephyr> git status
On branch main
Your branch is up to date with 'origin/main'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
        modified:   scripts/west_commands/runners/esp32.py

no changes added to commit (use "git add" and/or "git commit -a")
(.venv) PS C:\Users\<redacted>\zephyr> git diff .\scripts\
diff --git a/scripts/west_commands/runners/esp32.py b/scripts/west_commands/runners/esp32.py
index 619c16460e8..78ffd2311a5 100644
--- a/scripts/west_commands/runners/esp32.py
+++ b/scripts/west_commands/runners/esp32.py
@@ -91,9 +91,8 @@ class Esp32BinaryRunner(ZephyrBinaryRunner):
         if args.esp_tool:
             espidf = args.esp_tool
         else:
-            espidf = path.join(args.esp_idf_path, 'tools', 'esptool_py',
-                               'esptool.py')
-
+            espidf = os.path.normpath(path.join(args.esp_idf_path, 'tools', 'esptool_py',
+                                                'esptool.py')).replace('\\', '/')
         return Esp32BinaryRunner(
             cfg, args.esp_device, boot_address=args.esp_boot_address,
             part_table_address=args.esp_partition_table_address,
@@ -105,7 +104,10 @@ class Esp32BinaryRunner(ZephyrBinaryRunner):
             encrypt=args.esp_encrypt, no_stub=args.esp_no_stub)

     def do_run(self, command, **kwargs):
-        self.require(self.espidf)
+        if not os.path.exists(self.espidf):
+            self.logger.error(f"Expected esptool.py at: {self.espidf}")
+            raise RuntimeError("Cannot continue without esptool.py")
+

         # Add Python interpreter
         cmd_flash = [sys.executable, self.espidf, '--chip', 'auto']
(.venv) PS C:\Users\<redacted>\zephyr> west flash
-- west flash: rebuilding
ninja: no work to do.
-- west flash: using runner esp32
-- runners.esp32: reset after flashing requested
-- runners.esp32: Flashing esp32 chip on None (921600bps)
esptool.py v4.8.1
Found 2 serial ports
Serial port COM3
Connecting...
Detecting chip type... ESP32-C6
Chip is ESP32-C6FH4 (QFN32) (revision v0.1)
Features: WiFi 6, BT 5, IEEE802.15.4
Crystal is 40MHz
MAC: 54:32:04:ff:fe:11:ba:c4
BASE MAC: 54:32:04:11:ba:c4
MAC_EXT: ff:fe
Uploading stub...
Running stub...
Stub running...
Changing baud rate to 921600
Changed.
Configuring flash size...
Flash will be erased from 0x00000000 to 0x00023fff...
Wrote 147456 bytes at 0x00000000 in 1.1 seconds (1039.6 kbit/s)...
Hash of data verified.

Leaving...
Hard resetting via RTS pin...

@sylvioalves
Copy link
Collaborator

Did this PR fix the issue in your environment?

Yes.

I didn't apply the entire patch at first. This small change was enough to get past the issue:

diff --git a/scripts/west_commands/runners/esp32.py b/scripts/west_commands/runners/esp32.py
index 619c16460e8..009e32d161e 100644
--- a/scripts/west_commands/runners/esp32.py
+++ b/scripts/west_commands/runners/esp32.py
@@ -105,7 +105,7 @@ class Esp32BinaryRunner(ZephyrBinaryRunner):
encrypt=args.esp_encrypt, no_stub=args.esp_no_stub)

     def do_run(self, command, **kwargs):
-        self.require(self.espidf)
+        #self.require(self.espidf)

         # Add Python interpreter
         cmd_flash = [sys.executable, self.espidf, '--chip', 'auto']

For due diligence I also tried applying the entire patch, below are the build logs showing before/after.
Build Logs

Great, thanks for the confirmation. Sounds we can indeed merge this.

@sylvioalves
Copy link
Collaborator

@carlescufi, ping so you can review this back again.

@sylvioalves
Copy link
Collaborator

@kartben can we have this merged?

@kartben
Copy link
Collaborator

kartben commented Jun 2, 2025

@carlescufi please revisit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: West West utility platform: ESP32 Espressif ESP32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants