From 8af801ce974ba04bb533d60779633da798b8481b Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Mon, 15 Feb 2021 12:55:14 +0100 Subject: [PATCH 01/42] Fix the GCode queue when multiserial is used --- Marlin/src/core/language.h | 1 + Marlin/src/gcode/queue.cpp | 58 ++++++++++++++++++++++---------------- 2 files changed, 34 insertions(+), 25 deletions(-) diff --git a/Marlin/src/core/language.h b/Marlin/src/core/language.h index 923ad903cb55..a813ec4ba108 100644 --- a/Marlin/src/core/language.h +++ b/Marlin/src/core/language.h @@ -131,6 +131,7 @@ #define STR_WATCHDOG_FIRED "Watchdog timeout. Reset required." #define STR_ERR_KILLED "Printer halted. kill() called!" #define STR_ERR_STOPPED "Printer stopped due to errors. Fix the error and use M999 to restart. (Temperature is reset. Set it after restarting)" +#define STR_ERR_SERIAL_MISMATCH "Serial status mismatch" #define STR_BUSY_PROCESSING "busy: processing" #define STR_BUSY_PAUSED_FOR_USER "busy: paused for user" #define STR_BUSY_PAUSED_FOR_INPUT "busy: paused for input" diff --git a/Marlin/src/gcode/queue.cpp b/Marlin/src/gcode/queue.cpp index c49247912c06..4477b6233098 100644 --- a/Marlin/src/gcode/queue.cpp +++ b/Marlin/src/gcode/queue.cpp @@ -324,29 +324,19 @@ void GCodeQueue::flush_and_request_resend() { ok_to_send(); } -inline bool serial_data_available() { - byte data_available = 0; - if (MYSERIAL0.available()) data_available++; - #ifdef SERIAL_PORT_2 - const bool port2_open = TERN1(HAS_ETHERNET, ethernet.have_telnet_client); - if (port2_open && MYSERIAL1.available()) data_available++; - #endif - return data_available > 0; -} - -inline int read_serial(const uint8_t index) { - switch (index) { - case 0: return MYSERIAL0.read(); - case 1: { - #if HAS_MULTI_SERIAL - const bool port2_open = TERN1(HAS_ETHERNET, ethernet.have_telnet_client); - if (port2_open) return MYSERIAL1.read(); - #endif +// Multiserial already handle the dispatch to/from multiple port by itself +inline bool serial_data_available(uint8_t index = SERIAL_ALL) { + if (index == SERIAL_ALL) { + for (index = 0; index < NUM_SERIAL; index++) { + if (SERIAL_IMPL.available(index)) return true; } - default: return -1; + return false; } + return SERIAL_IMPL.available(index); } +inline int read_serial(const uint8_t index) { return SERIAL_IMPL.read(index); } + void GCodeQueue::gcode_line_error(PGM_P const err, const serial_index_t serial_ind) { PORT_REDIRECT(SERIAL_PORTMASK(serial_ind)); // Reply to the serial port that sent the command SERIAL_ERROR_START(); @@ -468,14 +458,32 @@ void GCodeQueue::get_serial_commands() { } #endif - /** - * Loop while serial characters are incoming and the queue is not full - */ - while (length < BUFSIZE && serial_data_available()) { + // Loop while serial characters are incoming and the queue is not full + bool hasData = true; + while (length < BUFSIZE && hasData) { + // Unless a serial port has data, this will exit on next iteration + hasData = false; + LOOP_L_N(p, NUM_SERIAL) { + // Is there data available on this specific port ? + if (!serial_data_available(p)) continue; + + hasData = true; // Let's make progress const int c = read_serial(p); - if (c < 0) continue; + if (c < 0) { + // This should never happen, let's log it + PORT_REDIRECT(SERIAL_PORTMASK(p)); // Reply to the serial port that sent the command + #if BOTH(MARLIN_DEV_MODE, POSTMORTEM_DEBUGGING) + // Crash here to get more information why it failed + BUG_ON("SP available but read -1"); + #else + SERIAL_ERROR_START(); + SERIAL_ECHOLNPGM(STR_ERR_SERIAL_MISMATCH); + SERIAL_FLUSH(); + continue; + #endif + } #if ENABLED(MEATPACK) meatpack.handle_rx_char(uint8_t(c), p); @@ -563,7 +571,7 @@ void GCodeQueue::get_serial_commands() { } #endif - #if defined(NO_TIMEOUTS) && NO_TIMEOUTS > 0 + #if NO_TIMEOUTS > 0 last_command_time = ms; #endif From 974ab8d477bd7206fb0a77ed633e1e48cfe08c5a Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Wed, 17 Feb 2021 18:24:18 +0100 Subject: [PATCH 02/42] Fix many bugs in the queue code and LPC176x serial port code Kick meatpack out of the queue code, and instead let it implement a serial interface like any other serial class (this should remove bugs in the queue code) --- Marlin/src/HAL/LPC1768/MarlinSerial.cpp | 42 ++++-- Marlin/src/HAL/LPC1768/MarlinSerial.h | 16 ++- Marlin/src/HAL/shared/Delay.cpp | 2 +- Marlin/src/core/serial.cpp | 5 + Marlin/src/core/serial.h | 17 ++- Marlin/src/core/serial_hook.h | 40 +++--- Marlin/src/feature/meatpack.h | 42 ++++++ Marlin/src/gcode/queue.cpp | 182 ++++++++++++------------ 8 files changed, 220 insertions(+), 126 deletions(-) diff --git a/Marlin/src/HAL/LPC1768/MarlinSerial.cpp b/Marlin/src/HAL/LPC1768/MarlinSerial.cpp index c636a40a120c..461986c5bd43 100644 --- a/Marlin/src/HAL/LPC1768/MarlinSerial.cpp +++ b/Marlin/src/HAL/LPC1768/MarlinSerial.cpp @@ -24,21 +24,47 @@ #include "../../inc/MarlinConfigPre.h" #include "MarlinSerial.h" + +#if ANY_SERIAL_IS(0) + MarlinSerial _MSerial(LPC_UART0); + MSerialT MSerial(true, _MSerial); + extern "C" void UART0_IRQHandler() { _MSerial.IRQHandler(); } +#endif +#if ANY_SERIAL_IS(1) + MarlinSerial _MSerial1((LPC_UART_TypeDef *) LPC_UART1); + MSerialT MSerial1(true, _MSerial1); + extern "C" void UART1_IRQHandler() { _MSerial1.IRQHandler(); } +#endif +#if ANY_SERIAL_IS(2) + MarlinSerial _MSerial2(LPC_UART2); + MSerialT MSerial2(true, _MSerial2); + extern "C" void UART2_IRQHandler() { _MSerial2.IRQHandler(); } +#endif +#if ANY_SERIAL_IS(3) + MarlinSerial _MSerial3(LPC_UART3); + MSerialT MSerial3(true, _MSerial3); + extern "C" void UART3_IRQHandler() { _MSerial3.IRQHandler(); } +#endif + +#if ENABLED(EMERGENCY_PARSER) + bool MarlinSerial::recv_callback(const char c) { + // Need to figure out which serial port we are and react in consequence (Marlin does not have CONTAINER_OF macro) + if (false) {} #if ANY_SERIAL_IS(0) - MSerialT MSerial(true, LPC_UART0); - extern "C" void UART0_IRQHandler() { MSerial.IRQHandler(); } + else if (this == &_MSerial) emergency_parser.update(MSerial.emergency_state, c); #endif #if ANY_SERIAL_IS(1) - MSerialT MSerial1(true, (LPC_UART_TypeDef *) LPC_UART1); - extern "C" void UART1_IRQHandler() { MSerial1.IRQHandler(); } + else if (this == &_MSerial1) emergency_parser.update(MSerial1.emergency_state, c); #endif #if ANY_SERIAL_IS(2) - MSerialT MSerial2(true, LPC_UART2); - extern "C" void UART2_IRQHandler() { MSerial2.IRQHandler(); } + else if (this == &_MSerial2) emergency_parser.update(MSerial2.emergency_state, c); #endif #if ANY_SERIAL_IS(3) - MSerialT MSerial3(true, LPC_UART3); - extern "C" void UART3_IRQHandler() { MSerial3.IRQHandler(); } + else if (this == &_MSerial3) emergency_parser.update(MSerial3.emergency_state, c); +#endif + return true; + } #endif + #endif // TARGET_LPC1768 diff --git a/Marlin/src/HAL/LPC1768/MarlinSerial.h b/Marlin/src/HAL/LPC1768/MarlinSerial.h index de0f62f006d2..65a9a6d1dcc5 100644 --- a/Marlin/src/HAL/LPC1768/MarlinSerial.h +++ b/Marlin/src/HAL/LPC1768/MarlinSerial.h @@ -47,15 +47,21 @@ class MarlinSerial : public HardwareSerial { void end() {} #if ENABLED(EMERGENCY_PARSER) - bool recv_callback(const char c) override { - emergency_parser.update(static_cast *>(this)->emergency_state, c); - return true; // do not discard character - } + bool recv_callback(const char c) override; #endif }; -typedef Serial0Type MSerialT; +// On LPC176x framework, HardwareSerial does not implement the same interface as Arduino's Serial, so overloads +// of 'available' and 'read' method are not used in this multiple inheritance scenario. +// Instead, use a ForwardSerial here that'll adapt the interface +typedef ForwardSerial0Type MSerialT; extern MSerialT MSerial; extern MSerialT MSerial1; extern MSerialT MSerial2; extern MSerialT MSerial3; + +// Consequently, we can't use a RuntimeSerial either. The workaround would be to use a RuntimeSerial> type here +// Right now, let's ignore this until it's actually required +#if ENABLED(SERIAL_RUNTIME_HOOK) + #error This configuration is not supported yet for LPC176x +#endif diff --git a/Marlin/src/HAL/shared/Delay.cpp b/Marlin/src/HAL/shared/Delay.cpp index 8ff5a88c6333..a8d9a5943edd 100644 --- a/Marlin/src/HAL/shared/Delay.cpp +++ b/Marlin/src/HAL/shared/Delay.cpp @@ -51,7 +51,7 @@ // Use hardware cycle counter instead, it's much safer void delay_dwt(uint32_t count) { // Reuse the ASM_CYCLES_PER_ITERATION variable to avoid wasting another useless variable - register uint32_t start = HW_REG(_DWT_CYCCNT) - ASM_CYCLES_PER_ITERATION, elapsed; + uint32_t start = HW_REG(_DWT_CYCCNT) - ASM_CYCLES_PER_ITERATION, elapsed; do { elapsed = HW_REG(_DWT_CYCCNT) - start; } while (elapsed < count); diff --git a/Marlin/src/core/serial.cpp b/Marlin/src/core/serial.cpp index 31f6d67e326e..e3ff187d2f7c 100644 --- a/Marlin/src/core/serial.cpp +++ b/Marlin/src/core/serial.cpp @@ -52,6 +52,11 @@ PGMSTR(SP_X_LBL, " X:"); PGMSTR(SP_Y_LBL, " Y:"); PGMSTR(SP_Z_LBL, " Z:"); PGMST #endif #endif +#if ENABLED(MEATPACK) + MeatPackSerial mpSerial(false, _SERIAL_IMPL); +#endif + + void serialprintPGM(PGM_P str) { while (const char c = pgm_read_byte(str++)) SERIAL_CHAR(c); } diff --git a/Marlin/src/core/serial.h b/Marlin/src/core/serial.h index 0fe843578963..298ff7902a61 100644 --- a/Marlin/src/core/serial.h +++ b/Marlin/src/core/serial.h @@ -24,6 +24,11 @@ #include "../inc/MarlinConfig.h" #include "serial_hook.h" +#if ENABLED(MEATPACK) + #include "../feature/meatpack.h" +#endif + + // Commonly-used strings in serial output extern const char NUL_STR[], SP_P_STR[], SP_T_STR[], X_STR[], Y_STR[], Z_STR[], E_STR[], @@ -70,14 +75,22 @@ typedef int8_t serial_index_t; typedef MultiSerial, decltype(MYSERIAL1)), 0> SerialOutputT; #endif extern SerialOutputT multiSerial; - #define SERIAL_IMPL multiSerial + #define _SERIAL_IMPL multiSerial #else #define _PORT_REDIRECT(n,p) NOOP #define _PORT_RESTORE(n) NOOP #define SERIAL_ASSERT(P) NOOP - #define SERIAL_IMPL MYSERIAL0 + #define _SERIAL_IMPL MYSERIAL0 #endif +#if ENABLED(MEATPACK) + extern MeatPackSerial mpSerial; + #define SERIAL_IMPL mpSerial +#else + #define SERIAL_IMPL _SERIAL_IMPL +#endif + + #define SERIAL_OUT(WHAT, V...) (void)SERIAL_IMPL.WHAT(V) #define PORT_REDIRECT(p) _PORT_REDIRECT(1,p) diff --git a/Marlin/src/core/serial_hook.h b/Marlin/src/core/serial_hook.h index ad8ec12b6e95..26da5ddbff06 100644 --- a/Marlin/src/core/serial_hook.h +++ b/Marlin/src/core/serial_hook.h @@ -35,10 +35,11 @@ struct BaseSerial : public SerialBase< BaseSerial >, public SerialT { void msgDone() {} - bool available(uint8_t index) { return index == 0 && SerialT::available(); } - int read(uint8_t index) { return index == 0 ? SerialT::read() : -1; } - bool connected() { return CALL_IF_EXISTS(bool, static_cast(this), connected);; } - void flushTX() { CALL_IF_EXISTS(void, static_cast(this), flushTX); } + // We don't care about indices here, since if one can call us, it's the right index anyway + bool available(uint8_t) { return (bool)SerialT::available(); } + int read(uint8_t) { return (int)SerialT::read(); } + bool connected() { return CALL_IF_EXISTS(bool, static_cast(this), connected);; } + void flushTX() { CALL_IF_EXISTS(void, static_cast(this), flushTX); } // We have 2 implementation of the same method in both base class, let's say which one we want using SerialT::available; @@ -65,18 +66,19 @@ struct ConditionalSerial : public SerialBase< ConditionalSerial > { bool & condition; SerialT & out; NO_INLINE size_t write(uint8_t c) { if (condition) return out.write(c); return 0; } - void flush() { if (condition) out.flush(); } - void begin(long br) { out.begin(br); } - void end() { out.end(); } + void flush() { if (condition) out.flush(); } + void begin(long br) { out.begin(br); } + void end() { out.end(); } void msgDone() {} - bool connected() { return CALL_IF_EXISTS(bool, &out, connected); } - void flushTX() { CALL_IF_EXISTS(void, &out, flushTX); } + bool connected() { return CALL_IF_EXISTS(bool, &out, connected); } + void flushTX() { CALL_IF_EXISTS(void, &out, flushTX); } - bool available(uint8_t index) { return index == 0 && out.available(); } - int read(uint8_t index) { return index == 0 ? out.read() : -1; } - using BaseClassT::available; - using BaseClassT::read; + bool available(uint8_t ) { return (bool)out.available(); } + int read(uint8_t ) { return (int)out.read(); } + bool available() { return (bool)out.available(); } + int read() { return (int)out.read(); } + ConditionalSerial(bool & conditionVariable, SerialT & out, const bool e) : BaseClassT(e), condition(conditionVariable), out(out) {} }; @@ -97,10 +99,10 @@ struct ForwardSerial : public SerialBase< ForwardSerial > { bool connected() { return Private::HasMember_connected::value ? CALL_IF_EXISTS(bool, &out, connected) : (bool)out; } void flushTX() { CALL_IF_EXISTS(void, &out, flushTX); } - bool available(uint8_t index) { return index == 0 && out.available(); } - int read(uint8_t index) { return index == 0 ? out.read() : -1; } - bool available() { return out.available(); } - int read() { return out.read(); } + bool available(uint8_t) { return (bool)out.available(); } + int read(uint8_t) { return (int)out.read(); } + bool available() { return (bool)out.available(); } + int read() { return (int)out.read(); } ForwardSerial(const bool e, SerialT & out) : BaseClassT(e), out(out) {} }; @@ -125,8 +127,8 @@ struct RuntimeSerial : public SerialBase< RuntimeSerial >, public Seria if (eofHook) eofHook(userPointer); } - bool available(uint8_t index) { return index == 0 && SerialT::available(); } - int read(uint8_t index) { return index == 0 ? SerialT::read() : -1; } + bool available(uint8_t) { return (bool)SerialT::available(); } + int read(uint8_t) { return (int)SerialT::read(); } using SerialT::available; using SerialT::read; using SerialT::flush; diff --git a/Marlin/src/feature/meatpack.h b/Marlin/src/feature/meatpack.h index 2641130bd880..ac39e5c24d8b 100644 --- a/Marlin/src/feature/meatpack.h +++ b/Marlin/src/feature/meatpack.h @@ -49,6 +49,7 @@ #pragma once #include +#include "../../core/serial_hook.h" /** * Commands sent to MeatPack to control its behavior. @@ -121,3 +122,44 @@ class MeatPack { }; extern MeatPack meatpack; + +// Implement the MeatPack serial class so it's transparent to rest of the code +template +struct MeatpackSerial : public SerialBase > { + typedef SerialBase< MeatPackSerial > BaseClassT; + + SerialT & out; + + uint8_t serialBuffer[2]; + uint8_t charCount; + + NO_INLINE size_t write(uint8_t c) { return out.write(c); } + void flush() { out.flush(); } + void begin(long br) { out.begin(br); } + void end() { out.end(); } + + void msgDone() { out.msgDone(); } + // Existing instances implement Arduino's operator bool, so use that if it's available + bool connected() { return Private::HasMember_connected::value ? CALL_IF_EXISTS(bool, &out, connected) : (bool)out; } + void flushTX() { CALL_IF_EXISTS(void, &out, flushTX); } + + bool available(uint8_t index) { + return charCount > 0 || (index == 0 && out.available()); + } + int readImpl(uint8_t index) { + if (charCount > 0) return serialBuffer[sizeof(serialBuffer) - (charCount--)]; + + int r = out.read(); + if (r == -1) return r; + + meatpack.handle_rx_char((uint8_t)c, index); + charCount = meatpack.get_result_char(serialBuffer); + return serialBuffer[sizeof(serialBuffer) - (charCount--)]; + } + + int read(uint8_t index) { return readImpl(index); } + bool available() { return charCount > 0 || out.available(); } + int read() { return readImpl(0); } + + MeatPackSerial(const bool e, SerialT & out) : BaseClassT(e), out(out) {} +}; diff --git a/Marlin/src/gcode/queue.cpp b/Marlin/src/gcode/queue.cpp index 4477b6233098..3c7862f4d04f 100644 --- a/Marlin/src/gcode/queue.cpp +++ b/Marlin/src/gcode/queue.cpp @@ -48,10 +48,6 @@ GCodeQueue queue; #include "../feature/binary_stream.h" #endif -#if ENABLED(MEATPACK) - #include "../feature/meatpack.h" -#endif - #if ENABLED(POWER_LOSS_RECOVERY) #include "../feature/powerloss.h" #endif @@ -321,7 +317,6 @@ void GCodeQueue::flush_and_request_resend() { SERIAL_FLUSH(); SERIAL_ECHOPGM(STR_RESEND); SERIAL_ECHOLN(last_N[serial_ind] + 1); - ok_to_send(); } // Multiserial already handle the dispatch to/from multiple port by itself @@ -459,16 +454,19 @@ void GCodeQueue::get_serial_commands() { #endif // Loop while serial characters are incoming and the queue is not full - bool hasData = true; - while (length < BUFSIZE && hasData) { + for (bool hadData = true; hadData; ) { // Unless a serial port has data, this will exit on next iteration - hasData = false; + hadData = false; LOOP_L_N(p, NUM_SERIAL) { - // Is there data available on this specific port ? + // Check if the queue is full, and exits if it is. + if (length >= BUFSIZE) return; + + // No data for this port ? Skip it if (!serial_data_available(p)) continue; - hasData = true; // Let's make progress + // Ok, we have some data to process, let's make progress here + hadData = true; const int c = read_serial(p); if (c < 0) { @@ -485,108 +483,110 @@ void GCodeQueue::get_serial_commands() { #endif } - #if ENABLED(MEATPACK) - meatpack.handle_rx_char(uint8_t(c), p); - char c_res[2] = { 0, 0 }; - const uint8_t char_count = meatpack.get_result_char(c_res); - #else - constexpr uint8_t char_count = 1; - #endif - LOOP_L_N(char_index, char_count) { - const char serial_char = TERN(MEATPACK, c_res[char_index], c); + const char serial_char = (char)c; - if (ISEOL(serial_char)) { + if (ISEOL(serial_char)) { - // Reset our state, continue if the line was empty - if (process_line_done(serial_input_state[p], serial_line_buffer[p], serial_count[p])) - continue; + // Reset our state, continue if the line was empty + if (process_line_done(serial_input_state[p], serial_line_buffer[p], serial_count[p])) + continue; - char* command = serial_line_buffer[p]; + char* command = serial_line_buffer[p]; - while (*command == ' ') command++; // Skip leading spaces - char *npos = (*command == 'N') ? command : nullptr; // Require the N parameter to start the line + while (*command == ' ') command++; // Skip leading spaces + char *npos = (*command == 'N') ? command : nullptr; // Require the N parameter to start the line - if (npos) { + if (npos) { - const bool M110 = !!strstr_P(command, PSTR("M110")); + const bool M110 = !!strstr_P(command, PSTR("M110")); - if (M110) { - char* n2pos = strchr(command + 4, 'N'); - if (n2pos) npos = n2pos; - } + if (M110) { + char* n2pos = strchr(command + 4, 'N'); + if (n2pos) npos = n2pos; + } - const long gcode_N = strtol(npos + 1, nullptr, 10); + const long gcode_N = strtol(npos + 1, nullptr, 10); - if (gcode_N != last_N[p] + 1 && !M110) - return gcode_line_error(PSTR(STR_ERR_LINE_NO), p); + if (gcode_N != last_N[p] + 1 && !M110) { + // In case of error on a serial port, don't prevent other serial port from making progress + gcode_line_error(PSTR(STR_ERR_LINE_NO), p); + break; + } - char *apos = strrchr(command, '*'); - if (apos) { - uint8_t checksum = 0, count = uint8_t(apos - command); - while (count) checksum ^= command[--count]; - if (strtol(apos + 1, nullptr, 10) != checksum) - return gcode_line_error(PSTR(STR_ERR_CHECKSUM_MISMATCH), p); + char *apos = strrchr(command, '*'); + if (apos) { + uint8_t checksum = 0, count = uint8_t(apos - command); + while (count) checksum ^= command[--count]; + if (strtol(apos + 1, nullptr, 10) != checksum) { + // In case of error on a serial port, don't prevent other serial port from making progress + gcode_line_error(PSTR(STR_ERR_CHECKSUM_MISMATCH), p); + break; } - else - return gcode_line_error(PSTR(STR_ERR_NO_CHECKSUM), p); - last_N[p] = gcode_N; } - #if ENABLED(SDSUPPORT) - // Pronterface "M29" and "M29 " has no line number - else if (card.flag.saving && !is_M29(command)) - return gcode_line_error(PSTR(STR_ERR_NO_CHECKSUM), p); - #endif + else { + // In case of error on a serial port, don't prevent other serial port from making progress + gcode_line_error(PSTR(STR_ERR_NO_CHECKSUM), p); + break; + } - // - // Movement commands give an alert when the machine is stopped - // - - if (IsStopped()) { - char* gpos = strchr(command, 'G'); - if (gpos) { - switch (strtol(gpos + 1, nullptr, 10)) { - case 0: case 1: - #if ENABLED(ARC_SUPPORT) - case 2: case 3: - #endif - #if ENABLED(BEZIER_CURVE_SUPPORT) - case 5: - #endif - PORT_REDIRECT(SERIAL_PORTMASK(p)); // Reply to the serial port that sent the command - SERIAL_ECHOLNPGM(STR_ERR_STOPPED); - LCD_MESSAGEPGM(MSG_STOPPED); - break; - } - } + last_N[p] = gcode_N; + } + #if ENABLED(SDSUPPORT) + // Pronterface "M29" and "M29 " has no line number + else if (card.flag.saving && !is_M29(command)) { + gcode_line_error(PSTR(STR_ERR_NO_CHECKSUM), p); + break; } + #endif - #if DISABLED(EMERGENCY_PARSER) - // Process critical commands early - if (command[0] == 'M') switch (command[3]) { - case '8': if (command[2] == '0' && command[1] == '1') { wait_for_heatup = false; TERN_(HAS_LCD_MENU, wait_for_user = false); } break; - case '2': if (command[2] == '1' && command[1] == '1') kill(M112_KILL_STR, nullptr, true); break; - case '0': if (command[1] == '4' && command[2] == '1') quickstop_stepper(); break; + // + // Movement commands give an alert when the machine is stopped + // + + if (IsStopped()) { + char* gpos = strchr(command, 'G'); + if (gpos) { + switch (strtol(gpos + 1, nullptr, 10)) { + case 0: case 1: + #if ENABLED(ARC_SUPPORT) + case 2: case 3: + #endif + #if ENABLED(BEZIER_CURVE_SUPPORT) + case 5: + #endif + PORT_REDIRECT(SERIAL_PORTMASK(p)); // Reply to the serial port that sent the command + SERIAL_ECHOLNPGM(STR_ERR_STOPPED); + LCD_MESSAGEPGM(MSG_STOPPED); + break; } - #endif - - #if NO_TIMEOUTS > 0 - last_command_time = ms; - #endif - - // Add the command to the queue - _enqueue(serial_line_buffer[p], true - #if HAS_MULTI_SERIAL - , p - #endif - ); + } } - else - process_stream_char(serial_char, serial_input_state[p], serial_line_buffer[p], serial_count[p]); - } // char_count loop + #if DISABLED(EMERGENCY_PARSER) + // Process critical commands early + if (command[0] == 'M') switch (command[3]) { + case '8': if (command[2] == '0' && command[1] == '1') { wait_for_heatup = false; TERN_(HAS_LCD_MENU, wait_for_user = false); } break; + case '2': if (command[2] == '1' && command[1] == '1') kill(M112_KILL_STR, nullptr, true); break; + case '0': if (command[1] == '4' && command[2] == '1') quickstop_stepper(); break; + } + #endif + + #if NO_TIMEOUTS > 0 + last_command_time = ms; + #endif + // Add the command to the queue + _enqueue(serial_line_buffer[p], true + #if HAS_MULTI_SERIAL + , p + #endif + ); + } + else + process_stream_char(serial_char, serial_input_state[p], serial_line_buffer[p], serial_count[p]); + } // NUM_SERIAL loop } // queue has space, serial has data } From 6a78211f72fa397d1f53fd943abda2ce4834d919 Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Wed, 17 Feb 2021 18:53:55 +0100 Subject: [PATCH 03/42] Fix building issue with Meatpack --- Marlin/src/core/serial.cpp | 2 +- Marlin/src/core/serial.h | 3 +-- Marlin/src/core/serial_hook.h | 3 +++ Marlin/src/feature/meatpack.h | 13 ++++++------- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/Marlin/src/core/serial.cpp b/Marlin/src/core/serial.cpp index e3ff187d2f7c..a2b29846591d 100644 --- a/Marlin/src/core/serial.cpp +++ b/Marlin/src/core/serial.cpp @@ -53,7 +53,7 @@ PGMSTR(SP_X_LBL, " X:"); PGMSTR(SP_Y_LBL, " Y:"); PGMSTR(SP_Z_LBL, " Z:"); PGMST #endif #if ENABLED(MEATPACK) - MeatPackSerial mpSerial(false, _SERIAL_IMPL); + MeatpackSerial mpSerial(false, _SERIAL_IMPL); #endif diff --git a/Marlin/src/core/serial.h b/Marlin/src/core/serial.h index 298ff7902a61..b7c26ed1892b 100644 --- a/Marlin/src/core/serial.h +++ b/Marlin/src/core/serial.h @@ -63,7 +63,6 @@ extern uint8_t marlin_debug_flags; // // Serial redirection // -typedef int8_t serial_index_t; #define SERIAL_ALL 0x7F #if HAS_MULTI_SERIAL #define _PORT_REDIRECT(n,p) REMEMBER(n,multiSerial.portMask,p) @@ -84,7 +83,7 @@ typedef int8_t serial_index_t; #endif #if ENABLED(MEATPACK) - extern MeatPackSerial mpSerial; + extern MeatpackSerial mpSerial; #define SERIAL_IMPL mpSerial #else #define SERIAL_IMPL _SERIAL_IMPL diff --git a/Marlin/src/core/serial_hook.h b/Marlin/src/core/serial_hook.h index 26da5ddbff06..86ff3796ddc8 100644 --- a/Marlin/src/core/serial_hook.h +++ b/Marlin/src/core/serial_hook.h @@ -240,3 +240,6 @@ struct MultiSerial : public SerialBase< MultiSerial #ifdef HAS_MULTI_SERIAL #define Serial1Type ConditionalSerial #endif + +// Used in multiple places +typedef int8_t serial_index_t; \ No newline at end of file diff --git a/Marlin/src/feature/meatpack.h b/Marlin/src/feature/meatpack.h index ac39e5c24d8b..ad24fdf058d3 100644 --- a/Marlin/src/feature/meatpack.h +++ b/Marlin/src/feature/meatpack.h @@ -49,7 +49,7 @@ #pragma once #include -#include "../../core/serial_hook.h" +#include "../core/serial_hook.h" /** * Commands sent to MeatPack to control its behavior. @@ -79,8 +79,6 @@ enum MeatPack_ConfigStateBits : uint8_t { }; class MeatPack { -private: - friend class GCodeQueue; // Utility definitions static const uint8_t kCommandByte = 0b11111111, @@ -100,6 +98,7 @@ class MeatPack { char_out_count; // Stores number of characters to be read out. static uint8_t char_out_buf[2]; // Output buffer for caching up to 2 characters +public: // Pass in a character rx'd by SD card or serial. Automatically parses command/ctrl sequences, // and will control state internally. static void handle_rx_char(const uint8_t c, const serial_index_t serial_ind); @@ -126,11 +125,11 @@ extern MeatPack meatpack; // Implement the MeatPack serial class so it's transparent to rest of the code template struct MeatpackSerial : public SerialBase > { - typedef SerialBase< MeatPackSerial > BaseClassT; + typedef SerialBase< MeatpackSerial > BaseClassT; SerialT & out; - uint8_t serialBuffer[2]; + char serialBuffer[2]; uint8_t charCount; NO_INLINE size_t write(uint8_t c) { return out.write(c); } @@ -152,7 +151,7 @@ struct MeatpackSerial : public SerialBase > { int r = out.read(); if (r == -1) return r; - meatpack.handle_rx_char((uint8_t)c, index); + meatpack.handle_rx_char((uint8_t)r, index); charCount = meatpack.get_result_char(serialBuffer); return serialBuffer[sizeof(serialBuffer) - (charCount--)]; } @@ -161,5 +160,5 @@ struct MeatpackSerial : public SerialBase > { bool available() { return charCount > 0 || out.available(); } int read() { return readImpl(0); } - MeatPackSerial(const bool e, SerialT & out) : BaseClassT(e), out(out) {} + MeatpackSerial(const bool e, SerialT & out) : BaseClassT(e), out(out) {} }; From 205ed9b5d869f5d13c25283b25d23ef19ede9e0b Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Wed, 17 Feb 2021 19:44:50 +0100 Subject: [PATCH 04/42] Fix building issue with Meatpack and Multiserial. It's not because it builds that it works. And I'm almost sure it does not. --- Marlin/src/feature/meatpack.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Marlin/src/feature/meatpack.h b/Marlin/src/feature/meatpack.h index ad24fdf058d3..982ccfd4a687 100644 --- a/Marlin/src/feature/meatpack.h +++ b/Marlin/src/feature/meatpack.h @@ -113,7 +113,6 @@ class MeatPack { static void reset_state(); static void report_state(); - static uint8_t unpacked_char(register const uint8_t in); static uint8_t unpack_chars(const uint8_t pk, uint8_t* __restrict const chars_out); static void handle_command(const MeatPack_Command c); static void handle_output_char(const uint8_t c); @@ -143,12 +142,16 @@ struct MeatpackSerial : public SerialBase > { void flushTX() { CALL_IF_EXISTS(void, &out, flushTX); } bool available(uint8_t index) { - return charCount > 0 || (index == 0 && out.available()); + // There is a potential issue here with multiserial, since it'll return its decoded buffer whatever the serial index here. + // So, instead of doing MeatpackSerial> we should do MultiSerial, MeatpackSerial<...>> + // TODO, let's fix this later on + return charCount > 0 || (bool)out.available(index); } int readImpl(uint8_t index) { + // DITTO if (charCount > 0) return serialBuffer[sizeof(serialBuffer) - (charCount--)]; - int r = out.read(); + int r = out.read(index); if (r == -1) return r; meatpack.handle_rx_char((uint8_t)r, index); @@ -157,7 +160,7 @@ struct MeatpackSerial : public SerialBase > { } int read(uint8_t index) { return readImpl(index); } - bool available() { return charCount > 0 || out.available(); } + bool available() { return charCount > 0 || out.available(0); } int read() { return readImpl(0); } MeatpackSerial(const bool e, SerialT & out) : BaseClassT(e), out(out) {} From 08adacf6f6dca246243cca7f50bd38f8d19704bb Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Wed, 17 Feb 2021 18:00:48 -0600 Subject: [PATCH 05/42] please install the editorconfig plugin --- Marlin/src/HAL/LPC1768/MarlinSerial.cpp | 1 - Marlin/src/HAL/LPC1768/MarlinSerial.h | 6 +++--- Marlin/src/core/serial.cpp | 1 - Marlin/src/core/serial.h | 1 - Marlin/src/core/serial_hook.h | 6 +++--- Marlin/src/feature/meatpack.h | 12 ++++++------ Marlin/src/gcode/queue.cpp | 4 +--- 7 files changed, 13 insertions(+), 18 deletions(-) diff --git a/Marlin/src/HAL/LPC1768/MarlinSerial.cpp b/Marlin/src/HAL/LPC1768/MarlinSerial.cpp index 461986c5bd43..612d420944e1 100644 --- a/Marlin/src/HAL/LPC1768/MarlinSerial.cpp +++ b/Marlin/src/HAL/LPC1768/MarlinSerial.cpp @@ -24,7 +24,6 @@ #include "../../inc/MarlinConfigPre.h" #include "MarlinSerial.h" - #if ANY_SERIAL_IS(0) MarlinSerial _MSerial(LPC_UART0); MSerialT MSerial(true, _MSerial); diff --git a/Marlin/src/HAL/LPC1768/MarlinSerial.h b/Marlin/src/HAL/LPC1768/MarlinSerial.h index 65a9a6d1dcc5..35d1689fc26a 100644 --- a/Marlin/src/HAL/LPC1768/MarlinSerial.h +++ b/Marlin/src/HAL/LPC1768/MarlinSerial.h @@ -51,9 +51,9 @@ class MarlinSerial : public HardwareSerial { #endif }; -// On LPC176x framework, HardwareSerial does not implement the same interface as Arduino's Serial, so overloads -// of 'available' and 'read' method are not used in this multiple inheritance scenario. -// Instead, use a ForwardSerial here that'll adapt the interface +// On LPC176x framework, HardwareSerial does not implement the same interface as Arduino's Serial, so overloads +// of 'available' and 'read' method are not used in this multiple inheritance scenario. +// Instead, use a ForwardSerial here that'll adapt the interface typedef ForwardSerial0Type MSerialT; extern MSerialT MSerial; extern MSerialT MSerial1; diff --git a/Marlin/src/core/serial.cpp b/Marlin/src/core/serial.cpp index a2b29846591d..01f850ba566b 100644 --- a/Marlin/src/core/serial.cpp +++ b/Marlin/src/core/serial.cpp @@ -56,7 +56,6 @@ PGMSTR(SP_X_LBL, " X:"); PGMSTR(SP_Y_LBL, " Y:"); PGMSTR(SP_Z_LBL, " Z:"); PGMST MeatpackSerial mpSerial(false, _SERIAL_IMPL); #endif - void serialprintPGM(PGM_P str) { while (const char c = pgm_read_byte(str++)) SERIAL_CHAR(c); } diff --git a/Marlin/src/core/serial.h b/Marlin/src/core/serial.h index b7c26ed1892b..132264a0aba4 100644 --- a/Marlin/src/core/serial.h +++ b/Marlin/src/core/serial.h @@ -89,7 +89,6 @@ extern uint8_t marlin_debug_flags; #define SERIAL_IMPL _SERIAL_IMPL #endif - #define SERIAL_OUT(WHAT, V...) (void)SERIAL_IMPL.WHAT(V) #define PORT_REDIRECT(p) _PORT_REDIRECT(1,p) diff --git a/Marlin/src/core/serial_hook.h b/Marlin/src/core/serial_hook.h index 86ff3796ddc8..a76a9522443d 100644 --- a/Marlin/src/core/serial_hook.h +++ b/Marlin/src/core/serial_hook.h @@ -78,7 +78,7 @@ struct ConditionalSerial : public SerialBase< ConditionalSerial > { int read(uint8_t ) { return (int)out.read(); } bool available() { return (bool)out.available(); } int read() { return (int)out.read(); } - + ConditionalSerial(bool & conditionVariable, SerialT & out, const bool e) : BaseClassT(e), condition(conditionVariable), out(out) {} }; @@ -99,7 +99,7 @@ struct ForwardSerial : public SerialBase< ForwardSerial > { bool connected() { return Private::HasMember_connected::value ? CALL_IF_EXISTS(bool, &out, connected) : (bool)out; } void flushTX() { CALL_IF_EXISTS(void, &out, flushTX); } - bool available(uint8_t) { return (bool)out.available(); } + bool available(uint8_t) { return (bool)out.available(); } int read(uint8_t) { return (int)out.read(); } bool available() { return (bool)out.available(); } int read() { return (int)out.read(); } @@ -242,4 +242,4 @@ struct MultiSerial : public SerialBase< MultiSerial #endif // Used in multiple places -typedef int8_t serial_index_t; \ No newline at end of file +typedef int8_t serial_index_t; diff --git a/Marlin/src/feature/meatpack.h b/Marlin/src/feature/meatpack.h index 982ccfd4a687..d6d8be802eeb 100644 --- a/Marlin/src/feature/meatpack.h +++ b/Marlin/src/feature/meatpack.h @@ -141,22 +141,22 @@ struct MeatpackSerial : public SerialBase > { bool connected() { return Private::HasMember_connected::value ? CALL_IF_EXISTS(bool, &out, connected) : (bool)out; } void flushTX() { CALL_IF_EXISTS(void, &out, flushTX); } - bool available(uint8_t index) { + bool available(uint8_t index) { // There is a potential issue here with multiserial, since it'll return its decoded buffer whatever the serial index here. // So, instead of doing MeatpackSerial> we should do MultiSerial, MeatpackSerial<...>> // TODO, let's fix this later on - return charCount > 0 || (bool)out.available(index); + return charCount > 0 || (bool)out.available(index); } int readImpl(uint8_t index) { - // DITTO + // DITTO if (charCount > 0) return serialBuffer[sizeof(serialBuffer) - (charCount--)]; - + int r = out.read(index); if (r == -1) return r; meatpack.handle_rx_char((uint8_t)r, index); - charCount = meatpack.get_result_char(serialBuffer); - return serialBuffer[sizeof(serialBuffer) - (charCount--)]; + charCount = meatpack.get_result_char(serialBuffer); + return serialBuffer[sizeof(serialBuffer) - (charCount--)]; } int read(uint8_t index) { return readImpl(index); } diff --git a/Marlin/src/gcode/queue.cpp b/Marlin/src/gcode/queue.cpp index 3c7862f4d04f..6cb21e6c61de 100644 --- a/Marlin/src/gcode/queue.cpp +++ b/Marlin/src/gcode/queue.cpp @@ -483,7 +483,6 @@ void GCodeQueue::get_serial_commands() { #endif } - const char serial_char = (char)c; if (ISEOL(serial_char)) { @@ -586,7 +585,7 @@ void GCodeQueue::get_serial_commands() { } else process_stream_char(serial_char, serial_input_state[p], serial_line_buffer[p], serial_count[p]); - + } // NUM_SERIAL loop } // queue has space, serial has data } @@ -702,5 +701,4 @@ void GCodeQueue::advance() { // The queue may be reset by a command handler or by code invoked by idle() within a handler --length; if (++index_r >= BUFSIZE) index_r = 0; - } From eeb73fb4aa3c3c0b11a2acbaa522e30dd5cf3d4c Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Wed, 17 Feb 2021 18:10:27 -0600 Subject: [PATCH 06/42] general cleanup --- Marlin/src/HAL/LPC1768/MarlinSerial.cpp | 27 +++++++++++++------------ Marlin/src/core/serial.h | 1 - Marlin/src/core/serial_hook.h | 6 +++--- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/Marlin/src/HAL/LPC1768/MarlinSerial.cpp b/Marlin/src/HAL/LPC1768/MarlinSerial.cpp index 612d420944e1..b0dfc0ae901c 100644 --- a/Marlin/src/HAL/LPC1768/MarlinSerial.cpp +++ b/Marlin/src/HAL/LPC1768/MarlinSerial.cpp @@ -46,24 +46,25 @@ #endif #if ENABLED(EMERGENCY_PARSER) + bool MarlinSerial::recv_callback(const char c) { // Need to figure out which serial port we are and react in consequence (Marlin does not have CONTAINER_OF macro) if (false) {} -#if ANY_SERIAL_IS(0) - else if (this == &_MSerial) emergency_parser.update(MSerial.emergency_state, c); -#endif -#if ANY_SERIAL_IS(1) - else if (this == &_MSerial1) emergency_parser.update(MSerial1.emergency_state, c); -#endif -#if ANY_SERIAL_IS(2) - else if (this == &_MSerial2) emergency_parser.update(MSerial2.emergency_state, c); -#endif -#if ANY_SERIAL_IS(3) - else if (this == &_MSerial3) emergency_parser.update(MSerial3.emergency_state, c); -#endif + #if ANY_SERIAL_IS(0) + else if (this == &_MSerial) emergency_parser.update(MSerial.emergency_state, c); + #endif + #if ANY_SERIAL_IS(1) + else if (this == &_MSerial1) emergency_parser.update(MSerial1.emergency_state, c); + #endif + #if ANY_SERIAL_IS(2) + else if (this == &_MSerial2) emergency_parser.update(MSerial2.emergency_state, c); + #endif + #if ANY_SERIAL_IS(3) + else if (this == &_MSerial3) emergency_parser.update(MSerial3.emergency_state, c); + #endif return true; } -#endif +#endif #endif // TARGET_LPC1768 diff --git a/Marlin/src/core/serial.h b/Marlin/src/core/serial.h index 132264a0aba4..60f873d473e5 100644 --- a/Marlin/src/core/serial.h +++ b/Marlin/src/core/serial.h @@ -28,7 +28,6 @@ #include "../feature/meatpack.h" #endif - // Commonly-used strings in serial output extern const char NUL_STR[], SP_P_STR[], SP_T_STR[], X_STR[], Y_STR[], Z_STR[], E_STR[], diff --git a/Marlin/src/core/serial_hook.h b/Marlin/src/core/serial_hook.h index a76a9522443d..63fa6bbbc85c 100644 --- a/Marlin/src/core/serial_hook.h +++ b/Marlin/src/core/serial_hook.h @@ -24,6 +24,9 @@ #include "macros.h" #include "serial_base.h" +// Used in multiple places +typedef int8_t serial_index_t; + // The most basic serial class: it dispatch to the base serial class with no hook whatsoever. This will compile to nothing but the base serial class template struct BaseSerial : public SerialBase< BaseSerial >, public SerialT { @@ -240,6 +243,3 @@ struct MultiSerial : public SerialBase< MultiSerial #ifdef HAS_MULTI_SERIAL #define Serial1Type ConditionalSerial #endif - -// Used in multiple places -typedef int8_t serial_index_t; From 954983b33a9427d964d7907eadfca38f3b7c659c Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Thu, 18 Feb 2021 12:12:42 +0100 Subject: [PATCH 07/42] Fix bad code in Delay.cpp not flushing the TX line but the RX line Make multiserial actually composable to handle multiple multiserial as child Document everything --- Marlin/src/HAL/shared/Delay.cpp | 2 +- Marlin/src/core/serial_hook.h | 32 +++++++++++++++++--------------- docs/Serial.md | 15 ++++++++++++++- 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/Marlin/src/HAL/shared/Delay.cpp b/Marlin/src/HAL/shared/Delay.cpp index a8d9a5943edd..8a021a2155ef 100644 --- a/Marlin/src/HAL/shared/Delay.cpp +++ b/Marlin/src/HAL/shared/Delay.cpp @@ -114,7 +114,7 @@ serialprintPGM(unit); SERIAL_ECHOLNPAIR(" took: ", total); serialprintPGM(unit); - if (do_flush) SERIAL_FLUSH(); + if (do_flush) SERIAL_FLUSHTX(); }; uint32_t s, e; diff --git a/Marlin/src/core/serial_hook.h b/Marlin/src/core/serial_hook.h index 63fa6bbbc85c..5226a51f167a 100644 --- a/Marlin/src/core/serial_hook.h +++ b/Marlin/src/core/serial_hook.h @@ -166,17 +166,18 @@ struct RuntimeSerial : public SerialBase< RuntimeSerial >, public Seria }; // A class that's duplicating its output conditionally to 2 serial interface -template -struct MultiSerial : public SerialBase< MultiSerial > { - typedef SerialBase< MultiSerial > BaseClassT; +template +struct MultiSerial : public SerialBase< MultiSerial > { + typedef SerialBase< MultiSerial > BaseClassT; uint8_t portMask; Serial0T & serial0; Serial1T & serial1; enum Masks { - FirstOutputMask = (1 << offset), - SecondOutputMask = (1 << (offset + 1)), + UsageMask = ((1 << step) - 1), // A bit mask containing as many bits as step + FirstOutputMask = (UsageMask << offset), + SecondOutputMask = (UsageMask << (offset + step)), AllMask = FirstOutputMask | SecondOutputMask, }; @@ -191,18 +192,18 @@ struct MultiSerial : public SerialBase< MultiSerial if (portMask & SecondOutputMask) serial1.msgDone(); } bool available(uint8_t index) { - switch(index) { - case 0 + offset: return serial0.available(); - case 1 + offset: return serial1.available(); - default: return false; - } + if (index >= 0 + offset && index < step + offset) + return serial0.available(index); + else if (index >= step + offset && index < 2 * step + offset) + return serial1.available(index); + return false; } NO_INLINE int read(uint8_t index) { - switch(index) { - case 0 + offset: return serial0.read(); - case 1 + offset: return serial1.read(); - default: return -1; - } + if (index >= 0 + offset && index < step + offset) + return serial0.read(index); + else if (index >= step + offset && index < 2 * step + offset) + return serial1.read(index); + return -1; } void begin(const long br) { if (portMask & FirstOutputMask) serial0.begin(br); @@ -243,3 +244,4 @@ struct MultiSerial : public SerialBase< MultiSerial #ifdef HAS_MULTI_SERIAL #define Serial1Type ConditionalSerial #endif + diff --git a/docs/Serial.md b/docs/Serial.md index 0db8175b3d5c..cdf7ff1a6938 100644 --- a/docs/Serial.md +++ b/docs/Serial.md @@ -30,11 +30,24 @@ In the `Marlin/src/core/serial_hook.h` file, the different serial feature are de Since all the types above are using CRTP, it's possible to combine them to get the appropriate functionality. This is easily done via type definition of the feature. -For example, to present a serial interface that's outputting to 2 serial port, the first one being hooked at runtime and the second one connected to a runtime switchable telnet client, you'll declare the type to use as: +For example, to present a serial interface that's outputting to 2 serial ports, the first one being hooked at runtime and the second one connected to a runtime switchable telnet client, you'll declare the type to use as: ```cpp typedef MultiSerial< RuntimeSerial, ConditionalSerial > Serial0Type; ``` +To send the same output to 4 serial ports, you'll then combine the MultiSerial this way: +```cpp +typedef MultiSerial< MultiSerial< BaseSerial, BaseSerial >, MultiSerial< BaseSerial, BaseSerial, 2, 1>, 0, 2> Serial0Type; +``` +The magical number here are the step and offset for computing the serial port, let's simplify a bit the above monster to: +```cpp +MS< A = MS, B = MS, offset = 0, step = 2> +``` +This means that the underlying multiserial A (outputting to `a,b`) is available from offset = 0 to offset = 1. +The multiserial B (outputting to `c, d`) is available from offset = 2 (the next step from the root multiserial) to offset 3. +In practice, the root multiserial will redirect any index/mask `0` to `step - 1` to its first leaf, and any index/mask `step = 2` to `2*step - 1` to its second leaf. + + ## Emergency parser By default, the serial base interface provide an emergency parser that's only enable for serial classes that support it. Because of this condition, all underlying type takes a first `bool emergencyParserEnabled` argument to their constructor. You must take into account this parameter when defining the actual type used. From b679069749d0e7120f138eb4e4b69db578ec5f28 Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Thu, 18 Feb 2021 12:20:35 +0100 Subject: [PATCH 08/42] Update documentation. I've installed the editorconfig plugin on VSCode. --- docs/Serial.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/Serial.md b/docs/Serial.md index cdf7ff1a6938..1663a65cbcd7 100644 --- a/docs/Serial.md +++ b/docs/Serial.md @@ -39,13 +39,13 @@ To send the same output to 4 serial ports, you'll then combine the MultiSerial t ```cpp typedef MultiSerial< MultiSerial< BaseSerial, BaseSerial >, MultiSerial< BaseSerial, BaseSerial, 2, 1>, 0, 2> Serial0Type; ``` -The magical number here are the step and offset for computing the serial port, let's simplify a bit the above monster to: +The magical numbers here are the step and offset for computing the serial port, let's simplify a bit the above monster to: ```cpp -MS< A = MS, B = MS, offset = 0, step = 2> +MS< A = MS, B = MS, offset = 0, step = 2> ``` -This means that the underlying multiserial A (outputting to `a,b`) is available from offset = 0 to offset = 1. -The multiserial B (outputting to `c, d`) is available from offset = 2 (the next step from the root multiserial) to offset 3. -In practice, the root multiserial will redirect any index/mask `0` to `step - 1` to its first leaf, and any index/mask `step = 2` to `2*step - 1` to its second leaf. +This means that the underlying multiserial A (outputting to `a,b`) is available from offset = 0 to offset + step = 1 (default value). +The multiserial B (outputting to `c, d`) is available from offset = 2 (the next step from the root multiserial) to offset + step = 3. +In practice, the root multiserial will redirect any index/mask `offset` to `offset + step - 1` to its first leaf, and any index/mask `offset + step` to `offset + 2*step - 1` to its second leaf. ## Emergency parser From 9f9b82ee6d40fa6958bcee01cfd47c400f84d059 Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Thu, 18 Feb 2021 16:38:52 +0100 Subject: [PATCH 09/42] WIP Used for debugging the issue (will be reverted in a later step), don't pull this. --- Marlin/src/gcode/queue.cpp | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/Marlin/src/gcode/queue.cpp b/Marlin/src/gcode/queue.cpp index 6cb21e6c61de..3f26b89a49bb 100644 --- a/Marlin/src/gcode/queue.cpp +++ b/Marlin/src/gcode/queue.cpp @@ -304,6 +304,10 @@ void GCodeQueue::ok_to_send() { SERIAL_EOL(); } +// WIP: TTTTTTTTTTTTT REMOVE THIS WHEN IT'S VALIDATED TTTTTTTTTTTTTTTT +static bool dumpUponError = false; +extern MarlinSerial _MSerial; + /** * Send a "Resend: nnn" message to the host to * indicate that a command needs to be re-sent. @@ -317,6 +321,7 @@ void GCodeQueue::flush_and_request_resend() { SERIAL_FLUSH(); SERIAL_ECHOPGM(STR_RESEND); SERIAL_ECHOLN(last_N[serial_ind] + 1); + dumpUponError = true; } // Multiserial already handle the dispatch to/from multiple port by itself @@ -420,6 +425,9 @@ inline bool process_line_done(uint8_t &sis, char (&buff)[MAX_CMD_SIZE], int &ind return is_empty; // Inform the caller } + + + /** * Get all commands waiting on the serial port and queue them. * Exit when the buffer is full or when no more characters are @@ -465,6 +473,13 @@ void GCodeQueue::get_serial_commands() { // No data for this port ? Skip it if (!serial_data_available(p)) continue; + if (dumpUponError) { + SERIAL_ECHO_START(); + // On LPC1768, available is returning the number of bytes that are available + if (p == 1) SERIAL_ECHOPAIR("A:", (int)_MSerial.available()); + else SERIAL_ECHOPAIR("A:", (int)MYSERIAL0.available()); + } + // Ok, we have some data to process, let's make progress here hadData = true; @@ -483,6 +498,10 @@ void GCodeQueue::get_serial_commands() { #endif } + if (dumpUponError) { + SERIAL_ECHOPAIR(" L", length, " R", p, " C", ISEOL(c) ? AS_CHAR('$') : AS_CHAR(c)); + } + const char serial_char = (char)c; if (ISEOL(serial_char)) { @@ -493,6 +512,10 @@ void GCodeQueue::get_serial_commands() { char* command = serial_line_buffer[p]; + if (dumpUponError) { + SERIAL_ECHOPAIR(" CMD:", command); + } + while (*command == ' ') command++; // Skip leading spaces char *npos = (*command == 'N') ? command : nullptr; // Require the N parameter to start the line @@ -506,6 +529,9 @@ void GCodeQueue::get_serial_commands() { } const long gcode_N = strtol(npos + 1, nullptr, 10); + if (dumpUponError) { + SERIAL_ECHOPAIR(" N:", gcode_N, " n:", npos+1, " lN:", last_N[p]); + } if (gcode_N != last_N[p] + 1 && !M110) { // In case of error on a serial port, don't prevent other serial port from making progress @@ -582,10 +608,18 @@ void GCodeQueue::get_serial_commands() { , p #endif ); + if (dumpUponError) { + SERIAL_ECHOPAIR(" El:", length); + } + } else process_stream_char(serial_char, serial_input_state[p], serial_line_buffer[p], serial_count[p]); + if (dumpUponError) { + SERIAL_ECHOLNPAIR(" SiS:", serial_input_state[p], " SLB:", serial_line_buffer[p], " SC:", serial_count[p]); + } + } // NUM_SERIAL loop } // queue has space, serial has data } From bc7687fbad96c8f1af3b66dc0d1cf86567b0b123 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Sun, 21 Feb 2021 04:25:22 -0600 Subject: [PATCH 10/42] fix F6 compilation --- Marlin/src/gcode/queue.cpp | 5 +---- docs/Serial.md | 16 +++++++--------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/Marlin/src/gcode/queue.cpp b/Marlin/src/gcode/queue.cpp index 3f26b89a49bb..06db4130eccd 100644 --- a/Marlin/src/gcode/queue.cpp +++ b/Marlin/src/gcode/queue.cpp @@ -306,7 +306,7 @@ void GCodeQueue::ok_to_send() { // WIP: TTTTTTTTTTTTT REMOVE THIS WHEN IT'S VALIDATED TTTTTTTTTTTTTTTT static bool dumpUponError = false; -extern MarlinSerial _MSerial; +extern MSerialT _MSerial; /** * Send a "Resend: nnn" message to the host to @@ -425,9 +425,6 @@ inline bool process_line_done(uint8_t &sis, char (&buff)[MAX_CMD_SIZE], int &ind return is_empty; // Inform the caller } - - - /** * Get all commands waiting on the serial port and queue them. * Exit when the buffer is full or when no more characters are diff --git a/docs/Serial.md b/docs/Serial.md index 1663a65cbcd7..b4079d765f73 100644 --- a/docs/Serial.md +++ b/docs/Serial.md @@ -30,27 +30,25 @@ In the `Marlin/src/core/serial_hook.h` file, the different serial feature are de Since all the types above are using CRTP, it's possible to combine them to get the appropriate functionality. This is easily done via type definition of the feature. -For example, to present a serial interface that's outputting to 2 serial ports, the first one being hooked at runtime and the second one connected to a runtime switchable telnet client, you'll declare the type to use as: +For example, to create a single serial interface with 2 serial outputs (one enabled at runtime and the other switchable): ```cpp typedef MultiSerial< RuntimeSerial, ConditionalSerial > Serial0Type; ``` -To send the same output to 4 serial ports, you'll then combine the MultiSerial this way: +To send the same output to 4 serial ports you could nest `MultiSerial` like this: ```cpp typedef MultiSerial< MultiSerial< BaseSerial, BaseSerial >, MultiSerial< BaseSerial, BaseSerial, 2, 1>, 0, 2> Serial0Type; ``` -The magical numbers here are the step and offset for computing the serial port, let's simplify a bit the above monster to: +The magical numbers here are the step and offset for computing the serial port. Simplifying the above monster a bit: ```cpp -MS< A = MS, B = MS, offset = 0, step = 2> +MS< A = MS, B=MS, offset=0, step=2> ``` -This means that the underlying multiserial A (outputting to `a,b`) is available from offset = 0 to offset + step = 1 (default value). -The multiserial B (outputting to `c, d`) is available from offset = 2 (the next step from the root multiserial) to offset + step = 3. +This means that the underlying multiserial A (with output to `a,b`) is available from offset = 0 to offset + step = 1 (default value). +The multiserial B (with output to `c,d`) is available from offset = 2 (the next step from the root multiserial) to offset + step = 3. In practice, the root multiserial will redirect any index/mask `offset` to `offset + step - 1` to its first leaf, and any index/mask `offset + step` to `offset + 2*step - 1` to its second leaf. - ## Emergency parser -By default, the serial base interface provide an emergency parser that's only enable for serial classes that support it. -Because of this condition, all underlying type takes a first `bool emergencyParserEnabled` argument to their constructor. You must take into account this parameter when defining the actual type used. +By default, the serial base interface provide an emergency parser that's only enable for serial classes that support it. Because of this condition, all underlying types take a first `bool emergencyParserEnabled` argument to their constructor. You must take into account this parameter when defining the actual type used. ## SERIAL macros The following macros are defined (in `serial.h`) to output data to the serial ports: From 574b51234844b3f890e5e2389bd509b190dafc65 Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Mon, 22 Feb 2021 11:31:19 +0100 Subject: [PATCH 11/42] Remove compiler warning with deprecated usage of "register" --- Marlin/src/feature/meatpack.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Marlin/src/feature/meatpack.cpp b/Marlin/src/feature/meatpack.cpp index cd6d8ce6b971..63aa6fb4bbe8 100644 --- a/Marlin/src/feature/meatpack.cpp +++ b/Marlin/src/feature/meatpack.cpp @@ -110,7 +110,7 @@ void MeatPack::handle_rx_char_inner(const uint8_t c) { if (TEST(state, MPConfig_Bit_Active)) { // Is MeatPack active? if (!full_char_count) { // No literal characters to fetch? uint8_t buf[2] = { 0, 0 }; - register const uint8_t res = unpack_chars(c, buf); // Decode the byte into one or two characters. + const uint8_t res = unpack_chars(c, buf); // Decode the byte into one or two characters. if (res & kFirstCharIsLiteral) { // The 1st character couldn't be packed. ++full_char_count; // So the next stream byte is a full character. if (res & kSecondCharIsLiteral) ++full_char_count; // The 2nd character couldn't be packed. Another stream byte is a full character. @@ -219,7 +219,7 @@ uint8_t MeatPack::get_result_char(char* const __restrict out) { if (char_out_count) { res = char_out_count; char_out_count = 0; - for (register uint8_t i = 0; i < res; ++i) + for (uint8_t i = 0; i < res; ++i) out[i] = (char)char_out_buf[i]; } return res; From 5e2ba89c989cc2efbcb91221373b2fe97a9a0037 Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Mon, 22 Feb 2021 11:33:27 +0100 Subject: [PATCH 12/42] Revert "WIP Used for debugging the issue (will be reverted in a later step), don't pull this." This reverts commit 9f9b82ee6d40fa6958bcee01cfd47c400f84d059. --- Marlin/src/gcode/queue.cpp | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/Marlin/src/gcode/queue.cpp b/Marlin/src/gcode/queue.cpp index 06db4130eccd..6cb21e6c61de 100644 --- a/Marlin/src/gcode/queue.cpp +++ b/Marlin/src/gcode/queue.cpp @@ -304,10 +304,6 @@ void GCodeQueue::ok_to_send() { SERIAL_EOL(); } -// WIP: TTTTTTTTTTTTT REMOVE THIS WHEN IT'S VALIDATED TTTTTTTTTTTTTTTT -static bool dumpUponError = false; -extern MSerialT _MSerial; - /** * Send a "Resend: nnn" message to the host to * indicate that a command needs to be re-sent. @@ -321,7 +317,6 @@ void GCodeQueue::flush_and_request_resend() { SERIAL_FLUSH(); SERIAL_ECHOPGM(STR_RESEND); SERIAL_ECHOLN(last_N[serial_ind] + 1); - dumpUponError = true; } // Multiserial already handle the dispatch to/from multiple port by itself @@ -470,13 +465,6 @@ void GCodeQueue::get_serial_commands() { // No data for this port ? Skip it if (!serial_data_available(p)) continue; - if (dumpUponError) { - SERIAL_ECHO_START(); - // On LPC1768, available is returning the number of bytes that are available - if (p == 1) SERIAL_ECHOPAIR("A:", (int)_MSerial.available()); - else SERIAL_ECHOPAIR("A:", (int)MYSERIAL0.available()); - } - // Ok, we have some data to process, let's make progress here hadData = true; @@ -495,10 +483,6 @@ void GCodeQueue::get_serial_commands() { #endif } - if (dumpUponError) { - SERIAL_ECHOPAIR(" L", length, " R", p, " C", ISEOL(c) ? AS_CHAR('$') : AS_CHAR(c)); - } - const char serial_char = (char)c; if (ISEOL(serial_char)) { @@ -509,10 +493,6 @@ void GCodeQueue::get_serial_commands() { char* command = serial_line_buffer[p]; - if (dumpUponError) { - SERIAL_ECHOPAIR(" CMD:", command); - } - while (*command == ' ') command++; // Skip leading spaces char *npos = (*command == 'N') ? command : nullptr; // Require the N parameter to start the line @@ -526,9 +506,6 @@ void GCodeQueue::get_serial_commands() { } const long gcode_N = strtol(npos + 1, nullptr, 10); - if (dumpUponError) { - SERIAL_ECHOPAIR(" N:", gcode_N, " n:", npos+1, " lN:", last_N[p]); - } if (gcode_N != last_N[p] + 1 && !M110) { // In case of error on a serial port, don't prevent other serial port from making progress @@ -605,18 +582,10 @@ void GCodeQueue::get_serial_commands() { , p #endif ); - if (dumpUponError) { - SERIAL_ECHOPAIR(" El:", length); - } - } else process_stream_char(serial_char, serial_input_state[p], serial_line_buffer[p], serial_count[p]); - if (dumpUponError) { - SERIAL_ECHOLNPAIR(" SiS:", serial_input_state[p], " SLB:", serial_line_buffer[p], " SC:", serial_count[p]); - } - } // NUM_SERIAL loop } // queue has space, serial has data } From adbbcc98685a9c6bcbaa6b0471f8ab6b454a15e2 Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Mon, 22 Feb 2021 13:51:43 +0100 Subject: [PATCH 13/42] Rewrite the ring buffer code in GCodeQueue so it's not spread over so many variables and instead use OOP where it's required Add serial rx buffer monitor feature to warn the user when the internal serial's buffer is too close to its limits Add BUG_ON macro so it's possible to stop the printer upon a bug condition --- Marlin/Configuration_adv.h | 7 + Marlin/src/HAL/DUE/MarlinSerialUSB.cpp | 11 +- Marlin/src/HAL/DUE/MarlinSerialUSB.h | 2 +- Marlin/src/MarlinCore.cpp | 2 +- Marlin/src/core/bug_on.h | 37 ++++ Marlin/src/core/macros.h | 26 +++ Marlin/src/core/serial_base.h | 2 +- Marlin/src/core/serial_hook.h | 24 +-- Marlin/src/gcode/gcode.cpp | 26 ++- Marlin/src/gcode/host/M110.cpp | 2 +- Marlin/src/gcode/queue.cpp | 174 +++++++----------- Marlin/src/gcode/queue.h | 138 +++++++++++--- .../lcd/extui/lib/mks_ui/draw_keyboard.cpp | 2 +- .../lcd/extui/lib/mks_ui/draw_manuaLevel.cpp | 2 +- .../lcd/extui/lib/mks_ui/draw_move_motor.cpp | 2 +- .../src/lcd/extui/lib/mks_ui/wifi_module.cpp | 4 +- 16 files changed, 285 insertions(+), 176 deletions(-) create mode 100644 Marlin/src/core/bug_on.h diff --git a/Marlin/Configuration_adv.h b/Marlin/Configuration_adv.h index 987ac293b2d2..1bc1fd4e55cc 100644 --- a/Marlin/Configuration_adv.h +++ b/Marlin/Configuration_adv.h @@ -2012,6 +2012,13 @@ //#define SERIAL_STATS_DROPPED_RX #endif +// Monitor RX buffer usage +// This will dump an error to the serial port in case of overflow of the serial receive buffer +// If it happens, increase the RX_BUFFER_SIZE value +// Not supported on all platform. +#define RX_BUFFER_MONITOR + + /** * Emergency Command Parser * diff --git a/Marlin/src/HAL/DUE/MarlinSerialUSB.cpp b/Marlin/src/HAL/DUE/MarlinSerialUSB.cpp index a04993ca35ed..fca677c7981e 100644 --- a/Marlin/src/HAL/DUE/MarlinSerialUSB.cpp +++ b/Marlin/src/HAL/DUE/MarlinSerialUSB.cpp @@ -92,12 +92,11 @@ int MarlinSerialUSB::read() { return c; } -bool MarlinSerialUSB::available() { - /* If Pending chars */ - return pending_char >= 0 || - /* or USB CDC enumerated and configured on the PC side and some - bytes where sent to us */ - (usb_task_cdc_isenabled() && udi_cdc_is_rx_ready()); +int MarlinSerialUSB::available() { + if (pending_char > 0) return pending_char; + return pending_char == 0 || + // or USB CDC enumerated and configured on the PC side and some bytes where sent to us */ + (usb_task_cdc_isenabled() && udi_cdc_is_rx_ready()); } void MarlinSerialUSB::flush() { } diff --git a/Marlin/src/HAL/DUE/MarlinSerialUSB.h b/Marlin/src/HAL/DUE/MarlinSerialUSB.h index 5281a006b155..f9cea2986960 100644 --- a/Marlin/src/HAL/DUE/MarlinSerialUSB.h +++ b/Marlin/src/HAL/DUE/MarlinSerialUSB.h @@ -39,7 +39,7 @@ struct MarlinSerialUSB { int peek(); int read(); void flush(); - bool available(); + int available(); size_t write(const uint8_t c); #if ENABLED(SERIAL_STATS_DROPPED_RX) diff --git a/Marlin/src/MarlinCore.cpp b/Marlin/src/MarlinCore.cpp index 8aa6cdd6c892..718d075e9c26 100644 --- a/Marlin/src/MarlinCore.cpp +++ b/Marlin/src/MarlinCore.cpp @@ -407,7 +407,7 @@ void startOrResumeJob() { */ inline void manage_inactivity(const bool ignore_stepper_queue=false) { - if (queue.length < BUFSIZE) queue.get_available_commands(); + queue.get_available_commands(); const millis_t ms = millis(); diff --git a/Marlin/src/core/bug_on.h b/Marlin/src/core/bug_on.h new file mode 100644 index 000000000000..8869be8d284f --- /dev/null +++ b/Marlin/src/core/bug_on.h @@ -0,0 +1,37 @@ +/** + * Marlin 3D Printer Firmware + * Copyright (c) 2021 MarlinFirmware [https://github.com/MarlinFirmware/Marlin] + * + * Copyright (c) 2021 X-Ryl669 [https://blog.cyril.by] + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ +#pragma once + +// We need SERIAL_ECHOPAIR and macros.h +#include "serial.h" + +#if ENABLED(POSTMORTEM_DEBUGGING) + // Useful macro for stopping the CPU on an unexpected condition + // This is used like SERIAL_ECHOPAIR, that is: a key-value call of the local variables you want + // to dump to the serial port before stopping the CPU. + #define BUG_ON(V...) do { SERIAL_ECHOPAIR(ONLY_FILENAME, __LINE__, ": "); SERIAL_ECHOLNPAIR(V); SERIAL_FLUSHTX(); *(char*)0 = 42; } while(0) +#elif ENABLED(MARLIN_DEV_MODE) + // Don't stop the CPU here, but at least dump the bug on the serial port + #define BUG_ON(V...) do { SERIAL_ECHOPAIR(ONLY_FILENAME, __LINE__, ": BUG!\n"); SERIAL_ECHOLNPAIR(V); SERIAL_FLUSHTX(); } while(0) +#else + // Release mode, let's ignore the bug + #define BUG_ON(V...) NOOP +#endif diff --git a/Marlin/src/core/macros.h b/Marlin/src/core/macros.h index 16e18ac9023d..c65aebdcdf85 100644 --- a/Marlin/src/core/macros.h +++ b/Marlin/src/core/macros.h @@ -349,6 +349,32 @@ #define CALL_IF_EXISTS(Return, That, Method, ...) \ static_cast(Private::Call_ ## Method(That, ##__VA_ARGS__)) + + // Compile time string manipulation + namespace CompileTimeString { + // Simple compile-time parser to find the position of the end of a string + constexpr const char* findStringEnd(const char *str) { + return *str ? findStringEnd(str + 1) : str; + } + + // Check whether a string contains a slash + constexpr bool containsSlash(const char *str) { + return *str == '/' ? true : (*str ? containsSlash(str + 1) : false); + } + // Find the last position of the slash + constexpr const char* findLastSlashPos(const char* str) { + return *str == '/' ? (str + 1) : findLastSlashPos(str - 1); + } + // Compile time evaluation of the last part of a file path + // Typically used to shorten the path to file in compiled strings + // CompileTimeString::fileName(__FILE__) returns "macros.h" and not /path/to/Marlin/src/core/macros.h + constexpr const char* fileName(const char* str) { + return containsSlash(str) ? findLastSlashPos(findStringEnd(str)) : str; + } + } + + #define ONLY_FILENAME CompileTimeString::fileName(__FILE__) + #else #define MIN_2(a,b) ((a)<(b)?(a):(b)) diff --git a/Marlin/src/core/serial_base.h b/Marlin/src/core/serial_base.h index 83b03cc7b608..418bb557e7cb 100644 --- a/Marlin/src/core/serial_base.h +++ b/Marlin/src/core/serial_base.h @@ -79,7 +79,7 @@ struct SerialBase { void end() { static_cast(this)->end(); } /** Check for available data from the port @param index The port index, usually 0 */ - bool available(uint8_t index = 0) { return static_cast(this)->available(index); } + int available(uint8_t index = 0) { return static_cast(this)->available(index); } /** Read a value from the port @param index The port index, usually 0 */ int read(uint8_t index = 0) { return static_cast(this)->read(index); } diff --git a/Marlin/src/core/serial_hook.h b/Marlin/src/core/serial_hook.h index 5226a51f167a..2fa58b27863c 100644 --- a/Marlin/src/core/serial_hook.h +++ b/Marlin/src/core/serial_hook.h @@ -39,7 +39,7 @@ struct BaseSerial : public SerialBase< BaseSerial >, public SerialT { void msgDone() {} // We don't care about indices here, since if one can call us, it's the right index anyway - bool available(uint8_t) { return (bool)SerialT::available(); } + int available(uint8_t) { return (int)SerialT::available(); } int read(uint8_t) { return (int)SerialT::read(); } bool connected() { return CALL_IF_EXISTS(bool, static_cast(this), connected);; } void flushTX() { CALL_IF_EXISTS(void, static_cast(this), flushTX); } @@ -74,13 +74,13 @@ struct ConditionalSerial : public SerialBase< ConditionalSerial > { void end() { out.end(); } void msgDone() {} - bool connected() { return CALL_IF_EXISTS(bool, &out, connected); } - void flushTX() { CALL_IF_EXISTS(void, &out, flushTX); } + bool connected() { return CALL_IF_EXISTS(bool, &out, connected); } + void flushTX() { CALL_IF_EXISTS(void, &out, flushTX); } - bool available(uint8_t ) { return (bool)out.available(); } - int read(uint8_t ) { return (int)out.read(); } - bool available() { return (bool)out.available(); } - int read() { return (int)out.read(); } + int available(uint8_t ) { return (int)out.available(); } + int read(uint8_t ) { return (int)out.read(); } + int available() { return (int)out.available(); } + int read() { return (int)out.read(); } ConditionalSerial(bool & conditionVariable, SerialT & out, const bool e) : BaseClassT(e), condition(conditionVariable), out(out) {} @@ -102,9 +102,9 @@ struct ForwardSerial : public SerialBase< ForwardSerial > { bool connected() { return Private::HasMember_connected::value ? CALL_IF_EXISTS(bool, &out, connected) : (bool)out; } void flushTX() { CALL_IF_EXISTS(void, &out, flushTX); } - bool available(uint8_t) { return (bool)out.available(); } + int available(uint8_t) { return (int)out.available(); } int read(uint8_t) { return (int)out.read(); } - bool available() { return (bool)out.available(); } + int available() { return (int)out.available(); } int read() { return (int)out.read(); } ForwardSerial(const bool e, SerialT & out) : BaseClassT(e), out(out) {} @@ -130,7 +130,7 @@ struct RuntimeSerial : public SerialBase< RuntimeSerial >, public Seria if (eofHook) eofHook(userPointer); } - bool available(uint8_t) { return (bool)SerialT::available(); } + int available(uint8_t) { return (int)SerialT::available(); } int read(uint8_t) { return (int)SerialT::read(); } using SerialT::available; using SerialT::read; @@ -191,14 +191,14 @@ struct MultiSerial : public SerialBase< MultiSerial= 0 + offset && index < step + offset) return serial0.available(index); else if (index >= step + offset && index < 2 * step + offset) return serial1.available(index); return false; } - NO_INLINE int read(uint8_t index) { + int read(uint8_t index) { if (index >= 0 + offset && index < step + offset) return serial0.read(index); else if (index >= step + offset && index < 2 * step + offset) diff --git a/Marlin/src/gcode/gcode.cpp b/Marlin/src/gcode/gcode.cpp index 77ac1fbff86b..b03223a2a32e 100644 --- a/Marlin/src/gcode/gcode.cpp +++ b/Marlin/src/gcode/gcode.cpp @@ -260,13 +260,6 @@ void GcodeSuite::dwell(millis_t time) { #endif // HAS_LEVELING && G29_RETRY_AND_RECOVER -// -// Placeholders for non-migrated codes -// -#if ENABLED(M100_FREE_MEMORY_WATCHER) - extern void M100_dump_routine(PGM_P const title, const char * const start, const char * const end); -#endif - /** * Process the parsed command and dispatch it to its handler */ @@ -995,25 +988,30 @@ void GcodeSuite::process_parsed_command(const bool no_ok/*=false*/) { * This is called from the main loop() */ void GcodeSuite::process_next_command() { - char * const current_command = queue.command_buffer[queue.index_r]; + GCodeQueue::CommandQueue & command = queue.peek_next_command(); - PORT_REDIRECT(SERIAL_PORTMASK(queue.port[queue.index_r])); + #if HAS_MULTI_SERIAL + PORT_REDIRECT(SERIAL_PORTMASK(command.port)); + #endif #if ENABLED(POWER_LOSS_RECOVERY) - recovery.queue_index_r = queue.index_r; + recovery.queue_index_r = queue.ring_buffer.index_r; #endif if (DEBUGGING(ECHO)) { SERIAL_ECHO_START(); - SERIAL_ECHOLN(current_command); + SERIAL_ECHOLN(command.buffer); #if ENABLED(M100_FREE_MEMORY_DUMPER) - SERIAL_ECHOPAIR("slot:", queue.index_r); - M100_dump_routine(PSTR(" Command Queue:"), &queue.command_buffer[0][0], &queue.command_buffer[BUFSIZE - 1][MAX_CMD_SIZE - 1]); + SERIAL_ECHOPAIR("slot:", queue.ring_buffer.index_r); + serialprintPGM(PSTR(" Command Queue:")); + SERIAL_EOL(); + for (GCodeQueue::CommandQueue * start = &queue.ring_buffer.commands[0]; start < &queue.ring_buffer.commands[BUFSIZE]; start++) + SERIAL_ECHOLN(start->buffer); #endif } // Parse the next command in the queue - parser.parse(current_command); + parser.parse(command.buffer); process_parsed_command(); } diff --git a/Marlin/src/gcode/host/M110.cpp b/Marlin/src/gcode/host/M110.cpp index b12b38ea0f12..2634b198978d 100644 --- a/Marlin/src/gcode/host/M110.cpp +++ b/Marlin/src/gcode/host/M110.cpp @@ -29,6 +29,6 @@ void GcodeSuite::M110() { if (parser.seenval('N')) - queue.last_N[queue.command_port()] = parser.value_long(); + queue.set_current_line_number(parser.value_long()); } diff --git a/Marlin/src/gcode/queue.cpp b/Marlin/src/gcode/queue.cpp index 6cb21e6c61de..b7aa9189fe28 100644 --- a/Marlin/src/gcode/queue.cpp +++ b/Marlin/src/gcode/queue.cpp @@ -35,6 +35,7 @@ GCodeQueue queue; #include "../module/planner.h" #include "../module/temperature.h" #include "../MarlinCore.h" +#include "../core/bug_on.h" #if ENABLED(PRINTER_EVENT_LEDS) #include "../feature/leds/printer_event_leds.h" @@ -59,44 +60,15 @@ GCodeQueue queue; // Frequently used G-code strings PGMSTR(G28_STR, "G28"); -/** - * GCode line number handling. Hosts may opt to include line numbers when - * sending commands to Marlin, and lines will be checked for sequentiality. - * M110 N sets the current line number. - */ -long GCodeQueue::last_N[NUM_SERIAL]; -/** - * GCode Command Queue - * A simple ring buffer of BUFSIZE command strings. - * - * Commands are copied into this buffer by the command injectors - * (immediate, serial, sd card) and they are processed sequentially by - * the main loop. The gcode.process_next_command method parses the next - * command and hands off execution to individual handler functions. - */ -uint8_t GCodeQueue::length = 0, // Count of commands in the queue - GCodeQueue::index_r = 0, // Ring buffer read position - GCodeQueue::index_w = 0; // Ring buffer write position +GCodeQueue::PerSerial GCodeQueue::per_serial[NUM_SERIAL] = { 0 }; +GCodeQueue::RingBuffer GCodeQueue::ring_buffer = { 0 }; -char GCodeQueue::command_buffer[BUFSIZE][MAX_CMD_SIZE]; - -/* - * The port that the command was received on - */ -#if HAS_MULTI_SERIAL - serial_index_t GCodeQueue::port[BUFSIZE]; -#endif /** * Serial command injection */ -// Number of characters read in the current line of serial input -static int serial_count[NUM_SERIAL] = { 0 }; - -bool send_ok[BUFSIZE]; - /** * Next Injected PROGMEM Command pointer. (nullptr == empty) * Internal commands are enqueued ahead of serial / SD commands. @@ -108,38 +80,16 @@ PGM_P GCodeQueue::injected_commands_P; // = nullptr */ char GCodeQueue::injected_commands[64]; // = { 0 } -GCodeQueue::GCodeQueue() { - // Send "ok" after commands by default - LOOP_L_N(i, COUNT(send_ok)) send_ok[i] = true; -} - -/** - * Check whether there are any commands yet to be executed - */ -bool GCodeQueue::has_commands_queued() { - return queue.length || injected_commands_P || injected_commands[0]; -} -/** - * Clear the Marlin command queue - */ -void GCodeQueue::clear() { - index_r = index_w = length = 0; -} - -/** - * Once a new command is in the ring buffer, call this to commit it - */ -void GCodeQueue::_commit_command(bool say_ok +void GCodeQueue::RingBuffer::commit_command(bool skip_ok #if HAS_MULTI_SERIAL , serial_index_t serial_ind/*=-1*/ #endif -) { - send_ok[index_w] = say_ok; - TERN_(HAS_MULTI_SERIAL, port[index_w] = serial_ind); + ) { + commands[index_w].skip_ok = skip_ok; + TERN_(HAS_MULTI_SERIAL, commands[index_w].port = serial_ind); TERN_(POWER_LOSS_RECOVERY, recovery.commit_sdpos(index_w)); - if (++index_w >= BUFSIZE) index_w = 0; - length++; + advance_pos(index_w, 1); } /** @@ -147,14 +97,14 @@ void GCodeQueue::_commit_command(bool say_ok * Return true if the command was successfully added. * Return false for a full buffer, or if the 'command' is a comment. */ -bool GCodeQueue::_enqueue(const char* cmd, bool say_ok/*=false*/ +bool GCodeQueue::RingBuffer::enqueue(const char* cmd, bool skip_ok/*=true*/ #if HAS_MULTI_SERIAL , serial_index_t serial_ind/*=-1*/ #endif ) { if (*cmd == ';' || length >= BUFSIZE) return false; - strcpy(command_buffer[index_w], cmd); - _commit_command(say_ok + strcpy(commands[index_w].buffer, cmd); + commit_command(skip_ok #if HAS_MULTI_SERIAL , serial_ind #endif @@ -174,7 +124,7 @@ bool GCodeQueue::enqueue_one(const char* cmd) { if (*cmd == 0 || ISEOL(*cmd)) return true; - if (_enqueue(cmd)) { + if (ring_buffer.enqueue(cmd)) { SERIAL_ECHO_MSG(STR_ENQUEUEING, cmd, "\""); return true; } @@ -252,7 +202,7 @@ bool GCodeQueue::enqueue_one_P(PGM_P const pgcode) { char cmd[i + 1]; memcpy_P(cmd, p, i); cmd[i] = '\0'; - return _enqueue(cmd); + return ring_buffer.enqueue(cmd); } /** @@ -283,16 +233,17 @@ void GCodeQueue::enqueue_now_P(PGM_P const pgcode) { * P Planner space remaining * B Block queue space remaining */ -void GCodeQueue::ok_to_send() { +void GCodeQueue::RingBuffer::ok_to_send() { + CommandQueue & command = commands[index_r]; #if HAS_MULTI_SERIAL - const serial_index_t serial_ind = command_port(); + const serial_index_t serial_ind = command.port; if (serial_ind < 0) return; PORT_REDIRECT(SERIAL_PORTMASK(serial_ind)); // Reply to the serial port that sent the command #endif - if (!send_ok[index_r]) return; + if (command.skip_ok) return; SERIAL_ECHOPGM(STR_OK); #if ENABLED(ADVANCED_OK) - char* p = command_buffer[index_r]; + char* p = command.buffer; if (*p == 'N') { SERIAL_CHAR(' ', *p++); while (NUMERIC_SIGNED(*p)) @@ -309,25 +260,42 @@ void GCodeQueue::ok_to_send() { * indicate that a command needs to be re-sent. */ void GCodeQueue::flush_and_request_resend() { - const serial_index_t serial_ind = command_port(); + const serial_index_t serial_ind = ring_buffer.command_port(); #if HAS_MULTI_SERIAL if (serial_ind < 0) return; // Never mind. Command came from SD or Flash Drive PORT_REDIRECT(SERIAL_PORTMASK(serial_ind)); // Reply to the serial port that sent the command #endif SERIAL_FLUSH(); SERIAL_ECHOPGM(STR_RESEND); - SERIAL_ECHOLN(last_N[serial_ind] + 1); + SERIAL_ECHOLN(per_serial[serial_ind].last_N + 1); } // Multiserial already handle the dispatch to/from multiple port by itself inline bool serial_data_available(uint8_t index = SERIAL_ALL) { if (index == SERIAL_ALL) { for (index = 0; index < NUM_SERIAL; index++) { - if (SERIAL_IMPL.available(index)) return true; + int a = SERIAL_IMPL.available(index); + #if BOTH(RX_BUFFER_MONITOR, RX_BUFFER_SIZE) + if (a > RX_BUFFER_SIZE - 2) { + PORT_REDIRECT(SERIAL_PORTMASK(index)); + SERIAL_ERROR_START(); + SERIAL_ECHOLNPAIR("RX BUF overflow, increase RX_BUFFER_SIZE: ", a); + } + #endif + if (a > 0) return true; } return false; } - return SERIAL_IMPL.available(index); + int a = SERIAL_IMPL.available(index); + #if BOTH(RX_BUFFER_MONITOR, RX_BUFFER_SIZE) + if (a > RX_BUFFER_SIZE - 2) { + PORT_REDIRECT(SERIAL_PORTMASK(index)); + SERIAL_ERROR_START(); + SERIAL_ECHOLNPAIR("RX BUF overflow, increase RX_BUFFER_SIZE: ", a); + } + #endif + + return a > 0; } inline int read_serial(const uint8_t index) { return SERIAL_IMPL.read(index); } @@ -336,10 +304,10 @@ void GCodeQueue::gcode_line_error(PGM_P const err, const serial_index_t serial_i PORT_REDIRECT(SERIAL_PORTMASK(serial_ind)); // Reply to the serial port that sent the command SERIAL_ERROR_START(); serialprintPGM(err); - SERIAL_ECHOLN(last_N[serial_ind]); - while (read_serial(serial_ind) != -1); // Clear out the RX buffer + SERIAL_ECHOLN(per_serial[serial_ind].last_N); + while (read_serial(serial_ind) != -1); // Clear out the RX buffer. Why don't use flush here ? flush_and_request_resend(); - serial_count[serial_ind] = 0; + per_serial[serial_ind].count = 0; } FORCE_INLINE bool is_M29(const char * const cmd) { // matches "M29" & "M29 ", but not "M290", etc @@ -426,10 +394,6 @@ inline bool process_line_done(uint8_t &sis, char (&buff)[MAX_CMD_SIZE], int &ind * left on the serial port. */ void GCodeQueue::get_serial_commands() { - static char serial_line_buffer[NUM_SERIAL][MAX_CMD_SIZE]; - - static uint8_t serial_input_state[NUM_SERIAL] = { PS_NORMAL }; - #if ENABLED(BINARY_FILE_TRANSFER) if (card.flag.binary_mode) { /** @@ -437,7 +401,7 @@ void GCodeQueue::get_serial_commands() { * receive buffer (which limits the packet size to MAX_CMD_SIZE). * The receive buffer also limits the packet size for reliable transmission. */ - binaryStream[card.transfer_port_index].receive(serial_line_buffer[card.transfer_port_index]); + binaryStream[card.transfer_port_index].receive(per_serial[card.transfer_port_index].line_buffer); return; } #endif @@ -460,7 +424,7 @@ void GCodeQueue::get_serial_commands() { LOOP_L_N(p, NUM_SERIAL) { // Check if the queue is full, and exits if it is. - if (length >= BUFSIZE) return; + if (ring_buffer.full()) return; // No data for this port ? Skip it if (!serial_data_available(p)) continue; @@ -472,26 +436,24 @@ void GCodeQueue::get_serial_commands() { if (c < 0) { // This should never happen, let's log it PORT_REDIRECT(SERIAL_PORTMASK(p)); // Reply to the serial port that sent the command - #if BOTH(MARLIN_DEV_MODE, POSTMORTEM_DEBUGGING) - // Crash here to get more information why it failed - BUG_ON("SP available but read -1"); - #else - SERIAL_ERROR_START(); - SERIAL_ECHOLNPGM(STR_ERR_SERIAL_MISMATCH); - SERIAL_FLUSH(); - continue; - #endif + // Crash here to get more information why it failed + BUG_ON("SP available but read -1"); + SERIAL_ERROR_START(); + SERIAL_ECHOLNPGM(STR_ERR_SERIAL_MISMATCH); + SERIAL_FLUSH(); + continue; } const char serial_char = (char)c; + PerSerial & serial = per_serial[p]; if (ISEOL(serial_char)) { // Reset our state, continue if the line was empty - if (process_line_done(serial_input_state[p], serial_line_buffer[p], serial_count[p])) + if (process_line_done(serial.input_state, serial.line_buffer, serial.count)) continue; - char* command = serial_line_buffer[p]; + char* command = serial.line_buffer; while (*command == ' ') command++; // Skip leading spaces char *npos = (*command == 'N') ? command : nullptr; // Require the N parameter to start the line @@ -507,7 +469,7 @@ void GCodeQueue::get_serial_commands() { const long gcode_N = strtol(npos + 1, nullptr, 10); - if (gcode_N != last_N[p] + 1 && !M110) { + if (gcode_N != serial.last_N + 1 && !M110) { // In case of error on a serial port, don't prevent other serial port from making progress gcode_line_error(PSTR(STR_ERR_LINE_NO), p); break; @@ -530,7 +492,7 @@ void GCodeQueue::get_serial_commands() { break; } - last_N[p] = gcode_N; + serial.last_N = gcode_N; } #if ENABLED(SDSUPPORT) // Pronterface "M29" and "M29 " has no line number @@ -577,14 +539,14 @@ void GCodeQueue::get_serial_commands() { #endif // Add the command to the queue - _enqueue(serial_line_buffer[p], true + ring_buffer.enqueue(serial.line_buffer, false #if HAS_MULTI_SERIAL , p #endif ); } else - process_stream_char(serial_char, serial_input_state[p], serial_line_buffer[p], serial_count[p]); + process_stream_char(serial_char, serial.input_state, serial.line_buffer, serial.count); } // NUM_SERIAL loop } // queue has space, serial has data @@ -604,33 +566,35 @@ void GCodeQueue::get_serial_commands() { if (!IS_SD_PRINTING()) return; int sd_count = 0; - while (length < BUFSIZE && !card.eof()) { + while (!ring_buffer.full() && !card.eof()) { const int16_t n = card.get(); const bool card_eof = card.eof(); if (n < 0 && !card_eof) { SERIAL_ERROR_MSG(STR_SD_ERR_READ); continue; } + CommandQueue & command = ring_buffer.commands[ring_buffer.index_w]; const char sd_char = (char)n; const bool is_eol = ISEOL(sd_char); if (is_eol || card_eof) { + // Reset stream state, terminate the buffer, and commit a non-empty command if (!is_eol && sd_count) ++sd_count; // End of file with no newline - if (!process_line_done(sd_input_state, command_buffer[index_w], sd_count)) { + if (!process_line_done(sd_input_state, command.buffer, sd_count)) { // M808 L saves the sdpos of the next line. M808 loops to a new sdpos. - TERN_(GCODE_REPEAT_MARKERS, repeat.early_parse_M808(command_buffer[index_w])); + TERN_(GCODE_REPEAT_MARKERS, repeat.early_parse_M808(command.buffer)); // Put the new command into the buffer (no "ok" sent) - _commit_command(false); + ring_buffer.commit_command(true); - // Prime Power-Loss Recovery for the NEXT _commit_command + // Prime Power-Loss Recovery for the NEXT commit_command TERN_(POWER_LOSS_RECOVERY, recovery.cmd_sdpos = card.getIndex()); } if (card.eof()) card.fileHasFinished(); // Handle end of file reached } else - process_stream_char(sd_char, sd_input_state, command_buffer[index_w], sd_count); + process_stream_char(sd_char, sd_input_state, command.buffer, sd_count); } } @@ -643,6 +607,7 @@ void GCodeQueue::get_serial_commands() { * - The SD card file being actively printed */ void GCodeQueue::get_available_commands() { + if (ring_buffer.full()) return; get_serial_commands(); @@ -658,12 +623,12 @@ void GCodeQueue::advance() { if (process_injected_command_P() || process_injected_command()) return; // Return if the G-code buffer is empty - if (!length) return; + if (ring_buffer.empty()) return; #if ENABLED(SDSUPPORT) if (card.flag.saving) { - char* command = command_buffer[index_r]; + char* command = ring_buffer.commands[ring_buffer.index_r].buffer; if (is_M29(command)) { // M29 closes the file card.closefile(); @@ -699,6 +664,5 @@ void GCodeQueue::advance() { #endif // SDSUPPORT // The queue may be reset by a command handler or by code invoked by idle() within a handler - --length; - if (++index_r >= BUFSIZE) index_r = 0; + ring_buffer.advance_pos(ring_buffer.index_r, -1); } diff --git a/Marlin/src/gcode/queue.h b/Marlin/src/gcode/queue.h index d677146a7dbe..8da5d83edbb6 100644 --- a/Marlin/src/gcode/queue.h +++ b/Marlin/src/gcode/queue.h @@ -31,41 +31,124 @@ class GCodeQueue { public: /** - * GCode line number handling. Hosts may include line numbers when sending - * commands to Marlin, and lines will be checked for sequentiality. - * M110 N sets the current line number. + * The buffers per serial port. */ + struct PerSerial + { + /** + * GCode line number handling. Hosts may include line numbers when sending + * commands to Marlin, and lines will be checked for sequentiality. + * M110 N sets the current line number. + */ + long last_N; + /** + * Number of characters read in the current line of serial input + */ + int count; + + /** + * The current line accumulator + */ + char line_buffer[MAX_CMD_SIZE]; + + /** + * The input state + */ + uint8_t input_state; + }; - static long last_N[NUM_SERIAL]; + /** + * The array of per serial state variable + */ + static PerSerial per_serial[NUM_SERIAL]; /** * GCode Command Queue - * A simple ring buffer of BUFSIZE command strings. + * A simple (circular) ring buffer of BUFSIZE command strings. * * Commands are copied into this buffer by the command injectors * (immediate, serial, sd card) and they are processed sequentially by * the main loop. The gcode.process_next_command method parses the next * command and hands off execution to individual handler functions. */ - static uint8_t length, // Count of commands in the queue - index_r; // Ring buffer read position + struct CommandQueue + { + /** + * The command buffer + */ + char buffer[MAX_CMD_SIZE]; + + /** + * Skip sending ok when command is processed ? + * The logic is inverted here to skip setting all the variables to true upon construction + */ + bool skip_ok; + + #if HAS_MULTI_SERIAL + /** + * The port that the command was received on + */ + serial_index_t port; + #endif + }; - static char command_buffer[BUFSIZE][MAX_CMD_SIZE]; /** - * The port that the command was received on + * The ring buffer head */ - #if HAS_MULTI_SERIAL - static serial_index_t port[BUFSIZE]; - #endif - static inline serial_index_t command_port() { return TERN0(HAS_MULTI_SERIAL, port[index_r]); } + struct RingBuffer + { + /** + * Number of commands in the queue + */ + uint8_t length; + /** + * Ring buffer's read position + */ + uint8_t index_r; + /** + * Ring buffer's write position + */ + uint8_t index_w; + /** + * The ring buffer of commands + */ + CommandQueue commands[BUFSIZE]; + + inline serial_index_t command_port() { return TERN0(HAS_MULTI_SERIAL, commands[index_r].port); } + + inline void clear() { length = index_r = index_w = 0; } + + void advance_pos(uint8_t & p, int inc) { if (++p >= BUFSIZE) p = 0; length += inc; } + + void commit_command(bool skip_ok + #if HAS_MULTI_SERIAL + , serial_index_t serial_ind = -1 + #endif + ); + + bool enqueue(const char* cmd, bool skip_ok = true + #if HAS_MULTI_SERIAL + , serial_index_t serial_ind = -1 + #endif + ); + + void ok_to_send(); + + inline bool full(uint8_t cmdCount = 1) const { return length > (BUFSIZE - cmdCount); } - GCodeQueue(); + inline bool empty() const { return length == 0; } + }; + + /** + * The ring buffer of commands + */ + static RingBuffer ring_buffer; /** * Clear the Marlin command queue */ - static void clear(); + static void clear() { ring_buffer.clear(); } /** * Next Injected Command (PROGMEM) pointer. (nullptr == empty) @@ -112,7 +195,7 @@ class GCodeQueue { /** * Check whether there are any commands yet to be executed */ - static bool has_commands_queued(); + static bool has_commands_queued() { return ring_buffer.length || injected_commands_P || injected_commands[0]; } /** * Get the next command in the queue, optionally log it to SD, then dispatch it @@ -136,7 +219,7 @@ class GCodeQueue { * P Planner space remaining * B Block queue space remaining */ - static void ok_to_send(); + static inline void ok_to_send() { ring_buffer.ok_to_send(); } /** * Clear the serial line and request a resend of @@ -144,9 +227,15 @@ class GCodeQueue { */ static void flush_and_request_resend(); + /** + * (Re)Set the current line number for the last received command + */ + static inline void set_current_line_number(long n) { per_serial[ring_buffer.command_port()].last_N = n; } + private: - static uint8_t index_w; // Ring buffer write position + /** Get the next command from the buffer (or NULL) */ + CommandQueue & peek_next_command() { return ring_buffer.commands[ring_buffer.index_r]; } static void get_serial_commands(); @@ -154,18 +243,6 @@ class GCodeQueue { static void get_sdcard_commands(); #endif - static void _commit_command(bool say_ok - #if HAS_MULTI_SERIAL - , serial_index_t serial_ind=-1 - #endif - ); - - static bool _enqueue(const char* cmd, bool say_ok=false - #if HAS_MULTI_SERIAL - , serial_index_t serial_ind=-1 - #endif - ); - // Process the next "immediate" command (PROGMEM) static bool process_injected_command_P(); @@ -180,6 +257,7 @@ class GCodeQueue { static void gcode_line_error(PGM_P const err, const serial_index_t serial_ind); + friend class GcodeSuite; }; extern GCodeQueue queue; diff --git a/Marlin/src/lcd/extui/lib/mks_ui/draw_keyboard.cpp b/Marlin/src/lcd/extui/lib/mks_ui/draw_keyboard.cpp index 2cf6f05a99ac..6e0d7814d02c 100644 --- a/Marlin/src/lcd/extui/lib/mks_ui/draw_keyboard.cpp +++ b/Marlin/src/lcd/extui/lib/mks_ui/draw_keyboard.cpp @@ -162,7 +162,7 @@ static void lv_kb_event_cb(lv_obj_t *kb, lv_event_t event) { draw_return_ui(); break; case GCodeCommand: - if (queue.length <= (BUFSIZE - 3)) { + if (!queue.ring_buffer.full(3)) { // Hook anything that goes to the serial port MYSERIAL0.setHook(lv_serial_capt_hook, lv_eom_hook, 0); queue.enqueue_one_now(ret_ta_txt); diff --git a/Marlin/src/lcd/extui/lib/mks_ui/draw_manuaLevel.cpp b/Marlin/src/lcd/extui/lib/mks_ui/draw_manuaLevel.cpp index 495acda06bda..dbeb3796e0cc 100644 --- a/Marlin/src/lcd/extui/lib/mks_ui/draw_manuaLevel.cpp +++ b/Marlin/src/lcd/extui/lib/mks_ui/draw_manuaLevel.cpp @@ -46,7 +46,7 @@ static void event_handler(lv_obj_t *obj, lv_event_t event) { switch (obj->mks_obj_id) { case ID_M_POINT1 ... ID_M_POINT5: - if (queue.length == 0) { + if (queue.ring_buffer.empty()) { if (uiCfg.leveling_first_time) { uiCfg.leveling_first_time = false; queue.inject_P(G28_STR); diff --git a/Marlin/src/lcd/extui/lib/mks_ui/draw_move_motor.cpp b/Marlin/src/lcd/extui/lib/mks_ui/draw_move_motor.cpp index 1c07583d53c1..2dec548af0f6 100644 --- a/Marlin/src/lcd/extui/lib/mks_ui/draw_move_motor.cpp +++ b/Marlin/src/lcd/extui/lib/mks_ui/draw_move_motor.cpp @@ -54,7 +54,7 @@ enum { static void event_handler(lv_obj_t *obj, lv_event_t event) { char str_1[16]; if (event != LV_EVENT_RELEASED) return; - if (queue.length <= (BUFSIZE - 3)) { + if (!queue.ring_buffer.full(3)) { bool do_inject = true; float dist = uiCfg.move_dist; switch (obj->mks_obj_id) { diff --git a/Marlin/src/lcd/extui/lib/mks_ui/wifi_module.cpp b/Marlin/src/lcd/extui/lib/mks_ui/wifi_module.cpp index 71cdb0f7d4d0..56229565791d 100644 --- a/Marlin/src/lcd/extui/lib/mks_ui/wifi_module.cpp +++ b/Marlin/src/lcd/extui/lib/mks_ui/wifi_module.cpp @@ -1613,7 +1613,7 @@ void wifi_rcv_handle() { if (wifiTransError.flag != 0x1) WIFI_IO1_RESET(); getDataF = 1; } - if (need_ok_later && (queue.length < BUFSIZE)) { + if (need_ok_later && !queue.ring_buffer.full()) { need_ok_later = false; send_to_wifi((uint8_t *)"ok\r\n", strlen("ok\r\n")); } @@ -1772,7 +1772,7 @@ void get_wifi_commands() { static int wifi_read_count = 0; if (espGcodeFifo.wait_tick > 5) { - while ((queue.length < BUFSIZE) && (espGcodeFifo.r != espGcodeFifo.w)) { + while (!queue.ring_buffer.full() && (espGcodeFifo.r != espGcodeFifo.w)) { espGcodeFifo.wait_tick = 0; From dc05645d00746af4d75df659a6ea3cc64f9191a7 Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Tue, 23 Feb 2021 10:58:35 +0100 Subject: [PATCH 14/42] Set the last command time at the appropriate place so we don't get a wait message when the printer is not actually waiting --- Marlin/src/core/serial_hook.h | 2 +- Marlin/src/gcode/queue.cpp | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/Marlin/src/core/serial_hook.h b/Marlin/src/core/serial_hook.h index 2fa58b27863c..f25465596461 100644 --- a/Marlin/src/core/serial_hook.h +++ b/Marlin/src/core/serial_hook.h @@ -162,7 +162,7 @@ struct RuntimeSerial : public SerialBase< RuntimeSerial >, public Seria // Forward constructor template - RuntimeSerial(const bool e, Args... args) : BaseClassT(e), SerialT(args...) {} + RuntimeSerial(const bool e, Args... args) : BaseClassT(e), SerialT(args...), writeHook(0), eofHook(0), userPointer(0) {} }; // A class that's duplicating its output conditionally to 2 serial interface diff --git a/Marlin/src/gcode/queue.cpp b/Marlin/src/gcode/queue.cpp index b7aa9189fe28..81441faa2a90 100644 --- a/Marlin/src/gcode/queue.cpp +++ b/Marlin/src/gcode/queue.cpp @@ -64,6 +64,9 @@ PGMSTR(G28_STR, "G28"); GCodeQueue::PerSerial GCodeQueue::per_serial[NUM_SERIAL] = { 0 }; GCodeQueue::RingBuffer GCodeQueue::ring_buffer = { 0 }; +#if NO_TIMEOUTS > 0 + static millis_t last_command_time = 0; +#endif /** * Serial command injection @@ -234,6 +237,10 @@ void GCodeQueue::enqueue_now_P(PGM_P const pgcode) { * B Block queue space remaining */ void GCodeQueue::RingBuffer::ok_to_send() { + #if NO_TIMEOUTS > 0 + // Start counting from the last command's execution + last_command_time = millis(); + #endif CommandQueue & command = commands[index_r]; #if HAS_MULTI_SERIAL const serial_index_t serial_ind = command.port; @@ -409,9 +416,8 @@ void GCodeQueue::get_serial_commands() { // If the command buffer is empty for too long, // send "wait" to indicate Marlin is still waiting. #if NO_TIMEOUTS > 0 - static millis_t last_command_time = 0; const millis_t ms = millis(); - if (length == 0 && !serial_data_available() && ELAPSED(ms, last_command_time + NO_TIMEOUTS)) { + if (ring_buffer.empty() && !serial_data_available() && ELAPSED(ms, last_command_time + NO_TIMEOUTS)) { SERIAL_ECHOLNPGM(STR_WAIT); last_command_time = ms; } From 371f9cae9055ad2790b31baaeb0e18596cd210d8 Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Wed, 24 Feb 2021 14:19:51 +0100 Subject: [PATCH 15/42] Fix meatpack logic so it reads the serial port in the available test so it can compute the actual number of byte available after decoding. --- Marlin/src/feature/meatpack.h | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/Marlin/src/feature/meatpack.h b/Marlin/src/feature/meatpack.h index d6d8be802eeb..520c0abcc99d 100644 --- a/Marlin/src/feature/meatpack.h +++ b/Marlin/src/feature/meatpack.h @@ -138,29 +138,36 @@ struct MeatpackSerial : public SerialBase > { void msgDone() { out.msgDone(); } // Existing instances implement Arduino's operator bool, so use that if it's available - bool connected() { return Private::HasMember_connected::value ? CALL_IF_EXISTS(bool, &out, connected) : (bool)out; } - void flushTX() { CALL_IF_EXISTS(void, &out, flushTX); } + bool connected() { return Private::HasMember_connected::value ? CALL_IF_EXISTS(bool, &out, connected) : (bool)out; } + void flushTX() { CALL_IF_EXISTS(void, &out, flushTX); } - bool available(uint8_t index) { + int available(uint8_t index) { // There is a potential issue here with multiserial, since it'll return its decoded buffer whatever the serial index here. // So, instead of doing MeatpackSerial> we should do MultiSerial, MeatpackSerial<...>> // TODO, let's fix this later on - return charCount > 0 || (bool)out.available(index); + if (charCount > 0) return charCount; + + // Don't read in read method, instead do it here, so we can make progress in the read method + while (out.available(index) > 0) { + int r = out.read(index); + if (r == -1) return 0; // This is an error from the underlying serial code + + meatpack.handle_rx_char((uint8_t)r, index); + charCount = meatpack.get_result_char(serialBuffer); + if (charCount) return charCount; + } + return 0; } int readImpl(uint8_t index) { // DITTO if (charCount > 0) return serialBuffer[sizeof(serialBuffer) - (charCount--)]; - - int r = out.read(index); - if (r == -1) return r; - - meatpack.handle_rx_char((uint8_t)r, index); - charCount = meatpack.get_result_char(serialBuffer); + // Not enough char to make progress ? + if (available(index) == 0) return -1; return serialBuffer[sizeof(serialBuffer) - (charCount--)]; } int read(uint8_t index) { return readImpl(index); } - bool available() { return charCount > 0 || out.available(0); } + int available() { return available(0); } int read() { return readImpl(0); } MeatpackSerial(const bool e, SerialT & out) : BaseClassT(e), out(out) {} From bc9031e8727f631d1fd22bb3e425ecaab25eb7cd Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Thu, 25 Feb 2021 10:27:28 +0100 Subject: [PATCH 16/42] Fix multiserial issue in Meatpack --- Marlin/src/feature/meatpack.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Marlin/src/feature/meatpack.cpp b/Marlin/src/feature/meatpack.cpp index 63aa6fb4bbe8..2ad46fab430a 100644 --- a/Marlin/src/feature/meatpack.cpp +++ b/Marlin/src/feature/meatpack.cpp @@ -200,7 +200,7 @@ void MeatPack::handle_rx_char(const uint8_t c, const serial_index_t serial_ind) } if (cmd_is_next) { // Were two command bytes received? - PORT_REDIRECT(serial_ind); + PORT_REDIRECT(SERIAL_PORTMASK(serial_ind)); handle_command((MeatPack_Command)c); // Then the byte is a MeatPack command cmd_is_next = false; return; From 449058db63eb6f249e110bf1657608267e7e7df2 Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Thu, 25 Feb 2021 10:34:53 +0100 Subject: [PATCH 17/42] Fix by @ldursw for MeatPack --- Marlin/src/feature/meatpack.cpp | 2 +- Marlin/src/feature/meatpack.h | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Marlin/src/feature/meatpack.cpp b/Marlin/src/feature/meatpack.cpp index 2ad46fab430a..9d5fadf1a1fb 100644 --- a/Marlin/src/feature/meatpack.cpp +++ b/Marlin/src/feature/meatpack.cpp @@ -148,7 +148,7 @@ void MeatPack::handle_output_char(const uint8_t c) { if (chars_decoded < 1024) { ++chars_decoded; DEBUG_ECHOPGM("RB: "); - MYSERIAL.print((char)c); + DEBUG_ECHO((char)c); DEBUG_EOL(); } #endif diff --git a/Marlin/src/feature/meatpack.h b/Marlin/src/feature/meatpack.h index 520c0abcc99d..d358fb0a2894 100644 --- a/Marlin/src/feature/meatpack.h +++ b/Marlin/src/feature/meatpack.h @@ -130,10 +130,11 @@ struct MeatpackSerial : public SerialBase > { char serialBuffer[2]; uint8_t charCount; + uint8_t readIndex; NO_INLINE size_t write(uint8_t c) { return out.write(c); } void flush() { out.flush(); } - void begin(long br) { out.begin(br); } + void begin(long br) { out.begin(br); readIndex = 0; } void end() { out.end(); } void msgDone() { out.msgDone(); } @@ -154,16 +155,17 @@ struct MeatpackSerial : public SerialBase > { meatpack.handle_rx_char((uint8_t)r, index); charCount = meatpack.get_result_char(serialBuffer); + readIndex = 0; if (charCount) return charCount; } return 0; } int readImpl(uint8_t index) { - // DITTO - if (charCount > 0) return serialBuffer[sizeof(serialBuffer) - (charCount--)]; // Not enough char to make progress ? - if (available(index) == 0) return -1; - return serialBuffer[sizeof(serialBuffer) - (charCount--)]; + if (charCount == 0 && available(index) == 0) return -1; + + charCount--; + return serialBuffer[readIndex++]; } int read(uint8_t index) { return readImpl(index); } From 5d21d9235299f09a78e2191f76806b0e35a544c4 Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Thu, 25 Feb 2021 10:40:37 +0100 Subject: [PATCH 18/42] Conflict resolution error --- Marlin/src/gcode/queue.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Marlin/src/gcode/queue.cpp b/Marlin/src/gcode/queue.cpp index 017783ecc77a..32da60b0b7bf 100644 --- a/Marlin/src/gcode/queue.cpp +++ b/Marlin/src/gcode/queue.cpp @@ -292,6 +292,7 @@ inline bool serial_data_available(uint8_t index = SERIAL_ALL) { SERIAL_ERROR_START(); SERIAL_ECHOLNPAIR("RX BUF overflow, increase RX_BUFFER_SIZE: ", a); } + #endif if (a > 0) return true; } return false; From 908e746ddd9a85021b785bfd72dfd657e58a4884 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 25 Feb 2021 04:56:20 -0600 Subject: [PATCH 19/42] Update Configuration_adv.h --- Marlin/Configuration_adv.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Marlin/Configuration_adv.h b/Marlin/Configuration_adv.h index f63c1b30eb7e..5f99ee2a67a0 100644 --- a/Marlin/Configuration_adv.h +++ b/Marlin/Configuration_adv.h @@ -2020,12 +2020,11 @@ #endif // Monitor RX buffer usage -// This will dump an error to the serial port in case of overflow of the serial receive buffer -// If it happens, increase the RX_BUFFER_SIZE value -// Not supported on all platform. +// Dump an error to the serial port if the serial receive buffer overflows. +// If you see these errors, increase the RX_BUFFER_SIZE value. +// Not supported on all platforms. #define RX_BUFFER_MONITOR - /** * Emergency Command Parser * From d87cea909081c0b0d967ad6472ba8e8c7d376bb5 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 25 Feb 2021 04:56:35 -0600 Subject: [PATCH 20/42] Update Configuration_adv.h --- Marlin/Configuration_adv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Marlin/Configuration_adv.h b/Marlin/Configuration_adv.h index 5f99ee2a67a0..1da31c6ed1e4 100644 --- a/Marlin/Configuration_adv.h +++ b/Marlin/Configuration_adv.h @@ -2023,7 +2023,7 @@ // Dump an error to the serial port if the serial receive buffer overflows. // If you see these errors, increase the RX_BUFFER_SIZE value. // Not supported on all platforms. -#define RX_BUFFER_MONITOR +//#define RX_BUFFER_MONITOR /** * Emergency Command Parser From e5c38a81b20a597b93579d6ebcd8d078b1f6222a Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 25 Feb 2021 05:00:07 -0600 Subject: [PATCH 21/42] Update macros.h --- Marlin/src/core/macros.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Marlin/src/core/macros.h b/Marlin/src/core/macros.h index c65aebdcdf85..33bcac029058 100644 --- a/Marlin/src/core/macros.h +++ b/Marlin/src/core/macros.h @@ -354,22 +354,22 @@ namespace CompileTimeString { // Simple compile-time parser to find the position of the end of a string constexpr const char* findStringEnd(const char *str) { - return *str ? findStringEnd(str + 1) : str; + return *str ? findStringEnd(str + 1) : str; } // Check whether a string contains a slash constexpr bool containsSlash(const char *str) { - return *str == '/' ? true : (*str ? containsSlash(str + 1) : false); + return *str == '/' ? true : (*str ? containsSlash(str + 1) : false); } // Find the last position of the slash constexpr const char* findLastSlashPos(const char* str) { - return *str == '/' ? (str + 1) : findLastSlashPos(str - 1); + return *str == '/' ? (str + 1) : findLastSlashPos(str - 1); } - // Compile time evaluation of the last part of a file path + // Compile-time evaluation of the last part of a file path // Typically used to shorten the path to file in compiled strings // CompileTimeString::fileName(__FILE__) returns "macros.h" and not /path/to/Marlin/src/core/macros.h constexpr const char* fileName(const char* str) { - return containsSlash(str) ? findLastSlashPos(findStringEnd(str)) : str; + return containsSlash(str) ? findLastSlashPos(findStringEnd(str)) : str; } } From bcd739e7c29f1263366edb5a9e304c7688a385ed Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 25 Feb 2021 05:00:31 -0600 Subject: [PATCH 22/42] Update serial_hook.h --- Marlin/src/core/serial_hook.h | 1 - 1 file changed, 1 deletion(-) diff --git a/Marlin/src/core/serial_hook.h b/Marlin/src/core/serial_hook.h index f25465596461..afd43892c720 100644 --- a/Marlin/src/core/serial_hook.h +++ b/Marlin/src/core/serial_hook.h @@ -244,4 +244,3 @@ struct MultiSerial : public SerialBase< MultiSerial Date: Thu, 25 Feb 2021 05:02:17 -0600 Subject: [PATCH 23/42] Update meatpack.cpp --- Marlin/src/feature/meatpack.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Marlin/src/feature/meatpack.cpp b/Marlin/src/feature/meatpack.cpp index 9d5fadf1a1fb..cb3979ea0acd 100644 --- a/Marlin/src/feature/meatpack.cpp +++ b/Marlin/src/feature/meatpack.cpp @@ -147,9 +147,7 @@ void MeatPack::handle_output_char(const uint8_t c) { #if ENABLED(MP_DEBUG) if (chars_decoded < 1024) { ++chars_decoded; - DEBUG_ECHOPGM("RB: "); - DEBUG_ECHO((char)c); - DEBUG_EOL(); + DEBUG_ECHOLNPAIR("RB: ", AS_CHAR(c)); } #endif } From a381ea6eeb3bc5b902b3ecb4e4ce4a4fb7d9debc Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 25 Feb 2021 05:08:27 -0600 Subject: [PATCH 24/42] Update macros.h --- Marlin/src/core/macros.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Marlin/src/core/macros.h b/Marlin/src/core/macros.h index 33bcac029058..3c4e660efcd0 100644 --- a/Marlin/src/core/macros.h +++ b/Marlin/src/core/macros.h @@ -349,8 +349,7 @@ #define CALL_IF_EXISTS(Return, That, Method, ...) \ static_cast(Private::Call_ ## Method(That, ##__VA_ARGS__)) - - // Compile time string manipulation + // Compile-time string manipulation namespace CompileTimeString { // Simple compile-time parser to find the position of the end of a string constexpr const char* findStringEnd(const char *str) { From 0ec9e45c4ab47a8ec8918363d5b23472a02a8d76 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 25 Feb 2021 05:25:34 -0600 Subject: [PATCH 25/42] equivalent but less --- Marlin/src/feature/meatpack.h | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/Marlin/src/feature/meatpack.h b/Marlin/src/feature/meatpack.h index d358fb0a2894..e439913da6e3 100644 --- a/Marlin/src/feature/meatpack.h +++ b/Marlin/src/feature/meatpack.h @@ -146,31 +146,30 @@ struct MeatpackSerial : public SerialBase > { // There is a potential issue here with multiserial, since it'll return its decoded buffer whatever the serial index here. // So, instead of doing MeatpackSerial> we should do MultiSerial, MeatpackSerial<...>> // TODO, let's fix this later on - if (charCount > 0) return charCount; - - // Don't read in read method, instead do it here, so we can make progress in the read method - while (out.available(index) > 0) { - int r = out.read(index); - if (r == -1) return 0; // This is an error from the underlying serial code - - meatpack.handle_rx_char((uint8_t)r, index); - charCount = meatpack.get_result_char(serialBuffer); - readIndex = 0; - if (charCount) return charCount; + if (!charCount) { + // Don't read in read method, instead do it here, so we can make progress in the read method + while (out.available(index) > 0) { + const int r = out.read(index); + if (r == -1) return 0; // This is an error from the underlying serial code + meatpack.handle_rx_char((uint8_t)r, index); + charCount = meatpack.get_result_char(serialBuffer); + readIndex = 0; + } } - return 0; + return charCount; } + int readImpl(uint8_t index) { - // Not enough char to make progress ? + // Not enough char to make progress? if (charCount == 0 && available(index) == 0) return -1; charCount--; return serialBuffer[readIndex++]; } - int read(uint8_t index) { return readImpl(index); } - int available() { return available(0); } - int read() { return readImpl(0); } + int read(uint8_t index) { return readImpl(index); } + int available() { return available(0); } + int read() { return readImpl(0); } MeatpackSerial(const bool e, SerialT & out) : BaseClassT(e), out(out) {} }; From f9104acc4706565f1dcd956a2b2ad3d6e0ce1f9e Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 25 Feb 2021 05:39:08 -0600 Subject: [PATCH 26/42] reduce --- Marlin/src/gcode/gcode.cpp | 15 ++++----------- Marlin/src/gcode/host/M118.cpp | 4 +--- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/Marlin/src/gcode/gcode.cpp b/Marlin/src/gcode/gcode.cpp index 07207e02d27c..f6777d651489 100644 --- a/Marlin/src/gcode/gcode.cpp +++ b/Marlin/src/gcode/gcode.cpp @@ -994,23 +994,16 @@ void GcodeSuite::process_parsed_command(const bool no_ok/*=false*/) { void GcodeSuite::process_next_command() { GCodeQueue::CommandQueue & command = queue.peek_next_command(); - #if HAS_MULTI_SERIAL - PORT_REDIRECT(SERIAL_PORTMASK(command.port)); - #endif + PORT_REDIRECT(SERIAL_PORTMASK(command.port)); - #if ENABLED(POWER_LOSS_RECOVERY) - recovery.queue_index_r = queue.ring_buffer.index_r; - #endif + TERN_(POWER_LOSS_RECOVERY, recovery.queue_index_r = queue.ring_buffer.index_r); if (DEBUGGING(ECHO)) { SERIAL_ECHO_START(); SERIAL_ECHOLN(command.buffer); #if ENABLED(M100_FREE_MEMORY_DUMPER) - SERIAL_ECHOPAIR("slot:", queue.ring_buffer.index_r); - serialprintPGM(PSTR(" Command Queue:")); - SERIAL_EOL(); - for (GCodeQueue::CommandQueue * start = &queue.ring_buffer.commands[0]; start < &queue.ring_buffer.commands[BUFSIZE]; start++) - SERIAL_ECHOLN(start->buffer); + SERIAL_ECHOLNPAIR("slot:", queue.ring_buffer.index_r, " Command Queue:"); + LOOP_L_N(i, BUFSIZE) SERIAL_ECHOLN(queue.ring_buffer.commands[i].buffer); #endif } diff --git a/Marlin/src/gcode/host/M118.cpp b/Marlin/src/gcode/host/M118.cpp index ef3c29374272..9982492f9348 100644 --- a/Marlin/src/gcode/host/M118.cpp +++ b/Marlin/src/gcode/host/M118.cpp @@ -52,9 +52,7 @@ void GcodeSuite::M118() { while (*p == ' ') ++p; } - #if HAS_MULTI_SERIAL - PORT_REDIRECT(WITHIN(port, 0, NUM_SERIAL) ? (port ? _BV(port - 1) : SERIAL_ALL) : multiSerial.portMask); - #endif + PORT_REDIRECT(WITHIN(port, 0, NUM_SERIAL) ? (port ? SERIAL_PORTMASK(port - 1) : SERIAL_ALL) : multiSerial.portMask); if (hasE) SERIAL_ECHO_START(); if (hasA) SERIAL_ECHOPGM("//"); From c9cadefcc187006af6fe9c73cf33b664e74341bf Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 25 Feb 2021 05:53:56 -0600 Subject: [PATCH 27/42] fix SERIAL_ECHOLNPGM_P --- Marlin/src/core/serial.h | 2 +- Marlin/src/gcode/calibrate/M100.cpp | 60 ++++++++++++++--------------- Marlin/src/gcode/gcode.cpp | 2 +- Marlin/src/gcode/queue.cpp | 4 +- 4 files changed, 32 insertions(+), 36 deletions(-) diff --git a/Marlin/src/core/serial.h b/Marlin/src/core/serial.h index 60f873d473e5..ec955a8deab9 100644 --- a/Marlin/src/core/serial.h +++ b/Marlin/src/core/serial.h @@ -305,7 +305,7 @@ void serialprintPGM(PGM_P str); #endif #define SERIAL_ECHOPGM_P(P) (serialprintPGM(P)) -#define SERIAL_ECHOLNPGM_P(P) (serialprintPGM(P "\n")) +#define SERIAL_ECHOLNPGM_P(P) do{ serialprintPGM(P); SERIAL_EOL(); }while(0) #define SERIAL_ECHOPGM(S) (serialprintPGM(PSTR(S))) #define SERIAL_ECHOLNPGM(S) (serialprintPGM(PSTR(S "\n"))) diff --git a/Marlin/src/gcode/calibrate/M100.cpp b/Marlin/src/gcode/calibrate/M100.cpp index 9ac2380e7964..2e7f5d3aa177 100644 --- a/Marlin/src/gcode/calibrate/M100.cpp +++ b/Marlin/src/gcode/calibrate/M100.cpp @@ -183,8 +183,7 @@ inline int32_t count_test_bytes(const char * const start_free_memory) { } void M100_dump_routine(PGM_P const title, const char * const start, const char * const end) { - serialprintPGM(title); - SERIAL_EOL(); + SERIAL_ECHOLNPGM_P(title); // // Round the start and end locations to produce full lines of output // @@ -197,24 +196,24 @@ inline int32_t count_test_bytes(const char * const start_free_memory) { #endif // M100_FREE_MEMORY_DUMPER inline int check_for_free_memory_corruption(PGM_P const title) { - serialprintPGM(title); + SERIAL_ECHOPGM_P(title); char *start_free_memory = free_memory_start, *end_free_memory = free_memory_end; int n = end_free_memory - start_free_memory; - SERIAL_ECHOPAIR("\nfmc() n=", n); - SERIAL_ECHOPAIR("\nfree_memory_start=", hex_address(free_memory_start)); - SERIAL_ECHOLNPAIR(" end_free_memory=", hex_address(end_free_memory)); + SERIAL_ECHOLNPAIR("\nfmc() n=", n, + "\nfree_memory_start=", hex_address(free_memory_start), + " end=", hex_address(end_free_memory)); if (end_free_memory < start_free_memory) { SERIAL_ECHOPGM(" end_free_memory < Heap "); - // SET_INPUT_PULLUP(63); // if the developer has a switch wired up to their controller board - // safe_delay(5); // this code can be enabled to pause the display as soon as the - // while ( READ(63)) // malfunction is detected. It is currently defaulting to a switch - // idle(); // being on pin-63 which is unassigend and available on most controller - // safe_delay(20); // boards. - // while ( !READ(63)) - // idle(); + //SET_INPUT_PULLUP(63); // if the developer has a switch wired up to their controller board + //safe_delay(5); // this code can be enabled to pause the display as soon as the + //while ( READ(63)) // malfunction is detected. It is currently defaulting to a switch + // idle(); // being on pin-63 which is unassigend and available on most controller + //safe_delay(20); // boards. + //while ( !READ(63)) + // idle(); serial_delay(20); #if ENABLED(M100_FREE_MEMORY_DUMPER) M100_dump_routine(PSTR(" Memory corruption detected with end_free_memory 8) { - SERIAL_ECHOPAIR("Found ", j); - SERIAL_ECHOLNPAIR(" bytes free at ", hex_address(addr)); + SERIAL_ECHOLNPAIR("Found ", j, " bytes free at ", hex_address(addr)); if (j > max_cnt) { max_cnt = j; max_addr = addr; @@ -280,11 +278,11 @@ inline void free_memory_pool_report(char * const start_free_memory, const int32_ } } } - if (block_cnt > 1) { - SERIAL_ECHOLNPGM("\nMemory Corruption detected in free memory area."); - SERIAL_ECHOPAIR("\nLargest free block is ", max_cnt); - SERIAL_ECHOLNPAIR(" bytes at ", hex_address(max_addr)); - } + if (block_cnt > 1) SERIAL_ECHOLNPAIR( + "\nMemory Corruption detected in free memory area." + "\nLargest free block is ", max_cnt + " bytes at ", hex_address(max_addr) + ); SERIAL_ECHOLNPAIR("check_for_free_memory_corruption() = ", check_for_free_memory_corruption(PSTR("M100 F "))); } @@ -299,7 +297,7 @@ inline void free_memory_pool_report(char * const start_free_memory, const int32_ const uint32_t near_top = top_of_stack() - start_free_memory - 250, // -250 to avoid interrupt activity that's altered the stack. j = near_top / (size + 1); - SERIAL_ECHOLNPGM("Corrupting free memory block.\n"); + SERIAL_ECHOLNPGM("Corrupting free memory block."); for (uint32_t i = 1; i <= size; i++) { char * const addr = start_free_memory + i * j; *addr = i; @@ -322,8 +320,8 @@ inline void init_free_memory(char *start_free_memory, int32_t size) { return; } - start_free_memory += 8; // move a few bytes away from the heap just because we don't want - // to be altering memory that close to it. + start_free_memory += 8; // move a few bytes away from the heap just because we + // don't want to be altering memory that close to it. memset(start_free_memory, TEST_BYTE, size); SERIAL_ECHO(size); @@ -342,16 +340,16 @@ inline void init_free_memory(char *start_free_memory, int32_t size) { * M100: Free Memory Check */ void GcodeSuite::M100() { - char *sp = top_of_stack(); if (!free_memory_end) free_memory_end = sp - MEMORY_END_CORRECTION; - SERIAL_ECHOPAIR("\nbss_end : ", hex_address(end_bss)); - if (heaplimit) SERIAL_ECHOPAIR("\n__heaplimit : ", hex_address(heaplimit)); - SERIAL_ECHOPAIR("\nfree_memory_start : ", hex_address(free_memory_start)); + SERIAL_ECHOPAIR("\nbss_end : ", hex_address(end_bss)); + if (heaplimit) SERIAL_ECHOPAIR("\n__heaplimit : ", hex_address(heaplimit)); + SERIAL_ECHOPAIR("\nfree_memory_start : ", hex_address(free_memory_start)); if (stacklimit) SERIAL_ECHOPAIR("\n__stacklimit : ", hex_address(stacklimit)); - SERIAL_ECHOPAIR("\nfree_memory_end : ", hex_address(free_memory_end)); - if (MEMORY_END_CORRECTION) SERIAL_ECHOPAIR("\nMEMORY_END_CORRECTION: ", MEMORY_END_CORRECTION); - SERIAL_ECHOLNPAIR("\nStack Pointer : ", hex_address(sp)); + SERIAL_ECHOPAIR("\nfree_memory_end : ", hex_address(free_memory_end)); + if (MEMORY_END_CORRECTION) + SERIAL_ECHOPAIR("\nMEMORY_END_CORRECTION : ", MEMORY_END_CORRECTION); + SERIAL_ECHOLNPAIR("\nStack Pointer : ", hex_address(sp)); // Always init on the first invocation of M100 static bool m100_not_initialized = true; @@ -369,10 +367,8 @@ void GcodeSuite::M100() { return free_memory_pool_report(free_memory_start, free_memory_end - free_memory_start); #if ENABLED(M100_FREE_MEMORY_CORRUPTOR) - if (parser.seen('C')) return corrupt_free_memory(free_memory_start, parser.value_int()); - #endif } diff --git a/Marlin/src/gcode/gcode.cpp b/Marlin/src/gcode/gcode.cpp index f6777d651489..926a1652a381 100644 --- a/Marlin/src/gcode/gcode.cpp +++ b/Marlin/src/gcode/gcode.cpp @@ -992,7 +992,7 @@ void GcodeSuite::process_parsed_command(const bool no_ok/*=false*/) { * This is called from the main loop() */ void GcodeSuite::process_next_command() { - GCodeQueue::CommandQueue & command = queue.peek_next_command(); + GCodeQueue::CommandQueue &command = queue.peek_next_command(); PORT_REDIRECT(SERIAL_PORTMASK(command.port)); diff --git a/Marlin/src/gcode/queue.cpp b/Marlin/src/gcode/queue.cpp index 32da60b0b7bf..dd440bc8f248 100644 --- a/Marlin/src/gcode/queue.cpp +++ b/Marlin/src/gcode/queue.cpp @@ -245,7 +245,7 @@ void GCodeQueue::RingBuffer::ok_to_send() { // Start counting from the last command's execution last_command_time = millis(); #endif - CommandQueue & command = commands[index_r]; + CommandQueue &command = commands[index_r]; #if HAS_MULTI_SERIAL const serial_index_t serial_ind = command.port; if (serial_ind < 0) return; @@ -581,7 +581,7 @@ void GCodeQueue::get_serial_commands() { const bool card_eof = card.eof(); if (n < 0 && !card_eof) { SERIAL_ERROR_MSG(STR_SD_ERR_READ); continue; } - CommandQueue & command = ring_buffer.commands[ring_buffer.index_w]; + CommandQueue &command = ring_buffer.commands[ring_buffer.index_w]; const char sd_char = (char)n; const bool is_eol = ISEOL(sd_char); if (is_eol || card_eof) { From e069722ba8688daae4f7d2bb62bc22b3dc70632a Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 25 Feb 2021 05:56:32 -0600 Subject: [PATCH 28/42] oops --- Marlin/src/gcode/calibrate/M100.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Marlin/src/gcode/calibrate/M100.cpp b/Marlin/src/gcode/calibrate/M100.cpp index 2e7f5d3aa177..9ea87806aece 100644 --- a/Marlin/src/gcode/calibrate/M100.cpp +++ b/Marlin/src/gcode/calibrate/M100.cpp @@ -280,8 +280,7 @@ inline void free_memory_pool_report(char * const start_free_memory, const int32_ } if (block_cnt > 1) SERIAL_ECHOLNPAIR( "\nMemory Corruption detected in free memory area." - "\nLargest free block is ", max_cnt - " bytes at ", hex_address(max_addr) + "\nLargest free block is ", max_cnt, " bytes at ", hex_address(max_addr) ); SERIAL_ECHOLNPAIR("check_for_free_memory_corruption() = ", check_for_free_memory_corruption(PSTR("M100 F "))); } From 052bc2451bff7f0312f2115a788b094b176a684b Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 25 Feb 2021 06:24:28 -0600 Subject: [PATCH 29/42] various style tweaks --- Marlin/src/gcode/gcode.cpp | 2 +- Marlin/src/gcode/queue.cpp | 32 ++++++------- Marlin/src/gcode/queue.h | 96 +++++++++++--------------------------- 3 files changed, 41 insertions(+), 89 deletions(-) diff --git a/Marlin/src/gcode/gcode.cpp b/Marlin/src/gcode/gcode.cpp index 926a1652a381..4a7820128a02 100644 --- a/Marlin/src/gcode/gcode.cpp +++ b/Marlin/src/gcode/gcode.cpp @@ -992,7 +992,7 @@ void GcodeSuite::process_parsed_command(const bool no_ok/*=false*/) { * This is called from the main loop() */ void GcodeSuite::process_next_command() { - GCodeQueue::CommandQueue &command = queue.peek_next_command(); + GCodeQueue::CommandLine &command = queue.peek_next_command(); PORT_REDIRECT(SERIAL_PORTMASK(command.port)); diff --git a/Marlin/src/gcode/queue.cpp b/Marlin/src/gcode/queue.cpp index dd440bc8f248..5a98ffab74b5 100644 --- a/Marlin/src/gcode/queue.cpp +++ b/Marlin/src/gcode/queue.cpp @@ -65,7 +65,7 @@ PGMSTR(G28_STR, "G28"); #endif -GCodeQueue::PerSerial GCodeQueue::per_serial[NUM_SERIAL] = { 0 }; +GCodeQueue::SerialState GCodeQueue::serial_state[NUM_SERIAL] = { 0 }; GCodeQueue::RingBuffer GCodeQueue::ring_buffer = { 0 }; #if NO_TIMEOUTS > 0 @@ -245,7 +245,7 @@ void GCodeQueue::RingBuffer::ok_to_send() { // Start counting from the last command's execution last_command_time = millis(); #endif - CommandQueue &command = commands[index_r]; + CommandLine &command = commands[index_r]; #if HAS_MULTI_SERIAL const serial_index_t serial_ind = command.port; if (serial_ind < 0) return; @@ -278,7 +278,7 @@ void GCodeQueue::flush_and_request_resend() { #endif SERIAL_FLUSH(); SERIAL_ECHOPGM(STR_RESEND); - SERIAL_ECHOLN(per_serial[serial_ind].last_N + 1); + SERIAL_ECHOLN(serial_state[serial_ind].last_N + 1); } // Multiserial already handle the dispatch to/from multiple port by itself @@ -289,8 +289,7 @@ inline bool serial_data_available(uint8_t index = SERIAL_ALL) { #if BOTH(RX_BUFFER_MONITOR, RX_BUFFER_SIZE) if (a > RX_BUFFER_SIZE - 2) { PORT_REDIRECT(SERIAL_PORTMASK(index)); - SERIAL_ERROR_START(); - SERIAL_ECHOLNPAIR("RX BUF overflow, increase RX_BUFFER_SIZE: ", a); + SERIAL_ERROR_MSG("RX BUF overflow, increase RX_BUFFER_SIZE: ", a); } #endif if (a > 0) return true; @@ -301,8 +300,7 @@ inline bool serial_data_available(uint8_t index = SERIAL_ALL) { #if BOTH(RX_BUFFER_MONITOR, RX_BUFFER_SIZE) if (a > RX_BUFFER_SIZE - 2) { PORT_REDIRECT(SERIAL_PORTMASK(index)); - SERIAL_ERROR_START(); - SERIAL_ECHOLNPAIR("RX BUF overflow, increase RX_BUFFER_SIZE: ", a); + SERIAL_ERROR_MSG("RX BUF overflow, increase RX_BUFFER_SIZE: ", a); } #endif @@ -314,11 +312,11 @@ inline int read_serial(const uint8_t index) { return SERIAL_IMPL.read(index); } void GCodeQueue::gcode_line_error(PGM_P const err, const serial_index_t serial_ind) { PORT_REDIRECT(SERIAL_PORTMASK(serial_ind)); // Reply to the serial port that sent the command SERIAL_ERROR_START(); - serialprintPGM(err); - SERIAL_ECHOLN(per_serial[serial_ind].last_N); + SERIAL_ECHOPGM_P(err); + SERIAL_ECHOLN(serial_state[serial_ind].last_N); while (read_serial(serial_ind) != -1); // Clear out the RX buffer. Why don't use flush here ? flush_and_request_resend(); - per_serial[serial_ind].count = 0; + serial_state[serial_ind].count = 0; } FORCE_INLINE bool is_M29(const char * const cmd) { // matches "M29" & "M29 ", but not "M290", etc @@ -412,7 +410,7 @@ void GCodeQueue::get_serial_commands() { * receive buffer (which limits the packet size to MAX_CMD_SIZE). * The receive buffer also limits the packet size for reliable transmission. */ - binaryStream[card.transfer_port_index].receive(per_serial[card.transfer_port_index].line_buffer); + binaryStream[card.transfer_port_index].receive(serial_state[card.transfer_port_index].line_buffer); return; } #endif @@ -428,12 +426,12 @@ void GCodeQueue::get_serial_commands() { #endif // Loop while serial characters are incoming and the queue is not full - for (bool hadData = true; hadData; ) { + for (bool hadData = true; hadData;) { // Unless a serial port has data, this will exit on next iteration hadData = false; LOOP_L_N(p, NUM_SERIAL) { - // Check if the queue is full, and exits if it is. + // Check if the queue is full and exit if it is. if (ring_buffer.full()) return; // No data for this port ? Skip it @@ -448,14 +446,13 @@ void GCodeQueue::get_serial_commands() { PORT_REDIRECT(SERIAL_PORTMASK(p)); // Reply to the serial port that sent the command // Crash here to get more information why it failed BUG_ON("SP available but read -1"); - SERIAL_ERROR_START(); - SERIAL_ECHOLNPGM(STR_ERR_SERIAL_MISMATCH); + SERIAL_ERROR_MSG(STR_ERR_SERIAL_MISMATCH); SERIAL_FLUSH(); continue; } const char serial_char = (char)c; - PerSerial & serial = per_serial[p]; + SerialState &serial = serial_state[p]; if (ISEOL(serial_char)) { @@ -494,7 +491,6 @@ void GCodeQueue::get_serial_commands() { gcode_line_error(PSTR(STR_ERR_CHECKSUM_MISMATCH), p); break; } - } else { // In case of error on a serial port, don't prevent other serial port from making progress @@ -581,7 +577,7 @@ void GCodeQueue::get_serial_commands() { const bool card_eof = card.eof(); if (n < 0 && !card_eof) { SERIAL_ERROR_MSG(STR_SD_ERR_READ); continue; } - CommandQueue &command = ring_buffer.commands[ring_buffer.index_w]; + CommandLine &command = ring_buffer.commands[ring_buffer.index_w]; const char sd_char = (char)n; const bool is_eol = ISEOL(sd_char); if (is_eol || card_eof) { diff --git a/Marlin/src/gcode/queue.h b/Marlin/src/gcode/queue.h index 8da5d83edbb6..196fdaef850e 100644 --- a/Marlin/src/gcode/queue.h +++ b/Marlin/src/gcode/queue.h @@ -33,34 +33,19 @@ class GCodeQueue { /** * The buffers per serial port. */ - struct PerSerial - { + struct SerialState { /** * GCode line number handling. Hosts may include line numbers when sending * commands to Marlin, and lines will be checked for sequentiality. * M110 N sets the current line number. */ long last_N; - /** - * Number of characters read in the current line of serial input - */ - int count; - - /** - * The current line accumulator - */ - char line_buffer[MAX_CMD_SIZE]; - - /** - * The input state - */ - uint8_t input_state; + int count; //!< Number of characters read in the current line of serial input + char line_buffer[MAX_CMD_SIZE]; //!< The current line accumulator + uint8_t input_state; //!< The input state }; - /** - * The array of per serial state variable - */ - static PerSerial per_serial[NUM_SERIAL]; + static SerialState serial_state[NUM_SERIAL]; //!< Serial states for each serial port /** * GCode Command Queue @@ -71,71 +56,42 @@ class GCodeQueue { * the main loop. The gcode.process_next_command method parses the next * command and hands off execution to individual handler functions. */ - struct CommandQueue - { - /** - * The command buffer - */ - char buffer[MAX_CMD_SIZE]; - - /** - * Skip sending ok when command is processed ? - * The logic is inverted here to skip setting all the variables to true upon construction - */ - bool skip_ok; - - #if HAS_MULTI_SERIAL - /** - * The port that the command was received on - */ - serial_index_t port; - #endif + struct CommandLine { + char buffer[MAX_CMD_SIZE]; //!< The command buffer + bool skip_ok; //!< Skip sending ok when command is processed? + TERN_(HAS_MULTI_SERIAL, serial_index_t port); //!< Serial port the command was received on }; - /** - * The ring buffer head + * A handy ring buffer type */ - struct RingBuffer - { - /** - * Number of commands in the queue - */ - uint8_t length; - /** - * Ring buffer's read position - */ - uint8_t index_r; - /** - * Ring buffer's write position - */ - uint8_t index_w; - /** - * The ring buffer of commands - */ - CommandQueue commands[BUFSIZE]; + struct RingBuffer { + uint8_t length; //!< Number of commands in the queue + uint8_t index_r; //!< Ring buffer's read position + uint8_t index_w; //!< Ring buffer's write position + CommandLine commands[BUFSIZE]; //!< The ring buffer of commands - inline serial_index_t command_port() { return TERN0(HAS_MULTI_SERIAL, commands[index_r].port); } + inline serial_index_t command_port() const { return TERN0(HAS_MULTI_SERIAL, commands[index_r].port); } inline void clear() { length = index_r = index_w = 0; } - void advance_pos(uint8_t & p, int inc) { if (++p >= BUFSIZE) p = 0; length += inc; } + void advance_pos(uint8_t &p, const int inc) { if (++p >= BUFSIZE) p = 0; length += inc; } void commit_command(bool skip_ok - #if HAS_MULTI_SERIAL - , serial_index_t serial_ind = -1 - #endif + #if HAS_MULTI_SERIAL + , serial_index_t serial_ind=-1 + #endif ); bool enqueue(const char* cmd, bool skip_ok = true - #if HAS_MULTI_SERIAL - , serial_index_t serial_ind = -1 - #endif + #if HAS_MULTI_SERIAL + , serial_index_t serial_ind=-1 + #endif ); void ok_to_send(); - inline bool full(uint8_t cmdCount = 1) const { return length > (BUFSIZE - cmdCount); } + inline bool full(uint8_t cmdCount=1) const { return length > (BUFSIZE - cmdCount); } inline bool empty() const { return length == 0; } }; @@ -230,12 +186,12 @@ class GCodeQueue { /** * (Re)Set the current line number for the last received command */ - static inline void set_current_line_number(long n) { per_serial[ring_buffer.command_port()].last_N = n; } + static inline void set_current_line_number(long n) { serial_state[ring_buffer.command_port()].last_N = n; } private: /** Get the next command from the buffer (or NULL) */ - CommandQueue & peek_next_command() { return ring_buffer.commands[ring_buffer.index_r]; } + CommandLine & peek_next_command() { return ring_buffer.commands[ring_buffer.index_r]; } static void get_serial_commands(); From 1a12202603a72694da2b94805eff1c3b1d9422f8 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 25 Feb 2021 07:07:18 -0600 Subject: [PATCH 30/42] misc --- Marlin/src/gcode/gcode.cpp | 2 +- Marlin/src/gcode/queue.cpp | 20 ++++++++------------ Marlin/src/gcode/queue.h | 9 +++++---- 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/Marlin/src/gcode/gcode.cpp b/Marlin/src/gcode/gcode.cpp index 4a7820128a02..7f7eebed28b9 100644 --- a/Marlin/src/gcode/gcode.cpp +++ b/Marlin/src/gcode/gcode.cpp @@ -992,7 +992,7 @@ void GcodeSuite::process_parsed_command(const bool no_ok/*=false*/) { * This is called from the main loop() */ void GcodeSuite::process_next_command() { - GCodeQueue::CommandLine &command = queue.peek_next_command(); + GCodeQueue::CommandLine &command = queue.ring_buffer.peek_next_command(); PORT_REDIRECT(SERIAL_PORTMASK(command.port)); diff --git a/Marlin/src/gcode/queue.cpp b/Marlin/src/gcode/queue.cpp index 5a98ffab74b5..f28c0586d9a1 100644 --- a/Marlin/src/gcode/queue.cpp +++ b/Marlin/src/gcode/queue.cpp @@ -64,7 +64,6 @@ PGMSTR(G28_STR, "G28"); static millis_t last_command_time = 0; #endif - GCodeQueue::SerialState GCodeQueue::serial_state[NUM_SERIAL] = { 0 }; GCodeQueue::RingBuffer GCodeQueue::ring_buffer = { 0 }; @@ -92,7 +91,7 @@ void GCodeQueue::RingBuffer::commit_command(bool skip_ok #if HAS_MULTI_SERIAL , serial_index_t serial_ind/*=-1*/ #endif - ) { +) { commands[index_w].skip_ok = skip_ok; TERN_(HAS_MULTI_SERIAL, commands[index_w].port = serial_ind); TERN_(POWER_LOSS_RECOVERY, recovery.commit_sdpos(index_w)); @@ -124,10 +123,7 @@ bool GCodeQueue::RingBuffer::enqueue(const char* cmd, bool skip_ok/*=true*/ * Return true if the command was consumed */ bool GCodeQueue::enqueue_one(const char* cmd) { - - //SERIAL_ECHOPGM("enqueue_one(\""); - //SERIAL_ECHO(cmd); - //SERIAL_ECHOPGM("\") \n"); + //SERIAL_ECHOLNPAIR("enqueue_one(\"", cmd, "\")"); if (*cmd == 0 || ISEOL(*cmd)) return true; @@ -285,7 +281,7 @@ void GCodeQueue::flush_and_request_resend() { inline bool serial_data_available(uint8_t index = SERIAL_ALL) { if (index == SERIAL_ALL) { for (index = 0; index < NUM_SERIAL; index++) { - int a = SERIAL_IMPL.available(index); + const int a = SERIAL_IMPL.available(index); #if BOTH(RX_BUFFER_MONITOR, RX_BUFFER_SIZE) if (a > RX_BUFFER_SIZE - 2) { PORT_REDIRECT(SERIAL_PORTMASK(index)); @@ -296,7 +292,7 @@ inline bool serial_data_available(uint8_t index = SERIAL_ALL) { } return false; } - int a = SERIAL_IMPL.available(index); + const int a = SERIAL_IMPL.available(index); #if BOTH(RX_BUFFER_MONITOR, RX_BUFFER_SIZE) if (a > RX_BUFFER_SIZE - 2) { PORT_REDIRECT(SERIAL_PORTMASK(index)); @@ -314,7 +310,7 @@ void GCodeQueue::gcode_line_error(PGM_P const err, const serial_index_t serial_i SERIAL_ERROR_START(); SERIAL_ECHOPGM_P(err); SERIAL_ECHOLN(serial_state[serial_ind].last_N); - while (read_serial(serial_ind) != -1); // Clear out the RX buffer. Why don't use flush here ? + while (read_serial(serial_ind) != -1) { /* nada */ } // Clear out the RX buffer. Why don't use flush here ? flush_and_request_resend(); serial_state[serial_ind].count = 0; } @@ -634,8 +630,8 @@ void GCodeQueue::advance() { #if ENABLED(SDSUPPORT) if (card.flag.saving) { - char* command = ring_buffer.commands[ring_buffer.index_r].buffer; - if (is_M29(command)) { + char * const cmd = ring_buffer.peek_next_command_string(); + if (is_M29(cmd)) { // M29 closes the file card.closefile(); SERIAL_ECHOLNPGM(STR_FILE_SAVED); @@ -653,7 +649,7 @@ void GCodeQueue::advance() { } else { // Write the string from the read buffer to SD - card.write_command(command); + card.write_command(cmd); if (card.flag.logging) gcode.process_next_command(); // The card is saving because it's logging else diff --git a/Marlin/src/gcode/queue.h b/Marlin/src/gcode/queue.h index 196fdaef850e..fcee71b19093 100644 --- a/Marlin/src/gcode/queue.h +++ b/Marlin/src/gcode/queue.h @@ -69,7 +69,7 @@ class GCodeQueue { uint8_t length; //!< Number of commands in the queue uint8_t index_r; //!< Ring buffer's read position uint8_t index_w; //!< Ring buffer's write position - CommandLine commands[BUFSIZE]; //!< The ring buffer of commands + CommandLine commands[BUFSIZE]; //!< The ring buffer of commands inline serial_index_t command_port() const { return TERN0(HAS_MULTI_SERIAL, commands[index_r].port); } @@ -94,6 +94,10 @@ class GCodeQueue { inline bool full(uint8_t cmdCount=1) const { return length > (BUFSIZE - cmdCount); } inline bool empty() const { return length == 0; } + + inline CommandLine& peek_next_command() { return commands[index_r]; } + + inline char* peek_next_command_string() { return peek_next_command().buffer; } }; /** @@ -190,9 +194,6 @@ class GCodeQueue { private: - /** Get the next command from the buffer (or NULL) */ - CommandLine & peek_next_command() { return ring_buffer.commands[ring_buffer.index_r]; } - static void get_serial_commands(); #if ENABLED(SDSUPPORT) From 9720d5ab645f1e18c224df2bb09b21ea554a6ff9 Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Thu, 25 Feb 2021 14:16:28 +0100 Subject: [PATCH 31/42] Restore command buffer dumping with M100 --- Marlin/src/gcode/calibrate/M100.cpp | 2 +- Marlin/src/gcode/gcode.cpp | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/Marlin/src/gcode/calibrate/M100.cpp b/Marlin/src/gcode/calibrate/M100.cpp index 9ea87806aece..68723320de02 100644 --- a/Marlin/src/gcode/calibrate/M100.cpp +++ b/Marlin/src/gcode/calibrate/M100.cpp @@ -151,7 +151,7 @@ inline int32_t count_test_bytes(const char * const start_free_memory) { * the block. If so, it may indicate memory corruption due to a bad pointer. * Unexpected bytes are flagged in the right column. */ - inline void dump_free_memory(char *start_free_memory, char *end_free_memory) { + void dump_free_memory(char *start_free_memory, char *end_free_memory) { // // Start and end the dump on a nice 16 byte boundary // (even though the values are not 16-byte aligned). diff --git a/Marlin/src/gcode/gcode.cpp b/Marlin/src/gcode/gcode.cpp index 7f7eebed28b9..fc28f3290ba9 100644 --- a/Marlin/src/gcode/gcode.cpp +++ b/Marlin/src/gcode/gcode.cpp @@ -987,6 +987,11 @@ void GcodeSuite::process_parsed_command(const bool no_ok/*=false*/) { SERIAL_OUT(msgDone); // Call the msgDone serial hook to signal command processing done } + +#if ENABLED(M100_FREE_MEMORY_DUMPER) + void dump_free_memory(char *start_free_memory, char *end_free_memory); +#endif + /** * Process a single command and dispatch it to its handler * This is called from the main loop() @@ -1003,7 +1008,10 @@ void GcodeSuite::process_next_command() { SERIAL_ECHOLN(command.buffer); #if ENABLED(M100_FREE_MEMORY_DUMPER) SERIAL_ECHOLNPAIR("slot:", queue.ring_buffer.index_r, " Command Queue:"); - LOOP_L_N(i, BUFSIZE) SERIAL_ECHOLN(queue.ring_buffer.commands[i].buffer); + + LOOP_L_N(i, BUFSIZE) { + dump_free_memory(queue.ring_buffer.commands[i].buffer, &queue.ring_buffer.commands[i].buffer[MAX_CMD_SIZE]); + } #endif } From d92c1b5c1b3231b6f12ad3a60842dfc57ecd241d Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 25 Feb 2021 07:38:31 -0600 Subject: [PATCH 32/42] revert --- Marlin/src/feature/meatpack.h | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/Marlin/src/feature/meatpack.h b/Marlin/src/feature/meatpack.h index e439913da6e3..e30a5ac979c1 100644 --- a/Marlin/src/feature/meatpack.h +++ b/Marlin/src/feature/meatpack.h @@ -146,20 +146,21 @@ struct MeatpackSerial : public SerialBase > { // There is a potential issue here with multiserial, since it'll return its decoded buffer whatever the serial index here. // So, instead of doing MeatpackSerial> we should do MultiSerial, MeatpackSerial<...>> // TODO, let's fix this later on - if (!charCount) { - // Don't read in read method, instead do it here, so we can make progress in the read method - while (out.available(index) > 0) { - const int r = out.read(index); - if (r == -1) return 0; // This is an error from the underlying serial code - meatpack.handle_rx_char((uint8_t)r, index); - charCount = meatpack.get_result_char(serialBuffer); - readIndex = 0; - } - } + + if (charCount) return charCount; // The buffer still has data + if (out.available(index) <= 0) return 0; // No data to read + + // Don't read in read method, instead do it here, so we can make progress in the read method + const int r = out.read(index); + if (r == -1) return 0; // This is an error from the underlying serial code + meatpack.handle_rx_char((uint8_t)r, index); + charCount = meatpack.get_result_char(serialBuffer); + readIndex = 0; + return charCount; } - int readImpl(uint8_t index) { + int readImpl(const uint8_t index) { // Not enough char to make progress? if (charCount == 0 && available(index) == 0) return -1; From 56bb0606229ead40a75266fc2e70429b976aa917 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 25 Feb 2021 07:46:18 -0600 Subject: [PATCH 33/42] dump the whole blob --- Marlin/src/gcode/gcode.cpp | 10 +++------- Marlin/src/gcode/queue.h | 6 +++--- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/Marlin/src/gcode/gcode.cpp b/Marlin/src/gcode/gcode.cpp index fc28f3290ba9..a0ca84d4ddb0 100644 --- a/Marlin/src/gcode/gcode.cpp +++ b/Marlin/src/gcode/gcode.cpp @@ -987,9 +987,8 @@ void GcodeSuite::process_parsed_command(const bool no_ok/*=false*/) { SERIAL_OUT(msgDone); // Call the msgDone serial hook to signal command processing done } - #if ENABLED(M100_FREE_MEMORY_DUMPER) - void dump_free_memory(char *start_free_memory, char *end_free_memory); + void M100_dump_routine(PGM_P const title, const char * const start, const char * const end); #endif /** @@ -1007,11 +1006,8 @@ void GcodeSuite::process_next_command() { SERIAL_ECHO_START(); SERIAL_ECHOLN(command.buffer); #if ENABLED(M100_FREE_MEMORY_DUMPER) - SERIAL_ECHOLNPAIR("slot:", queue.ring_buffer.index_r, " Command Queue:"); - - LOOP_L_N(i, BUFSIZE) { - dump_free_memory(queue.ring_buffer.commands[i].buffer, &queue.ring_buffer.commands[i].buffer[MAX_CMD_SIZE]); - } + SERIAL_ECHOPAIR("slot:", queue.ring_buffer.index_r); + M100_dump_routine(PSTR(" Command Queue:"), &queue.ring_buffer, &queue.ring_buffer + sizeof(queue.ring_buffer) - 1); #endif } diff --git a/Marlin/src/gcode/queue.h b/Marlin/src/gcode/queue.h index fcee71b19093..778f9a7f67d0 100644 --- a/Marlin/src/gcode/queue.h +++ b/Marlin/src/gcode/queue.h @@ -66,9 +66,9 @@ class GCodeQueue { * A handy ring buffer type */ struct RingBuffer { - uint8_t length; //!< Number of commands in the queue - uint8_t index_r; //!< Ring buffer's read position - uint8_t index_w; //!< Ring buffer's write position + uint8_t length, //!< Number of commands in the queue + index_r, //!< Ring buffer's read position + index_w; //!< Ring buffer's write position CommandLine commands[BUFSIZE]; //!< The ring buffer of commands inline serial_index_t command_port() const { return TERN0(HAS_MULTI_SERIAL, commands[index_r].port); } From d8e2342c405d6e15d2484ba12e7ee8a9b7e1356d Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Thu, 25 Feb 2021 14:49:00 +0100 Subject: [PATCH 34/42] Add documentation for the Queue design --- docs/Queue.md | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 docs/Queue.md diff --git a/docs/Queue.md b/docs/Queue.md new file mode 100644 index 000000000000..a0a8312e5328 --- /dev/null +++ b/docs/Queue.md @@ -0,0 +1,61 @@ +# Marlin's command queue concept + +Marlin is a software that process commands encoded in G-Code coming from multiple different source (serial port, SD card, WIFI connection, and other). +It's also processing this command by converting them to actions on many physical actuators (motors, resistances, lasers, RGB leds and other). + +Since physical actuators are limited by physic, a synchronization must be achieved between the source of the commands and the physical movement or action. + +The synchronization is done via a queue that's buffering/adapting the next commands in memory so that the command processor is never starved (whenever possible). + +There are multiple buffers in the system to achieve this goal: +``` ++------+ Marlin's GCodeQueue +| | +--------------------------------------+ +-----------+ +| Host | | SerialState RingBuffer | | | +| | Marlin | NUM_SERIAL BUF_SIZE | | Marlin | ++--+---+ R/TX_BUFFER_SIZE | +---+ +------------------+ | | | + | +------------+ | | | | | | | GCode | + | | | | | | | MAX_CMD_SIZE +-+-----> processor | + | | Platform | | | | On EOL | +--------------+ | r_pos | | + +-------------> serial's +-----------> +--------> | | | | +-----------+ + | buffer | | | | w_pos | | CommandQueue | | | + | | | | | | | | | | + +------------+ | +---+ | +--------------+ | | + | Line buffer | x BUF_SIZE | | + | | | | + | | | | + | | | | + | | | | + | +------------------+ | + | | + | | + | | + +--------------------------------------+ +``` + +Marlin is not a multithread or multiprocess software, so the processing loop performs the following actions sequentially: +1. Check if data is available in the platform's serial buffer and in that case, move them into the per-serial line buffer +2. If the data in the line buffer contains a End Of Line character, it's committed to the ring buffer of CommandQueue +3. Loop to 1 until there is no more data in the serial buffer +4. Run the G-Code processor by pop'ing from the CommandQueue ring buffer and execute one command +5. Loop to 1 + + +## Synchronization +To achieve synchronization, Marlin sends a `ok` answer to the host when it's done processing a command (end of G-Code processor task). +Most host will then wait until this answer is received to feed more command into the serial's buffer. + + +In some case, if no data is available in the serial buffer for a long time, Marlin can ask the host to send more data via the `wait` message. + + +## Limitation of the design + +Please notice few limitations here: +1. While the G-Code processor is busy processing a command, the G-Code queue is not running. +2. If the host is sending data to the serial buffer, then the serial buffer will fill up (by interrupting the CPU). +3. This means that the serial buffer must be able to contain at least a complete line of data (the size of the serial buffer must be greater than the maximum G-Code command line the host can generate). The default value `MAX_CMD_SIZE` is 96 bytes for Marlin's CommandQueue lines, so a serial buffer that's smaller than 96 bytes can loose data. +4. Since serial buffer size are likely used as ring buffer themselves, their size should be a power of 2 (64 or 128 bytes recommanded) +5. A host generating too many G-Code command simultaneously will saturate the GCodeQueue but will not improve the processing rate of Marlin since only one command is processed per loop iteration. +6. Because of this, having a large BUF_SIZE is likely useless. The default value of 4 is good for a single serial port. + From 49fc4e197116e1209daaea471cf166fd8e56b660 Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Thu, 25 Feb 2021 14:54:17 +0100 Subject: [PATCH 35/42] Fix CI --- Marlin/src/gcode/gcode.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Marlin/src/gcode/gcode.cpp b/Marlin/src/gcode/gcode.cpp index a0ca84d4ddb0..a8363c4c4380 100644 --- a/Marlin/src/gcode/gcode.cpp +++ b/Marlin/src/gcode/gcode.cpp @@ -1007,7 +1007,7 @@ void GcodeSuite::process_next_command() { SERIAL_ECHOLN(command.buffer); #if ENABLED(M100_FREE_MEMORY_DUMPER) SERIAL_ECHOPAIR("slot:", queue.ring_buffer.index_r); - M100_dump_routine(PSTR(" Command Queue:"), &queue.ring_buffer, &queue.ring_buffer + sizeof(queue.ring_buffer) - 1); + M100_dump_routine(PSTR(" Command Queue:"), (const char*)&queue.ring_buffer, (const char*)&queue.ring_buffer + sizeof(queue.ring_buffer) - 1); #endif } From 965230c51b91801adbb289bb2d822a29a37682af Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 25 Feb 2021 07:56:08 -0600 Subject: [PATCH 36/42] dumper size --- Marlin/src/gcode/calibrate/M100.cpp | 15 +++++++-------- Marlin/src/gcode/gcode.cpp | 4 ++-- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/Marlin/src/gcode/calibrate/M100.cpp b/Marlin/src/gcode/calibrate/M100.cpp index 68723320de02..890d38ddc804 100644 --- a/Marlin/src/gcode/calibrate/M100.cpp +++ b/Marlin/src/gcode/calibrate/M100.cpp @@ -51,7 +51,7 @@ * Also, there are two support functions that can be called from a developer's C code. * * uint16_t check_for_free_memory_corruption(PGM_P const free_memory_start); - * void M100_dump_routine(PGM_P const title, const char * const start, const char * const end); + * void M100_dump_routine(PGM_P const title, const char * const start, const uint32_t size); * * Initial version by Roxy-3D */ @@ -182,11 +182,12 @@ inline int32_t count_test_bytes(const char * const start_free_memory) { } } - void M100_dump_routine(PGM_P const title, const char * const start, const char * const end) { + void M100_dump_routine(PGM_P const title, const char * const start, const uint32_t size) { SERIAL_ECHOLNPGM_P(title); // // Round the start and end locations to produce full lines of output // + const char * const end = start + size - 1; dump_free_memory( (char*)(uintptr_t(uint32_t(start) & ~0xFUL)), // Align to 16-byte boundary (char*)(uintptr_t(uint32_t(end) | 0xFUL)) // Align end_free_memory to the 15th byte (at or above end_free_memory) @@ -216,7 +217,7 @@ inline int check_for_free_memory_corruption(PGM_P const title) { // idle(); serial_delay(20); #if ENABLED(M100_FREE_MEMORY_DUMPER) - M100_dump_routine(PSTR(" Memory corruption detected with end_free_memory 8) { - // SERIAL_ECHOPAIR("Found ", j); - // SERIAL_ECHOLNPAIR(" bytes free at ", hex_address(start_free_memory + i)); + //SERIAL_ECHOPAIR("Found ", j); + //SERIAL_ECHOLNPAIR(" bytes free at ", hex_address(start_free_memory + i)); i += j; block_cnt++; - SERIAL_ECHOPAIR(" (", block_cnt); - SERIAL_ECHOPAIR(") found=", j); - SERIAL_ECHOLNPGM(" "); + SERIAL_ECHOLNPAIR(" (", block_cnt, ") found=", j); } } } diff --git a/Marlin/src/gcode/gcode.cpp b/Marlin/src/gcode/gcode.cpp index a8363c4c4380..c7bba574213a 100644 --- a/Marlin/src/gcode/gcode.cpp +++ b/Marlin/src/gcode/gcode.cpp @@ -988,7 +988,7 @@ void GcodeSuite::process_parsed_command(const bool no_ok/*=false*/) { } #if ENABLED(M100_FREE_MEMORY_DUMPER) - void M100_dump_routine(PGM_P const title, const char * const start, const char * const end); + void M100_dump_routine(PGM_P const title, const char * const start, const uint32_t size); #endif /** @@ -1007,7 +1007,7 @@ void GcodeSuite::process_next_command() { SERIAL_ECHOLN(command.buffer); #if ENABLED(M100_FREE_MEMORY_DUMPER) SERIAL_ECHOPAIR("slot:", queue.ring_buffer.index_r); - M100_dump_routine(PSTR(" Command Queue:"), (const char*)&queue.ring_buffer, (const char*)&queue.ring_buffer + sizeof(queue.ring_buffer) - 1); + M100_dump_routine(PSTR(" Command Queue:"), &queue.ring_buffer, sizeof(queue.ring_buffer)); #endif } From bcf322dfef17b984db027c252afbe0469676af57 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 25 Feb 2021 07:58:43 -0600 Subject: [PATCH 37/42] smaller type on avr --- Marlin/src/gcode/calibrate/M100.cpp | 6 +++--- Marlin/src/gcode/gcode.cpp | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Marlin/src/gcode/calibrate/M100.cpp b/Marlin/src/gcode/calibrate/M100.cpp index 890d38ddc804..ee572e033d75 100644 --- a/Marlin/src/gcode/calibrate/M100.cpp +++ b/Marlin/src/gcode/calibrate/M100.cpp @@ -51,7 +51,7 @@ * Also, there are two support functions that can be called from a developer's C code. * * uint16_t check_for_free_memory_corruption(PGM_P const free_memory_start); - * void M100_dump_routine(PGM_P const title, const char * const start, const uint32_t size); + * void M100_dump_routine(PGM_P const title, const char * const start, const uintptr_t size); * * Initial version by Roxy-3D */ @@ -182,7 +182,7 @@ inline int32_t count_test_bytes(const char * const start_free_memory) { } } - void M100_dump_routine(PGM_P const title, const char * const start, const uint32_t size) { + void M100_dump_routine(PGM_P const title, const char * const start, const uintptr_t size) { SERIAL_ECHOLNPGM_P(title); // // Round the start and end locations to produce full lines of output @@ -290,7 +290,7 @@ inline void free_memory_pool_report(char * const start_free_memory, const int32_ * Corrupt locations in the free memory pool and report the corrupt addresses. * This is useful to check the correctness of the M100 D and the M100 F commands. */ - inline void corrupt_free_memory(char *start_free_memory, const uint32_t size) { + inline void corrupt_free_memory(char *start_free_memory, const uintptr_t size) { start_free_memory += 8; const uint32_t near_top = top_of_stack() - start_free_memory - 250, // -250 to avoid interrupt activity that's altered the stack. j = near_top / (size + 1); diff --git a/Marlin/src/gcode/gcode.cpp b/Marlin/src/gcode/gcode.cpp index c7bba574213a..008a0c50455c 100644 --- a/Marlin/src/gcode/gcode.cpp +++ b/Marlin/src/gcode/gcode.cpp @@ -988,7 +988,7 @@ void GcodeSuite::process_parsed_command(const bool no_ok/*=false*/) { } #if ENABLED(M100_FREE_MEMORY_DUMPER) - void M100_dump_routine(PGM_P const title, const char * const start, const uint32_t size); + void M100_dump_routine(PGM_P const title, const char * const start, const uintptr_t size); #endif /** From 1608c891dfadcf9be40680d1da4f8d91958cd216 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 25 Feb 2021 08:03:44 -0600 Subject: [PATCH 38/42] fix type --- Marlin/src/gcode/gcode.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Marlin/src/gcode/gcode.cpp b/Marlin/src/gcode/gcode.cpp index 008a0c50455c..d7535dd4ff6b 100644 --- a/Marlin/src/gcode/gcode.cpp +++ b/Marlin/src/gcode/gcode.cpp @@ -1007,7 +1007,7 @@ void GcodeSuite::process_next_command() { SERIAL_ECHOLN(command.buffer); #if ENABLED(M100_FREE_MEMORY_DUMPER) SERIAL_ECHOPAIR("slot:", queue.ring_buffer.index_r); - M100_dump_routine(PSTR(" Command Queue:"), &queue.ring_buffer, sizeof(queue.ring_buffer)); + M100_dump_routine(PSTR(" Command Queue:"), (const char*)&queue.ring_buffer, sizeof(queue.ring_buffer)); #endif } From 94a13f3245aeb75270e833ae7bb816b13a498676 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 25 Feb 2021 21:29:04 -0600 Subject: [PATCH 39/42] editorial changes --- docs/Queue.md | 52 +++++++++++++++++++++++++-------------------------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/docs/Queue.md b/docs/Queue.md index a0a8312e5328..e1ba9d1c17e0 100644 --- a/docs/Queue.md +++ b/docs/Queue.md @@ -1,13 +1,18 @@ # Marlin's command queue concept -Marlin is a software that process commands encoded in G-Code coming from multiple different source (serial port, SD card, WIFI connection, and other). -It's also processing this command by converting them to actions on many physical actuators (motors, resistances, lasers, RGB leds and other). +Marlin Firmware processes G-code commands as they arrive from multiple sources, including the SD card and one or more serial ports such as USB-connected hosts, WiFi, Bluetooth, and so on. -Since physical actuators are limited by physic, a synchronization must be achieved between the source of the commands and the physical movement or action. +Marlin is also continuously processing the commands at the front of the queue, converting them into signals for many physical actuators such as motors, heaters, lasers, and RGB LEDs. -The synchronization is done via a queue that's buffering/adapting the next commands in memory so that the command processor is never starved (whenever possible). +The firmware needs to maintain continuity and timing so the command senders remain unblocked, while still performing physical movements and other actions in real-time, respecting the physical limits of stepper motors and other peripherals. -There are multiple buffers in the system to achieve this goal: +To keep things flowing Marlin feeds a single queue of G-code commands from all inputs, inserting them in the order received. Movement commands immediately go into the Planner Buffer, if there is room. The buffering of a move is considered the completion of the command, so if a non-movement command has to occur after a move is done, and not just after a move is buffered, then there has to be an `M400` to wait for the Planner Buffer to finish. + +Whenever the command queue gets full the sender has to wait and may need to re-send the last command again, so Marlin does some handshaking to keep the host informed during a print job, with "wait" + +An opposite problem called "planner starvation" occurs if the Planner Buffer is full of short and fast moves so the host can't send commands fast enough to prevent the Planner Buffer from emptying out. This causes "stuttering" and is common on an overloaded deltabot. Marlin has strategies to mitigate this, but sometimes a model has to be re-sliced to stay within the machine's inherent processing limits. + +Here's a basic flowchart of Marlin command processing: ``` +------+ Marlin's GCodeQueue | | +--------------------------------------+ +-----------+ @@ -17,9 +22,9 @@ There are multiple buffers in the system to achieve this goal: | +------------+ | | | | | | | GCode | | | | | | | | MAX_CMD_SIZE +-+-----> processor | | | Platform | | | | On EOL | +--------------+ | r_pos | | - +-------------> serial's +-----------> +--------> | | | | +-----------+ - | buffer | | | | w_pos | | CommandQueue | | | - | | | | | | | | | | + +-------------> serial's +-----------> +--------> | G-code | | | +-----------+ + | buffer | | | | w_pos | | command | | | + | | | | | | | line | | | +------------+ | +---+ | +--------------+ | | | Line buffer | x BUF_SIZE | | | | | | @@ -33,29 +38,22 @@ There are multiple buffers in the system to achieve this goal: +--------------------------------------+ ``` -Marlin is not a multithread or multiprocess software, so the processing loop performs the following actions sequentially: -1. Check if data is available in the platform's serial buffer and in that case, move them into the per-serial line buffer -2. If the data in the line buffer contains a End Of Line character, it's committed to the ring buffer of CommandQueue -3. Loop to 1 until there is no more data in the serial buffer -4. Run the G-Code processor by pop'ing from the CommandQueue ring buffer and execute one command -5. Loop to 1 - +Marlin is a single-threaded application with a main `loop()` that manages the command queue and an `idle()` routine that manages the hardware. The command queue is handled in two stages: +1. The `idle()` routine reads all inputs and attempts to enqueue any completed command lines. +2. The `loop()` gets the command at the front the G-code queue (if any) and runs it. The command will block the main loop and prevent the queue from advancing until it returns. ## Synchronization -To achieve synchronization, Marlin sends a `ok` answer to the host when it's done processing a command (end of G-Code processor task). -Most host will then wait until this answer is received to feed more command into the serial's buffer. - -In some case, if no data is available in the serial buffer for a long time, Marlin can ask the host to send more data via the `wait` message. +To maintain synchronization Marlin replies "`ok`" to the host as soon as the command has been enqueued. This lets the host know that it can send another command, and well-behaved hosts will wait for this message. With `ADVANCED_OK` enabled the `ok` message includes extra information (such as the number of slots left in the queue). +If no data is available on the serial buffer, Marlin can be configured to periodically send a "`wait`" message to the host. This was the only method of "host keepalive" provided in Marlin 1.0, but today the better options are `HOST_KEEPALIVE` and `ADVANCED_OK`. ## Limitation of the design -Please notice few limitations here: -1. While the G-Code processor is busy processing a command, the G-Code queue is not running. -2. If the host is sending data to the serial buffer, then the serial buffer will fill up (by interrupting the CPU). -3. This means that the serial buffer must be able to contain at least a complete line of data (the size of the serial buffer must be greater than the maximum G-Code command line the host can generate). The default value `MAX_CMD_SIZE` is 96 bytes for Marlin's CommandQueue lines, so a serial buffer that's smaller than 96 bytes can loose data. -4. Since serial buffer size are likely used as ring buffer themselves, their size should be a power of 2 (64 or 128 bytes recommanded) -5. A host generating too many G-Code command simultaneously will saturate the GCodeQueue but will not improve the processing rate of Marlin since only one command is processed per loop iteration. -6. Because of this, having a large BUF_SIZE is likely useless. The default value of 4 is good for a single serial port. - +Some limitations to the design are evident: +1. Whenever the G-code processor is busy processing a command, the G-code queue cannot advance. +2. A long command like `G29` causes commands to pile up and to fill the queue, making the host wait. +3. Each serial input requires a buffer large enough for a complete G-code line. This is set by `MAX_CMD_SIZE` with a default value of 96. +4. Since serial buffer sizes are likely used as ring buffers themselves, as an optimization their sizes must be a power of 2 (64 or 128 bytes recommended). +5. If a host sends too much G-code at once it can saturate the `GCodeQueue`. This doesn't do anything to improve the processing rate of Marlin since only one command can be dispatched per loop iteration. +6. With the previous point in mind, it's clear that the longstanding wisdom that you don't need a large `BUF_SIZE` is not just apocryphal. The default value of 4 is typically just fine for a single serial port. (And, if you decide to send a `G25` to pause the machine, the wait will be much shorter!) From e03bd35d3e9d97eabe291c03e134b15af1198884 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 25 Feb 2021 21:35:55 -0600 Subject: [PATCH 40/42] etc. --- docs/Queue.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/Queue.md b/docs/Queue.md index e1ba9d1c17e0..4bef1b44b3e2 100644 --- a/docs/Queue.md +++ b/docs/Queue.md @@ -8,9 +8,9 @@ The firmware needs to maintain continuity and timing so the command senders rema To keep things flowing Marlin feeds a single queue of G-code commands from all inputs, inserting them in the order received. Movement commands immediately go into the Planner Buffer, if there is room. The buffering of a move is considered the completion of the command, so if a non-movement command has to occur after a move is done, and not just after a move is buffered, then there has to be an `M400` to wait for the Planner Buffer to finish. -Whenever the command queue gets full the sender has to wait and may need to re-send the last command again, so Marlin does some handshaking to keep the host informed during a print job, with "wait" +Whenever the command queue gets full the sender needs to wait for space to open up, and the host may need to re-send the last command again. Marlin does some handshaking to keep the host informed during a print job, described below. -An opposite problem called "planner starvation" occurs if the Planner Buffer is full of short and fast moves so the host can't send commands fast enough to prevent the Planner Buffer from emptying out. This causes "stuttering" and is common on an overloaded deltabot. Marlin has strategies to mitigate this, but sometimes a model has to be re-sliced to stay within the machine's inherent processing limits. +An opposite problem called "planner starvation" occurs when Marlin receives many short and fast moves in a row so the Planner Buffer gets completed very quickly. In this case the host can't send commands fast enough to prevent the Planner Buffer from emptying out. Planner starvation causes obvious stuttering and is commonly seen on overloaded deltabots during small curves. Marlin has strategies to mitigate this issue, but sometimes a model has to be re-sliced (or the G-code has to be post-processed with Arc Welder) just to stay within the machine's inherent limits. Here's a basic flowchart of Marlin command processing: ``` From 107797fd56faf6fedc2a216b35cc8ef77200838a Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Fri, 26 Feb 2021 16:36:31 -0600 Subject: [PATCH 41/42] use common name --- Marlin/src/core/macros.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Marlin/src/core/macros.h b/Marlin/src/core/macros.h index 3c4e660efcd0..9b07af3618b7 100644 --- a/Marlin/src/core/macros.h +++ b/Marlin/src/core/macros.h @@ -366,13 +366,13 @@ } // Compile-time evaluation of the last part of a file path // Typically used to shorten the path to file in compiled strings - // CompileTimeString::fileName(__FILE__) returns "macros.h" and not /path/to/Marlin/src/core/macros.h - constexpr const char* fileName(const char* str) { + // CompileTimeString::baseName(__FILE__) returns "macros.h" and not /path/to/Marlin/src/core/macros.h + constexpr const char* baseName(const char* str) { return containsSlash(str) ? findLastSlashPos(findStringEnd(str)) : str; } } - #define ONLY_FILENAME CompileTimeString::fileName(__FILE__) + #define ONLY_FILENAME CompileTimeString::baseName(__FILE__) #else From 30cbc591361df5a42cd89aea83526b33f854fc9f Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Fri, 26 Feb 2021 16:53:29 -0600 Subject: [PATCH 42/42] Update Queue.md --- docs/Queue.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Queue.md b/docs/Queue.md index 4bef1b44b3e2..bce68b0551a7 100644 --- a/docs/Queue.md +++ b/docs/Queue.md @@ -40,7 +40,7 @@ Here's a basic flowchart of Marlin command processing: Marlin is a single-threaded application with a main `loop()` that manages the command queue and an `idle()` routine that manages the hardware. The command queue is handled in two stages: 1. The `idle()` routine reads all inputs and attempts to enqueue any completed command lines. -2. The `loop()` gets the command at the front the G-code queue (if any) and runs it. The command will block the main loop and prevent the queue from advancing until it returns. +2. The main `loop()` gets the command at the front the G-code queue (if any) and runs it. Each G-code command blocks the main loop, preventing the queue from advancing until it returns. To keep essential tasks and the UI running, any commands that run a long process need to call `idle()` frequently. ## Synchronization