diff --git a/src/LibBGCode/binarize/meatpack.cpp b/src/LibBGCode/binarize/meatpack.cpp index 2915603..abb779f 100644 --- a/src/LibBGCode/binarize/meatpack.cpp +++ b/src/LibBGCode/binarize/meatpack.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include namespace MeatPack { @@ -64,6 +65,49 @@ 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; +} + +// 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') { + 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); + } + } + } + + return line; +} + MPBinarizer::LookupTables MPBinarizer::s_lookup_tables = { { 0 }, { 0 }, false, 0 }; MPBinarizer::MPBinarizer(uint8_t flags) : m_flags(flags) {} @@ -88,47 +132,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 +167,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; @@ -429,18 +436,19 @@ 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()) { // 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 +457,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/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_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); + } +} diff --git a/tests/binarize/meatpack_whitespace_tests.cpp b/tests/binarize/meatpack_whitespace_tests.cpp new file mode 100644 index 0000000..c892620 --- /dev/null +++ b/tests/binarize/meatpack_whitespace_tests.cpp @@ -0,0 +1,123 @@ +#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 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); + 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::vector& packed_lines) +{ + constexpr uint8_t EnablePackingCommand = 251; + constexpr uint8_t EnableNoSpacesCommand = 247; + + std::vector stream = { 0xFF, 0xFF, EnablePackingCommand, 0xFF, 0xFF, EnableNoSpacesCommand }; + 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)); + + 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; + std::string packed_by_previous_versions; + std::string decoded_from_previous_versions; +}; + +static const std::vector compaction_cases = { + // 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 })); + } +} + +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); +}