Skip to content

Commit 300330c

Browse files
committed
update comments
1 parent 1ef6508 commit 300330c

File tree

6 files changed

+49
-50
lines changed

6 files changed

+49
-50
lines changed

builder/frameworks/arduino.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -886,7 +886,7 @@ def get_frameworks_in_current_env():
886886
if flag_custom_sdkconfig and not flag_any_custom_sdkconfig:
887887
call_compile_libs()
888888

889-
# Main logic for Arduino Framework
889+
# Arduino framework configuration and build logic
890890
pioframework = env.subst("$PIOFRAMEWORK")
891891
arduino_lib_compile_flag = env.subst("$ARDUINO_LIB_COMPILE_FLAG")
892892

builder/frameworks/component_manager.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ def _get_or_create_component_yml(self) -> str:
252252
Returns:
253253
Absolute path to the component YAML file
254254
"""
255-
# Try Arduino framework first
255+
# Check Arduino framework directory first
256256
afd = self.config.arduino_framework_dir
257257
framework_yml = str(Path(afd) / "idf_component.yml") if afd else ""
258258
if framework_yml and os.path.exists(framework_yml):

builder/frameworks/espidf.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1672,8 +1672,8 @@ def get_python_exe():
16721672

16731673
ensure_python_venv_available()
16741674

1675-
# ESP-IDF package doesn't contain .git folder, instead package version is specified
1676-
# in a special file "version.h" in the root folder of the package
1675+
# ESP-IDF package version is determined from version.h file
1676+
# since the package distribution doesn't include .git metadata
16771677

16781678
create_version_file()
16791679

@@ -1712,7 +1712,7 @@ def get_python_exe():
17121712

17131713

17141714
#
1715-
# Current build script limitations
1715+
# Known build system limitations
17161716
#
17171717

17181718
if any(" " in p for p in (FRAMEWORK_DIR, BUILD_DIR)):
@@ -1751,12 +1751,12 @@ def get_python_exe():
17511751
LIBSOURCE_DIRS=[str(Path(ARDUINO_FRAMEWORK_DIR) / "libraries")]
17521752
)
17531753

1754-
# Set ESP-IDF version environment variables (needed for proper Kconfig processing)
1754+
# Configure ESP-IDF version environment variables for Kconfig processing
17551755
framework_version = get_framework_version()
17561756
major_version = framework_version.split('.')[0] + '.' + framework_version.split('.')[1]
17571757
os.environ["ESP_IDF_VERSION"] = major_version
17581758

1759-
# Configure CMake arguments with ESP-IDF version
1759+
# Setup CMake configuration arguments
17601760
extra_cmake_args = [
17611761
"-DIDF_TARGET=" + idf_variant,
17621762
"-DPYTHON_DEPS_CHECKED=1",
@@ -1850,7 +1850,7 @@ def get_python_exe():
18501850
env.Depends("$BUILD_DIR/$PROGNAME$PROGSUFFIX", build_bootloader(sdk_config))
18511851

18521852
#
1853-
# Target: ESP-IDF menuconfig
1853+
# ESP-IDF menuconfig target implementation
18541854
#
18551855

18561856
env.AddPlatformTarget(
@@ -1995,8 +1995,8 @@ def _skip_prj_source_files(node):
19951995
):
19961996
project_env = env.Clone()
19971997
if project_target_name != "__idf_main":
1998-
# Manually add dependencies to CPPPATH since ESP-IDF build system doesn't generate
1999-
# this info if the folder with sources is not named 'main'
1998+
# Add dependencies to CPPPATH for non-main source directories
1999+
# ESP-IDF build system requires manual dependency handling for custom source folders
20002000
# https://docs.espressif.com/projects/esp-idf/en/latest/api-guides/build-system.html#rename-main
20012001
project_env.AppendUnique(CPPPATH=app_includes["plain_includes"])
20022002

@@ -2044,7 +2044,7 @@ def _skip_prj_source_files(node):
20442044
#
20452045

20462046
extra_elf2bin_flags = "--elf-sha256-offset 0xb0"
2047-
# https://github.com/espressif/esp-idf/blob/master/components/esptool_py/project_include.cmake#L58
2047+
# Reference: ESP-IDF esptool_py component configuration
20482048
# For chips that support configurable MMU page size feature
20492049
# If page size is configured to values other than the default "64KB" in menuconfig,
20502050
mmu_page_size = "64KB"

builder/main.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
from platformio.util import get_serial_ports
3636
from platformio.compat import IS_WINDOWS
3737

38-
# Initialize environment and configuration
38+
# Initialize SCons environment and project configuration
3939
env = DefaultEnvironment()
4040
platform = env.PioPlatform()
4141
projectconfig = env.GetProjectConfig()
@@ -45,10 +45,10 @@
4545
core_dir = projectconfig.get("platformio", "core_dir")
4646
build_dir = Path(projectconfig.get("platformio", "build_dir"))
4747

48-
# Setup Python virtual environment and get executable paths
48+
# Configure Python environment through centralized platform management
4949
PYTHON_EXE, esptool_binary_path = platform.setup_python_env(env)
5050

51-
# Initialize board configuration and MCU settings
51+
# Load board configuration and determine MCU architecture
5252
board = env.BoardConfig()
5353
board_id = env.subst("$BOARD")
5454
mcu = board.get("build.mcu", "esp32")
@@ -450,7 +450,7 @@ def switch_off_ldf():
450450
if not is_xtensa:
451451
toolchain_arch = "riscv32-esp"
452452

453-
# Initialize integration extra data if not present
453+
# Ensure integration extra data structure exists
454454
if "INTEGRATION_EXTRA_DATA" not in env:
455455
env["INTEGRATION_EXTRA_DATA"] = {}
456456

@@ -460,7 +460,7 @@ def switch_off_ldf():
460460
if ' ' in esptool_binary_path
461461
else esptool_binary_path
462462
)
463-
# Configure build tools and environment variables
463+
# Configure SCons build tools and compiler settings
464464
env.Replace(
465465
__get_board_boot_mode=_get_board_boot_mode,
466466
__get_board_f_flash=_get_board_f_flash,
@@ -636,7 +636,7 @@ def firmware_metrics(target, source, env):
636636
if env.GetProjectOption("custom_esp_idf_size_verbose", False):
637637
print(f"Running command: {' '.join(cmd)}")
638638

639-
# Call esp-idf-size with modified environment
639+
# Execute esp-idf-size with current environment
640640
result = subprocess.run(cmd, check=False, capture_output=False, env=os.environ)
641641

642642
if result.returncode != 0:

builder/penv_setup.py

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141
re.IGNORECASE,
4242
)
4343

44-
# Python dependencies required for the build process
44+
# Python dependencies required for ESP32 platform builds
4545
python_deps = {
4646
"platformio": "https://github.com/pioarduino/platformio-core/archive/refs/tags/v6.1.18.zip",
4747
"pyyaml": ">=6.0.2",
@@ -89,7 +89,7 @@ def setup_pipenv_in_package(env, penv_dir):
8989
str or None: Path to uv executable if uv was used, None if python -m venv was used
9090
"""
9191
if not os.path.exists(penv_dir):
92-
# First try to create virtual environment with uv
92+
# Attempt virtual environment creation using uv package manager
9393
uv_success = False
9494
uv_cmd = None
9595
try:
@@ -125,8 +125,8 @@ def setup_pipenv_in_package(env, penv_dir):
125125
)
126126
)
127127

