Merge branch 'fix/serial_port_target_mismatch' into 'master'

fix(tools): Detect serial port matching the project target

Closes IDF-15356

See merge request espressif/esp-idf!51466

tools/idf_py_actions/serial_ext.py

@@ -20,8 +20,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
@@ -96,6 +98,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,
@@ -112,6 +157,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'])
 
@@ -121,11 +201,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')
@@ -156,52 +238,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,
@@ -265,8 +322,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(

tools/idf_py_actions/tools.py

@@ -137,26 +137,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
@@ -164,6 +176,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
+
+
 def debug_print_idf_version() -> None:
     log.note(f'ESP-IDF {idf_version() or "version unknown"}')
 
@@ -431,7 +490,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