Implement combined factory bin creation with filesystem support #5274
Conversation
WalkthroughAdds two PlatformIO post-build scripts: Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @pio-scripts/pos-esp32.py:
- Around line 145-162: Add a timeout to the HTTP call and close the file via a
context manager: change the requests.get call in the download block
(requests.get(url)) to include a sensible timeout (e.g., timeout=10 or a
configurable constant) and replace the raw open(target, "wb").write(...) with a
with open(target, "wb") as f: f.write(response.content) pattern to avoid leaking
file handles; keep the existing filename/rename logic and error handling intact.
- Around line 173-177: The subprocess return code from calling mklittlefs
(assigned to returncode when invoking subprocess.call with cmd built from tool,
filesystem_dir, fs_size, and littlefs.bin) is ignored; update the function
(where tool, cmd, and returncode are defined) to check returncode and handle
failures (e.g., log/error and return False or raise an exception) instead of
always returning True so callers can detect mklittlefs errors.
🧹 Nitpick comments (10)
pio-scripts/copy_factory.py (5)
1-5: Remove unused import.The
gzipmodule is imported but never used in this file.Proposed fix
Import('env') import os import shutil -import gzip import json
10-16: Handle non-tuple items in CPPDEFINES.
CPPDEFINEScan contain plain strings (not just tuples). Accessingitem[0]on a string would compare individual characters, potentially causing incorrect matches or unexpected behavior.Proposed fix
def _get_cpp_define_value(env, define): - define_list = [item[-1] for item in env["CPPDEFINES"] if item[0] == define] + define_list = [ + item[-1] for item in env["CPPDEFINES"] + if isinstance(item, (list, tuple)) and len(item) >= 2 and item[0] == define + ] if define_list: return define_list[0] return None
18-20: Avoid mutable default argument.Using a mutable list as a default argument can cause unexpected behavior if it gets modified. Use
Noneand initialize inside the function.Proposed fix
-def _create_dirs(dirs=["factory"]): +def _create_dirs(dirs=None): + if dirs is None: + dirs = ["factory"] for d in dirs: os.makedirs(os.path.join(OUTPUT_DIR, d), exist_ok=True)
22-31: Add error handling for package.json parsing.If
package.jsonis missing or contains invalid JSON, this will raise an unhandled exception, causing the build to fail with an unclear error message.Proposed fix
def get_release_basename(env): release_name_def = _get_cpp_define_value(env, "WLED_RELEASE_NAME") if not release_name_def: return None release_name = release_name_def.replace("\\\"", "") - with open("package.json", "r") as package: - version = json.load(package)["version"] + try: + with open("package.json", "r") as package: + version = json.load(package)["version"] + except (FileNotFoundError, json.JSONDecodeError, KeyError) as e: + print(f"Warning: Could not read version from package.json: {e}") + return None return f"WLED_{version}_{release_name}"
50-53: Fix grammatical issue in log message.Missing punctuation makes the message harder to read.
Proposed fix
# copy factory binary if os.path.isfile(source_factory): - print(f"Found factory binary Copying {source_factory} to {factory_file}") + print(f"Found factory binary. Copying {source_factory} to {factory_file}") shutil.copy(source_factory, factory_file)pio-scripts/pos-esp32.py (5)
64-88: Improve membership tests and variable naming.Lines 66 and 68 use
not "x" in yinstead of the preferred"x" not in y. Line 75 useslas a variable name which is ambiguous (can be confused with1).Proposed fix
def esp32_detect_flashsize(): uploader = env.subst("$UPLOADER") - if not "upload" in COMMAND_LINE_TARGETS: + if "upload" not in COMMAND_LINE_TARGETS: return "4MB",False - if not "esptool" in uploader: + if "esptool" not in uploader: return "4MB",False else: esptool_flags = ["flash-id"] esptool_cmd = [env["PYTHONEXE"], esptoolpy] + esptool_flags try: output = subprocess.run(esptool_cmd, capture_output=True).stdout.splitlines() - for l in output: - if l.decode().startswith("Detected flash size: "): - size = (l.decode().split(": ")[1]) + for line in output: + if line.decode().startswith("Detected flash size: "): + size = (line.decode().split(": ")[1]) print("Did get flash size:", size)
92-108: Move import to top of file.The
hashlibimport on line 96 is inside the function. While functional, imports are conventionally placed at the top of the file for better readability and to avoid repeated import overhead.Proposed fix
Add at top of file with other imports:
import hashlibThen remove from inside the function:
def patch_partitions_bin(size_string): partition_bin_path = os.path.normpath(join(env.subst("$BUILD_DIR"), "partitions.bin")) with open(partition_bin_path, 'r+b') as file: binary_data = file.read(0xb0) - import hashlib bin_list = list(binary_data)
26-26: Useos.path.existsinstead ofgenericpath.exists.Importing
existsfromgenericpathis unusual. The standard approach is to useos.path.existswhich is already available sinceosis imported.Proposed fix
-from genericpath import existsThen at line 269, change:
- if exists(fs_bin): + if os.path.exists(fs_bin):
188-191: Add error handling for partition file.If the partition file specified in
build.partitionsdoesn't exist, this will raise an unhandledFileNotFoundError, causing the build to fail with an unclear error.Proposed fix
- with open(env.BoardConfig().get("build.partitions")) as csv_file: + partitions_file = env.BoardConfig().get("build.partitions") + if not os.path.exists(partitions_file): + print(Fore.RED + f"Partition file not found: {partitions_file}") + return + with open(partitions_file) as csv_file: print() - print("Read partitions from ",env.BoardConfig().get("build.partitions")) + print("Read partitions from ", partitions_file)
292-297: Check subprocess result for merge_bin.The subprocess result is checked for non-zero return code, but only prints an error without stopping the build. Consider raising an exception or returning early to prevent downstream issues with a corrupted/missing factory binary.
Proposed fix
if("safeboot" not in firmware_name): cmdline = [env.subst("$PYTHONEXE")] + [env.subst("$OBJCOPY")] + normalize_paths(cmd) # print('Command Line: %s' % cmdline) result = subprocess.run(cmdline, text=True, check=False, stdout=subprocess.DEVNULL) if result.returncode != 0: print(Fore.RED + f"esptool create firmware failed with exit code: {result.returncode}") + raise Exception(f"Failed to create factory binary (exit code: {result.returncode})")
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pio-scripts/copy_factory.pypio-scripts/pos-esp32.pyplatformio_override.sample.ini
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to platformio.ini : Use platformio.ini as the single source of truth for hardware build targets and settings
Applied to files:
platformio_override.sample.ini
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Build at least one hardware environment locally (e.g., `pio run -e esp32dev`) before marking work complete
Applied to files:
platformio_override.sample.ini
📚 Learning: 2025-11-22T20:33:14.840Z
Learnt from: softhack007
Repo: wled/WLED PR: 5107
File: boards/adafruit_matrixportal_esp32s3.json:1-66
Timestamp: 2025-11-22T20:33:14.840Z
Learning: In WLED, board JSON files in the boards/ directory may reference partition files that don't exist or have slightly different names than actual files in tools/. This is intentional because platformio.ini or platformio_override.sample.ini explicitly override the partition file using the board_build.partitions setting, which takes precedence over the board JSON partition reference. This pattern exists in multiple local board definitions and should not be flagged as an issue.
Applied to files:
pio-scripts/pos-esp32.py
🧬 Code graph analysis (1)
pio-scripts/copy_factory.py (1)
tools/cdata.js (1)
version(102-102)
🪛 Ruff (0.14.10)
pio-scripts/copy_factory.py
1-1: Undefined name Import
(F821)
18-18: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
33-33: Unused function argument: source
(ARG001)
33-33: Unused function argument: target
(ARG001)
55-55: Undefined name env
(F821)
pio-scripts/pos-esp32.py
44-44: Undefined name DefaultEnvironment
(F821)
66-66: Test for membership should be not in
Convert to not in
(E713)
68-68: Test for membership should be not in
Convert to not in
(E713)
72-72: Consider [env["PYTHONEXE"], esptoolpy, *esptool_flags] instead of concatenation
Replace with [env["PYTHONEXE"], esptoolpy, *esptool_flags]
(RUF005)
74-74: subprocess call: check for execution of untrusted input
(S603)
75-75: Ambiguous variable name: l
(E741)
85-85: Consider moving this statement to an else block
(TRY300)
104-104: Probable use of insecure hash functions in hashlib: md5
(S324)
114-114: Do not catch blind exception: Exception
(BLE001)
145-145: Probable use of requests call without timeout
(S113)
175-175: Local variable returncode is assigned to but never used
Remove assignment to unused variable returncode
(F841)
175-175: subprocess call: check for execution of untrusted input
(S603)
180-180: Unused function argument: source
(ARG001)
180-180: Unused function argument: target
(ARG001)
248-248: Create your own exception
(TRY002)
293-293: Consider iterable unpacking instead of concatenation
Replace with iterable unpacking
(RUF005)
295-295: subprocess call: check for execution of untrusted input
(S603)
300-300: Unused lambda argument: target
(ARG005)
300-300: Unused lambda argument: source
(ARG005)
300-300: Unused lambda argument: env
(ARG005)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: wled_build / Build Environments (esp8266_2m)
- GitHub Check: wled_build / Build Environments (esp32S3_wroom2)
- GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
- GitHub Check: wled_build / Build Environments (esp32c3dev_qio)
- GitHub Check: wled_build / Build Environments (esp32s3_4M_qspi)
- GitHub Check: wled_build / Build Environments (usermods)
- GitHub Check: wled_build / Build Environments (esp32s3dev_16MB_opi)
- GitHub Check: wled_build / Build Environments (esp32_wrover)
- GitHub Check: wled_build / Build Environments (esp32_eth)
- GitHub Check: wled_build / Build Environments (lolin_s2_mini)
- GitHub Check: wled_build / Build Environments (esp32dev_debug)
- GitHub Check: wled_build / Build Environments (esp32dev)
🔇 Additional comments (1)
platformio_override.sample.ini (1)
649-656: Configuration is correct.The factory build environment properly extends
esp32devand adds the necessary components for factory binary generation. The script filenamepos-esp32.pyis the actual file in the repository (not a typo), and all references are correct.
|
@spiro-c nice addition, thanks 👍 While playing with your PR, i got a build error with below buildenv, but only if I start completely "fresh" - delete buildenv [env:esps3_WROOM2_factory]
extends = env:esp32S3_wroom2
board_build.filesystem = littlefs
extra_scripts = post:pio-scripts/pos-esp32.py
post:pio-scripts/copy_factory.py
${scripts_defaults.extra_scripts}
custom_files_upload = ;; https://raw.githubusercontent.com/LedFx/LedFx/main/ledfx_assets/gifs/cat-space.gif ;; if set to no_files, then no files are uploaded
; upload/presets.json ;; uncomment to upload local file from folder
wled00/data/favicon.ico ;; uncomment to upload local file from folderVSCode output window Compiling .pio\build\esps3_WROOM2_factory\FrameworkArduino\wiring_pulse.c.o
Compiling .pio\build\esps3_WROOM2_factory\FrameworkArduino\wiring_shift.c.o
Archiving .pio\build\esps3_WROOM2_factory\libFrameworkArduino.a
Linking .pio\build\esps3_WROOM2_factory\firmware.elf
Checking linked optional modules (usermods) in map file
INFO: 1 libraries linked as WLED optional/user modules
INFO: 1 usermod object entries
Retrieving maximum program size .pio\build\esps3_WROOM2_factory\firmware.elf
Checking size .pio\build\esps3_WROOM2_factory\firmware.elf
Advanced Memory Usage is available via "PlatformIO Home > Project Inspect"
RAM: [= ] 14.5% (used 47444 bytes from 327680 bytes)
Flash: [==== ] 38.8% (used 1221977 bytes from 3145728 bytes)
Building .pio\build\esps3_WROOM2_factory\firmware.bin
esptool.py v4.6.2
Creating esp32s3 image...
Merged 2 ELF sections
Successfully created esp32s3 image.
Read partitions from tools/WLED_ESP32_16MB_9MB_FS.csv
-----------*** [.pio\build\esps3_WROOM2_factory\firmware.bin] No such file or directory
---------------------------------------------------------
nvs, data, nvs, 0x9000, 0x5000,
otadata data ota 0xe000 0x2000
app0 app ota_0 0x10000 0x300000
app1 app ota_1 0x310000 0x300000
spiffs data spiffs 0x610000 0x9E0000
============================================================================================================ [FAILED] Took 77.28 secondsany idea? |
|
@softhack007 I was able to reproduce your error look like the problem is how the files are defined in this is working no problem [env:esps3_WROOM2_factory]
extends = env:esp32S3_wroom2
board_build.filesystem = littlefs
extra_scripts = post:pio-scripts/pos-esp32.py
post:pio-scripts/copy_factory.py
${scripts_defaults.extra_scripts}
custom_files_upload = wled00/data/favicon.ico
; https://raw.githubusercontent.com/LedFx/LedFx/main/ledfx_assets/gifs/cat-space.gif ;; if set to no_files, then no files are uploaded
; upload/presets.json ;; uncomment to upload local file from folder
; wled00/data/favicon.ico ;; uncomment to upload local file from folderBuilding in release mode
Compiling .pio\build\esps3_WROOM2_factory\src\wled_server.cpp.o
Linking .pio\build\esps3_WROOM2_factory\firmware.elf
Checking linked optional modules (usermods) in map file
INFO: 1 libraries linked as WLED optional/user modules
INFO: 1 usermod object entries
Retrieving maximum program size .pio\build\esps3_WROOM2_factory\firmware.elf
Checking size .pio\build\esps3_WROOM2_factory\firmware.elf
Advanced Memory Usage is available via "PlatformIO Home > Project Inspect"
RAM: [= ] 14.5% (used 47444 bytes from 327680 bytes)
Flash: [==== ] 38.8% (used 1221977 bytes from 3145728 bytes)
Building .pio\build\esps3_WROOM2_factory\firmware.bin
esptool.py v4.6.2
Creating esp32s3 image...
Merged 2 ELF sections
Successfully created esp32s3 image.
Read partitions from tools/WLED_ESP32_16MB_9MB_FS.csv
--------------------------------------------------------------------
nvs, data, nvs, 0x9000, 0x5000,
otadata data ota 0xe000 0x2000
app0 app ota_0 0x10000 0x300000
app1 app ota_1 0x310000 0x300000
spiffs data spiffs 0x610000 0x9E0000
wled00/data/favicon.ico
/favicon.ico
coredump data coredump 64K
Offset | File
- 0x0000 | D:\Git\spiroc\WLED-my\.pio\build\esps3_WROOM2_factory\bootloader.bin
- 0x8000 | D:\Git\spiroc\WLED-my\.pio\build\esps3_WROOM2_factory\partitions.bin
- 0xe000 | C:\Users\blago\.platformio\packages\framework-arduinoespressif32\tools\partitions\boot_app0.bin
- 0x10000 | D:\Git\spiroc\WLED-my\.pio\build\esps3_WROOM2_factory\firmware.bin
- 0x610000 | D:\Git\spiroc\WLED-my\.pio\build\esps3_WROOM2_factory\littlefs.bin
Will use custom upload command for flashing operation to add file system defined for this build target.
copy_factory([".pio\build\esps3_WROOM2_factory\firmware.bin"], [".pio\build\esps3_WROOM2_factory\firmware.elf"])
Found factory binary Copying D:\Git\spiroc\WLED-my\.pio\build\esps3_WROOM2_factory\firmware.factory.bin to build_output\factory\WLED_0.16.0-alpha_ESP32-S3_WROOM-2.factory.bin
bin_rename_copy([".pio\build\esps3_WROOM2_factory\firmware.bin"], [".pio\build\esps3_WROOM2_factory\firmware.elf"])
Copying .pio\build\esps3_WROOM2_factory\firmware.bin to build_output\release\WLED_0.16.0-alpha_ESP32-S3_WROOM-2.bin
Found linker mapfile D:\Git\spiroc\WLED-my\.pio\build\esps3_WROOM2_factory\firmware.map
========================================================================== [SUCCESS] Took 32.92 seconds ==========================================================================
Environment Status Duration
-------------------- -------- ------------
esps3_WROOM2_factory SUCCESS 00:00:32.920 |
spiro-c
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
pio-scripts/pos-esp32.py (1)
158-159: File handle leak: use context manager for writing downloaded content.The file is opened without a context manager, which can leak the file handle if an error occurs.
Proposed fix
target = os.path.normpath(join(filesystem_dir, filename)) - open(target, "wb").write(response.content) + with open(target, "wb") as f: + f.write(response.content)
🧹 Nitpick comments (7)
pio-scripts/copy_factory.py (3)
17-19: Avoid mutable default argument.Using a mutable default argument (
dirs=["factory"]) is a Python anti-pattern. While safe here since the list isn't modified, it's better to useNoneand initialize inside the function.Proposed fix
-def _create_dirs(dirs=["factory"]): +def _create_dirs(dirs=None): + if dirs is None: + dirs = ["factory"] for d in dirs: os.makedirs(os.path.join(OUTPUT_DIR, d), exist_ok=True)
21-30: Consider adding error handling for package.json access.If
package.jsonis missing or malformed, this function will raise an unhandled exception and fail the build. Consider wrapping in try/except to gracefully fall back toNone.Proposed fix
def get_release_basename(env): release_name_def = _get_cpp_define_value(env, "WLED_RELEASE_NAME") if not release_name_def: return None release_name = release_name_def.replace("\\\"", "") - with open("package.json", "r") as package: - version = json.load(package)["version"] - - return f"WLED_{version}_{release_name}" + try: + with open("package.json", "r") as package: + version = json.load(package)["version"] + return f"WLED_{version}_{release_name}" + except (FileNotFoundError, json.JSONDecodeError, KeyError) as e: + print(f"Warning: Could not read version from package.json: {e}") + return None
49-52: Minor: Fix punctuation in log message.The print statement is missing punctuation between sentences.
Proposed fix
# copy factory binary if os.path.isfile(source_factory): - print(f"Found factory binary Copying {source_factory} to {factory_file}") + print(f"Found factory binary. Copying {source_factory} to {factory_file}") shutil.copy(source_factory, factory_file)pio-scripts/pos-esp32.py (4)
26-39: Consider usingos.path.existsinstead of importing fromgenericpath.The
existsfunction is imported fromgenericpathbutos.path.existsis more conventional and already available sinceosis imported.Proposed fix
-from genericpath import existsThen on line 271, change:
- if exists(fs_bin): + if os.path.exists(fs_bin):
64-88: Fix exception handling and style issues.
- Lines 66, 68: Use
not ininstead ofnot X in Yfor readability.- Line 75: Rename variable
lto avoid ambiguity with1.- Line 86:
CalledProcessErroris never raised becausesubprocess.run()doesn't raise it unlesscheck=Trueis passed. The exception block is dead code.Proposed fix
def esp32_detect_flashsize(): uploader = env.subst("$UPLOADER") - if not "upload" in COMMAND_LINE_TARGETS: + if "upload" not in COMMAND_LINE_TARGETS: return "4MB",False - if not "esptool" in uploader: + if "esptool" not in uploader: return "4MB",False else: esptool_flags = ["flash-id"] esptool_cmd = [env["PYTHONEXE"], esptoolpy] + esptool_flags try: output = subprocess.run(esptool_cmd, capture_output=True).stdout.splitlines() - for l in output: - if l.decode().startswith("Detected flash size: "): - size = (l.decode().split(": ")[1]) + for line in output: + if line.decode().startswith("Detected flash size: "): + size = (line.decode().split(": ")[1]) print("Did get flash size:", size) stored_flash_size_mb = env.BoardConfig().get("upload.flash_size") stored_flash_size = int(stored_flash_size_mb.split("MB")[0]) * 0x100000 detected_flash_size = int(size.split("MB")[0]) * 0x100000 if detected_flash_size > stored_flash_size: env.BoardConfig().update("upload.flash_size", size) return size, True return "4MB",False - except subprocess.CalledProcessError as exc: - print(Fore.YELLOW + "Did get chip info failed with " + str(exc)) + except Exception as exc: + print(Fore.YELLOW + "Flash size detection failed: " + str(exc)) return "4MB",False
249-250: Consider using a more specific exception type.Raising a base
Exceptionworks but a more descriptive exception (e.g.,RuntimeErroror a customBuildError) would be cleaner.Proposed fix
if (fw_size > max_size): - raise Exception(Fore.RED + "firmware binary too large: %d > %d" % (fw_size, max_size)) + raise RuntimeError(Fore.RED + "firmware binary too large: %d > %d" % (fw_size, max_size))
190-194: Specify encoding when opening CSV file.For cross-platform compatibility, explicitly specify the encoding when opening the partitions CSV file.
Proposed fix
- with open(env.BoardConfig().get("build.partitions")) as csv_file: + with open(env.BoardConfig().get("build.partitions"), encoding="utf-8") as csv_file:
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pio-scripts/copy_factory.pypio-scripts/pos-esp32.py
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-29T00:22:34.115Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.cpp:40-46
Timestamp: 2025-08-29T00:22:34.115Z
Learning: For ESP32/ESP8266 HTTP requests in WLED, set timeouts on the underlying WiFiClient (using client.setTimeout()) before calling http_.begin(), rather than using HTTPClient::setTimeout(). This pattern is used successfully in the Klipper usermod to prevent connection hangs.
Applied to files:
pio-scripts/pos-esp32.py
📚 Learning: 2025-08-29T00:22:34.115Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.cpp:40-46
Timestamp: 2025-08-29T00:22:34.115Z
Learning: The HTTPClient::setTimeout() method in ESP32/ESP8266 may not prevent hanging in all scenarios, particularly during connection establishment or DNS resolution phases.
Applied to files:
pio-scripts/pos-esp32.py
📚 Learning: 2025-11-22T20:33:14.840Z
Learnt from: softhack007
Repo: wled/WLED PR: 5107
File: boards/adafruit_matrixportal_esp32s3.json:1-66
Timestamp: 2025-11-22T20:33:14.840Z
Learning: In WLED, board JSON files in the boards/ directory may reference partition files that don't exist or have slightly different names than actual files in tools/. This is intentional because platformio.ini or platformio_override.sample.ini explicitly override the partition file using the board_build.partitions setting, which takes precedence over the board JSON partition reference. This pattern exists in multiple local board definitions and should not be flagged as an issue.
Applied to files:
pio-scripts/pos-esp32.py
🪛 Ruff (0.14.10)
pio-scripts/pos-esp32.py
44-44: Undefined name DefaultEnvironment
(F821)
66-66: Test for membership should be not in
Convert to not in
(E713)
68-68: Test for membership should be not in
Convert to not in
(E713)
72-72: Consider [env["PYTHONEXE"], esptoolpy, *esptool_flags] instead of concatenation
Replace with [env["PYTHONEXE"], esptoolpy, *esptool_flags]
(RUF005)
74-74: subprocess call: check for execution of untrusted input
(S603)
75-75: Ambiguous variable name: l
(E741)
85-85: Consider moving this statement to an else block
(TRY300)
104-104: Probable use of insecure hash functions in hashlib: md5
(S324)
114-114: Do not catch blind exception: Exception
(BLE001)
175-175: subprocess call: check for execution of untrusted input
(S603)
182-182: Unused function argument: source
(ARG001)
182-182: Unused function argument: target
(ARG001)
250-250: Create your own exception
(TRY002)
295-295: Consider iterable unpacking instead of concatenation
Replace with iterable unpacking
(RUF005)
297-297: subprocess call: check for execution of untrusted input
(S603)
302-302: Unused lambda argument: target
(ARG005)
302-302: Unused lambda argument: source
(ARG005)
302-302: Unused lambda argument: env
(ARG005)
pio-scripts/copy_factory.py
1-1: Undefined name Import
(F821)
17-17: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
32-32: Unused function argument: source
(ARG001)
32-32: Unused function argument: target
(ARG001)
54-54: Undefined name env
(F821)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: wled_build / Build Environments (esp8266_2m_compat)
- GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
- GitHub Check: wled_build / Build Environments (usermods)
🔇 Additional comments (6)
pio-scripts/copy_factory.py (2)
1-7: Imports and OUTPUT_DIR setup look good.The
Import('env')is the standard PlatformIO/SCons idiom for importing the build environment. The commented alternative on line 7 can be removed to reduce clutter.
9-15: LGTM!The function correctly extracts CPPDEFINE values. The logic handles the case where the define is not found by returning
None.pio-scripts/pos-esp32.py (4)
58-62: LGTM!Simple utility function that correctly normalizes path separators for cross-platform compatibility.
92-108: LGTM!The MD5 hash usage here is correct—ESP32 partition tables use MD5 for integrity verification per ESP-IDF specification, not for security purposes. The static analysis warning about insecure hash is a false positive in this context.
173-179: Good: subprocess return code is now checked.The return code from
mklittlefsis properly checked and the function returnsFalseon failure. This addresses the concern from the previous review.
301-303: LGTM!Standard pattern for registering a silent post-action in PlatformIO/SCons. The unused lambda arguments are required by the SCons
strfunctionsignature.
|
That sounds really promising. If you have that i could make a test webinstaller fork that uses those to make testing a bit easier |
|
This need to work also for c3 and s2 i dont have that boards to test |
|
I was trouble shooting problem Quindor was having when using the scrip to build FS for the if "MKSPIFFSTOOL" in env:
print()
print(Fore.YELLOW + "Replacing MKSPIFFSTOOL to use mklittlefs")
print()
env.Replace( MKSPIFFSTOOL=platform.get_package_dir("tool-mklittlefs") + '/mklittlefs' )
tool = env.subst(env["MKSPIFFSTOOL"])
else:
print()
print(Fore.GREEN + "Using MKFSTOOL to create littlefs.bin")
print()
tool = env.subst(env["MKFSTOOL"])
cmd = (tool,"-c",filesystem_dir,"-s",fs_size,join(env.subst("$BUILD_DIR"),"littlefs.bin"))also the |
3 participants
This modified version of the tasmota post_esp32.py script for building combine binary file that is including
bootloader.binpartition.binboot_app0.binfirmware.binandlittlefs.binif thecustom_files_uploadis used to build the filesystem and uploaded on the espalso there is helper script to pull the factory bin file in the
build_output\factoryfolderI test it on standard esp32 4M esp32-s3-8MB-8MB psram and esp32s3-8MB no psram wall was working good
I also build the environments using all the esp32
board_build.partitionsfrom default to extriime it correctly read the offsetsSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.