From c88150ff5f8eaf4f72b9be7b2951fa3e08873320 Mon Sep 17 00:00:00 2001 From: Vojtech Piroch Date: Mon, 3 Aug 2026 13:37:33 +0200 Subject: [PATCH] fix(tools): Detect serial port matching the project target Auto-detect used to pick the first Espressif device, even when it did not match IDF_TARGET. With several boards attached, flash/monitor could talk to the wrong chip. Pass the project target into esptool so unmatched ports are skipped. Resolve the port after ensure_build_directory() so the target is known. For monitor on an unconfigured project, probe the connected chip and pass it directly to idf_monitor without configuring the project. --- .gitlab/ci/host-test.yml | 1 + docs/en/api-guides/tools/idf-monitor.rst | 7 + tools/idf_py_actions/serial_ext.py | 147 +++++++++++++------ tools/idf_py_actions/tools.py | 71 +++++++++- tools/test_idf_py/test_serial_ext.py | 172 +++++++++++++++++++++++ 5 files changed, 347 insertions(+), 51 deletions(-) create mode 100644 tools/test_idf_py/test_serial_ext.py diff --git a/.gitlab/ci/host-test.yml b/.gitlab/ci/host-test.yml index 4e58c294ace..e9a39b2f004 100644 --- a/.gitlab/ci/host-test.yml +++ b/.gitlab/ci/host-test.yml @@ -205,6 +205,7 @@ test_tools: - run_cmd pytest --noconftest test_hints.py --junitxml=${IDF_PATH}/XUNIT_HINTS.xml --ignore-result-files ${KNOWN_FAILURE_CASES_FILE_NAME} || stat=1 - run_cmd pytest --noconftest test_idf_qemu.py --junitxml=${IDF_PATH}/XUNIT_IDF_PY_QEMU.xml --ignore-result-files ${KNOWN_FAILURE_CASES_FILE_NAME} || stat=1 - run_cmd pytest --noconftest test_mcp_ext.py --junitxml=${IDF_PATH}/XUNIT_MCP_EXT.xml --ignore-result-files ${KNOWN_FAILURE_CASES_FILE_NAME} || stat=1 + - run_cmd pytest --noconftest test_serial_ext.py --junitxml=${IDF_PATH}/XUNIT_SERIAL_EXT.xml --ignore-result-files ${KNOWN_FAILURE_CASES_FILE_NAME} || stat=1 - cd ${IDF_PATH}/tools/test_bsasm - run_cmd pytest --noconftest test_bsasm.py --junitxml=${IDF_PATH}/XUNIT_BSASM.xml --ignore-result-files ${KNOWN_FAILURE_CASES_FILE_NAME} || stat=1 - cd ${IDF_PATH}/tools/test_mkdfu diff --git a/docs/en/api-guides/tools/idf-monitor.rst b/docs/en/api-guides/tools/idf-monitor.rst index 9e2f5014955..2a467181f30 100644 --- a/docs/en/api-guides/tools/idf-monitor.rst +++ b/docs/en/api-guides/tools/idf-monitor.rst @@ -241,6 +241,13 @@ The ROM ELF file is automatically loaded from a location based on the ``IDF_PATH Set environment variable ``ESP_MONITOR_DECODE`` to ``0`` or call esp_idf_monitor with specific command line option: ``python -m esp_idf_monitor --disable-address-decoding`` to disable address decoding. +.. _idf-monitor-target-detection: + +Automatic Target Detection +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +By default, ``idf.py monitor`` connects to the target that has been set for the project. However, on an unbuilt project where no target has been configured, it can connect to any target — it will automatically detect the chip on the default serial port and pass it to the monitor. This allows running ``idf.py monitor`` on a clean project without having to call ``idf.py set-target`` first. + Target Reset on Connection ~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tools/idf_py_actions/serial_ext.py b/tools/idf_py_actions/serial_ext.py index 4685708b12c..831a9330095 100644 --- a/tools/idf_py_actions/serial_ext.py +++ b/tools/idf_py_actions/serial_ext.py @@ -17,8 +17,10 @@ from idf_py_actions.global_options import global_options from idf_py_actions.tools import PropertyDict from idf_py_actions.tools import RunTool from idf_py_actions.tools import ensure_build_directory +from idf_py_actions.tools import get_default_esp from idf_py_actions.tools import get_default_serial_port from idf_py_actions.tools import get_sdkconfig_value +from idf_py_actions.tools import get_selected_target from idf_py_actions.tools import run_target PYTHON = sys.executable @@ -98,6 +100,49 @@ def action_extensions(base_actions: dict, project_path: str) -> dict: return result + def _run_monitor( + monitor_args: list, + args: PropertyDict, + print_filter: str, + encrypted: bool, + no_reset: bool, + timestamps: bool, + timestamp_format: str, + force_color: bool, + disable_auto_color: bool, + ) -> None: + if print_filter: + monitor_args += ['--print_filter', print_filter] + if encrypted: + monitor_args += ['--encrypted'] + if no_reset: + monitor_args += ['--no-reset'] + if timestamps: + monitor_args += ['--timestamps'] + if timestamp_format: + monitor_args += ['--timestamp-format', timestamp_format] + if force_color or os.name == 'nt': + monitor_args += ['--force-color'] + if disable_auto_color: + monitor_args += ['--disable-auto-color'] + hints = not args.no_hints and os.path.isdir(args.build_dir) + + # Temporally ignore SIGINT, which is used in idf_monitor to spawn gdb. + old_handler = signal.getsignal(signal.SIGINT) + signal.signal(signal.SIGINT, signal.SIG_IGN) + try: + RunTool( + 'idf_monitor', + monitor_args, + args.project_dir, + build_dir=args.build_dir, + hints=hints, + interactive=True, + convert_output=True, + )() + finally: + signal.signal(signal.SIGINT, old_handler) + def monitor( action: str, ctx: Context, @@ -114,6 +159,41 @@ def action_extensions(base_actions: dict, project_path: str) -> dict: """ Run esp_idf_monitor to watch build output """ + project_built = os.path.exists(os.path.join(args.build_dir, 'project_description.json')) + target_selected = project_built or get_selected_target(args) + + detected_target = None + + if not project_built: + if no_reset and args.port is None: + raise FatalError( + '--no-reset is only supported when used with a port. ' + 'Please specify the port with the --port argument to use this option.' + ) + if not target_selected and args.port is None: + esp = get_default_esp() + args.port = esp.serial_port + detected_target = str(esp.CHIP_NAME.lower().replace('-', '')) + else: + detected_target = get_selected_target(args) + idf_monitor = os.path.join(os.environ['IDF_PATH'], 'tools/idf_monitor.py') + monitor_args = [PYTHON, idf_monitor] + monitor_args += ['-p', args.port or get_default_serial_port(detected_target)] + if detected_target: + monitor_args += ['--target', detected_target] + _run_monitor( + monitor_args, + args, + print_filter, + encrypted, + no_reset, + timestamps, + timestamp_format, + force_color, + disable_auto_color, + ) + return + project_desc = _get_project_desc(ctx, args) elf_file = os.path.join(args.build_dir, project_desc['app_elf']) @@ -123,11 +203,13 @@ def action_extensions(base_actions: dict, project_path: str) -> dict: if project_desc['target'] != 'linux': if no_reset and args.port is None: raise FatalError( - 'Error: --no-reset is only supported when used with a port.' - 'Please specify the port with the --port argument in order to use this option.' + '--no-reset is only supported when used with a port. ' + 'Please specify the port with the --port argument to use this option.' ) - args.port = args.port or get_default_serial_port() + # The target is passed explicitly, because ensure_build_directory(), + # which sets the build context, is not called for an already built project. + args.port = args.port or get_default_serial_port(project_desc['target']) monitor_args += ['-p', args.port] baud = monitor_baud or os.getenv('IDF_MONITOR_BAUD') or os.getenv('MONITORBAUD') @@ -158,52 +240,27 @@ def action_extensions(base_actions: dict, project_path: str) -> dict: if target_arch_riscv: monitor_args += ['--decode-panic', 'backtrace'] - if print_filter is not None: - monitor_args += ['--print_filter', print_filter] - elf_list = [str(elf) for elf in Path(args.build_dir).rglob('*.elf')] - if elf_file and elf_file in elf_list: - # prepend the main app elf file to the list; make sure it is the first one - elf_list.insert(0, elf_list.pop(elf_list.index(elf_file))) - monitor_args.extend(elf_list) - - if encrypted: - monitor_args += ['--encrypted'] - - if no_reset: - monitor_args += ['--no-reset'] - - if timestamps: - monitor_args += ['--timestamps'] - - if timestamp_format: - monitor_args += ['--timestamp-format', timestamp_format] - - if force_color or os.name == 'nt': - monitor_args += ['--force-color'] - - if disable_auto_color: - monitor_args += ['--disable-auto-color'] + if elf_list: + if elf_file and elf_file in elf_list: + # prepend the main app elf file to the list; make sure it is the first one + elf_list.insert(0, elf_list.pop(elf_list.index(elf_file))) + monitor_args.extend(elf_list) idf_py = [PYTHON] + _get_commandline_options(ctx) # commands to re-run idf.py monitor_args += ['-m', ' '.join(f"'{a}'" for a in idf_py)] - hints = not args.no_hints - # Temporally ignore SIGINT, which is used in idf_monitor to spawn gdb. - old_handler = signal.getsignal(signal.SIGINT) - signal.signal(signal.SIGINT, signal.SIG_IGN) - try: - RunTool( - 'idf_monitor', - monitor_args, - args.project_dir, - build_dir=args.build_dir, - hints=hints, - interactive=True, - convert_output=True, - )() - finally: - signal.signal(signal.SIGINT, old_handler) + _run_monitor( + monitor_args, + args, + print_filter, + encrypted, + no_reset, + timestamps, + timestamp_format, + force_color, + disable_auto_color, + ) def flash( action: str, @@ -267,8 +324,8 @@ def action_extensions(base_actions: dict, project_path: str) -> dict: Calls ensure_build_directory() which will run cmake to generate a build directory (with the specified generator) as needed. """ - args.port = args.port or get_default_serial_port() ensure_build_directory(args, ctx.info_name) + args.port = args.port or get_default_serial_port() run_target(target_name, args, {'ESPBAUD': str(args.baud), 'ESPPORT': args.port}, interactive=True) def merge_bin( diff --git a/tools/idf_py_actions/tools.py b/tools/idf_py_actions/tools.py index 61f993e1e5e..f8735833c45 100644 --- a/tools/idf_py_actions/tools.py +++ b/tools/idf_py_actions/tools.py @@ -129,26 +129,38 @@ def idf_version() -> str | None: return version -def get_default_serial_port() -> Any: +def get_default_esp(target: str | None = None) -> Any: + """ + Detect a connected Espressif device. + + If the target is not given, it is taken from the build context, which is empty unless ensure_build_directory() was + called. Without a known target any Espressif device is accepted. + """ # Import is done here in order to move it after the check_environment() # ensured that pyserial has been installed try: import esptool + target = target or get_build_context().get('proj_desc', {}).get('target') + ports = esptool.get_port_list() # high baud rate could cause the failure of creation of the connection esp = esptool.get_default_connected_device( - serial_list=ports, port=None, connect_attempts=4, initial_baud=115200 + serial_list=ports, + port=None, + connect_attempts=4, + initial_baud=115200, + chip=target or 'auto', ) if esp is None: + device = f'{target} device' if target else 'serial port' raise NoSerialPortFoundError( - "No serial ports found. Connect a device, or use '-p PORT' option to set a specific port." + f"No {device} found. Connect a device, or use '-p PORT' option to set a specific port." ) - serial_port = esp.serial_port esp._port.close() - return serial_port + return esp except NoSerialPortFoundError: raise @@ -156,6 +168,53 @@ def get_default_serial_port() -> Any: raise FatalError(f'An exception occurred during detection of the serial port: {e}') +def get_default_serial_port(target: str | None = None) -> Any: + """ + Detect a serial port with a connected device. + + Ports with a device not matching the target are skipped. If the target is not given, + it is taken from the build context, which is empty unless ensure_build_directory() was + called. Without a known target any Espressif device is accepted. + """ + return get_default_esp(target).serial_port + + +def get_selected_target(args: 'PropertyDict') -> str | None: + """ + Return the target name if a project target has been explicitly selected instead of + relying on the implicit default target (esp32). Return None otherwise. + + The target may come from the IDF_TARGET environment variable, a -DIDF_TARGET command + line define, the project sdkconfig, a sdkconfig.defaults file, or the CMakeCache.txt + from a previous build (mirroring how CMake guesses the target). This has to be + evaluated before the project is (re)configured, because configuration generates a + sdkconfig pinned to the (possibly default) target. + """ + cache_cmdl = _parse_cmdl_cmakecache(args.define_cache_entry) + + target = ( + os.environ.get('IDF_TARGET') + or cache_cmdl.get('IDF_TARGET') + or get_sdkconfig_value(get_sdkconfig_filename(args, cache_cmdl), 'CONFIG_IDF_TARGET') + ) + if target: + return target + + sdkconfig_defaults = cache_cmdl.get('SDKCONFIG_DEFAULTS') or os.environ.get('SDKCONFIG_DEFAULTS') + default_files = sdkconfig_defaults.split(';') if sdkconfig_defaults else ['sdkconfig.defaults'] + for default_file in default_files: + default_file = os.path.join(args.project_dir, default_file) + target = get_sdkconfig_value(default_file, 'CONFIG_IDF_TARGET') + if target: + return target + + cache_path = os.path.join(args.build_dir, 'CMakeCache.txt') + if os.path.exists(cache_path): + return _parse_cmakecache(cache_path).get('IDF_TARGET') + + return None + + # function prints warning when autocompletion is not being performed # set argument stream to sys.stderr for errors and exceptions def print_warning(message: str, stream: TextIO | None = None) -> None: @@ -453,7 +512,7 @@ class RunTool: and of the command, the id of the process, paths to captured output""" log_dir_name = 'log' try: - os.mkdir(os.path.join(self.build_dir, log_dir_name)) + os.makedirs(os.path.join(self.build_dir, log_dir_name), exist_ok=True) except FileExistsError: pass # Note: we explicitly pass in os.environ here, as we may have set IDF_PATH there during startup diff --git a/tools/test_idf_py/test_serial_ext.py b/tools/test_idf_py/test_serial_ext.py new file mode 100644 index 00000000000..6f403d3ba2f --- /dev/null +++ b/tools/test_idf_py/test_serial_ext.py @@ -0,0 +1,172 @@ +# SPDX-FileCopyrightText: 2026 Espressif Systems (Shanghai) CO LTD +# SPDX-License-Identifier: Apache-2.0 +import os +import pathlib +import sys +from typing import Any +from unittest import mock + +import pytest + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..')) + +from idf_py_actions.tools import PropertyDict # noqa: E402 +from idf_py_actions.tools import get_selected_target # noqa: E402 + + +@pytest.fixture +def project_dir(tmp_path: pathlib.Path) -> str: + return str(tmp_path) + + +@pytest.fixture +def args(project_dir: str, tmp_path: pathlib.Path) -> PropertyDict: + build_dir = str(tmp_path / 'build') + os.makedirs(build_dir, exist_ok=True) + return PropertyDict( + project_dir=project_dir, + build_dir=build_dir, + port=None, + baud=460800, + define_cache_entry=[], + no_hints=False, + ) + + +class TestGetSelectedTarget: + def test_returns_none_when_nothing_set(self, args: PropertyDict) -> None: + with mock.patch.dict(os.environ, {}, clear=True): + os.environ.pop('IDF_TARGET', None) + os.environ.pop('SDKCONFIG_DEFAULTS', None) + assert get_selected_target(args) is None + + def test_returns_target_from_env(self, args: PropertyDict) -> None: + with mock.patch.dict(os.environ, {'IDF_TARGET': 'esp32s3'}): + assert get_selected_target(args) == 'esp32s3' + + def test_returns_target_from_define_cache(self, args: PropertyDict) -> None: + args.define_cache_entry = ['IDF_TARGET=esp32c3'] + with mock.patch.dict(os.environ, {}, clear=True): + os.environ.pop('IDF_TARGET', None) + os.environ.pop('SDKCONFIG_DEFAULTS', None) + assert get_selected_target(args) == 'esp32c3' + + def test_returns_target_from_sdkconfig(self, args: PropertyDict, project_dir: str) -> None: + sdkconfig_path = os.path.join(project_dir, 'sdkconfig') + with open(sdkconfig_path, 'w') as f: + f.write('CONFIG_IDF_TARGET="esp32h2"\n') + with mock.patch.dict(os.environ, {}, clear=True): + os.environ.pop('IDF_TARGET', None) + os.environ.pop('SDKCONFIG_DEFAULTS', None) + assert get_selected_target(args) == 'esp32h2' + + def test_returns_target_from_sdkconfig_defaults(self, args: PropertyDict, project_dir: str) -> None: + defaults_path = os.path.join(project_dir, 'sdkconfig.defaults') + with open(defaults_path, 'w') as f: + f.write('CONFIG_IDF_TARGET="esp32c6"\n') + with mock.patch.dict(os.environ, {}, clear=True): + os.environ.pop('IDF_TARGET', None) + os.environ.pop('SDKCONFIG_DEFAULTS', None) + assert get_selected_target(args) == 'esp32c6' + + def test_returns_target_from_cmake_cache(self, args: PropertyDict) -> None: + cache_path = os.path.join(args.build_dir, 'CMakeCache.txt') + with open(cache_path, 'w') as f: + f.write('IDF_TARGET:STRING=esp32s2\n') + with mock.patch.dict(os.environ, {}, clear=True): + os.environ.pop('IDF_TARGET', None) + os.environ.pop('SDKCONFIG_DEFAULTS', None) + assert get_selected_target(args) == 'esp32s2' + + def test_env_takes_priority_over_cache(self, args: PropertyDict) -> None: + cache_path = os.path.join(args.build_dir, 'CMakeCache.txt') + with open(cache_path, 'w') as f: + f.write('IDF_TARGET:STRING=esp32s2\n') + with mock.patch.dict(os.environ, {'IDF_TARGET': 'esp32c3'}): + assert get_selected_target(args) == 'esp32c3' + + +class TestMonitorPortDetection: + """Test that the monitor function builds correct args for idf_monitor.""" + + @pytest.fixture + def mock_esp(self) -> Any: + esp = mock.MagicMock() + esp.serial_port = '/dev/ttyUSB0' + esp.CHIP_NAME = 'ESP32-C5' + return esp + + def _get_monitor_fn(self) -> Any: + from idf_py_actions.serial_ext import action_extensions + + ext = action_extensions({}, '') + return ext['actions']['monitor']['callback'] + + def _call_monitor(self, args: PropertyDict, port: str | None = None, no_reset: bool = False) -> None: + args.port = port + monitor_fn = self._get_monitor_fn() + + ctx = mock.MagicMock() + ctx.info_name = 'idf.py' + ctx._parameter_source = {'baud': mock.MagicMock()} + + monitor_fn( + 'monitor', + ctx, + args, + print_filter=None, + monitor_baud=None, + encrypted=False, + no_reset=no_reset, + timestamps=False, + timestamp_format=None, + force_color=False, + disable_auto_color=False, + ) + + def test_clean_project_autodetects_port_and_target(self, args: PropertyDict, mock_esp: Any) -> None: + with mock.patch.dict(os.environ, {'IDF_PATH': '/idf'}, clear=False): + os.environ.pop('IDF_TARGET', None) + with mock.patch('idf_py_actions.serial_ext.get_default_esp', return_value=mock_esp): + with mock.patch('idf_py_actions.serial_ext.RunTool') as mock_run: + self._call_monitor(args) + + call_args = mock_run.call_args[0][1] + assert '-p' in call_args + assert '/dev/ttyUSB0' in call_args + assert '--target' in call_args + assert 'esp32c5' in call_args + + def test_clean_project_with_explicit_port(self, args: PropertyDict) -> None: + with mock.patch.dict(os.environ, {'IDF_PATH': '/idf'}, clear=False): + os.environ.pop('IDF_TARGET', None) + with mock.patch('idf_py_actions.serial_ext.get_default_serial_port', return_value='/dev/ttyACM0'): + with mock.patch('idf_py_actions.serial_ext.RunTool') as mock_run: + self._call_monitor(args, port='/dev/ttyACM1') + + call_args = mock_run.call_args[0][1] + assert '/dev/ttyACM1' in call_args + + def test_clean_project_with_target_set_uses_correct_port(self, args: PropertyDict) -> None: + with mock.patch.dict(os.environ, {'IDF_PATH': '/idf', 'IDF_TARGET': 'esp32s3'}): + with mock.patch( + 'idf_py_actions.serial_ext.get_default_serial_port', return_value='/dev/ttyUSB2' + ) as mock_port: + with mock.patch('idf_py_actions.serial_ext.RunTool') as mock_run: + self._call_monitor(args) + + mock_port.assert_called_with('esp32s3') + call_args = mock_run.call_args[0][1] + assert '/dev/ttyUSB2' in call_args + assert '--target' in call_args + assert 'esp32s3' in call_args + + def test_clean_project_with_target_set_no_autodetect(self, args: PropertyDict, mock_esp: Any) -> None: + """When target is already set, get_default_esp should NOT be called.""" + with mock.patch.dict(os.environ, {'IDF_PATH': '/idf', 'IDF_TARGET': 'esp32s3'}): + with mock.patch('idf_py_actions.serial_ext.get_default_esp', return_value=mock_esp) as mock_detect: + with mock.patch('idf_py_actions.serial_ext.get_default_serial_port', return_value='/dev/ttyUSB0'): + with mock.patch('idf_py_actions.serial_ext.RunTool'): + self._call_monitor(args) + + mock_detect.assert_not_called()