From 60e2cc2aefda8eb58af12dd9565754ec130585b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Hejl?= Date: Fri, 25 Sep 2026 11:50:11 +0200 Subject: [PATCH 1/4] MeatPack: Fix unbinarize losing a character when its output buffer grows. --- src/LibBGCode/binarize/meatpack.cpp | 10 ++++++---- tests/binarize/meatpack_overflow_tests.cpp | 23 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/LibBGCode/binarize/meatpack.cpp b/src/LibBGCode/binarize/meatpack.cpp index 2915603..4cc0943 100644 --- a/src/LibBGCode/binarize/meatpack.cpp +++ b/src/LibBGCode/binarize/meatpack.cpp @@ -439,8 +439,9 @@ void unbinarize(const std::vector& src, std::string& dst) ++it_unbin_end; if (it_unbin_end == unbin_buffer.end()) { // the buffer is not big enough, resize it - unbin_buffer.resize(2 * unbin_buffer.size(), 0); - it_unbin_end = unbin_buffer.begin() + curr_unbin_buffer_length + 1; + const size_t unbin_buffer_length = unbin_buffer.size(); + unbin_buffer.resize(2 * unbin_buffer_length, 0); + it_unbin_end = unbin_buffer.begin() + unbin_buffer_length; } } @@ -449,8 +450,9 @@ void unbinarize(const std::vector& src, std::string& dst) ++it_unbin_end; if (it_unbin_end == unbin_buffer.end()) { // the buffer is not big enough, resize it - unbin_buffer.resize(2 * unbin_buffer.size(), 0); - it_unbin_end = unbin_buffer.begin() + curr_unbin_buffer_length + 1; + const size_t unbin_buffer_length = unbin_buffer.size(); + unbin_buffer.resize(2 * unbin_buffer_length, 0); + it_unbin_end = unbin_buffer.begin() + unbin_buffer_length; } } } diff --git a/tests/binarize/meatpack_overflow_tests.cpp b/tests/binarize/meatpack_overflow_tests.cpp index 9575380..39c2fe0 100644 --- a/tests/binarize/meatpack_overflow_tests.cpp +++ b/tests/binarize/meatpack_overflow_tests.cpp @@ -54,3 +54,26 @@ TEST_CASE("reading a crafted MeatPack gcode block stays in bounds", "[Binarize][ REQUIRE((result == EResult::Success || result == EResult::GCodeDecodingError)); } + +TEST_CASE("Unbinarize keeps the parameter letter after a reinserted space when its output buffer grows", "[Binarize][MeatPack]") { + const std::string gcode_line = "G1 X10.5 E1.25\n"; + for (size_t line_count = 1; line_count <= 64; ++line_count) { + CAPTURE(line_count); + + MeatPack::MPBinarizer binarizer(MeatPack::Flag_OmitWhitespaces); + std::vector binarized_data; + binarizer.initialize(binarized_data); + + std::string gcode; + for (size_t i = 0; i < line_count; ++i) { + REQUIRE(binarizer.binarize_line(gcode_line, binarized_data)); + gcode += gcode_line; + } + + binarizer.finalize(binarized_data); + + std::string out; + unbinarize(binarized_data, out); + CHECK(out == gcode); + } +} From 6539dddd951241ff1f801e9002e94289b8363fd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Hejl?= Date: Fri, 25 Sep 2026 11:50:19 +0200 Subject: [PATCH 2/4] MeatPack: Extract the G-code line compaction into functions. --- src/LibBGCode/binarize/meatpack.cpp | 87 ++++++++++++++--------------- 1 file changed, 43 insertions(+), 44 deletions(-) diff --git a/src/LibBGCode/binarize/meatpack.cpp b/src/LibBGCode/binarize/meatpack.cpp index 4cc0943..a730d4d 100644 --- a/src/LibBGCode/binarize/meatpack.cpp +++ b/src/LibBGCode/binarize/meatpack.cpp @@ -64,6 +64,42 @@ static std::string_view ltrim(const std::string_view& str) return start == std::string_view::npos ? std::string_view() : str.substr(start); } +static std::string compact_gcode_line_for_packing(const std::string& line, bool uppercase_e) +{ + std::string result = line; + if (uppercase_e) { + std::replace(result.begin(), result.end(), 'e', 'E'); + } + + std::replace(result.begin(), result.end(), 'x', 'X'); + std::replace(result.begin(), result.end(), 'g', 'G'); + result.erase(std::remove(result.begin(), result.end(), ' '), result.end()); + if (result.find('*') != std::string::npos) { + size_t checksum = 0; + result.erase(std::remove(result.begin(), result.end(), '*'), result.end()); + for (const char line_char : result) { + checksum ^= static_cast(line_char); + } + + result += "*" + std::to_string(checksum); + } + + result += '\n'; + return result; +} + +static std::string compact_gcode_line_if_it_contains_g_command(const std::string& line, bool uppercase_e) +{ + const std::string::size_type g_idx = line.find('G'); + if (g_idx != std::string::npos) { + if (g_idx + 1 < line.size() && line[g_idx + 1] >= '0' && line[g_idx + 1] <= '9') { + return compact_gcode_line_for_packing(line, uppercase_e); + } + } + + return line; +} + MPBinarizer::LookupTables MPBinarizer::s_lookup_tables = { { 0 }, { 0 }, false, 0 }; MPBinarizer::MPBinarizer(uint8_t flags) : m_flags(flags) {} @@ -88,47 +124,6 @@ void MPBinarizer::finalize(std::vector& dst) bool MPBinarizer::binarize_line(const std::string& line, std::vector& dst) { - auto unified_method = [this](const std::string& line) { - const std::string::size_type g_idx = line.find('G'); - if (g_idx != std::string::npos) { - if (g_idx + 1 < line.size() && line[g_idx + 1] >= '0' && line[g_idx + 1] <= '9') { - if ((m_flags & Flag_OmitWhitespaces) != 0) { - std::string result = line; - std::replace(result.begin(), result.end(), 'e', 'E'); - std::replace(result.begin(), result.end(), 'x', 'X'); - std::replace(result.begin(), result.end(), 'g', 'G'); - result.erase(std::remove(result.begin(), result.end(), ' '), result.end()); - if (result.find('*') != std::string::npos) { - size_t checksum = 0; - result.erase(std::remove(result.begin(), result.end(), '*'), result.end()); - for (size_t i = 0; i < result.size(); ++i) { - checksum ^= static_cast(result[i]); - } - result += "*" + std::to_string(checksum); - } - result += '\n'; - return result; - } - else { - std::string result = line; - std::replace(result.begin(), result.end(), 'x', 'X'); - std::replace(result.begin(), result.end(), 'g', 'G'); - result.erase(std::remove(result.begin(), result.end(), ' '), result.end()); - if (result.find('*') != std::string::npos) { - size_t checksum = 0; - result.erase(std::remove(result.begin(), result.end(), '*'), result.end()); - for (size_t i = 0; i < result.size(); ++i) { - checksum ^= static_cast(result[i]); - } - result += "*" + std::to_string(checksum); - } - result += '\n'; - return result; - } - } - } - return line; - }; auto is_packable = [](char c) { return (s_lookup_tables.packable[static_cast(c)] != 0); }; @@ -164,11 +159,15 @@ bool MPBinarizer::binarize_line(const std::string& line, std::vector& d return true; std::string modifiedLine = std::string(trim(std::string_view(line.substr(0, line.find(';'))))); - if (modifiedLine.empty()) + if (modifiedLine.empty()) { return true; - modifiedLine = unified_method(modifiedLine); - if (modifiedLine.back() != '\n') + } + + modifiedLine = compact_gcode_line_if_it_contains_g_command(modifiedLine, (m_flags & Flag_OmitWhitespaces) != 0); + if (modifiedLine.back() != '\n') { modifiedLine.push_back('\n'); + } + // 0xFF is the decoder's signal byte; it cannot be represented in the packed output. if (modifiedLine.find('\xff') != std::string::npos) return false; From 67a3f103f4c258e9d8f1bfde81b221b7d0e2bfe7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Hejl?= Date: Fri, 25 Sep 2026 11:50:34 +0200 Subject: [PATCH 3/4] MeatPack: Compact only the move commands G0-G3. --- src/LibBGCode/binarize/meatpack.cpp | 10 ++- tests/binarize/CMakeLists.txt | 1 + tests/binarize/meatpack_whitespace_tests.cpp | 82 ++++++++++++++++++++ 3 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 tests/binarize/meatpack_whitespace_tests.cpp diff --git a/src/LibBGCode/binarize/meatpack.cpp b/src/LibBGCode/binarize/meatpack.cpp index a730d4d..76799dd 100644 --- a/src/LibBGCode/binarize/meatpack.cpp +++ b/src/LibBGCode/binarize/meatpack.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include namespace MeatPack { @@ -88,12 +89,19 @@ static std::string compact_gcode_line_for_packing(const std::string& line, bool return result; } +// Only G0 to G3 lines are compacted because the decoder reliably reinserts spaces only before their parameters. static std::string compact_gcode_line_if_it_contains_g_command(const std::string& line, bool uppercase_e) { const std::string::size_type g_idx = line.find('G'); if (g_idx != std::string::npos) { if (g_idx + 1 < line.size() && line[g_idx + 1] >= '0' && line[g_idx + 1] <= '9') { - return compact_gcode_line_for_packing(line, uppercase_e); + int gcode_number = 0; + const std::from_chars_result gcode_number_result = + std::from_chars(line.data() + g_idx + 1, line.data() + line.size(), gcode_number); + const bool is_g0_to_g3_line = g_idx == 0 && gcode_number_result.ec == std::errc() && gcode_number <= 3; + if (is_g0_to_g3_line) { + return compact_gcode_line_for_packing(line, uppercase_e); + } } } diff --git a/tests/binarize/CMakeLists.txt b/tests/binarize/CMakeLists.txt index 2d6f73a..18e0092 100644 --- a/tests/binarize/CMakeLists.txt +++ b/tests/binarize/CMakeLists.txt @@ -5,6 +5,7 @@ add_executable(binarize_tests heatshrink_roundtrip_tests.cpp meatpack_encode_tests.cpp meatpack_overflow_tests.cpp + meatpack_whitespace_tests.cpp metadata_iterator_tests.cpp ) diff --git a/tests/binarize/meatpack_whitespace_tests.cpp b/tests/binarize/meatpack_whitespace_tests.cpp new file mode 100644 index 0000000..6792c1a --- /dev/null +++ b/tests/binarize/meatpack_whitespace_tests.cpp @@ -0,0 +1,82 @@ +#include + +#include "binarize/meatpack.hpp" + +#include +#include +#include + +static std::vector binarize_gcode_line(const std::string& gcode_line, uint8_t meatpack_flags) +{ + MeatPack::MPBinarizer binarizer(meatpack_flags); + std::vector binarized_data; + binarizer.initialize(binarized_data); + REQUIRE(binarizer.binarize_line(gcode_line, binarized_data)); + binarizer.finalize(binarized_data); + return binarized_data; +} + +static uint8_t four_bit_code_in_no_spaces_mode(char character) +{ + const size_t index = std::string("0123456789.E\nGX").find(character); + return index == std::string::npos ? 0xF : static_cast(index); +} + +// Written according to the MeatPack specification, independently of the library. +static std::vector expected_stream(const std::string& packed_text) +{ + constexpr uint8_t EnablePackingCommand = 251; + constexpr uint8_t EnableNoSpacesCommand = 247; + + std::vector stream = { 0xFF, 0xFF, EnablePackingCommand, 0xFF, 0xFF, EnableNoSpacesCommand }; + for (size_t i = 0; i < packed_text.size(); i += 2) { + const char first = packed_text[i]; + const char second = (i + 1 < packed_text.size()) ? packed_text[i + 1] : '\n'; + const uint8_t first_code = four_bit_code_in_no_spaces_mode(first); + const uint8_t second_code = four_bit_code_in_no_spaces_mode(second); + + stream.push_back(static_cast((second_code << 4) | first_code)); + + if (first_code == 0xF) { + stream.push_back(static_cast(first)); + } + + if (second_code == 0xF) { + stream.push_back(static_cast(second)); + } + } + + return stream; +} + +struct CompactionCase +{ + std::string gcode_line; + std::string packed_line; +}; + +static const std::vector compaction_cases = { + // gcode line Packed + { "M104 S200\n", "M104 S200\n" }, + { "G1 X10 Y10 E1.5 F3000\n", "G1X10Y10E1.5F3000\n\n" }, + { "G01 X1 Y2\n", "G01X1Y2\n\n" }, + { "G2 X10 Y10 I5 J5\n", "G2X10Y10I5J5\n\n" }, + { "G10\n", "G10\n" }, + { "G28 W\n", "G28 W\n" }, + { "G28 XY\n", "G28 XY\n" }, + { "N5 G1 X10\n", "N5 G1 X10\n" }, + { "G12 P1 S3 T5\n", "G12 P1 S3 T5\n" }, + { "G12 Quick clean\n", "G12 Quick clean\n" }, + { "G12 Quick_Clean\n", "G12 Quick_Clean\n" }, + { "G12 QUICK STOP\n", "G12 QUICK STOP\n" }, + { "G FS\n", "G FS\n" }, + { "g1 x10 y10\n", "g1 x10 y10\n" }, +}; + +TEST_CASE("MeatPack compacts only G0-G3 lines", "[Binarize][MeatPack]") +{ + for (const CompactionCase& compaction_case : compaction_cases) { + CAPTURE(compaction_case.gcode_line); + CHECK(binarize_gcode_line(compaction_case.gcode_line, MeatPack::Flag_OmitWhitespaces) == expected_stream(compaction_case.packed_line)); + } +} From e01e0eb22e77741744af2e28b3f4d58879c49aef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Hejl?= Date: Fri, 25 Sep 2026 15:07:24 +0200 Subject: [PATCH 4/4] MeatPack: Reinsert spaces only into compacted lines. --- src/LibBGCode/binarize/meatpack.cpp | 8 +- tests/binarize/meatpack_whitespace_tests.cpp | 97 ++++++++++++++------ 2 files changed, 73 insertions(+), 32 deletions(-) diff --git a/src/LibBGCode/binarize/meatpack.cpp b/src/LibBGCode/binarize/meatpack.cpp index 76799dd..abb779f 100644 --- a/src/LibBGCode/binarize/meatpack.cpp +++ b/src/LibBGCode/binarize/meatpack.cpp @@ -436,12 +436,12 @@ void unbinarize(const std::vector& src, std::string& dst) if (c_unbin[i] == 'G' && (curr_unbin_buffer_length == 0 || *std::prev(it_unbin_end, 1) == '\n')) { add_space = true; new_line = true; - } - else if (c_unbin[i] == '\n') + } else if (c_unbin[i] == '\n' || c_unbin[i] == ' ') { + // A line containing a space was stored unchanged by the encoder. add_space = false; + } - if (!new_line && add_space && (curr_unbin_buffer_length == 0 || *std::prev(it_unbin_end, 1) != ' ') && - is_gline_parameter(c_unbin[i])) { + if (!new_line && add_space && is_gline_parameter(c_unbin[i])) { *it_unbin_end = ' '; ++it_unbin_end; if (it_unbin_end == unbin_buffer.end()) { diff --git a/tests/binarize/meatpack_whitespace_tests.cpp b/tests/binarize/meatpack_whitespace_tests.cpp index 6792c1a..c892620 100644 --- a/tests/binarize/meatpack_whitespace_tests.cpp +++ b/tests/binarize/meatpack_whitespace_tests.cpp @@ -16,6 +16,13 @@ static std::vector binarize_gcode_line(const std::string& gcode_line, u return binarized_data; } +static std::string unbinarize_to_text(const std::vector& binarized_data) +{ + std::string text; + MeatPack::unbinarize(binarized_data, text); + return text; +} + static uint8_t four_bit_code_in_no_spaces_mode(char character) { const size_t index = std::string("0123456789.E\nGX").find(character); @@ -23,26 +30,28 @@ static uint8_t four_bit_code_in_no_spaces_mode(char character) } // Written according to the MeatPack specification, independently of the library. -static std::vector expected_stream(const std::string& packed_text) +static std::vector expected_stream(const std::vector& packed_lines) { constexpr uint8_t EnablePackingCommand = 251; constexpr uint8_t EnableNoSpacesCommand = 247; std::vector stream = { 0xFF, 0xFF, EnablePackingCommand, 0xFF, 0xFF, EnableNoSpacesCommand }; - for (size_t i = 0; i < packed_text.size(); i += 2) { - const char first = packed_text[i]; - const char second = (i + 1 < packed_text.size()) ? packed_text[i + 1] : '\n'; - const uint8_t first_code = four_bit_code_in_no_spaces_mode(first); - const uint8_t second_code = four_bit_code_in_no_spaces_mode(second); + for (const std::string& packed_line : packed_lines) { + for (size_t i = 0; i < packed_line.size(); i += 2) { + const char first = packed_line[i]; + const char second = (i + 1 < packed_line.size()) ? packed_line[i + 1] : '\n'; + const uint8_t first_code = four_bit_code_in_no_spaces_mode(first); + const uint8_t second_code = four_bit_code_in_no_spaces_mode(second); - stream.push_back(static_cast((second_code << 4) | first_code)); + stream.push_back(static_cast((second_code << 4) | first_code)); - if (first_code == 0xF) { - stream.push_back(static_cast(first)); - } + if (first_code == 0xF) { + stream.push_back(static_cast(first)); + } - if (second_code == 0xF) { - stream.push_back(static_cast(second)); + if (second_code == 0xF) { + stream.push_back(static_cast(second)); + } } } @@ -53,30 +62,62 @@ struct CompactionCase { std::string gcode_line; std::string packed_line; + std::string packed_by_previous_versions; + std::string decoded_from_previous_versions; }; static const std::vector compaction_cases = { - // gcode line Packed - { "M104 S200\n", "M104 S200\n" }, - { "G1 X10 Y10 E1.5 F3000\n", "G1X10Y10E1.5F3000\n\n" }, - { "G01 X1 Y2\n", "G01X1Y2\n\n" }, - { "G2 X10 Y10 I5 J5\n", "G2X10Y10I5J5\n\n" }, - { "G10\n", "G10\n" }, - { "G28 W\n", "G28 W\n" }, - { "G28 XY\n", "G28 XY\n" }, - { "N5 G1 X10\n", "N5 G1 X10\n" }, - { "G12 P1 S3 T5\n", "G12 P1 S3 T5\n" }, - { "G12 Quick clean\n", "G12 Quick clean\n" }, - { "G12 Quick_Clean\n", "G12 Quick_Clean\n" }, - { "G12 QUICK STOP\n", "G12 QUICK STOP\n" }, - { "G FS\n", "G FS\n" }, - { "g1 x10 y10\n", "g1 x10 y10\n" }, + // gcode line Packed Previous versions Decoded from previous versions + { "M104 S200\n", "M104 S200\n", "M104 S200\n", "M104 S200\n" }, + { "G1 X10 Y10 E1.5 F3000\n", "G1X10Y10E1.5F3000\n\n", "G1X10Y10E1.5F3000\n\n", "G1 X10 Y10 E1.5 F3000\n" }, + { "G01 X1 Y2\n", "G01X1Y2\n\n", "G01X1Y2\n\n", "G01 X1 Y2\n" }, + { "G2 X10 Y10 I5 J5\n", "G2X10Y10I5J5\n\n", "G2X10Y10I5J5\n\n", "G2 X10 Y10 I5 J5\n" }, + { "G10\n", "G10\n", "G10\n\n", "G10\n" }, + { "G28 W\n", "G28 W\n", "G28W\n\n", "G28 W\n" }, + { "G28 XY\n", "G28 XY\n", "G28XY\n\n", "G28 X Y\n" }, + { "N5 G1 X10\n", "N5 G1 X10\n", "N5G1X10\n\n", "N5G1X10\n" }, + { "G12 P1 S3 T5\n", "G12 P1 S3 T5\n", "G12P1S3T5\n\n", "G12 P1 S3T5\n" }, + { "G12 Quick clean\n", "G12 Quick clean\n", "G12QuickclEan\n\n", "G12Quickcl Ean\n" }, + { "G12 Quick_Clean\n", "G12 Quick_Clean\n", "G12Quick_ClEan\n\n", "G12Quick_ Cl Ean\n" }, + { "G12 QUICK STOP\n", "G12 QUICK STOP\n", "G12QUICKSTOP\n\n", "G12QU I CK STO P\n" }, + { "G FS\n", "G FS\n", "G FS\n", "G FS\n" }, + { "g1 x10 y10\n", "g1 x10 y10\n", "g1 x10 y10\n", "g1 x10 y10\n" }, }; TEST_CASE("MeatPack compacts only G0-G3 lines", "[Binarize][MeatPack]") { for (const CompactionCase& compaction_case : compaction_cases) { CAPTURE(compaction_case.gcode_line); - CHECK(binarize_gcode_line(compaction_case.gcode_line, MeatPack::Flag_OmitWhitespaces) == expected_stream(compaction_case.packed_line)); + CHECK(binarize_gcode_line(compaction_case.gcode_line, MeatPack::Flag_OmitWhitespaces) == expected_stream({ compaction_case.packed_line })); + } +} + +TEST_CASE("MeatPack decodes every line back exactly", "[Binarize][MeatPack]") +{ + std::vector packed_lines; + std::string gcode_lines; + for (const CompactionCase& compaction_case : compaction_cases) { + CAPTURE(compaction_case.gcode_line); + CHECK(unbinarize_to_text(binarize_gcode_line(compaction_case.gcode_line, MeatPack::Flag_OmitWhitespaces)) == compaction_case.gcode_line); + + packed_lines.push_back(compaction_case.packed_line); + gcode_lines += compaction_case.gcode_line; + } + + CHECK(unbinarize_to_text(expected_stream(packed_lines)) == gcode_lines); +} + +TEST_CASE("MeatPack decodes streams of previous versions as before", "[Binarize][MeatPack]") +{ + std::vector packed_lines; + std::string decoded_lines; + for (const CompactionCase& compaction_case : compaction_cases) { + CAPTURE(compaction_case.gcode_line); + CHECK(unbinarize_to_text(expected_stream({ compaction_case.packed_by_previous_versions })) == compaction_case.decoded_from_previous_versions); + + packed_lines.push_back(compaction_case.packed_by_previous_versions); + decoded_lines += compaction_case.decoded_from_previous_versions; } + + CHECK(unbinarize_to_text(expected_stream(packed_lines)) == decoded_lines); }