-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Fix up usermod libArchive settings #4669
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
Merged
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
7998650
Fix up usermod libArchive settings
willmmiles 6464c62
load_usermods: Enforce CPPPATH type
willmmiles a8dd243
Revert "Usermods: Remove libArchive"
willmmiles 849d5e6
Add libArchive to other usermods
willmmiles ee38641
load_usermods: Make missing libArchive an error
willmmiles 999637f
Validate usermods at link time
willmmiles 24ab295
Add unambiguous usermod list to info
willmmiles ac61eb4
validate_usermods: Fix inverted check
willmmiles 358e38e
validate_usermods: Ensure map file is created
willmmiles 242df4b
validate_usermods: Fix old ESP32 platform
willmmiles 7ea510e
validate_usermods: Improve message
willmmiles 792a7aa
load_usermods: Resolve folder paths
willmmiles 75cd411
Improve all-usermod handling
willmmiles 0a7d3a9
load_usermods: Simplify load code
willmmiles 75c95d8
usermods/*/setup_deps.py: Check lib_deps for deps
willmmiles 309c8d6
Generalize module link validation
willmmiles e80a7c6
usermod_v2_HttpPullLightControl: Add usermod object
willmmiles f362315
Usermod script cleanup
willmmiles File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| import re | ||
| from pathlib import Path # For OS-agnostic path manipulation | ||
| from typing import Iterable | ||
| from click import secho | ||
| from SCons.Script import Action, Exit | ||
| from platformio.builder.tools.piolib import LibBuilderBase | ||
|
|
||
|
|
||
| def is_wled_module(env, dep: LibBuilderBase) -> bool: | ||
| """Returns true if the specified library is a wled module | ||
| """ | ||
| usermod_dir = Path(env["PROJECT_DIR"]).resolve() / "usermods" | ||
| return usermod_dir in Path(dep.src_dir).parents or str(dep.name).startswith("wled-") | ||
|
|
||
|
|
||
| def read_lines(p: Path): | ||
| """ Read in the contents of a file for analysis """ | ||
| with p.open("r", encoding="utf-8", errors="ignore") as f: | ||
| return f.readlines() | ||
|
|
||
|
|
||
| def check_map_file_objects(map_file: list[str], dirs: Iterable[str]) -> set[str]: | ||
| """ Identify which dirs contributed to the final build | ||
|
|
||
| Returns the (sub)set of dirs that are found in the output ELF | ||
| """ | ||
| # Pattern to match symbols in object directories | ||
| # Join directories into alternation | ||
| usermod_dir_regex = "|".join([re.escape(dir) for dir in dirs]) | ||
| # Matches nonzero address, any size, and any path in a matching directory | ||
| object_path_regex = re.compile(r"0x0*[1-9a-f][0-9a-f]*\s+0x[0-9a-f]+\s+\S+[/\\](" + usermod_dir_regex + r")[/\\]\S+\.o") | ||
|
|
||
| found = set() | ||
| for line in map_file: | ||
| matches = object_path_regex.findall(line) | ||
| for m in matches: | ||
| found.add(m) | ||
| return found | ||
|
|
||
|
|
||
| def count_usermod_objects(map_file: list[str]) -> int: | ||
| """ Returns the number of usermod objects in the usermod list """ | ||
| # Count the number of entries in the usermods table section | ||
| return len([x for x in map_file if ".dtors.tbl.usermods.1" in x]) | ||
|
|
||
|
|
||
| def validate_map_file(source, target, env): | ||
| """ Validate that all modules appear in the output build """ | ||
| build_dir = Path(env.subst("$BUILD_DIR")) | ||
| map_file_path = build_dir / env.subst("${PROGNAME}.map") | ||
|
|
||
| if not map_file_path.exists(): | ||
| secho(f"ERROR: Map file not found: {map_file_path}", fg="red", err=True) | ||
| Exit(1) | ||
|
|
||
| # Identify the WLED module source directories | ||
| module_lib_builders = [builder for builder in env.GetLibBuilders() if is_wled_module(env, builder)] | ||
|
|
||
| if env.GetProjectOption("custom_usermods","") == "*": | ||
| # All usermods build; filter non-platform-OK modules | ||
| module_lib_builders = [builder for builder in module_lib_builders if env.IsCompatibleLibBuilder(builder)] | ||
| else: | ||
| incompatible_builders = [builder for builder in module_lib_builders if not env.IsCompatibleLibBuilder(builder)] | ||
| if incompatible_builders: | ||
| secho( | ||
| f"ERROR: Modules {[b.name for b in incompatible_builders]} are not compatible with this platform!", | ||
| fg="red", | ||
| err=True) | ||
| Exit(1) | ||
|
|
||
| # Extract the values we care about | ||
| modules = {Path(builder.build_dir).name: builder.name for builder in module_lib_builders} | ||
| secho(f"INFO: {len(modules)} libraries linked as WLED optional/user modules") | ||
|
|
||
| # Now parse the map file | ||
| map_file_contents = read_lines(map_file_path) | ||
| usermod_object_count = count_usermod_objects(map_file_contents) | ||
| secho(f"INFO: {usermod_object_count} usermod object entries") | ||
|
|
||
| confirmed_modules = check_map_file_objects(map_file_contents, modules.keys()) | ||
| missing_modules = [modname for mdir, modname in modules.items() if mdir not in confirmed_modules] | ||
| if missing_modules: | ||
| secho( | ||
| f"ERROR: No object files from {missing_modules} found in linked output!", | ||
| fg="red", | ||
| err=True) | ||
| Exit(1) | ||
| return None | ||
|
|
||
| Import("env") | ||
| env.Append(LINKFLAGS=[env.subst("-Wl,--Map=${BUILD_DIR}/${PROGNAME}.map")]) | ||
| env.AddPostAction("$BUILD_DIR/${PROGNAME}.elf", Action(validate_map_file, cmdstr='Checking linked optional modules (usermods) in map file')) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| { | ||
| "name": "AHT10_v2", | ||
| "build": { "libArchive": false }, | ||
| "dependencies": { | ||
| "enjoyneering/AHT10":"~1.1.0" | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| { | ||
| "name": "Analog_Clock" | ||
| "name": "Analog_Clock", | ||
| "build": { "libArchive": false } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| { | ||
| "name": "Animated_Staircase" | ||
| "name": "Animated_Staircase", | ||
| "build": { "libArchive": false } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| { | ||
| "name": "BH1750_v2", | ||
| "build": { "libArchive": false }, | ||
| "dependencies": { | ||
| "claws/BH1750":"^1.2.0" | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| { | ||
| "name": "BME280_v2", | ||
| "build": { "libArchive": false }, | ||
| "dependencies": { | ||
| "finitespace/BME280":"~3.0.0" | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| { | ||
| "name": "Battery" | ||
| "name": "Battery", | ||
| "build": { "libArchive": false } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| { | ||
| "name": "Cronixie" | ||
| "name": "Cronixie", | ||
| "build": { "libArchive": false } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| { | ||
| "name": "EXAMPLE", | ||
| "build": { "libArchive": false }, | ||
| "dependencies": {} | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| { | ||
| "name:": "EleksTube_IPS", | ||
| "build": { "libArchive": false }, | ||
| "dependencies": { | ||
| "TFT_eSPI" : "2.5.33" | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| { | ||
| "name": "INA226_v2", | ||
| "build": { "libArchive": false }, | ||
| "dependencies": { | ||
| "wollewald/INA226_WE":"~1.2.9" | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| { | ||
| "name": "Internal_Temperature_v2" | ||
| "name": "Internal_Temperature_v2", | ||
| "build": { "libArchive": false } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| { | ||
| "name": "LD2410_v2", | ||
| "build": { "libArchive": false }, | ||
| "dependencies": { | ||
| "ncmreynolds/ld2410":"^0.1.3" | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| { | ||
| "name": "LDR_Dusk_Dawn_v2" | ||
| "name": "LDR_Dusk_Dawn_v2", | ||
| "build": { "libArchive": false } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| { | ||
| "name": "MY9291", | ||
| "build": { "libArchive": false }, | ||
| "platforms": ["espressif8266"] | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| { | ||
| "name": "PIR_sensor_switch" | ||
| "name": "PIR_sensor_switch", | ||
| "build": { "libArchive": false } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| { | ||
| "name": "PWM_fan", | ||
| "build": { | ||
| "libArchive": false, | ||
| "extraScript": "setup_deps.py" | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,12 @@ | ||
| from platformio.package.meta import PackageSpec | ||
| Import('env') | ||
|
|
||
|
|
||
| usermods = env.GetProjectOption("custom_usermods","").split() | ||
| libs = [PackageSpec(lib).name for lib in env.GetProjectOption("lib_deps",[])] | ||
| # Check for dependencies | ||
| if "Temperature" in usermods: | ||
| if "Temperature" in libs: | ||
| env.Append(CPPDEFINES=[("USERMOD_DALLASTEMPERATURE")]) | ||
| elif "sht" in usermods: | ||
| elif "sht" in libs: | ||
| env.Append(CPPDEFINES=[("USERMOD_SHT")]) | ||
| elif "PWM_fan" in usermods: # The script can be run if this module was previously selected | ||
| elif "PWM_fan" in libs: # The script can be run if this module was previously selected | ||
| raise RuntimeError("PWM_fan usermod requires Temperature or sht to be enabled") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| { | ||
| "name": "RTC" | ||
| "name": "RTC", | ||
| "build": { "libArchive": false } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
🛠️ Refactor suggestion
Consider adding error handling for file operations.
The file reading function should handle potential I/O errors more robustly.
def read_lines(p: Path): """ Read in the contents of a file for analysis """ - with p.open("r", encoding="utf-8", errors="ignore") as f: - return f.readlines() + try: + with p.open("r", encoding="utf-8", errors="ignore") as f: + return f.readlines() + except (OSError, IOError) as e: + secho(f"ERROR: Failed to read file {p}: {e}", fg="red", err=True) + Exit(1)🤖 Prompt for AI Agents
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.
In this use case, a file read error indicates something deeply wrong on the user's build platform, since the script itself adds the required parameters to produce the only file we open. While it's definitely possible to add more sophisticated error messages, I don't judge that it's worth the extra code (and future maintainence) given that the only way to get there is either a hardware failure on the user's build platform or they're messing around with unsupported build options.
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.
@willmmiles Thank you for the clarification! That makes perfect sense - since the script itself adds the linker flags to generate the map file, a file read error would indeed indicate a fundamental build system issue rather than a typical I/O error. Your cost-benefit analysis is spot-on; adding error handling for what would essentially be unrecoverable platform failures isn't worth the maintenance overhead.
I appreciate you taking the time to explain the context and design rationale.
✏️ Learnings added