From 242ef2b2b4f02740e95cb518c0d084e1c2bcce7e Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Wed, 10 Mar 2021 21:22:20 +0100 Subject: [PATCH] Fix MeatPack with per-serial-port instances (#21306) --- Marlin/Configuration_adv.h | 4 ++- Marlin/src/core/serial.cpp | 29 ++++++++-------- Marlin/src/core/serial.h | 56 +++++++++++++++++++++++------- Marlin/src/feature/meatpack.cpp | 12 ++----- Marlin/src/feature/meatpack.h | 39 ++++++++++----------- Marlin/src/gcode/host/M115.cpp | 2 +- Marlin/src/inc/Conditionals_post.h | 4 +++ Marlin/src/inc/SanityCheck.h | 6 ++-- platformio.ini | 2 +- 9 files changed, 90 insertions(+), 64 deletions(-) diff --git a/Marlin/Configuration_adv.h b/Marlin/Configuration_adv.h index 868b0c239..9ab3e2d04 100644 --- a/Marlin/Configuration_adv.h +++ b/Marlin/Configuration_adv.h @@ -3373,7 +3373,9 @@ //#define GCODE_QUOTED_STRINGS // Support for quoted string parameters #endif -//#define MEATPACK // Support for MeatPack G-code compression (https://github.com/scottmudge/OctoPrint-MeatPack) +// Support for MeatPack G-code compression (https://github.com/scottmudge/OctoPrint-MeatPack) +//#define MEATPACK_ON_SERIAL_PORT_1 +//#define MEATPACK_ON_SERIAL_PORT_2 //#define GCODE_CASE_INSENSITIVE // Accept G-code sent to the firmware in lowercase diff --git a/Marlin/src/core/serial.cpp b/Marlin/src/core/serial.cpp index 326baf0c5..dcbfd608b 100644 --- a/Marlin/src/core/serial.cpp +++ b/Marlin/src/core/serial.cpp @@ -37,23 +37,22 @@ PGMSTR(SP_A_STR, " A"); PGMSTR(SP_B_STR, " B"); PGMSTR(SP_C_STR, " C"); PGMSTR(SP_X_STR, " X"); PGMSTR(SP_Y_STR, " Y"); PGMSTR(SP_Z_STR, " Z"); PGMSTR(SP_E_STR, " E"); PGMSTR(SP_X_LBL, " X:"); PGMSTR(SP_Y_LBL, " Y:"); PGMSTR(SP_Z_LBL, " Z:"); PGMSTR(SP_E_LBL, " E:"); -#if HAS_MULTI_SERIAL - #ifdef SERIAL_CATCHALL - SerialOutputT multiSerial(MYSERIAL, SERIAL_CATCHALL); - #else - #if HAS_ETHERNET - // Runtime checking of the condition variable - ConditionalSerial serialOut2(ethernet.have_telnet_client, MYSERIAL2, false); // Takes reference here - #else - // Don't pay for runtime checking a true variable, instead use the output directly - #define serialOut2 MYSERIAL2 - #endif - SerialOutputT multiSerial(MYSERIAL1, serialOut2); - #endif +// Hook Meatpack if it's enabled on the first leaf +#if ENABLED(MEATPACK_ON_SERIAL_PORT_1) + SerialLeafT1 mpSerial1(false, _SERIAL_LEAF_1); +#endif +#if ENABLED(MEATPACK_ON_SERIAL_PORT_2) + SerialLeafT2 mpSerial2(false, _SERIAL_LEAF_2); #endif -#if ENABLED(MEATPACK) - MeatpackSerial mpSerial(false, _SERIAL_IMPL); +// Step 2: For multiserial, handle the second serial port as well +#if HAS_MULTI_SERIAL + #if HAS_ETHERNET + // We need a definition here + SerialLeafT2 msSerial2(ethernet.have_telnet_client, MYSERIAL2, false); + #endif + + SerialOutputT multiSerial(SERIAL_LEAF_1, SERIAL_LEAF_2); #endif void serialprintPGM(PGM_P str) { diff --git a/Marlin/src/core/serial.h b/Marlin/src/core/serial.h index 87c2a390d..2f23e4e3c 100644 --- a/Marlin/src/core/serial.h +++ b/Marlin/src/core/serial.h @@ -24,7 +24,7 @@ #include "../inc/MarlinConfig.h" #include "serial_hook.h" -#if ENABLED(MEATPACK) +#if HAS_MEATPACK #include "../feature/meatpack.h" #endif @@ -62,29 +62,59 @@ extern uint8_t marlin_debug_flags; // // Serial redirection // +// Step 1: Find what's the first serial leaf +#if BOTH(HAS_MULTI_SERIAL, SERIAL_CATCHALL) + #define _SERIAL_LEAF_1 MYSERIAL +#else + #define _SERIAL_LEAF_1 MYSERIAL1 +#endif + +// Hook Meatpack if it's enabled on the first leaf +#if ENABLED(MEATPACK_ON_SERIAL_PORT_1) + typedef MeatpackSerial SerialLeafT1; + extern SerialLeafT1 mpSerial1; + #define SERIAL_LEAF_1 mpSerial1 +#else + #define SERIAL_LEAF_1 _SERIAL_LEAF_1 +#endif + +// Step 2: For multiserial, handle the second serial port as well #if HAS_MULTI_SERIAL #define _PORT_REDIRECT(n,p) REMEMBER(n,multiSerial.portMask,p) #define _PORT_RESTORE(n,p) RESTORE(n) #define SERIAL_ASSERT(P) if(multiSerial.portMask!=(P)){ debugger(); } + // If we have a catchall, use that directly #ifdef SERIAL_CATCHALL - typedef MultiSerial SerialOutputT; + #define _SERIAL_LEAF_2 SERIAL_CATCHALL #else - typedef MultiSerial, decltype(MYSERIAL2)), 0> SerialOutputT; + #if HAS_ETHERNET + // We need to create an instance here + typedef ConditionalSerial SerialLeafT2; + extern SerialLeafT2 msSerial2; + #define _SERIAL_LEAF_2 msSerial2 + #else + // Don't create a useless instance here, directly use the existing instance + #define _SERIAL_LEAF_2 MYSERIAL2 + #endif #endif - extern SerialOutputT multiSerial; - #define _SERIAL_IMPL multiSerial + + // Hook Meatpack if it's enabled on the second leaf + #if ENABLED(MEATPACK_ON_SERIAL_PORT_2) + typedef MeatpackSerial SerialLeafT2; + extern SerialLeafT2 mpSerial2; + #define SERIAL_LEAF_2 mpSerial2 + #else + #define SERIAL_LEAF_2 _SERIAL_LEAF_2 + #endif + + typedef MultiSerial SerialOutputT; + extern SerialOutputT 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 MYSERIAL1 -#endif - -#if ENABLED(MEATPACK) - extern MeatpackSerial mpSerial; - #define SERIAL_IMPL mpSerial -#else - #define SERIAL_IMPL _SERIAL_IMPL + #define SERIAL_IMPL SERIAL_LEAF_1 #endif #define SERIAL_OUT(WHAT, V...) (void)SERIAL_IMPL.WHAT(V) diff --git a/Marlin/src/feature/meatpack.cpp b/Marlin/src/feature/meatpack.cpp index 7e81dbed7..178831c9b 100644 --- a/Marlin/src/feature/meatpack.cpp +++ b/Marlin/src/feature/meatpack.cpp @@ -39,7 +39,7 @@ #include "../inc/MarlinConfig.h" -#if ENABLED(MEATPACK) +#if HAS_MEATPACK #include "meatpack.h" MeatPack meatpack; @@ -50,14 +50,6 @@ MeatPack meatpack; #define DEBUG_OUT ENABLED(MP_DEBUG) #include "../core/debug_out.h" -bool MeatPack::cmd_is_next = false; // A command is pending -uint8_t MeatPack::state = 0; // Configuration state OFF -uint8_t MeatPack::second_char = 0; // The unpacked 2nd character from an out-of-sequence packed pair -uint8_t MeatPack::cmd_count = 0, // Counts how many command bytes are received (need 2) - MeatPack::full_char_count = 0, // Counts how many full-width characters are to be received - MeatPack::char_out_count = 0; // Stores number of characters to be read out. -uint8_t MeatPack::char_out_buf[2]; // Output buffer for caching up to 2 characters - // The 15 most-common characters used in G-code, ~90-95% of all G-code uses these characters // Stored in SRAM for performance. uint8_t meatPackLookupTable[16] = { @@ -223,4 +215,4 @@ uint8_t MeatPack::get_result_char(char* const __restrict out) { return res; } -#endif // MEATPACK +#endif // HAS_MEATPACK diff --git a/Marlin/src/feature/meatpack.h b/Marlin/src/feature/meatpack.h index 96004cfea..80f4570e0 100644 --- a/Marlin/src/feature/meatpack.h +++ b/Marlin/src/feature/meatpack.h @@ -90,18 +90,18 @@ class MeatPack { static const uint8_t kSpaceCharIdx = 11; static const char kSpaceCharReplace = 'E'; - static bool cmd_is_next; // A command is pending - static uint8_t state; // Configuration state - static uint8_t second_char; // Buffers a character if dealing with out-of-sequence pairs - static uint8_t cmd_count, // Counter of command bytes received (need 2) - full_char_count, // Counter for full-width characters to be received - 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 + bool cmd_is_next; // A command is pending + uint8_t state; // Configuration state + uint8_t second_char; // Buffers a character if dealing with out-of-sequence pairs + uint8_t cmd_count, // Counter of command bytes received (need 2) + full_char_count, // Counter for full-width characters to be received + char_out_count; // Stores number of characters to be read out. + 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); + void handle_rx_char(const uint8_t c, const serial_index_t serial_ind); /** * After passing in rx'd char using above method, call this to get characters out. @@ -109,24 +109,25 @@ public: * @param out [in] Output pointer for unpacked/processed data. * @return Number of characters returned. Range from 0 to 2. */ - static uint8_t get_result_char(char* const __restrict out); + uint8_t get_result_char(char* const __restrict out); - static void reset_state(); - static void report_state(); - 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); - static void handle_rx_char_inner(const uint8_t c); + void reset_state(); + void report_state(); + uint8_t unpack_chars(const uint8_t pk, uint8_t* __restrict const chars_out); + void handle_command(const MeatPack_Command c); + void handle_output_char(const uint8_t c); + void handle_rx_char_inner(const uint8_t c); + + MeatPack() : cmd_is_next(false), state(0), second_char(0), cmd_count(0), full_char_count(0), char_out_count(0) {} }; -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; + MeatPack meatpack; char serialBuffer[2]; uint8_t charCount; @@ -143,10 +144,6 @@ struct MeatpackSerial : public SerialBase > { void flushTX() { CALL_IF_EXISTS(void, &out, flushTX); } int available(serial_index_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 - if (charCount) return charCount; // The buffer still has data if (out.available(index) <= 0) return 0; // No data to read diff --git a/Marlin/src/gcode/host/M115.cpp b/Marlin/src/gcode/host/M115.cpp index 4b7b26868..cd6fcbaf8 100644 --- a/Marlin/src/gcode/host/M115.cpp +++ b/Marlin/src/gcode/host/M115.cpp @@ -145,7 +145,7 @@ void GcodeSuite::M115() { cap_line(PSTR("COOLER_TEMPERATURE"), ENABLED(HAS_COOLER)); // MEATPACK Compression - cap_line(PSTR("MEATPACK"), ENABLED(MEATPACK)); + cap_line(PSTR("MEATPACK"), ENABLED(HAS_MEATPACK)); // Machine Geometry #if ENABLED(M115_GEOMETRY_REPORT) diff --git a/Marlin/src/inc/Conditionals_post.h b/Marlin/src/inc/Conditionals_post.h index e8ed0d5a9..7dc75db1c 100644 --- a/Marlin/src/inc/Conditionals_post.h +++ b/Marlin/src/inc/Conditionals_post.h @@ -2912,3 +2912,7 @@ #elif NUM_SERIAL > 1 #define HAS_MULTI_SERIAL 1 #endif + +#if ENABLED(MEATPACK_ON_SERIAL_PORT_1) || BOTH(HAS_MULTI_SERIAL, MEATPACK_ON_SERIAL_PORT_2) + #define HAS_MEATPACK 1 +#endif diff --git a/Marlin/src/inc/SanityCheck.h b/Marlin/src/inc/SanityCheck.h index 8c92cf2c6..6802bff1d 100644 --- a/Marlin/src/inc/SanityCheck.h +++ b/Marlin/src/inc/SanityCheck.h @@ -551,6 +551,8 @@ #error "UNKNOWN_Z_NO_RAISE is replaced by setting Z_IDLE_HEIGHT to Z_MAX_POS." #elif defined(Z_AFTER_DEACTIVATE) #error "Z_AFTER_DEACTIVATE is replaced by Z_IDLE_HEIGHT." +#elif defined(MEATPACK) + #error "MEATPACK is now enabled with MEATPACK_ON_SERIAL_PORT_1, MEATPACK_ON_SERIAL_PORT_2, etc." #endif /** @@ -3340,8 +3342,8 @@ static_assert( _ARR_TEST(3,0) && _ARR_TEST(3,1) && _ARR_TEST(3,2) /** * Sanity Check for MEATPACK and BINARY_FILE_TRANSFER Features */ -#if BOTH(MEATPACK, BINARY_FILE_TRANSFER) - #error "Either enable MEATPACK or BINARY_FILE_TRANSFER, not both." +#if BOTH(HAS_MEATPACK, BINARY_FILE_TRANSFER) + #error "Either enable MEATPACK_ON_SERIAL_PORT_* or BINARY_FILE_TRANSFER, not both." #endif /** diff --git a/platformio.ini b/platformio.ini index b8235c84f..3b1604a29 100644 --- a/platformio.ini +++ b/platformio.ini @@ -332,7 +332,7 @@ PCA9632 = src_filter=+ PRINTER_EVENT_LEDS = src_filter=+ TEMP_STAT_LEDS = src_filter=+ MAX7219_DEBUG = src_filter=+ + -MEATPACK = src_filter=+ +HAS_MEATPACK = src_filter=+ MIXING_EXTRUDER = src_filter=+ + HAS_PRUSA_MMU1 = src_filter=+ HAS_PRUSA_MMU2 = src_filter=+ +