Fix MeatPack with per-serial-port instances (#21306)

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
 

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
+// Hook Meatpack if it's enabled on the first leaf
-  #ifdef SERIAL_CATCHALL
+#if ENABLED(MEATPACK_ON_SERIAL_PORT_1)
-    SerialOutputT multiSerial(MYSERIAL, SERIAL_CATCHALL);
+  SerialLeafT1 mpSerial1(false, _SERIAL_LEAF_1);
-  #else
+#endif
-    #if HAS_ETHERNET
+#if ENABLED(MEATPACK_ON_SERIAL_PORT_2)
-      // Runtime checking of the condition variable
+  SerialLeafT2 mpSerial2(false, _SERIAL_LEAF_2);
-      ConditionalSerial<decltype(MYSERIAL2)> 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
 #endif
 
-#if ENABLED(MEATPACK)
+// Step 2: For multiserial, handle the second serial port as well
-  MeatpackSerial<decltype(_SERIAL_IMPL)> mpSerial(false, _SERIAL_IMPL);
+#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) {

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<decltype(_SERIAL_LEAF_1)> 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<decltype(MYSERIAL), decltype(SERIAL_CATCHALL), 0> SerialOutputT;
+    #define _SERIAL_LEAF_2 SERIAL_CATCHALL
   #else
-    typedef MultiSerial<decltype(MYSERIAL1), TERN(HAS_ETHERNET, ConditionalSerial<decltype(MYSERIAL2)>, decltype(MYSERIAL2)), 0> SerialOutputT;
+    #if HAS_ETHERNET
+      // We need to create an instance here
+      typedef ConditionalSerial<decltype(MYSERIAL2)> 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<decltype(_SERIAL_LEAF_2)> SerialLeafT2;
+    extern SerialLeafT2 mpSerial2;
+    #define SERIAL_LEAF_2 mpSerial2
+  #else
+    #define SERIAL_LEAF_2 _SERIAL_LEAF_2
+  #endif
+
+  typedef MultiSerial<decltype(SERIAL_LEAF_1), decltype(SERIAL_LEAF_2), 0> 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
+  #define SERIAL_IMPL         SERIAL_LEAF_1
-#endif
-
-#if ENABLED(MEATPACK)
-  extern MeatpackSerial<decltype(_SERIAL_IMPL)> mpSerial;
-  #define SERIAL_IMPL mpSerial
-#else
-  #define SERIAL_IMPL _SERIAL_IMPL
 #endif
 
 #define SERIAL_OUT(WHAT, V...)  (void)SERIAL_IMPL.WHAT(V)

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

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
+  bool cmd_is_next;        // A command is pending
-  static uint8_t state;           // Configuration state
+  uint8_t state;           // Configuration state
-  static uint8_t second_char;     // Buffers a character if dealing with out-of-sequence pairs
+  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)
+  uint8_t cmd_count,       // Counter of command bytes received (need 2)
-                 full_char_count, // Counter for full-width characters to be received
+          full_char_count, // Counter for full-width characters to be received
-                 char_out_count;  // Stores number of characters to be read out.
+          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
+  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();
+  void reset_state();
-  static void report_state();
+  void report_state();
-  static uint8_t unpack_chars(const uint8_t pk, uint8_t* __restrict const chars_out);
+  uint8_t unpack_chars(const uint8_t pk, uint8_t* __restrict const chars_out);
-  static void handle_command(const MeatPack_Command c);
+  void handle_command(const MeatPack_Command c);
-  static void handle_output_char(const uint8_t c);
+  void handle_output_char(const uint8_t c);
-  static void handle_rx_char_inner(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 <typename SerialT>
 struct MeatpackSerial : public SerialBase <MeatpackSerial < SerialT >> {
   typedef SerialBase< MeatpackSerial<SerialT> > BaseClassT;
 
   SerialT & out;
+  MeatPack meatpack;
 
   char serialBuffer[2];
   uint8_t charCount;
@@ -143,10 +144,6 @@ struct MeatpackSerial : public SerialBase <MeatpackSerial < SerialT >> {
   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<MultiSerial<...>> we should do MultiSerial<MeatpackSerial<...>, 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
 

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)

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

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)
+#if BOTH(HAS_MEATPACK, BINARY_FILE_TRANSFER)
-  #error "Either enable MEATPACK or BINARY_FILE_TRANSFER, not both."
+  #error "Either enable MEATPACK_ON_SERIAL_PORT_* or BINARY_FILE_TRANSFER, not both."
 #endif
 
 /**

platformio.ini

@@ -332,7 +332,7 @@ PCA9632                 = src_filter=+<src/feature/leds/pca9632.cpp>
 PRINTER_EVENT_LEDS      = src_filter=+<src/feature/leds/printer_event_leds.cpp>
 TEMP_STAT_LEDS          = src_filter=+<src/feature/leds/tempstat.cpp>
 MAX7219_DEBUG           = src_filter=+<src/feature/max7219.cpp> +<src/gcode/feature/leds/M7219.cpp>
-MEATPACK                = src_filter=+<src/feature/meatpack.cpp>
+HAS_MEATPACK            = src_filter=+<src/feature/meatpack.cpp>
 MIXING_EXTRUDER         = src_filter=+<src/feature/mixing.cpp> +<src/gcode/feature/mixing/M163-M165.cpp>
 HAS_PRUSA_MMU1          = src_filter=+<src/feature/mmu/mmu.cpp>
 HAS_PRUSA_MMU2          = src_filter=+<src/feature/mmu/mmu2.cpp> +<src/gcode/feature/prusa_MMU2>