128-
# Verify that the virtual environment was created properly
129-
# Check for python executable
128+
# Validate virtual environment creation
129+
# Ensure Python executable is available
130130
assert os.path.isfile(
131131
get_executable_path(penv_dir, "python")
132132
), f"Error: Failed to create a proper virtual environment. Missing the `python` binary! Created with uv: {uv_success}"
@@ -432,7 +432,7 @@ def _setup_python_environment_core(env, platform, platformio_dir, should_install
432432
"""
433433
penv_dir = str(Path(platformio_dir) / "penv")
434434

435-
# Setup virtual environment if needed
435+
# Create virtual environment if not present
436436
if env is not None:
437437
# SCons version
438438
used_uv_executable = setup_pipenv_in_package(env, penv_dir)
@@ -457,21 +457,21 @@ def _setup_python_environment_core(env, platform, platformio_dir, should_install
457457
esptool_binary_path = get_executable_path(penv_dir, "esptool")
458458
uv_executable = get_executable_path(penv_dir, "uv")
459459

460-
# Install espressif32 Python dependencies
460+
# Install required Python dependencies for ESP32 platform
461461
if has_internet_connection() or github_actions:
462462
if not install_python_deps(penv_python, used_uv_executable):
463463
sys.stderr.write("Error: Failed to install Python dependencies into penv\n")
464464
sys.exit(1)
465465
else:
466466
print("Warning: No internet connection detected, Python dependency check will be skipped.")
467467

468-
# Install esptool after dependencies (if requested)
468+
# Install esptool package if required
469469
if should_install_esptool:
470470
if env is not None:
471471
# SCons version
472472
install_esptool(env, platform, penv_python, uv_executable)
473473
else:
474-
# Minimal version - install esptool from tl-install provided path
474+
# Minimal setup - install esptool from tool package
475475
_install_esptool_from_tl_install(platform, penv_python, uv_executable)
476476

477477
# Setup certifi environment variables
@@ -491,7 +491,7 @@ def _setup_pipenv_minimal(penv_dir):
491491
str or None: Path to uv executable if uv was used, None if python -m venv was used
492492
"""
493493
if not os.path.exists(penv_dir):
494-
# First try to create virtual environment with uv
494+
# Attempt virtual environment creation using uv package manager
495495
uv_success = False
496496
uv_cmd = None
497497
try:
@@ -528,8 +528,8 @@ def _setup_pipenv_minimal(penv_dir):
528528
sys.stderr.write(f"Error: Failed to create virtual environment: {e}\n")
529529
sys.exit(1)
530530

531-
# Verify that the virtual environment was created properly
532-
# Check for python executable
531+
# Validate virtual environment creation
532+
# Ensure Python executable is available
533533
assert os.path.isfile(
534534
get_executable_path(penv_dir, "python")
535535
), f"Error: Failed to create a proper virtual environment. Missing the `python` binary! Created with uv: {uv_success}"

platform.py

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
from platformio.package.manager.tool import ToolPackageManager
4646

4747

48-
# Import penv_setup functionality using explicit module loading
48+
# Import penv_setup functionality using explicit module loading for centralized Python environment management
4949
penv_setup_path = Path(__file__).parent / "builder" / "penv_setup.py"
5050
spec = importlib.util.spec_from_file_location("penv_setup", str(penv_setup_path))
5151
penv_setup_module = importlib.util.module_from_spec(spec)
@@ -226,7 +226,7 @@ def _check_tl_install_version(self) -> bool:
226226
logger.debug(f"No version check required for {tl_install_name}")
227227
return True
228228

229-
# Check if tool is already installed
229+
# Check current installation status
230230
tl_install_path = self.packages_dir / tl_install_name
231231
package_json_path = tl_install_path / "package.json"
232232

@@ -244,10 +244,10 @@ def _check_tl_install_version(self) -> bool:
244244
logger.warning(f"Installed version for {tl_install_name} unknown, installing {required_version}")
245245
return self._install_tl_install(required_version)
246246

247-
# IMPORTANT: Compare versions correctly
247+
# Compare versions to avoid unnecessary reinstallation
248248
if self._compare_tl_install_versions(installed_version, required_version):
249249
logger.debug(f"{tl_install_name} version {installed_version} is already correctly installed")
250-
# IMPORTANT: Set package as available, but do NOT reinstall
250+
# Mark package as available without reinstalling
251251
self.packages[tl_install_name]["optional"] = True
252252
return True
253253
else:
@@ -305,8 +305,7 @@ def _extract_version_from_url(self, version_string: str) -> str:
305305

306306
def _install_tl_install(self, version: str) -> bool:
307307
"""
308-
Install tool-esp_install ONLY when necessary
309-
and handles backwards compatibility for tl-install.
308+
Install tool-esp_install with version validation and legacy compatibility.
310309
311310
Args:
312311
version: Version string or URL to install
@@ -320,7 +319,7 @@ def _install_tl_install(self, version: str) -> bool:
320319
try:
321320
old_tl_install_exists = old_tl_install_path.exists()
322321
if old_tl_install_exists:
323-
# remove outdated tl-install
322+
# Remove legacy tl-install directory
324323
safe_remove_directory(old_tl_install_path)
325324

326325
if tl_install_path.exists():
@@ -331,17 +330,17 @@ def _install_tl_install(self, version: str) -> bool:
331330
self.packages[tl_install_name]["optional"] = False
332331
self.packages[tl_install_name]["version"] = version
333332
pm.install(version)
334-
# Ensure backward compatibility by removing pio install status indicator
333+
# Remove PlatformIO install marker to prevent version conflicts
335334
tl_piopm_path = tl_install_path / ".piopm"
336335
safe_remove_file(tl_piopm_path)
337336

338337
if (tl_install_path / "package.json").exists():
339338
logger.info(f"{tl_install_name} successfully installed and verified")
340339
self.packages[tl_install_name]["optional"] = True
341340

342-
# Handle old tl-install to keep backwards compatibility
341+
# Maintain backwards compatibility with legacy tl-install references
343342
if old_tl_install_exists:
344-
# Copy tool-esp_install content to tl-install location
343+
# Copy tool-esp_install content to legacy tl-install location
345344
if safe_copy_directory(tl_install_path, old_tl_install_path):
346345
logger.info(f"Content copied from {tl_install_name} to old tl-install location")
347346
else:
@@ -450,7 +449,7 @@ def _run_idf_tools_install(self, tools_json_path: str, idf_tools_path: str, penv
450449

451450
def _check_tool_version(self, tool_name: str) -> bool:
452451
"""Check if the installed tool version matches the required version."""
453-
# Clean up versioned directories FIRST, before any version checks
452+
# Clean up versioned directories before version checks to prevent conflicts
454453
self._cleanup_versioned_tool_directories(tool_name)
455454

456455
paths = self._get_tool_paths(tool_name)
@@ -489,14 +488,14 @@ def install_tool(self, tool_name: str) -> bool:
489488
paths = self._get_tool_paths(tool_name)
490489
status = self._check_tool_status(tool_name)
491490

492-
# Get penv python if available
491+
# Use centrally configured Python executable if available
493492
penv_python = getattr(self, '_penv_python', None)
494493

495-
# Case 1: New installation with idf_tools
494+
# Case 1: Fresh installation using idf_tools.py
496495
if status['has_idf_tools'] and status['has_tools_json']:
497496
return self._install_with_idf_tools(tool_name, paths, penv_python)
498497

499-
# Case 2: Tool already installed, version check
498+
# Case 2: Tool already installed, perform version validation
500499
if (status['has_idf_tools'] and status['has_piopm'] and
501500
not status['has_tools_json']):
502501
return self._handle_existing_tool(tool_name, paths)
@@ -511,7 +510,7 @@ def _install_with_idf_tools(self, tool_name: str, paths: Dict[str, str], penv_py
511510
):
512511
return False
513512

514-
# Copy tool files
513+
# Copy tool metadata to IDF tools directory
515514
target_package_path = Path(IDF_TOOLS_PATH) / "tools" / tool_name / "package.json"
516515

517516
if not safe_copy_file(paths['package_path'], target_package_path):
@@ -534,7 +533,7 @@ def _handle_existing_tool(self, tool_name: str, paths: Dict[str, str]) -> bool:
534533
logger.debug(f"Tool {tool_name} found with correct version")
535534
return True
536535

537-
# Wrong version, reinstall - cleanup is already done in _check_tool_version
536+
# Version mismatch detected, reinstall tool (cleanup already performed)
538537
logger.info(f"Reinstalling {tool_name} due to version mismatch")
539538

540539
# Remove the main tool directory (if it still exists after cleanup)
@@ -623,7 +622,7 @@ def _configure_installer(self) -> None:
623622
logger.error("Error during tool-esp_install version check / installation")
624623
return
625624

626-
# Remove pio install marker to avoid issues when switching versions
625+
# Remove legacy PlatformIO install marker to prevent version conflicts
627626
old_tl_piopm_path = Path(self.packages_dir) / "tl-install" / ".piopm"
628627
if old_tl_piopm_path.exists():
629628
safe_remove_file(old_tl_piopm_path)
@@ -735,17 +734,17 @@ def _configure_filesystem_tools(self, variables: Dict, targets: List[str]) -> No
735734
self._install_filesystem_tool(filesystem, for_download=True)
736735

737736
def setup_python_env(self, env):
738-
"""Setup Python virtual environment and return executable paths."""
739-
# Penv should already be set up in configure_default_packages
737+
"""Configure SCons environment with centrally managed Python executable paths."""
738+
# Python environment is centrally managed in configure_default_packages
740739
if hasattr(self, '_penv_python') and hasattr(self, '_esptool_path'):
741-
# Update SCons environment with penv python
740+
# Update SCons environment with centrally configured Python executable
742741
env.Replace(PYTHONEXE=self._penv_python)
743742
return self._penv_python, self._esptool_path
744743

745744
# This should not happen, but provide fallback
746745
logger.warning("Penv not set up in configure_default_packages, setting up now")
747746

748-
# Use centralized minimal setup as a fallback and propagate into SCons
747+
# Fallback to minimal setup if centralized configuration failed
749748
config = ProjectConfig.get_instance()
750749
core_dir = config.get("platformio", "core_dir")
751750
penv_python, esptool_path = setup_penv_minimal(self, core_dir, install_esptool=True)
@@ -769,7 +768,7 @@ def configure_default_packages(self, variables: Dict, targets: List[str]) -> Any
769768
self._configure_installer()
770769
self._install_esptool_package()
771770

772-
# THEN: Setup Python virtual environment completely
771+
# Complete Python virtual environment setup
773772
config = ProjectConfig.get_instance()
774773
core_dir = config.get("platformio", "core_dir")
775774

0 commit comments

Comments
 (0)