From e477ce93e9257d5700f25df6366308682f00bb5a Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Fri, 17 Mar 2017 18:41:18 +0800 Subject: [PATCH 1/5] idf_monitor: Small fixes (baud rate, EOL, /dev/tty.X on macOS, Ctrl-T on failure) * "make monitor" not passed the configured baud rate Closes #436 https://github.com/espressif/esp-idf/issues/436 * Pass toolchain prefix from sdkconfig into monitor tool * Allow setting EOL in idf_monitor.py, use CRLF by default * Detect if /dev/tty.X is used on macOS, warn and replace with /dev/cu.X * If a build fails or gdb exits, ignore Ctrl-T (allowing Ctrl-T Ctrl-A/F to be same key sequence everywhere) * Add a note about winpty on Windows Ref https://github.com/espressif/esp-idf/commit/02fdf8271de3a6db2ab9d4d6f023c160c84ebdbe#commitcomment-21369196 --- components/esptool_py/Makefile.projbuild | 4 ++- docs/idf-monitor.rst | 1 + tools/idf_monitor.py | 41 ++++++++++++++++++++---- 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/components/esptool_py/Makefile.projbuild b/components/esptool_py/Makefile.projbuild index 860e0383830..b35dc111262 100644 --- a/components/esptool_py/Makefile.projbuild +++ b/components/esptool_py/Makefile.projbuild @@ -83,12 +83,14 @@ endif simple_monitor: $(call prereq_if_explicit,%flash) $(MONITOR_PYTHON) -m serial.tools.miniterm --rts 0 --dtr 0 --raw $(ESPPORT) $(MONITORBAUD) +MONITOR_OPTS := --baud $(MONITORBAUD) --port $(ESPPORT) --toolchain-prefix $(CONFIG_TOOLPREFIX) --make "$(MAKE)" + monitor: $(call prereq_if_explicit,%flash) $(summary) MONITOR [ -f $(APP_ELF) ] || echo "*** 'make monitor' target requires an app to be compiled and flashed first." [ -f $(APP_ELF) ] || echo "*** Run 'make flash monitor' to build, flash and monitor" [ -f $(APP_ELF) ] || echo "*** Or alternatively 'make simple_monitor' to view the serial port as-is." [ -f $(APP_ELF) ] || exit 1 - $(MONITOR_PYTHON) $(IDF_PATH)/tools/idf_monitor.py --port $(ESPPORT) --make "$(MAKE)" $(APP_ELF) + $(MONITOR_PYTHON) $(IDF_PATH)/tools/idf_monitor.py $(MONITOR_OPTS) $(APP_ELF) .PHONY: erase_flash diff --git a/docs/idf-monitor.rst b/docs/idf-monitor.rst index a0a2c5cba69..61dfd0afd31 100644 --- a/docs/idf-monitor.rst +++ b/docs/idf-monitor.rst @@ -102,6 +102,7 @@ Known Issues with idf_monitor Issues Observed on Windows ~~~~~~~~~~~~~~~~~~~~~~~~~~ +- If you are using the supported Windows environment and receive the error "winpty: command not found" then run ``pacman -S winpty`` to fix. - Arrow keys and some other special keys in gdb don't work, due to Windows Console limitations. - Occasionally when "make" exits, it may stall for up to 30 seconds before idf_monitor resumes. - Occasionally when "gdb" is run, it may stall for a short time before it begins communicating with the gdbstub. diff --git a/tools/idf_monitor.py b/tools/idf_monitor.py index 0bd5f3368e3..a8ddf77e796 100644 --- a/tools/idf_monitor.py +++ b/tools/idf_monitor.py @@ -70,6 +70,8 @@ # regex matches an potential PC value (0x4xxxxxxx) MATCH_PCADDR = re.compile(r'0x4[0-9a-f]{7}', re.IGNORECASE) +DEFAULT_TOOLCHAIN_PREFIX = "xtensa-esp32-elf-" + class StoppableThread(object): """ Provide a Thread-like class which can be 'cancelled' via a subclass-provided @@ -193,7 +195,7 @@ class Monitor(object): Main difference is that all event processing happens in the main thread, not the worker threads. """ - def __init__(self, serial_instance, elf_file, make="make"): + def __init__(self, serial_instance, elf_file, make="make", toolchain_prefix=DEFAULT_TOOLCHAIN_PREFIX, eol="CRLF"): super(Monitor, self).__init__() self.event_queue = queue.Queue() self.console = miniterm.Console() @@ -207,9 +209,16 @@ def __init__(self, serial_instance, elf_file, make="make"): self.serial_reader = SerialReader(self.serial, self.event_queue) self.elf_file = elf_file self.make = make + self.toolchain_prefix = DEFAULT_TOOLCHAIN_PREFIX self.menu_key = CTRL_T self.exit_key = CTRL_RBRACKET + self.translate_eol = { + "CRLF": lambda c: c.replace(b"\n", b"\r\n"), + "CR": lambda c: c.replace(b"\n", b"\r"), + "LF": lambda c: c.replace(b"\r", b"\n"), + }[eol] + # internal state self._pressed_menu_key = False self._read_line = b"" @@ -246,6 +255,7 @@ def handle_key(self, key): self.serial_reader.stop() else: try: + key = self.translate_eol(key) self.serial.write(codecs.encode(key)) except serial.SerialException: pass # this shouldn't happen, but sometimes port has closed in serial thread @@ -327,7 +337,9 @@ def prompt_next_action(self, reason): .format(key_description(CTRL_A))) sys.stderr.write("--- Press any other key to resume monitor (resets target).\n") sys.stderr.write(ANSI_NORMAL) - k = self.console.getkey() + k = CTRL_T # ignore CTRL-T here, so people can muscle-memory Ctrl-T Ctrl-F, etc. + while k == CTRL_T: + k = self.console.getkey() finally: self.console.cleanup() if k == self.exit_key: @@ -350,8 +362,8 @@ def run_make(self, target): def lookup_pc_address(self, pc_addr): translation = subprocess.check_output( - ["xtensa-esp32-elf-addr2line", "-pfia", - "-e", self.elf_file, pc_addr], + ["%saddr2line" % self.toolchain_prefix, + "-pfia", "-e", self.elf_file, pc_addr], cwd=".") if not "?? ??:0" in translation: sys.stderr.write(ANSI_YELLOW + translation + ANSI_NORMAL) @@ -375,7 +387,7 @@ def run_gdb(self): with self: # disable console control sys.stderr.write(ANSI_NORMAL) try: - subprocess.call(["xtensa-esp32-elf-gdb", + subprocess.call(["%sgdb" % self.toolchain_prefix, "-ex", "set serial baud %d" % self.serial.baudrate, "-ex", "target remote %s" % self.serial.port, "-ex", "interrupt", # monitor has already parsed the first 'reason' command, need a second @@ -404,12 +416,29 @@ def main(): help='Command to run make', type=str, default='make') + parser.add_argument( + '--toolchain-prefix', + help="Triplet prefix to add before cross-toolchain names", + default=DEFAULT_TOOLCHAIN_PREFIX) + + parser.add_argument( + "--eol", + choices=['CR', 'LF', 'CRLF'], + type=lambda c: c.upper(), + help="End of line to use when sending to the serial port", + default='CRLF') + parser.add_argument( 'elf_file', help='ELF file of application', type=argparse.FileType('r')) args = parser.parse_args() + if args.port.startswith("/dev/tty."): + args.port = args.port.replace("/dev/tty.", "/dev/cu.") + sys.stderr.write("WARNING: Serial ports accessed as /dev/tty.* will hang gdb if launched. ") + sys.stderr.write("Using %s instead...\n" % args.port) + serial_instance = serial.serial_for_url(args.port, args.baud, do_not_open=True) serial_instance.dtr = False @@ -428,7 +457,7 @@ def main(): except KeyError: pass # not running a make jobserver - monitor = Monitor(serial_instance, args.elf_file.name, args.make) + monitor = Monitor(serial_instance, args.elf_file.name, args.make, args.eol) sys.stderr.write('--- idf_monitor on {p.name} {p.baudrate} ---\n'.format( p=serial_instance)) From 5f3b9876b8088f89f5223484ad000cb4480d03f6 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Tue, 21 Mar 2017 16:08:06 +0800 Subject: [PATCH 2/5] idf_monitor: Fix issues using Ctrl-F/Ctrl-A/gdb with older pyserial Previously error was "AttributeError: 'Console' object has no attribute 'cancel'" --- tools/idf_monitor.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tools/idf_monitor.py b/tools/idf_monitor.py index a8ddf77e796..824a2f8e1bb 100644 --- a/tools/idf_monitor.py +++ b/tools/idf_monitor.py @@ -148,7 +148,15 @@ def run(self): self.console.cleanup() def _cancel(self): - self.console.cancel() + if hasattr(self.console, "cancel"): + self.console.cancel() + elif os.name == 'posix': + # this is the way cancel() is implemented in pyserial 3.1 or newer, + # older pyserial doesn't have this method, hence this hack. + # + # on Windows there is a different (also hacky) fix, applied above. + import fcntl, termios + fcntl.ioctl(self.console.fd, termios.TIOCSTI, b'\0') class SerialReader(StoppableThread): """ Read serial data from the serial port and push to the From 6afea0e81c55a91879e349fc5ed14824915276e7 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Tue, 21 Mar 2017 16:29:57 +0800 Subject: [PATCH 3/5] linker scripts: Add explicit symbols for _iram_start and _flash_cache_start This is to avoid confusion when idf_monitor prints the first symbol in each section, ie "WindowOverflow4" or similar, when bootloader prints the section mapping address. Closes #447 https://github.com/espressif/esp-idf/issues/447 --- components/esp32/ld/esp32.common.ld | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/components/esp32/ld/esp32.common.ld b/components/esp32/ld/esp32.common.ld index bc28e5ca99c..4b1a359631b 100644 --- a/components/esp32/ld/esp32.common.ld +++ b/components/esp32/ld/esp32.common.ld @@ -71,6 +71,10 @@ SECTIONS *(.init.literal) *(.init) _init_end = ABSOLUTE(.); + + /* This goes here, not at top of linker script, so addr2line finds it last, + and uses it in preference to the first symbol in IRAM */ + _iram_start = ABSOLUTE(0); } > iram0_0_seg .iram0.text : @@ -193,5 +197,11 @@ SECTIONS *(.gnu.version) _text_end = ABSOLUTE(.); _etext = .; + + /* Similar to _iram_start, this symbol goes here so it is + resolved by addr2line in preference to the first symbol in + the flash.text segment. + */ + _flash_cache_start = ABSOLUTE(0); } >iram0_2_seg } From a3ce95e482a31d407e5a637c52d563ffc19d50d9 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Tue, 21 Mar 2017 16:46:48 +0800 Subject: [PATCH 4/5] build system: Add explicit DEBUG_FLAGS variable, pass to assembler also --- make/component_wrapper.mk | 2 +- make/project.mk | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/make/component_wrapper.mk b/make/component_wrapper.mk index 31e3c276546..87c05479add 100644 --- a/make/component_wrapper.mk +++ b/make/component_wrapper.mk @@ -165,7 +165,7 @@ $(1)/%.o: $$(COMPONENT_PATH)/$(1)/%.cpp $(COMMON_MAKEFILES) $(COMPONENT_MAKEFILE $(1)/%.o: $$(COMPONENT_PATH)/$(1)/%.S $(COMMON_MAKEFILES) $(COMPONENT_MAKEFILE) | $(1) $$(summary) AS $$@ - $$(CC) $$(CPPFLAGS) $$(addprefix -I ,$$(COMPONENT_INCLUDES)) $$(addprefix -I ,$$(COMPONENT_EXTRA_INCLUDES)) -I$(1) -c $$< -o $$@ + $$(CC) $$(CPPFLAGS) $$(DEBUG_FLAGS) $$(addprefix -I ,$$(COMPONENT_INCLUDES)) $$(addprefix -I ,$$(COMPONENT_EXTRA_INCLUDES)) -I$(1) -c $$< -o $$@ # CWD is build dir, create the build subdirectory if it doesn't exist $(1): diff --git a/make/project.mk b/make/project.mk index e1a23e96e96..617e8aedd06 100644 --- a/make/project.mk +++ b/make/project.mk @@ -241,13 +241,14 @@ OPTIMIZATION_FLAGS = -Og endif # Enable generation of debugging symbols -OPTIMIZATION_FLAGS += -ggdb +# (we generate even in Release mode, as this has no impact on final binary size.) +DEBUG_FLAGS ?= -ggdb # List of flags to pass to C compiler # If any flags are defined in application Makefile, add them at the end. CFLAGS := $(strip \ -std=gnu99 \ - $(OPTIMIZATION_FLAGS) \ + $(OPTIMIZATION_FLAGS) $(DEBUG_FLAGS) \ $(COMMON_FLAGS) \ $(COMMON_WARNING_FLAGS) -Wno-old-style-declaration \ $(CFLAGS) \ @@ -259,7 +260,7 @@ CXXFLAGS := $(strip \ -std=gnu++11 \ -fno-exceptions \ -fno-rtti \ - $(OPTIMIZATION_FLAGS) \ + $(OPTIMIZATION_FLAGS) $(DEBUG_FLAGS) \ $(COMMON_FLAGS) \ $(COMMON_WARNING_FLAGS) \ $(CXXFLAGS) \ From e88226c256a26170c41d1d607aa9dbe0163c554c Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 23 Mar 2017 10:41:14 +0800 Subject: [PATCH 5/5] idf_monitor: Use ANSI color codes for all output we inject into stderr --- tools/idf_monitor.py | 48 ++++++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/tools/idf_monitor.py b/tools/idf_monitor.py index 824a2f8e1bb..46d6e32bd98 100644 --- a/tools/idf_monitor.py +++ b/tools/idf_monitor.py @@ -56,11 +56,20 @@ CTRL_RBRACKET = '\x1d' # Ctrl+] # ANSI terminal codes -ANSI_BLUE = '\033[0;34m' ANSI_RED = '\033[1;31m' ANSI_YELLOW = '\033[0;33m' ANSI_NORMAL = '\033[0m' +def color_print(message, color): + """ Print a message to stderr with colored highlighting """ + sys.stderr.write("%s%s%s\n" % (color, message, ANSI_NORMAL)) + +def yellow_print(message): + color_print(message, ANSI_YELLOW) + +def red_print(message): + color_print(message, ANSI_RED) + __version__ = "1.0" # Tags for tuples in queues @@ -288,7 +297,7 @@ def handle_menu_key(self, c): if c == self.exit_key or c == self.menu_key: # send verbatim self.serial.write(codecs.encode(c)) elif c in [ CTRL_H, 'h', 'H', '?' ]: - sys.stderr.write(self.get_help_text()) + red_print(self.get_help_text()) elif c == CTRL_R: # Reset device via RTS self.serial.setRTS(True) time.sleep(0.2) @@ -298,7 +307,7 @@ def handle_menu_key(self, c): elif c == CTRL_A: # Recompile & upload app only self.run_make("app-flash") else: - sys.stderr.write('--- unknown menu character {} --\n'.format(key_description(c))) + red_print('--- unknown menu character {} --'.format(key_description(c))) def get_help_text(self): return """ @@ -335,16 +344,15 @@ def __exit__(self, *args, **kwargs): def prompt_next_action(self, reason): self.console.setup() # set up console to trap input characters try: - sys.stderr.write(ANSI_RED) - sys.stderr.write("--- {}\n".format(reason)) - sys.stderr.write("--- Press {} to exit monitor.\n" - .format(key_description(self.exit_key))) - sys.stderr.write("--- Press {} to run 'make flash'.\n" - .format(key_description(CTRL_F))) - sys.stderr.write("--- Press {} to run 'make app-flash'.\n" - .format(key_description(CTRL_A))) - sys.stderr.write("--- Press any other key to resume monitor (resets target).\n") - sys.stderr.write(ANSI_NORMAL) + red_print(""" +--- {} +--- Press {} to exit monitor. +--- Press {} to run 'make flash'. +--- Press {} to run 'make app-flash'. +--- Press any other key to resume monitor (resets target).""".format(reason, + key_description(self.exit_key), + key_description(CTRL_F), + key_description(CTRL_A))) k = CTRL_T # ignore CTRL-T here, so people can muscle-memory Ctrl-T Ctrl-F, etc. while k == CTRL_T: k = self.console.getkey() @@ -358,7 +366,7 @@ def prompt_next_action(self, reason): def run_make(self, target): with self: - sys.stderr.write("%s--- Running make %s...\n" % (ANSI_NORMAL, target)) + yellow_print("Running make %s..." % target) p = subprocess.Popen([self.make, target ]) try: @@ -374,7 +382,7 @@ def lookup_pc_address(self, pc_addr): "-pfia", "-e", self.elf_file, pc_addr], cwd=".") if not "?? ??:0" in translation: - sys.stderr.write(ANSI_YELLOW + translation + ANSI_NORMAL) + yellow_print(translation) def check_gdbstub_trigger(self, c): self._gdb_buffer = self._gdb_buffer[-6:] + c # keep the last 7 characters seen @@ -388,7 +396,7 @@ def check_gdbstub_trigger(self, c): if chsum == calc_chsum: self.run_gdb() else: - sys.stderr.write("Malformed gdb message... calculated checksum %02x received %02x\n" % (chsum, calc_chsum)) + red_print("Malformed gdb message... calculated checksum %02x received %02x" % (chsum, calc_chsum)) def run_gdb(self): @@ -444,8 +452,8 @@ def main(): if args.port.startswith("/dev/tty."): args.port = args.port.replace("/dev/tty.", "/dev/cu.") - sys.stderr.write("WARNING: Serial ports accessed as /dev/tty.* will hang gdb if launched. ") - sys.stderr.write("Using %s instead...\n" % args.port) + yellow_print("--- WARNING: Serial ports accessed as /dev/tty.* will hang gdb if launched.") + yellow_print("--- Using %s instead..." % args.port) serial_instance = serial.serial_for_url(args.port, args.baud, do_not_open=True) @@ -467,9 +475,9 @@ def main(): monitor = Monitor(serial_instance, args.elf_file.name, args.make, args.eol) - sys.stderr.write('--- idf_monitor on {p.name} {p.baudrate} ---\n'.format( + yellow_print('--- idf_monitor on {p.name} {p.baudrate} ---'.format( p=serial_instance)) - sys.stderr.write('--- Quit: {} | Menu: {} | Help: {} followed by {} ---\n'.format( + yellow_print('--- Quit: {} | Menu: {} | Help: {} followed by {} ---'.format( key_description(monitor.exit_key), key_description(monitor.menu_key), key_description(monitor.menu_key),