Fix statusbar corruption

src/commands/global/cls.c

@@ -17,6 +17,7 @@ static const char* const usage[] = {
 static const struct ui_help_options options[] = {};
 
 void ui_display_clear(struct command_result* res) {
+    BP_ASSERT_CORE0();
     if (ui_help_show(res->help_flag, usage, count_of(usage), &options[0], count_of(options))) {
         return;
     }
@@ -30,6 +31,6 @@ void ui_display_clear(struct command_result* res) {
     ui_term_init();   // Initialize VT100 if ANSI terminal
     if (system_config.terminal_ansi_color && system_config.terminal_ansi_statusbar) {
         ui_statusbar_init();
-        ui_statusbar_update(UI_UPDATE_ALL);
+        ui_statusbar_update_blocking();
     }
 }

src/commands/global/script.c

@@ -96,7 +96,7 @@ bool script_exec(char* location, bool pause_for_input, bool show_comments, bool
                 if (file[i] == '\r' || file[i] == '\n' || file[i] == '\0') {
                     break;
                 }
-                rx_fifo_add(&file[i]);
+                rx_fifo_add(&file[i]); // BUGBUG -- breaks FIFO queue only being added to by Core1
                 // cmdln_try_add(&file[i]);
                 // tx_fifo_put(&file[i]);
             }

src/debug_rtt.h

@@ -202,37 +202,38 @@ static inline bool bp_debug_should_print(bp_debug_level_t level, bp_debug_catego
 void bp_mark_system_initialized(void);
 bool bp_is_system_initialized(void);
 
-#ifdef NDEBUG
-
-// These assertions only assert after the system is fully initialized.
+// Verify execution is on Core0 ... but only warn if system is not fully initialized.
 #define BP_ASSERT_CORE0() \
-    do {                                                                  \
-        if ((get_core_num() != 0) && bp_is_system_initialized()) {        \
-            PRINT_FATAL("ASSERTION FAILED: %s:%d\n", __FILE__, __LINE__); \
-            hard_assert(false);                                           \
-        }                                                                 \
+    do {                                                                                       \
+        if (get_core_num() != 0) {                                                             \
+            if (bp_is_system_initialized()) {                                                  \
+                PRINT_FATAL("Not on Core0: %s:%d\n", __FILE__, __LINE__);                      \
+                hard_assert(false);                                                            \
+            } else {                                                                           \
+                PRINT_FATAL("Warning: Not on Core0 during init: %s:%d\n", __FILE__, __LINE__); \
+            }                                                                                  \
+        }                                                                                      \
     } while (0)
+
+// Verify execution is on Core1 ... but only warn if system is not fully initialized.
 #define BP_ASSERT_CORE1() \
-    do {                                                                  \
-        if ((get_core_num() != 1) && bp_is_system_initialized()) {        \
-            PRINT_FATAL("ASSERTION FAILED: %s:%d\n", __FILE__, __LINE__); \
-            hard_assert(false);                                           \
-        }                                                                 \
+    do {                                                                                       \
+        if (get_core_num() != 1) {                                                             \
+            if (bp_is_system_initialized()) {                                                  \
+                PRINT_FATAL("Not on Core1: %s:%d\n", __FILE__, __LINE__);                      \
+                hard_assert(false);                                                            \
+            } else {                                                                           \
+                PRINT_FATAL("Warning: Not on Core1 during init: %s:%d\n", __FILE__, __LINE__); \
+            }                                                                                  \
+        }                                                                                      \
     } while (0)
+
+// Ensures an assertion is checked and file/line number are printed (even in NDEBUG compilation).
+// If used hard_assert() directly, file/line number would not be shown.
 #define BP_ASSERT(_COND) \
     do {                                                                  \
         if (!(_COND)) {                                                   \
             PRINT_FATAL("ASSERTION FAILED: %s:%d\n", __FILE__, __LINE__); \
             hard_assert(false);                                           \
         }                                                                 \
     } while (0)
-
-#else
-
-// These assertions only assert after the system is fully initialized.
-#define BP_ASSERT_CORE0() assert((get_core_num() == 0) || !bp_is_system_initialized())
-#define BP_ASSERT_CORE1() assert((get_core_num() == 1) || !bp_is_system_initialized())
-#define BP_ASSERT(_COND)  assert(_COND)
-#endif
-
-

src/mode/binloopback.c

@@ -75,13 +75,13 @@ void binloopback_close(struct _bytecode* result, struct _bytecode* next) {
 
 void binloopback_write(struct _bytecode* result, struct _bytecode* next) {
     char c = result->out_data;
-    bin_rx_fifo_add(&c);
+    bin_rx_fifo_add(&c); // ?? BUGBUG -- should this be bin_tx_fifo_add() ??
 }
 
 void binloopback_read(struct _bytecode* result, struct _bytecode* next) {
     uint32_t timeout = 0xfff;
     char c;
-    while (!bin_tx_fifo_try_get(&c)) {
+    while (!bin_tx_fifo_try_get(&c)) { // ?? BUGBUG -- should this be bin_rx_fifo_try_get() ??
         timeout--;
         if (!timeout) {
             result->error = SERR_ERROR;

src/pirate.c

@@ -522,7 +522,7 @@ static void core0_infinite_loop(void) {
                             // and does the initial painting of the full statusbar
                             if (system_config.terminal_ansi_statusbar) {
                                 ui_statusbar_init();
-                                ui_statusbar_update(UI_UPDATE_ALL);
+                                ui_statusbar_update_blocking();
                             }
                             break;
                         default:
@@ -768,7 +768,7 @@ static void core1_infinite_loop(void) {
                 system_config.terminal_ansi_statusbar &&
                 system_config.terminal_ansi_statusbar_update &&
                 !system_config.terminal_ansi_statusbar_pause) {
-                ui_statusbar_update(update_flags);
+                ui_statusbar_update_from_core1(update_flags);
             }
 
 #if (BP_VER == 5 && BP_REV <= 9)

src/ui/ui_config.c

@@ -153,6 +153,8 @@ uint32_t ui_config_action_ansi_color(uint32_t a, uint32_t b) {
 }
 
 uint32_t ui_config_action_ansi_toolbar(uint32_t a, uint32_t b) {
+    BP_ASSERT_CORE0();
+
     // NOTE: `b` is treated as a boolean value
     b = !!b;
 
@@ -164,7 +166,7 @@ uint32_t ui_config_action_ansi_toolbar(uint32_t a, uint32_t b) {
         }
         ui_term_detect(); // Do we detect a VT100 ANSI terminal? what is the size?
         ui_term_init();   // Initialize VT100 if ANSI terminal
-        ui_statusbar_update(UI_UPDATE_ALL);
+        ui_statusbar_update_blocking();
     }
 }
 

src/ui/ui_statusbar.c

@@ -11,6 +11,7 @@
 #include "ui/ui_flags.h"
 #include "system_monitor.h"
 #include "display/scope.h"
+#include "pirate/intercore_helpers.h"
 
 uint32_t ui_statusbar_info(char* buf, size_t buffLen) {
     uint32_t len = 0;
@@ -209,8 +210,13 @@ uint32_t ui_statusbar_value(char* buf, size_t buffLen) {
 
     return (do_update ? len : 0);
 }
+void ui_statusbar_update_blocking() {
+    BP_ASSERT_CORE0(); // if called from core1, this will deadlock
+    icm_core0_send_message_synchronous(BP_ICM_FORCE_LCD_UPDATE);
+}
+void ui_statusbar_update_from_core1(uint32_t update_flags) {
+    BP_ASSERT_CORE1();
 
-void ui_statusbar_update(uint32_t update_flags) {
     uint32_t len = 0;
     size_t buffLen = sizeof(tx_sb_buf);
 

src/ui/ui_statusbar.h

@@ -1,3 +1,5 @@
-void ui_statusbar_update(uint32_t update_flags);
+#pragma once
+void ui_statusbar_update_blocking();
+void ui_statusbar_update_from_core1(uint32_t update_flags);
 void ui_statusbar_init(void);
-void ui_statusbar_deinit(void);
+void ui_statusbar_deinit(void);

src/usb_rx.c

@@ -26,19 +26,24 @@ void rx_uart_irq_handler(void);
 
 queue_t rx_fifo;
 queue_t bin_rx_fifo;
-#define RX_FIFO_LENGTH_IN_BITS 7 // 2^n buffer size. 2^3=8, 2^9=512
+#define RX_FIFO_LENGTH_IN_BITS 7 // tinyUSB requires 2^n buffer size, so declare in bits.   2^3=8, 2^7=128, 2^9=512, etc.
 #define RX_FIFO_LENGTH_IN_BYTES (0x0001 << RX_FIFO_LENGTH_IN_BITS)
 char rx_buf[RX_FIFO_LENGTH_IN_BYTES];
 char bin_rx_buf[RX_FIFO_LENGTH_IN_BYTES];
 
 // init buffer (and IRQ for UART debug mode)
 void rx_fifo_init(void) {
+    // OK to call from either core
     queue2_init(&rx_fifo, rx_buf, RX_FIFO_LENGTH_IN_BYTES); // buffer size must be 2^n for queue AND DMA rollover
     queue2_init(&bin_rx_fifo, bin_rx_buf, RX_FIFO_LENGTH_IN_BYTES);
 }
 
 // enables receive interrupt for ALREADY configured debug uarts (see init in debug.c)
 void rx_uart_init_irq(void) {
+    // RX FIFO (whether from UART, CDC, RTT, ...) should only be added to from core1 (deadlock risk)
+    // irq_set_exclusive_handler() sets the interrupt to be exclusive to the current core.
+    // therefore, this function must be called from Core1.
+    BP_ASSERT_CORE1();
     // rx interrupt
     irq_set_exclusive_handler(debug_uart[system_config.terminal_uart_number].irq, rx_uart_irq_handler);
     // irq_set_priority(debug_uart[system_config.terminal_uart_number].irq, 0xFF);
@@ -48,14 +53,16 @@ void rx_uart_init_irq(void) {
 
 // UART interrupt handler
 void rx_uart_irq_handler(void) {
+    BP_ASSERT_CORE1(); // RX FIFO (whether from UART, CDC, RTT, ...) should only be added to from core1 (deadlock risk)
     // while bytes available shove them in the buffer
     while (uart_is_readable(debug_uart[system_config.terminal_uart_number].uart)) {
         uint8_t c = uart_getc(debug_uart[system_config.terminal_uart_number].uart);
-        queue2_add_blocking(&rx_fifo, &c);
+        queue2_add_blocking(&rx_fifo, &c); // BUGBUG -- blocking call from ISR!
     }
 }
 
 void rx_usb_init(void) {
+    BP_ASSERT_CORE1();
     tusb_init();
 }
 
@@ -64,6 +71,8 @@ void rx_usb_init(void) {
 // If RX queue is full, store the last character in a local static variable.  Always use that character
 // (if positive) first when entering the function.  Thus, no RTT input characters are ever lost.
 void rx_from_rtt_terminal(void) {
+    BP_ASSERT_CORE1(); // RX FIFO (whether from UART, CDC, RTT, ...) should only be added to from core1 (deadlock risk)
+
     static int last_character = -1;
 
     // if prior callback left a prior character, use it first.
@@ -88,14 +97,17 @@ void rx_from_rtt_terminal(void) {
 // USB (tinyUSB) interrupt handler
 // Invoked when CDC interface received data from host
 void tud_cdc_rx_cb(uint8_t itf) {
+
+    BP_ASSERT_CORE1(); // RX FIFO (whether from UART, CDC, RTT, ...) should only be added to from core1 (deadlock risk)
+
     char buf[64];
 
     if (itf == 0 && tud_cdc_n_available(0)) {
         uint32_t count = tud_cdc_n_read(0, buf, 64);
 
         // while bytes available shove them in the buffer
         for (uint8_t i = 0; i < count; i++) {
-            queue2_add_blocking(&rx_fifo, &buf[i]);
+            queue2_add_blocking(&rx_fifo, &buf[i]); // BUGBUG -- blocking call from ISR!
         }
     }
 
@@ -104,53 +116,65 @@ void tud_cdc_rx_cb(uint8_t itf) {
 
         // while bytes available shove them in the buffer
         for (uint8_t i = 0; i < count; i++) {
-            queue2_add_blocking(&bin_rx_fifo, &buf[i]);
+            queue2_add_blocking(&bin_rx_fifo, &buf[i]); // BUGBUG -- blocking call from ISR!
         }
     }
 }
 
 // Invoked when cdc when line state changed e.g connected/disconnected
 void tud_cdc_line_state_cb(uint8_t itf, bool dtr, bool rts) {
+    BP_DEBUG_PRINT(BP_DEBUG_LEVEL_VERBOSE, BP_DEBUG_CAT_TEMP,
+        "--> tud_cdc_line_state_cb(%d, %d, %d)\n", itf, dtr, rts
+        );
     system_config.rts = rts;
 }
 
 // insert a byte into the queue
 void rx_fifo_add(char* c) {
+    BP_ASSERT_CORE1(); // RX FIFO (whether from UART, CDC, RTT, ...) should only be added to from core1 (deadlock risk)
     queue2_add_blocking(&rx_fifo, c);
 }
 
 // functions to access the ring buffer from other code
 // block until a byte is available, remove from buffer
 void rx_fifo_get_blocking(char* c) {
+    BP_ASSERT_CORE0(); // RX FIFO (whether from UART, CDC, RTT, ...) should only be drained from core0 (deadlock risk)
     queue2_remove_blocking(&rx_fifo, c);
 }
 // try to get a byte, remove from buffer if available, return false if no byte
 bool rx_fifo_try_get(char* c) {
+    // OK to call from either core
     return queue2_try_remove(&rx_fifo, c);
 }
 // block until a byte is available, return byte but leave in buffer
 void rx_fifo_peek_blocking(char* c) {
+    BP_ASSERT_CORE0(); // RX FIFO (whether from UART, CDC, RTT, ...) should only be drained from core0 (deadlock risk)
     queue2_peek_blocking(&rx_fifo, c);
 }
 // try to peek at next byte, return byte but leave in buffer, return false if no byte
 bool rx_fifo_try_peek(char* c) {
+    // OK to call from either core
     return queue2_try_peek(&rx_fifo, c);
 }
 
 // BINMODE queue
 void bin_rx_fifo_add(char* c) {
+    BP_ASSERT_CORE1();
     queue2_add_blocking(&bin_rx_fifo, c);
 }
 
 void bin_rx_fifo_get_blocking(char* c) {
+    BP_ASSERT_CORE0(); // RX FIFO (whether from UART, CDC, RTT, ...) should only be drained from core0 (deadlock risk)
     queue2_remove_blocking(&bin_rx_fifo, c);
 }
 
 void bin_rx_fifo_available_bytes(uint16_t* cnt) {
+    // OK to call from either core
     queue_available_bytes(&bin_rx_fifo, cnt);
 }
 
 bool bin_rx_fifo_try_get(char* c) {
+    // OK to call from either core
     bool result = queue2_try_remove(&bin_rx_fifo, c);
     // if(result) printf("%.2x ", (*c));
     return result;

src/usb_tx.c

@@ -30,18 +30,23 @@ queue_t bin_tx_fifo;
 char tx_buf[TX_FIFO_LENGTH_IN_BYTES] __attribute__((aligned(2048)));
 char bin_tx_buf[TX_FIFO_LENGTH_IN_BYTES] __attribute__((aligned(2048)));
 
-char tx_sb_buf[1024];
+#define MAXIMUM_STATUS_BAR_BUFFER_BYTES 1024
+char tx_sb_buf[MAXIMUM_STATUS_BAR_BUFFER_BYTES];
 uint16_t tx_sb_buf_cnt = 0;
 uint16_t tx_sb_buf_index = 0;
 bool tx_sb_buf_ready = false;
 
 void tx_fifo_init(void) {
+    // OK to call from either core
     queue2_init(&tx_fifo, tx_buf, TX_FIFO_LENGTH_IN_BYTES); // buffer size must be 2^n for queue AND DMA rollover
     queue2_init(&bin_tx_fifo, bin_tx_buf, TX_FIFO_LENGTH_IN_BYTES); // buffer size must be 2^n for queue AND DMA
                                                                     // rollover
 }
 
 void tx_sb_start(uint32_t valid_characters_in_status_bar) {
+
+    BP_ASSERT_CORE1();
+    BP_ASSERT(valid_characters_in_status_bar <= MAXIMUM_STATUS_BAR_BUFFER_BYTES);
     tx_sb_buf_cnt = valid_characters_in_status_bar;
     tx_sb_buf_ready = true;
 }
@@ -145,27 +150,22 @@ void tx_fifo_service(void) {
 }
 
 void tx_fifo_put(char* c) {
-    uint core = get_core_num();
-    assert(core == 0); // tx fifo should not be shared between cores
-    assert(core != 1); // tx fifo can deadlock if called from core1
+    BP_ASSERT_CORE0(); // tx fifo shoudl only be added to from core 0 (deadlock risk)
     queue2_add_blocking(&tx_fifo, c);
 }
 
 void bin_tx_fifo_put(const char c) {
-    uint core = get_core_num();
-    assert(core == 0); // binmode fifo should not be shared between cores
-    assert(core != 1); // binmode fifo can deadlock if called from core1
+    BP_ASSERT_CORE0(); // tx fifo shoudl only be added to from core 0 (deadlock risk)
     queue2_add_blocking(&bin_tx_fifo, &c);
 }
 
 bool bin_tx_fifo_try_get(char* c) {
+    BP_ASSERT_CORE1(); // tx fifo is drained from core1 only
     return queue2_try_remove(&bin_tx_fifo, c);
 }
 
-// #if(0)
 void bin_tx_fifo_service(void) {
-    uint core = get_core_num();
-    assert(core == 1); // binmode fifo is drained from core1 only
+    BP_ASSERT_CORE1(); // tx fifo is drained from core1 only
 
     uint16_t bytes_available;
     char data[64];
@@ -190,10 +190,12 @@ void bin_tx_fifo_service(void) {
     tud_cdc_n_write(1, &data, i);
     tud_cdc_n_write_flush(1);
 }
-// #endif
 
 bool bin_tx_not_empty(void) {
+    // OK to check empty from either core
     uint16_t cnt;
+    // gets the number of additional bytes that can be added to the queue
     queue_available_bytes(&bin_tx_fifo, &cnt);
-    return TX_FIFO_LENGTH_IN_BYTES - cnt;
+    // If that differs from the total size of the queue, then it's not empty
+    return cnt != TX_FIFO_LENGTH_IN_BYTES;
 }