-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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
base: main
Are you sure you want to change the base?
Conversation
Hello @rnicolas, and thank you very much for your first pull request to the Zephyr project! |
@@ -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): |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
...
Squash your commits and force push, no fixup commits allowed. |
@rnicolas Your email does not match the signed-off. Can you update accordingly? |
@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? |
@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:
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>
I recently ran into this issue as well. It was quite easy to just remove the |
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... |
Great, thanks for the confirmation. Sounds we can indeed merge this. |
@carlescufi, ping so you can review this back again. |
@kartben can we have this merged? |
@carlescufi please revisit |
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.