From 65a89591d655dedf374cc98e776fc310ff8ec2db Mon Sep 17 00:00:00 2001 From: Antonino Di Guardo <64427768+digant73@users.noreply.github.com> Date: Thu, 24 Aug 2023 03:55:24 +0200 Subject: [PATCH] Fixed random pause/hesitation when printing (#2827) * Fixed random pause/hesitation when printing * added volatile type for wIndex on Serial.h * minor cleanup --- TFT/src/User/API/Printing.c | 2 +- TFT/src/User/API/SerialConnection.c | 47 ++++++++++++-------------- TFT/src/User/API/SerialConnection.h | 6 ++++ TFT/src/User/Hal/gd32f20x/Serial.c | 9 +---- TFT/src/User/Hal/gd32f20x/Serial.h | 7 ++-- TFT/src/User/Hal/stm32f10x/Serial.c | 9 +---- TFT/src/User/Hal/stm32f10x/Serial.h | 7 ++-- TFT/src/User/Hal/stm32f2_f4xx/Serial.c | 9 +---- TFT/src/User/Hal/stm32f2_f4xx/Serial.h | 7 ++-- TFT/src/User/main.h | 14 ++++---- 10 files changed, 51 insertions(+), 66 deletions(-) diff --git a/TFT/src/User/API/Printing.c b/TFT/src/User/API/Printing.c index 1e5ca80900..10d409c102 100644 --- a/TFT/src/User/API/Printing.c +++ b/TFT/src/User/API/Printing.c @@ -105,7 +105,7 @@ void loopBreakToCondition(CONDITION_CALLBACK condCallback) // from that command. Than another "M108" will be sent to unlock a next possible blocking command. // This way enough "M108" will be sent to unlock all possible blocking command(s) (ongoing or enqueued) but not too much and // not too fast one after another to overload/overrun the serial communication - TASK_LOOP_WHILE(condCallback(), if (infoHost.rx_ok[SERIAL_PORT] == true) sendEmergencyCmd("M108\n")) + TASK_LOOP_WHILE(condCallback(), if (Serial_Available(SERIAL_PORT) != 0) sendEmergencyCmd("M108\n")); // remove any enqueued command that could come from a supplementary serial port or TFT media // (if printing from remote host or TFT media) during the loop above diff --git a/TFT/src/User/API/SerialConnection.c b/TFT/src/User/API/SerialConnection.c index 7129f1112d..6c3dd9dfb9 100644 --- a/TFT/src/User/API/SerialConnection.c +++ b/TFT/src/User/API/SerialConnection.c @@ -112,7 +112,7 @@ void Serial_Forward(SERIAL_PORT_INDEX portIndex, const char * msg) Serial_Put(SERIAL_DEBUG_PORT, msg); #endif - uint8_t portCount = SERIAL_PORT_COUNT; // by default, select all the serial ports + uint8_t portCount = SERIAL_PORT_COUNT; // by default, select all the serial ports if (portIndex == ALL_PORTS) // if ALL_PORTS, forward the message to all the enabled serial ports portIndex = PORT_1; @@ -136,55 +136,52 @@ void Serial_Forward(SERIAL_PORT_INDEX portIndex, const char * msg) } } +uint16_t Serial_Available(SERIAL_PORT_INDEX portIndex) +{ + if (!WITHIN(portIndex, PORT_1, SERIAL_PORT_COUNT - 1)) + return 0; + + return (dmaL1Data[portIndex].cacheSize + dmaL1Data[portIndex].wIndex - dmaL1Data[portIndex].rIndex) % dmaL1Data[portIndex].cacheSize; +} + uint16_t Serial_Get(SERIAL_PORT_INDEX portIndex, char * buf, uint16_t bufSize) { // if port index is out of range or no data to read from L1 cache - if (!WITHIN(portIndex, PORT_1, SERIAL_PORT_COUNT - 1) || !infoHost.rx_ok[portIndex]) + if (!WITHIN(portIndex, PORT_1, SERIAL_PORT_COUNT - 1) || dmaL1Data[portIndex].flag == dmaL1Data[portIndex].wIndex) return 0; - // make access to dinamically changed (by L1 cache's interrupt handler) variables/attributes faster and also reducing the code DMA_CIRCULAR_BUFFER * dmaL1Data_ptr = &dmaL1Data[portIndex]; - uint16_t * wIndex_ptr = &dmaL1Data_ptr->wIndex; - // L1 cache's reading index (not dinamically changed (by L1 cache's interrupt handler) variables/attributes) + // make a static access to dynamically changed (by L1 cache's interrupt handler) variables/attributes + uint16_t wIndex = dmaL1Data_ptr->wIndex; + + // L1 cache's reading index (not dynamically changed (by L1 cache's interrupt handler) variables/attributes) uint16_t rIndex = dmaL1Data_ptr->rIndex; - while (dmaL1Data_ptr->cache[rIndex] == ' ' && rIndex != *wIndex_ptr) // remove leading empty space, if any + while (dmaL1Data_ptr->cache[rIndex] == ' ' && rIndex != wIndex) // remove leading empty space, if any { rIndex = (rIndex + 1) % dmaL1Data_ptr->cacheSize; } - for (uint16_t i = 0; i < (bufSize - 1) && rIndex != *wIndex_ptr; ) // retrieve data until buf is full or L1 cache is empty + for (uint16_t i = 0; i < (bufSize - 1) && rIndex != wIndex; ) // retrieve data until buf is full or L1 cache is empty { buf[i] = dmaL1Data_ptr->cache[rIndex]; rIndex = (rIndex + 1) % dmaL1Data_ptr->cacheSize; if (buf[i++] == '\n') // if data end marker is found { - buf[i] = '\0'; // end character - dmaL1Data_ptr->rIndex = rIndex; // update queue's index - - if (rIndex == dmaL1Data_ptr->wIndex) // if L1 cache is empty, mark the port as containing no more data - infoHost.rx_ok[portIndex] = false; + buf[i] = '\0'; // end character + dmaL1Data_ptr->flag = dmaL1Data_ptr->rIndex = rIndex; // update queue's custom flag and reading index with rIndex return i; // return the number of bytes stored in buf } } // if here, a partial message is present on the L1 cache (message not terminated by "\n"). - // We temporary skip the message until it is fully received - // - // NOTES: - // - this scenario typically happens when the TFT receives a burst of messages (e.g. the output for "M420 V1 T1"). - // The first fully received message (terminated by "\n") is enough to trigger the L1 cache as available - // (infoHost.rx_ok[portIndex] set to "true") for reading - // - it is more safe to leave the following code line commented out just to avoid any possibility the - // L1 cache's interrupt handler is receiving (and triggering L1 cache as available) in the meanwhile - // (more safe to perform an active polling on the next invokation of this function than the possibility - // to not be able to read the message (if infoHost.rx_ok[portIndex] is found set to "false") until a - // next message (if any) becomes available) - // -// infoHost.rx_ok[portIndex] = false; // mark the port as containing no more (or partial) data + // We temporary skip the message until it is fully received updating also dmaL1Data_ptr->flag to + // prevent to read again (multiple times) the same partial message on next function invokation + + dmaL1Data_ptr->flag = wIndex; // update queue's custom flag with wIndex return 0; // return the number of bytes stored in buf } diff --git a/TFT/src/User/API/SerialConnection.h b/TFT/src/User/API/SerialConnection.h index 92053788bd..de1d055664 100644 --- a/TFT/src/User/API/SerialConnection.h +++ b/TFT/src/User/API/SerialConnection.h @@ -62,6 +62,12 @@ void Serial_DeInit(SERIAL_PORT_INDEX portIndex); // - specific port index: specific serial port void Serial_Forward(SERIAL_PORT_INDEX portIndex, const char * msg); +// retrieve the number of bytes available on the provided serial port: +// - portIndex: index of serial port +// +// - return value: number of bytes available on serial port +uint16_t Serial_Available(SERIAL_PORT_INDEX portIndex); + // retrieve a message from the provided serial port, if any: // - portIndex: index of serial port where data are read from // - buf: buffer where data are stored diff --git a/TFT/src/User/Hal/gd32f20x/Serial.c b/TFT/src/User/Hal/gd32f20x/Serial.c index b47096edef..2b9badedcb 100644 --- a/TFT/src/User/Hal/gd32f20x/Serial.c +++ b/TFT/src/User/Hal/gd32f20x/Serial.c @@ -69,15 +69,13 @@ void Serial_DMA_Config(uint8_t port) void Serial_ClearData(uint8_t port) { - dmaL1Data[port].rIndex = dmaL1Data[port].wIndex = dmaL1Data[port].cacheSize = 0; + dmaL1Data[port].wIndex = dmaL1Data[port].rIndex = dmaL1Data[port].flag = dmaL1Data[port].cacheSize = 0; if (dmaL1Data[port].cache != NULL) { free(dmaL1Data[port].cache); dmaL1Data[port].cache = NULL; } - - infoHost.rx_ok[port] = false; } void Serial_Config(uint8_t port, uint16_t cacheSize, uint32_t baudrate) @@ -109,11 +107,6 @@ void USART_IRQHandler(uint8_t port) USART_DATA(Serial[port].uart); dmaL1Data[port].wIndex = dmaL1Data[port].cacheSize - DMA_CHCNT(Serial[port].dma_stream, Serial[port].dma_channel); - uint16_t wIndex = (dmaL1Data[port].wIndex == 0) ? dmaL1Data[port].cacheSize : dmaL1Data[port].wIndex; - if (dmaL1Data[port].cache[wIndex - 1] == '\n') // Receive completed - { - infoHost.rx_ok[port] = true; - } } } diff --git a/TFT/src/User/Hal/gd32f20x/Serial.h b/TFT/src/User/Hal/gd32f20x/Serial.h index 9888133082..ca48a22082 100644 --- a/TFT/src/User/Hal/gd32f20x/Serial.h +++ b/TFT/src/User/Hal/gd32f20x/Serial.h @@ -5,11 +5,12 @@ #include "variants.h" // for uint32_t etc... #include "uart.h" -typedef struct +typedef volatile struct // precautionally declared as volatile due to access from interrupt handler and main thread { char *cache; - uint16_t wIndex; - uint16_t rIndex; + uint16_t wIndex; // writing index + uint16_t rIndex; // reading index + uint16_t flag; // custom flag (for custom usage by the application) uint16_t cacheSize; } DMA_CIRCULAR_BUFFER; diff --git a/TFT/src/User/Hal/stm32f10x/Serial.c b/TFT/src/User/Hal/stm32f10x/Serial.c index 489dfdff55..bbe60e14c8 100644 --- a/TFT/src/User/Hal/stm32f10x/Serial.c +++ b/TFT/src/User/Hal/stm32f10x/Serial.c @@ -41,15 +41,13 @@ void Serial_DMA_Config(uint8_t port) void Serial_ClearData(uint8_t port) { - dmaL1Data[port].rIndex = dmaL1Data[port].wIndex = dmaL1Data[port].cacheSize = 0; + dmaL1Data[port].wIndex = dmaL1Data[port].rIndex = dmaL1Data[port].flag = dmaL1Data[port].cacheSize = 0; if (dmaL1Data[port].cache != NULL) { free(dmaL1Data[port].cache); dmaL1Data[port].cache = NULL; } - - infoHost.rx_ok[port] = false; } void Serial_Config(uint8_t port, uint16_t cacheSize, uint32_t baudrate) @@ -80,11 +78,6 @@ void USART_IRQHandler(uint8_t port) Serial[port].uart->DR; dmaL1Data[port].wIndex = dmaL1Data[port].cacheSize - Serial[port].dma_chanel->CNDTR; - uint16_t wIndex = (dmaL1Data[port].wIndex == 0) ? dmaL1Data[port].cacheSize : dmaL1Data[port].wIndex; - if (dmaL1Data[port].cache[wIndex - 1] == '\n') // Receive completed - { - infoHost.rx_ok[port] = true; - } } } diff --git a/TFT/src/User/Hal/stm32f10x/Serial.h b/TFT/src/User/Hal/stm32f10x/Serial.h index 9888133082..ca48a22082 100644 --- a/TFT/src/User/Hal/stm32f10x/Serial.h +++ b/TFT/src/User/Hal/stm32f10x/Serial.h @@ -5,11 +5,12 @@ #include "variants.h" // for uint32_t etc... #include "uart.h" -typedef struct +typedef volatile struct // precautionally declared as volatile due to access from interrupt handler and main thread { char *cache; - uint16_t wIndex; - uint16_t rIndex; + uint16_t wIndex; // writing index + uint16_t rIndex; // reading index + uint16_t flag; // custom flag (for custom usage by the application) uint16_t cacheSize; } DMA_CIRCULAR_BUFFER; diff --git a/TFT/src/User/Hal/stm32f2_f4xx/Serial.c b/TFT/src/User/Hal/stm32f2_f4xx/Serial.c index 7635ab63e6..fe931f69c2 100644 --- a/TFT/src/User/Hal/stm32f2_f4xx/Serial.c +++ b/TFT/src/User/Hal/stm32f2_f4xx/Serial.c @@ -70,15 +70,13 @@ void Serial_DMA_Config(uint8_t port) void Serial_ClearData(uint8_t port) { - dmaL1Data[port].rIndex = dmaL1Data[port].wIndex = dmaL1Data[port].cacheSize = 0; + dmaL1Data[port].wIndex = dmaL1Data[port].rIndex = dmaL1Data[port].flag = dmaL1Data[port].cacheSize = 0; if (dmaL1Data[port].cache != NULL) { free(dmaL1Data[port].cache); dmaL1Data[port].cache = NULL; } - - infoHost.rx_ok[port] = false; } void Serial_Config(uint8_t port, uint16_t cacheSize, uint32_t baudrate) @@ -110,11 +108,6 @@ void USART_IRQHandler(uint8_t port) Serial[port].uart->DR; dmaL1Data[port].wIndex = dmaL1Data[port].cacheSize - Serial[port].dma_stream->NDTR; - uint16_t wIndex = (dmaL1Data[port].wIndex == 0) ? dmaL1Data[port].cacheSize : dmaL1Data[port].wIndex; - if (dmaL1Data[port].cache[wIndex - 1] == '\n') // Receive completed - { - infoHost.rx_ok[port] = true; - } } } diff --git a/TFT/src/User/Hal/stm32f2_f4xx/Serial.h b/TFT/src/User/Hal/stm32f2_f4xx/Serial.h index 9888133082..ca48a22082 100644 --- a/TFT/src/User/Hal/stm32f2_f4xx/Serial.h +++ b/TFT/src/User/Hal/stm32f2_f4xx/Serial.h @@ -5,11 +5,12 @@ #include "variants.h" // for uint32_t etc... #include "uart.h" -typedef struct +typedef volatile struct // precautionally declared as volatile due to access from interrupt handler and main thread { char *cache; - uint16_t wIndex; - uint16_t rIndex; + uint16_t wIndex; // writing index + uint16_t rIndex; // reading index + uint16_t flag; // custom flag (for custom usage by the application) uint16_t cacheSize; } DMA_CIRCULAR_BUFFER; diff --git a/TFT/src/User/main.h b/TFT/src/User/main.h index aefceef929..78744cf391 100644 --- a/TFT/src/User/main.h +++ b/TFT/src/User/main.h @@ -10,13 +10,14 @@ extern "C" { #include "variants.h" // for RCC_ClocksTypeDef #include "uart.h" // for _UART_CNT -#define MAX_MENU_DEPTH 10 // Max sub menu depth +#define MAX_MENU_DEPTH 10 // max sub menu depth + typedef void (* FP_MENU)(void); typedef struct { - FP_MENU menu[MAX_MENU_DEPTH]; // Menu function buffer - uint8_t cur; // Current menu index in buffer + FP_MENU menu[MAX_MENU_DEPTH]; // menu function buffer + uint8_t cur; // current menu index in buffer } MENU; typedef enum @@ -30,10 +31,9 @@ typedef enum typedef struct { - bool wait; // Whether wait for Marlin's response - bool rx_ok[_UART_CNT]; // Whether receive Marlin's response or get gcode by other UART (ESP3D/OctoPrint) - bool connected; // Whether have connected to Marlin - HOST_STATUS status; // Whether the host is busy in printing execution. (USB serial printing and gcode print from onboard) + bool wait; // whether wait for Marlin's response + bool connected; // whether have connected to Marlin + HOST_STATUS status; // whether the host is busy in printing execution. (USB serial printing and gcode print from onboard) } HOST; typedef struct