Merge branch 'monitor_win_color-v5.0' into 'release/v5.0'

bug(idf_monitor): fix color on windows with hints (v5.0)

See merge request espressif/esp-idf!23348

components/console/linenoise/linenoise.c

@@ -1046,7 +1046,7 @@ int linenoiseProbe(void) {
     flushWrite();
 
     /* Try to read response */
-    int timeout_ms = 300;
+    int timeout_ms = 500;
     const int retry_ms = 10;
     size_t read_bytes = 0;
     while (timeout_ms > 0 && read_bytes < 4) { // response is ESC[0n or ESC[3n

docs/en/api-guides/tools/idf-py.rst

@@ -146,7 +146,7 @@ or partition table as applicable.
 Hints on how to resolve errors
 ==============================
 
-``idf.py`` will try to suggest hints on how to resolve errors. It works with a database of hints stored in :idf_file:`tools/idf_py_actions/hints.yml` and the hints will be printed if a match is found for the given error. The monitor, menuconfig, gdb and openocd targets are not supported at the moment by automatic hints on resolving errors.
+``idf.py`` will try to suggest hints on how to resolve errors. It works with a database of hints stored in :idf_file:`tools/idf_py_actions/hints.yml` and the hints will be printed if a match is found for the given error. The menuconfig, gdb and openocd targets are not supported at the moment by automatic hints on resolving errors.
 
 The ``--no-hints`` argument of ``idf.py`` can be used to turn the hints off in case they are not desired.
 

tools/idf_monitor.py

@@ -35,7 +35,7 @@ from typing import Any, List, Optional, Type, Union
 import serial
 import serial.tools.list_ports
 # Windows console stuff
-from idf_monitor_base.ansi_color_converter import get_converter
+from idf_monitor_base.ansi_color_converter import get_ansi_converter
 from idf_monitor_base.argument_parser import get_parser
 from idf_monitor_base.console_parser import ConsoleParser
 from idf_monitor_base.console_reader import ConsoleReader
@@ -91,9 +91,9 @@ class Monitor:
         self.cmd_queue = queue.Queue()  # type: queue.Queue
         self.console = miniterm.Console()
         # if the variable is set ANSI will be printed even if we do not print to terminal
-        sys.stderr = get_converter(sys.stderr, decode_output=True, force_color=force_color)
-        self.console.output = get_converter(self.console.output, force_color=force_color)
-        self.console.byte_output = get_converter(self.console.byte_output, force_color=force_color)
+        sys.stderr = get_ansi_converter(sys.stderr, force_color=force_color)  # type: ignore
+        self.console.output = get_ansi_converter(self.console.output, force_color=force_color)
+        self.console.byte_output = get_ansi_converter(self.console.byte_output, force_color=force_color)
 
         self.elf_file = elf_file or ''
         self.elf_exists = os.path.exists(self.elf_file)

tools/idf_monitor_base/ansi_color_converter.py

@@ -28,15 +28,14 @@ if os.name == 'nt':
     SetConsoleTextAttribute = ctypes.windll.kernel32.SetConsoleTextAttribute  # type: ignore
 
 
-def get_converter(orig_output_method=None, decode_output=False, force_color=False):
-    # type: (Any[TextIO, Optional[TextIOBase]], bool, bool) -> Union[ANSIColorConverter, Optional[TextIOBase]]
+def get_ansi_converter(orig_output_method=None, force_color=False):
+    # type: (Any[TextIO, Optional[TextIOBase]], bool) -> Union[ANSIColorConverter, Optional[TextIOBase]]
     """
     Returns an ANSIColorConverter on Windows and the original output method (orig_output_method) on other platforms.
-    The ANSIColorConverter with decode_output=True will decode the bytes before passing them to the output
-    The ANSIColorConverter with force_color=True will be forced to convert ANSI in windows format
+    The ANSIColorConverter with force_color=True will be forced to use ANSI color codes
     """
-    if os.name == 'nt':
-        return ANSIColorConverter(orig_output_method, decode_output, force_color)
+    if os.name == 'nt' and not force_color:
+        return ANSIColorConverter(orig_output_method, force_color)
     return orig_output_method
 
 
@@ -51,10 +50,16 @@ class ANSIColorConverter(object):
     least-bad working solution, as winpty doesn't support any "passthrough" mode for raw output.
     """
 
-    def __init__(self, output=None, decode_output=False, force_color=False):
-        # type: (TextIOBase, bool, bool) -> None
+    def __init__(self, output=None, force_color=False):
+        # type: (TextIOBase, bool) -> None
         self.output = output
-        self.decode_output = decode_output
+        # check if output supports writing bytes or if decoding before writing is necessary
+        try:
+            output.write(b'')  # type: ignore
+        except Exception:
+            self.decode_output = True
+        else:
+            self.decode_output = False
         self.handle = GetStdHandle(STD_ERROR_HANDLE if self.output == sys.stderr else STD_OUTPUT_HANDLE)
         self.matched = b''
         self.force_color = force_color  # always print ANSI for colors if true
@@ -82,6 +87,7 @@ class ANSIColorConverter(object):
             data = bytearray(data)
         else:
             data = bytearray(data, 'utf-8')
+
         for b in data:
             b = bytes([b])
             length = len(self.matched)
@@ -91,12 +97,17 @@ class ANSIColorConverter(object):
                 self.matched = b
             elif (length == 1 and b == b'[') or (1 < length < 7):
                 self.matched += b
-                if not self.force_color and self.matched == ANSI_NORMAL.encode('latin-1'):  # reset console
+                if self.matched == ANSI_NORMAL.encode('latin-1'):  # reset console
                     # Flush is required only with Python3 - switching color before it is printed would mess up the console
                     self.flush()
                     SetConsoleTextAttribute(self.handle, FOREGROUND_GREY)
                     self.matched = b''
-                elif self.force_color or len(self.matched) == 7:  # could be an ANSI sequence
+                elif (length == 2 and b not in [b'0', b'1']) or (length == 3 and b != b';'):
+                    # other escape sequences like cursor position and status reports
+                    self._output_write(self.matched)
+                    self.flush()
+                    self.matched = b''
+                elif length == 6:  # could be an ANSI sequence
                     m = re.match(RE_ANSI_COLOR, self.matched)
                     if m is not None:
                         color = ANSI_TO_WINDOWS_COLOR[int(m.group(2))]
@@ -107,6 +118,7 @@ class ANSIColorConverter(object):
                         SetConsoleTextAttribute(self.handle, color)
                     else:
                         self._output_write(self.matched)  # not an ANSI color code, display verbatim
+                        self.flush()
                     self.matched = b''
             else:
                 self._output_write(b)

tools/idf_monitor_base/constants.py

@@ -19,6 +19,9 @@ CTRL_X = '\x18'
 CTRL_L = '\x0c'
 CTRL_RBRACKET = '\x1d'  # Ctrl+]
 
+# VT100 escape sequences
+CONSOLE_STATUS_QUERY = b'\x1b[5n'
+
 # Command parsed from console inputs
 CMD_STOP = 1
 CMD_RESET = 2

tools/idf_monitor_base/serial_handler.py

@@ -16,9 +16,9 @@ from .chip_specific_config import get_chip_config
 from .console_parser import ConsoleParser, prompt_next_action  # noqa: F401
 from .console_reader import ConsoleReader  # noqa: F401
 from .constants import (CMD_APP_FLASH, CMD_ENTER_BOOT, CMD_MAKE, CMD_OUTPUT_TOGGLE, CMD_RESET, CMD_STOP,
-                        CMD_TOGGLE_LOGGING, CMD_TOGGLE_TIMESTAMPS, PANIC_DECODE_DISABLE, PANIC_END, PANIC_IDLE,
-                        PANIC_READING, PANIC_STACK_DUMP, PANIC_START)
-from .coredump import CoreDump
+                        CMD_TOGGLE_LOGGING, CMD_TOGGLE_TIMESTAMPS, CONSOLE_STATUS_QUERY, PANIC_DECODE_DISABLE,
+                        PANIC_END, PANIC_IDLE, PANIC_READING, PANIC_STACK_DUMP, PANIC_START)
+from .coredump import CoreDump  # noqa: F401
 from .exceptions import SerialStopException
 from .gdbhelper import GDBHelper
 from .line_matcher import LineMatcher
@@ -106,6 +106,11 @@ class SerialHandler:
                     self.logger.handle_possible_pc_address_in_line(line)
             check_gdb_stub_and_run(line)
             self._force_line_print = False
+
+        if self._last_line_part.startswith(CONSOLE_STATUS_QUERY):
+            self.logger.print(CONSOLE_STATUS_QUERY)
+            self._last_line_part = self._last_line_part[len(CONSOLE_STATUS_QUERY):]
+
         # Now we have the last part (incomplete line) in _last_line_part. By
         # default we don't touch it and just wait for the arrival of the rest
         # of the line. But after some time when we didn't received it we need
@@ -255,6 +260,10 @@ class SerialHandlerNoElf(SerialHandler):
             self.compare_elf_sha256(line.decode(errors='ignore'))
             self._force_line_print = False
 
+        if self._last_line_part.startswith(CONSOLE_STATUS_QUERY):
+            self.logger.print(CONSOLE_STATUS_QUERY)
+            self._last_line_part = self._last_line_part[len(CONSOLE_STATUS_QUERY):]
+
         force_print_or_matched = any((
             self._force_line_print,
             (finalize_line and line_matcher.match(self._last_line_part.decode(errors='ignore')))

tools/idf_py_actions/serial_ext.py

@@ -1,4 +1,4 @@
-# SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD
+# SPDX-FileCopyrightText: 2021-2023 Espressif Systems (Shanghai) CO LTD
 # SPDX-License-Identifier: Apache-2.0
 
 import json
@@ -155,9 +155,9 @@ def action_extensions(base_actions: Dict, project_path: str) -> Dict:
 
         idf_py = [PYTHON] + _get_commandline_options(ctx)  # commands to re-run idf.py
         monitor_args += ['-m', ' '.join("'%s'" % a for a in idf_py)]
-        hints = False  # Temporarily disabled because of https://github.com/espressif/esp-idf/issues/9610
+        hints = not args.no_hints
 
-        RunTool('idf_monitor', monitor_args, args.project_dir, build_dir=args.build_dir, hints=hints, interactive=True)()
+        RunTool('idf_monitor', monitor_args, args.project_dir, build_dir=args.build_dir, hints=hints, interactive=True, convert_output=True)()
 
     def flash(action: str, ctx: click.core.Context, args: PropertyDict, force: bool, extra_args: str) -> None:
         """

tools/idf_py_actions/tools.py

@@ -12,6 +12,7 @@ from typing import Any, Dict, List, Match, Optional, TextIO, Tuple, Union
 
 import click
 import yaml
+from idf_monitor_base.ansi_color_converter import get_ansi_converter
 
 from .constants import GENERATORS
 from .errors import FatalError
@@ -167,7 +168,7 @@ def fit_text_in_terminal(out: str) -> str:
 
 class RunTool:
     def __init__(self, tool_name: str, args: List, cwd: str, env: Dict=None, custom_error_handler: FunctionType=None, build_dir: str=None,
-                 hints: bool=True, force_progression: bool=False, interactive: bool=False) -> None:
+                 hints: bool=True, force_progression: bool=False, interactive: bool=False, convert_output: bool=False) -> None:
         self.tool_name = tool_name
         self.args = args
         self.cwd = cwd
@@ -178,6 +179,7 @@ class RunTool:
         self.hints = hints
         self.force_progression = force_progression
         self.interactive = interactive
+        self.convert_output = convert_output
 
     def __call__(self) -> None:
         def quote_arg(arg: str) -> str:
@@ -276,6 +278,9 @@ class RunTool:
                         # and still can not decode it we can just ignore some bytes
                         return buffer.decode(errors='ignore')
 
+        # use ANSI color converter for Monitor on Windows
+        output_converter = get_ansi_converter(output_stream) if self.convert_output else output_stream
+
         try:
             with open(output_filename, 'w', encoding='utf8') as output_file:
                 while True:
@@ -298,8 +303,8 @@ class RunTool:
                         # print output in progression way but only the progression related (that started with '[') and if verbose flag is not set
                         print_progression(output)
                     else:
-                        output_stream.write(output)
-                        output_stream.flush()
+                        output_converter.write(output)
+                        output_converter.flush()
         except (RuntimeError, EnvironmentError) as e:
             yellow_print('WARNING: The exception {} was raised and we can\'t capture all your {} and '
                          'hints on how to resolve errors can be not accurate.'.format(e, output_stream.name.strip('<>')))

tools/test_idf_monitor/tests/in2f1.txt

@@ -9,7 +9,7 @@
 I (793) mdf_device_handle: [mdf_device_init_handle, 908]:silicon revision : 0
 I (801) mdf_device_handle: [mdf_device_init_handle, 910]:flash            : 4 MB external
 I (810) mdf_device_handle: [mdf_device_init_handle, 911]:***************************************************
-E (45488) mesh: [esp_mesh_push_to_nwk_queue,5360] null args
+E (45488) mesh: [esp_mesh_push_to_nwk_queue,5360] null args
 I (50495) mdf_device_handle: [mdf_show_sysinfo_timercb, 73]:parent: 00:00:00:00:00:00, mac:30:ae:a4:00:43:84, layer: -1, free heap: 87712, compile time: May 26 2018 00:22:11
 I (55495) mdf_device_handle: [mdf_show_sysinfo_timercb, 73]:parent: 20:a6:80:98:dc:b4, mac:30:ae:a4:00:43:84, layer: 1, free heap: 88452, compile time: May 26 2018 00:22:11
 I (60495) mdf_device_handle: [mdf_show_sysinfo_timercb, 73]:parent: 20:a6:80:98:dc:b4, mac:30:ae:a4:00:43:84, layer: 1, free heap: 71792, compile time: May 26 2018 00:22:11

tools/test_idf_monitor/tests/in2f2.txt

@@ -1,5 +1,5 @@
 D (318) vfs: esp_vfs_register_fd_range is successful for range <54; 64) and VFS ID 1
-D (59342) vfs: esp_vfs_select starts with nfds = 55
+D (59342) vfs: esp_vfs_select starts with nfds = 55
 D (59343) vfs: FDs in readfds =
 D (59343) vfs: 54
 D (59355) vfs: calling socket_select with the following FDs