From 713040db60d02bce3cea39ddb999d7d4d8c6d7ab Mon Sep 17 00:00:00 2001 From: M Hightower <27247790+mhightower83@users.noreply.github.com> Date: Fri, 20 Sep 2019 17:07:05 -0700 Subject: [PATCH 1/3] Add typedef for putc1, fn_putc1_t. Replaced relevent usage of `(void *)` with `fn_putc1_t`. Correct usage of `ets_putc()`, returning 0, in libc_replacement.cpp This PR assumes PR https://github.com/esp8266/Arduino/pull/6489#issue-315018841 has merged and removes `uart_buff_switch` from `umm_performance.cpp` Updated method of defining `_rom_putc1` to be more acceptable (I hope) to the new compiler. --- cores/esp8266/core_esp8266_postmortem.cpp | 2 +- cores/esp8266/libc_replacements.cpp | 3 ++- cores/esp8266/uart.cpp | 4 ++-- cores/esp8266/umm_malloc/umm_performance.cpp | 22 +++++++------------- tools/sdk/include/ets_sys.h | 22 +++++++++++++++++--- 5 files changed, 31 insertions(+), 22 deletions(-) diff --git a/cores/esp8266/core_esp8266_postmortem.cpp b/cores/esp8266/core_esp8266_postmortem.cpp index bbe67f1d4d..cb6b35d501 100644 --- a/cores/esp8266/core_esp8266_postmortem.cpp +++ b/cores/esp8266/core_esp8266_postmortem.cpp @@ -109,7 +109,7 @@ void __wrap_system_restart_local() { rst_info.reason = s_user_reset_reason; // TODO: ets_install_putc1 definition is wrong in ets_sys.h, need cast - ets_install_putc1((void *)&uart_write_char_d); + ets_install_putc1(&uart_write_char_d); if (s_panic_line) { ets_printf_P(PSTR("\nPanic %S:%d %S"), s_panic_file, s_panic_line, s_panic_func); diff --git a/cores/esp8266/libc_replacements.cpp b/cores/esp8266/libc_replacements.cpp index 1d591abf0c..b3beb07dd0 100644 --- a/cores/esp8266/libc_replacements.cpp +++ b/cores/esp8266/libc_replacements.cpp @@ -100,7 +100,8 @@ int ICACHE_RAM_ATTR _putc_r(struct _reent* r, int c, FILE* file) __attribute__(( int ICACHE_RAM_ATTR _putc_r(struct _reent* r, int c, FILE* file) { (void) r; if (file->_file == STDOUT_FILENO) { - return ets_putc(c); + ets_putc(c); + return c; } return EOF; } diff --git a/cores/esp8266/uart.cpp b/cores/esp8266/uart.cpp index 30b375cbad..08988f2d3c 100644 --- a/cores/esp8266/uart.cpp +++ b/cores/esp8266/uart.cpp @@ -899,7 +899,7 @@ void uart_set_debug(int uart_nr) { s_uart_debug_nr = uart_nr; - void (*func)(char) = NULL; + fp_putc_t func = NULL; switch(s_uart_debug_nr) { case UART0: @@ -921,7 +921,7 @@ uart_set_debug(int uart_nr) } else { system_set_os_print(0); } - ets_install_putc1((void *) func); + ets_install_putc1(func); } } diff --git a/cores/esp8266/umm_malloc/umm_performance.cpp b/cores/esp8266/umm_malloc/umm_performance.cpp index 4b9ec4e6b3..10d7ef34f1 100644 --- a/cores/esp8266/umm_malloc/umm_performance.cpp +++ b/cores/esp8266/umm_malloc/umm_performance.cpp @@ -40,24 +40,16 @@ bool ICACHE_FLASH_ATTR get_umm_get_perf_data(struct _UMM_TIME_STATS *p, size_t s when interrupts are disabled and w/o availability of heap resources. */ -// ROM _putc1, ignores CRs and sends CR/LF for LF, newline. -// Always returns character sent. -int constexpr (*_rom_putc1)(int) = (int (*)(int))0x40001dcc; -void uart_buff_switch(uint8_t); +/* + * ROM builtin "print character function" _putc1, ignores CRs and sends CR/LF + * for LF, newline. This function is used internally by ets_uart_printf. It is + * also the function that gets installed by ets_uart_install_printf through a + * call to ets_install_putc1. + */ +#define _rom_putc1 ((fp_putc_t)0x40001dcc) int _isr_safe_printf_P(const char *fmt, ...) __attribute__((format(printf, 1, 2))); int ICACHE_RAM_ATTR _isr_safe_printf_P(const char *fmt, ...) { -#ifdef DEBUG_ESP_PORT -#define VALUE(x) __STRINGIFY(x) - // Preprocessor and compiler together will optimize away the if. - if (strcmp("Serial1", VALUE(DEBUG_ESP_PORT)) == 0) { - uart_buff_switch(1U); - } else { - uart_buff_switch(0U); - } -#else - uart_buff_switch(0U); // Side effect, clears RX FIFO -#endif /* To use ets_strlen() and ets_memcpy() safely with PROGMEM, flash storage, the PROGMEM address must be word (4 bytes) aligned. The destination diff --git a/tools/sdk/include/ets_sys.h b/tools/sdk/include/ets_sys.h index da1f3897a4..2af99f7537 100644 --- a/tools/sdk/include/ets_sys.h +++ b/tools/sdk/include/ets_sys.h @@ -33,6 +33,17 @@ extern "C" { #endif +/* + * This "print character function" prototype is modeled after the argument for + * ets_install_putc1() found in "ESP8266_NONOS_SDK/include/osapi.h". This + * deviates away from the familiar C library definition of putchar; however, it + * agrees with the code we are working with. Note, in the ROM some "character + * print functions" always return 0 (uart_tx_one_char and ets_putc), some return + * last character printed (buildin _putc1), and some return nothing + * (ets_write_char). Using a void return type safely represents them all. + */ +typedef void (*fp_putc_t)(char); + typedef uint32_t ETSSignal; typedef uint32_t ETSParam; @@ -205,15 +216,20 @@ char *ets_strstr(const char *haystack, const char *needle); int ets_sprintf(char *str, const char *format, ...) __attribute__ ((format (printf, 2, 3))); int os_snprintf(char *str, size_t size, const char *format, ...) __attribute__ ((format (printf, 3, 4))); int ets_printf(const char *format, ...) __attribute__ ((format (printf, 1, 2))); -void ets_install_putc1(void* routine); +void ets_install_putc1(fp_putc_t routine); void ets_isr_mask(int intr); void ets_isr_unmask(int intr); void ets_isr_attach(int intr, int_handler_t handler, void *arg); void ets_intr_lock(); void ets_intr_unlock(); int ets_vsnprintf(char * s, size_t n, const char * format, va_list arg) __attribute__ ((format (printf, 3, 0))); -int ets_vprintf(int (*print_function)(int), const char * format, va_list arg) __attribute__ ((format (printf, 2, 0))); -int ets_putc(int); +int ets_vprintf(fp_putc_t print_function, const char * format, va_list arg) __attribute__ ((format (printf, 2, 0))); +/* + * ets_putc(), a "print character function" in ROM, prints a character to a + * UART. It always returns 0. The use of this function requires a prior setup + * call to uart_buff_switch() to select the UART. + */ +int ets_putc(char); bool ets_task(ETSTask task, uint8 prio, ETSEvent *queue, uint8 qlen); bool ets_post(uint8 prio, ETSSignal sig, ETSParam par); void ets_update_cpu_frequency(uint32_t ticks_per_us); From ee822deaa0f8a55afa69b7364066f66649715825 Mon Sep 17 00:00:00 2001 From: M Hightower <27247790+mhightower83@users.noreply.github.com> Date: Tue, 24 Sep 2019 11:01:51 -0700 Subject: [PATCH 2/3] Use PROVIDE to expose ROM function entry point, ets_uart_putc1. Added comments to ets_putc() and ets_uart_putc1() to explain their differences. Change prototype of ets_putc() to conform with fp_putc_t. Updated _isr_safe_printf_P to use new definition, ets_uart_putc1. --- cores/esp8266/umm_malloc/umm_performance.cpp | 11 +------ tools/sdk/include/ets_sys.h | 31 ++++++++++++++++---- tools/sdk/ld/eagle.rom.addr.v6.ld | 2 ++ 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/cores/esp8266/umm_malloc/umm_performance.cpp b/cores/esp8266/umm_malloc/umm_performance.cpp index 10d7ef34f1..b2417b02ea 100644 --- a/cores/esp8266/umm_malloc/umm_performance.cpp +++ b/cores/esp8266/umm_malloc/umm_performance.cpp @@ -39,15 +39,6 @@ bool ICACHE_FLASH_ATTR get_umm_get_perf_data(struct _UMM_TIME_STATS *p, size_t s Objective: To be able to print "last gasp" diagnostic messages when interrupts are disabled and w/o availability of heap resources. */ - -/* - * ROM builtin "print character function" _putc1, ignores CRs and sends CR/LF - * for LF, newline. This function is used internally by ets_uart_printf. It is - * also the function that gets installed by ets_uart_install_printf through a - * call to ets_install_putc1. - */ -#define _rom_putc1 ((fp_putc_t)0x40001dcc) - int _isr_safe_printf_P(const char *fmt, ...) __attribute__((format(printf, 1, 2))); int ICACHE_RAM_ATTR _isr_safe_printf_P(const char *fmt, ...) { /* @@ -63,7 +54,7 @@ int ICACHE_RAM_ATTR _isr_safe_printf_P(const char *fmt, ...) { ets_memcpy(ram_buf, fmt, buf_len); va_list argPtr; va_start(argPtr, fmt); - int result = ets_vprintf(_rom_putc1, ram_buf, argPtr); + int result = ets_vprintf(ets_uart_putc1, ram_buf, argPtr); va_end(argPtr); return result; } diff --git a/tools/sdk/include/ets_sys.h b/tools/sdk/include/ets_sys.h index 2af99f7537..e30607eed7 100644 --- a/tools/sdk/include/ets_sys.h +++ b/tools/sdk/include/ets_sys.h @@ -37,9 +37,9 @@ extern "C" { * This "print character function" prototype is modeled after the argument for * ets_install_putc1() found in "ESP8266_NONOS_SDK/include/osapi.h". This * deviates away from the familiar C library definition of putchar; however, it - * agrees with the code we are working with. Note, in the ROM some "character - * print functions" always return 0 (uart_tx_one_char and ets_putc), some return - * last character printed (buildin _putc1), and some return nothing + * agrees with the code we are working with. Note, in the ROM some "printf + * character functions" always return 0 (uart_tx_one_char and ets_putc), some + * return last character printed (buildin _putc1), and some return nothing * (ets_write_char). Using a void return type safely represents them all. */ typedef void (*fp_putc_t)(char); @@ -224,12 +224,33 @@ void ets_intr_lock(); void ets_intr_unlock(); int ets_vsnprintf(char * s, size_t n, const char * format, va_list arg) __attribute__ ((format (printf, 3, 0))); int ets_vprintf(fp_putc_t print_function, const char * format, va_list arg) __attribute__ ((format (printf, 2, 0))); + /* * ets_putc(), a "print character function" in ROM, prints a character to a - * UART. It always returns 0. The use of this function requires a prior setup + * UART. It always returns 0; however, the prototype here is defined with void + * return to make compatible with other usages of fp_putc_t. ets_putc() provides + * a "raw", print as is, interface. '\r' and '\n' are each printed exactly as is + * w/o addition. For a "cooked" interface use ets_uart_putc1(). + * The use of this function requires a prior setup call to uart_buff_switch() to + * select the UART. + */ +void ets_putc(char); + +/* + * ets_uart_putc1(), a "print character function" in ROM, prints a character to + * a UART. It returns the character printed; however, the prototype here is + * defined with void return to make compatible with other usages of fp_putc_t. + * This function provides additional processing to characters '\r' and '\n'. It + * filters out '\r'. When called with '\n', it prints characters '\r' and '\n'. + * This is sometimes refered to as a "cooked" interface. For a "raw", print as + * is, interface use ets_putc(). The use of this function requires a prior setup * call to uart_buff_switch() to select the UART. + * ets_uart_putc1() is used internally by ets_uart_printf. It is also the + * function that gets installed by ets_uart_install_printf through a call to + * ets_install_putc1. */ -int ets_putc(char); +void ets_uart_putc1(char); + bool ets_task(ETSTask task, uint8 prio, ETSEvent *queue, uint8 qlen); bool ets_post(uint8 prio, ETSSignal sig, ETSParam par); void ets_update_cpu_frequency(uint32_t ticks_per_us); diff --git a/tools/sdk/ld/eagle.rom.addr.v6.ld b/tools/sdk/ld/eagle.rom.addr.v6.ld index 144c72b557..84c5e3acf5 100644 --- a/tools/sdk/ld/eagle.rom.addr.v6.ld +++ b/tools/sdk/ld/eagle.rom.addr.v6.ld @@ -119,6 +119,8 @@ PROVIDE ( ets_install_external_printf = 0x40002450 ); PROVIDE ( ets_install_putc1 = 0x4000242c ); PROVIDE ( ets_install_putc2 = 0x4000248c ); PROVIDE ( ets_install_uart_printf = 0x40002438 ); +/* Undocumented function to print character to UART */ +PROVIDE ( ets_uart_putc1 = 0x40001dcc ); /* permanently hide reimplemented ets_intr_*lock(), see #6484 PROVIDE ( ets_intr_lock = 0x40000f74 ); PROVIDE ( ets_intr_unlock = 0x40000f80 ); From 2da628fa7693de2a32c26d9fb0fc1ea30393f035 Mon Sep 17 00:00:00 2001 From: M Hightower <27247790+mhightower83@users.noreply.github.com> Date: Tue, 24 Sep 2019 17:19:30 -0700 Subject: [PATCH 3/3] Deleted obsolete comment. --- cores/esp8266/core_esp8266_postmortem.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/cores/esp8266/core_esp8266_postmortem.cpp b/cores/esp8266/core_esp8266_postmortem.cpp index cb6b35d501..0b254b9b73 100644 --- a/cores/esp8266/core_esp8266_postmortem.cpp +++ b/cores/esp8266/core_esp8266_postmortem.cpp @@ -108,7 +108,6 @@ void __wrap_system_restart_local() { else rst_info.reason = s_user_reset_reason; - // TODO: ets_install_putc1 definition is wrong in ets_sys.h, need cast ets_install_putc1(&uart_write_char_d); if (s_panic_line) {