From 507f3fae91de6912056669a37e30699883b949c5 Mon Sep 17 00:00:00 2001 From: Roberto Martin Fantini Date: Fri, 28 Feb 2025 18:00:46 +0100 Subject: [PATCH 1/4] Add reusable the encoder and decoder test class --- tests/encoder_decoder_test.cpp | 68 ++++++----------------------- tests/encoder_decoder_test_v2.cpp | 69 ++++++------------------------ tests/fast_test_coding_case.hpp | 52 ++++++++++++++++++++++ tests/fast_test_coding_case_v2.hpp | 49 +++++++++++++++++++++ 4 files changed, 127 insertions(+), 111 deletions(-) create mode 100644 tests/fast_test_coding_case.hpp create mode 100644 tests/fast_test_coding_case_v2.hpp diff --git a/tests/encoder_decoder_test.cpp b/tests/encoder_decoder_test.cpp index 7b9899a7..8c3d1d87 100644 --- a/tests/encoder_decoder_test.cpp +++ b/tests/encoder_decoder_test.cpp @@ -1,58 +1,16 @@ #include "catch.hpp" #include -#include -#include + +#include "fast_test_coding_case.hpp" #include "byte_stream.h" #include "simple12.h" -namespace -{ -class fast_test_coding_case -{ - public: - fast_test_coding_case() - { - encoder_.include({simple12::description()}); - decoder_.include({simple12::description()}); - } - - bool encoding(const mfast::message_cref& msg_ref, const byte_stream& result, bool reset=false) - { - const int buffer_size = 128; - char buffer[buffer_size]; - - std::size_t encoded_size = encoder_.encode(msg_ref, - buffer, - buffer_size, - reset); - - if (result == byte_stream(buffer, encoded_size)) - return true; - - std::cout << byte_stream(buffer, encoded_size); - - INFO( "Got \"" << byte_stream(buffer, encoded_size) << "\" instead." ); - return false; - } - - bool decoding(const byte_stream& bytes, const mfast::message_cref& result, bool reset=false) - { - const char* first = bytes.data(); - mfast::message_cref msg = decoder_.decode(first, first+bytes.size(), reset); - - return (msg == result); - } - - private: - mfast::fast_encoder encoder_; - mfast::fast_decoder decoder_; -}; -} +using namespace test::coding; -TEST_CASE("simple field and sequence optional encoder","[field_sequence_optional_sequence_encoder]") +TEST_CASE("simple field and sequence optional encoder/decoder","[field_sequence_optional_encoder_decoder]") { - fast_test_coding_case test_case; + fast_test_coding_case test_case; SECTION("encode field") { @@ -110,9 +68,9 @@ TEST_CASE("simple field and sequence optional encoder","[field_sequence_optional } } -TEST_CASE("simple field encoder","[field_optional_encoder]") +TEST_CASE("simple field encoder/decoder","[field_optional_encoder_decoder]") { - fast_test_coding_case test_case; + fast_test_coding_case test_case; SECTION("encode fields") { @@ -130,9 +88,9 @@ TEST_CASE("simple field encoder","[field_optional_encoder]") } } -TEST_CASE("simple field and sequence encoder","[field_sequence_encoder]") +TEST_CASE("simple field and sequence encoder/decoder","[field_sequence_encoder_decoder]") { - fast_test_coding_case test_case; + fast_test_coding_case test_case; SECTION("encode fields") { @@ -156,9 +114,9 @@ TEST_CASE("simple field and sequence encoder","[field_sequence_encoder]") } } -TEST_CASE("simple field optional encoder","[field_optional_encoder]") +TEST_CASE("simple field optional encoder/decoder","[field_optional_encoder_decoder]") { - fast_test_coding_case test_case; + fast_test_coding_case test_case; SECTION("encode fields") { @@ -204,9 +162,9 @@ TEST_CASE("simple field optional encoder","[field_optional_encoder]") } } -TEST_CASE("simple field group optional encoder","[field_group_optional_encoder]") +TEST_CASE("simple field group optional encoder/decoder","[field_group_optional_encoder_decoder]") { - fast_test_coding_case test_case; + fast_test_coding_case test_case; SECTION("encode field") { diff --git a/tests/encoder_decoder_test_v2.cpp b/tests/encoder_decoder_test_v2.cpp index f46ba5e8..5261a673 100644 --- a/tests/encoder_decoder_test_v2.cpp +++ b/tests/encoder_decoder_test_v2.cpp @@ -1,54 +1,16 @@ #include "catch.hpp" #include -#include -#include +#include "fast_test_coding_case_v2.hpp" #include "byte_stream.h" #include "simple12.h" -namespace -{ -class fast_test_coding_case_v2 -{ - public: - fast_test_coding_case_v2(): - encoder_v2_(simple12::description()), - decoder_v2_(simple12::description()) - {} - - bool - encoding(const mfast::message_cref& msg_ref, const byte_stream& result, bool reset=false) - { - const int buffer_size = 128; - char buffer[buffer_size]; - - std::size_t encoded_size = encoder_v2_.encode(msg_ref, buffer, buffer_size, reset); - if (result == byte_stream(buffer, encoded_size)) - return true; - - INFO( "Got \"" << byte_stream(buffer, encoded_size) << "\" instead." ); - return false; - } - - bool - decoding(const byte_stream& bytes, const mfast::message_cref& result, bool reset=false) - { - const char* first = bytes.data(); - mfast::message_cref msg = decoder_v2_.decode(first, first+bytes.size(), reset); +using namespace test::coding; - return (msg == result); - } - - private: - mfast::fast_encoder_v2 encoder_v2_; - mfast::fast_decoder_v2<0> decoder_v2_; -}; -} - -TEST_CASE("simple field and sequence optional encoder_v2","[field_sequence_optional_sequence_encoder_v2]") +TEST_CASE("simple field and sequence optional encoder_v2/decoder_v2","[field_sequence_optional_sequence_encoder_v2_decoder2]") { - fast_test_coding_case_v2 test_case; + fast_test_coding_case_v2 test_case; SECTION("encode field") { @@ -106,9 +68,9 @@ TEST_CASE("simple field and sequence optional encoder_v2","[field_sequence_optio } } -TEST_CASE("simple field encoder_v2","[field_optional_encoder_v2]") +TEST_CASE("simple field encoder_v2/decoder_v2","[field_optional_encoder_v2]") { - fast_test_coding_case_v2 test_case; + fast_test_coding_case_v2 test_case; SECTION("encode fields") { @@ -126,9 +88,9 @@ TEST_CASE("simple field encoder_v2","[field_optional_encoder_v2]") } } -TEST_CASE("simple field and sequence encoder_v2","[field_sequence_encoder_v2]") +TEST_CASE("simple field and sequence encoder_v2/decoder_v2","[field_sequence_encoder_v2_decoder_v2]") { - fast_test_coding_case_v2 test_case; + fast_test_coding_case_v2 test_case; SECTION("encode fields") { @@ -152,9 +114,9 @@ TEST_CASE("simple field and sequence encoder_v2","[field_sequence_encoder_v2]") } } -TEST_CASE("simple field optional encoder_v2","[field__optional_encoder_v2]") +TEST_CASE("simple field optional encoder_v2/decoder_v2","[field__optional_encoder_v2_decoder_v2]") { - fast_test_coding_case_v2 test_case; + fast_test_coding_case_v2 test_case; SECTION("encode fields") { @@ -200,25 +162,20 @@ TEST_CASE("simple field optional encoder_v2","[field__optional_encoder_v2]") } } -TEST_CASE("simple field group optional encoder_v2","[field_group_optional_encoder_v2]") +TEST_CASE("simple field group optional encoder_v2/decoder_v2","[field_group_optional_encoder_v2_decoder_v2]") { - fast_test_coding_case_v2 test_case; + fast_test_coding_case_v2 test_case; SECTION("encode field") { simple12::Test_5 test_5; simple12::Test_5_mref test_5_mref = test_5.mref(); test_5_mref.set_field_5_1().as(1); - // Error: Expecting value is \xE0 : 1110 + // \xE0 : 1110 : OK // 1 : Stop Bit. // 1 : Set Template Id. // 1 : Set Field field_5_1 // 0 : Not Set test_5_group - // Get value: F0, expecting E0 - // 1 : Stop Bit. - // 1 : Set Template Id. - // 1 : Set Field field_5_1 - // 1 : Set test_5_group -> False! REQUIRE(test_case.encoding(test_5.cref(),"\xE0\x85\x82",true)); REQUIRE(test_case.decoding("\xE0\x85\x82",test_5.cref(),true)); } diff --git a/tests/fast_test_coding_case.hpp b/tests/fast_test_coding_case.hpp new file mode 100644 index 00000000..0a70a07f --- /dev/null +++ b/tests/fast_test_coding_case.hpp @@ -0,0 +1,52 @@ +#pragma once + +#include +#include +#include +#include "byte_stream.h" + +namespace test +{ +namespace coding +{ +template +class fast_test_coding_case +{ + public: + fast_test_coding_case() + { + encoder_.include({DESC::instance()}); + decoder_.include({DESC::instance()}); + } + + bool encoding(const mfast::message_cref& msg_ref, const byte_stream& result, bool reset=false) + { + const int buffer_size = 128; + char buffer[buffer_size]; + + std::size_t encoded_size = encoder_.encode(msg_ref, + buffer, + buffer_size, + reset); + + if (result == byte_stream(buffer, encoded_size)) + return true; + + INFO( "Got \"" << byte_stream(buffer, encoded_size) << "\" instead." ); + return false; + } + + bool decoding(const byte_stream& bytes, const mfast::message_cref& result, bool reset=false) + { + const char* first = bytes.data(); + mfast::message_cref msg = decoder_.decode(first, first+bytes.size(), reset); + + return (msg == result); + } + + private: + mfast::fast_encoder encoder_; + mfast::fast_decoder decoder_; +}; +} // namespace coding +} // namespace test diff --git a/tests/fast_test_coding_case_v2.hpp b/tests/fast_test_coding_case_v2.hpp new file mode 100644 index 00000000..be06cf30 --- /dev/null +++ b/tests/fast_test_coding_case_v2.hpp @@ -0,0 +1,49 @@ +#pragma once + +#include +#include +#include +#include "byte_stream.h" + +namespace test +{ +namespace coding +{ +template +class fast_test_coding_case_v2 +{ + public: + fast_test_coding_case_v2(): + encoder_v2_(DESC::instance()), + decoder_v2_(DESC::instance()) + {} + + bool + encoding(const mfast::message_cref& msg_ref, const byte_stream& result, bool reset=false) + { + const int buffer_size = 128; + char buffer[buffer_size]; + + std::size_t encoded_size = encoder_v2_.encode(msg_ref, buffer, buffer_size, reset); + if (result == byte_stream(buffer, encoded_size)) + return true; + + INFO( "Got \"" << byte_stream(buffer, encoded_size) << "\" instead." ); + return false; + } + + bool + decoding(const byte_stream& bytes, const mfast::message_cref& result, bool reset=false) + { + const char* first = bytes.data(); + mfast::message_cref msg = decoder_v2_.decode(first, first+bytes.size(), reset); + + return (msg == result); + } + + private: + mfast::fast_encoder_v2 encoder_v2_; + mfast::fast_decoder_v2<0> decoder_v2_; +}; +} // namespace coding +} // namespace test From 5dc37624336bed33947cb63fc014a74fc09371c9 Mon Sep 17 00:00:00 2001 From: Roberto Martin Fantini Date: Fri, 28 Feb 2025 16:40:45 +0100 Subject: [PATCH 2/4] Revert the change which control the pointer --- src/mfast/aggregate_ref.h | 1 - src/mfast/ext_ref.h | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/mfast/aggregate_ref.h b/src/mfast/aggregate_ref.h index 676a5ef5..9d03c432 100644 --- a/src/mfast/aggregate_ref.h +++ b/src/mfast/aggregate_ref.h @@ -57,7 +57,6 @@ class aggregate_cref { this->instruction_ = other.instruction_; this->storage_array_ = other.storage_array_; } - bool content() const { return storage_array_->of_group.content_ != nullptr; } class iterator : public boost::iterator_facade explicit ext_cref(const field_cref &base) : base_(base) {} explicit ext_cref(const aggregate_cref &base) : base_(base) {} cref_type get() const { return base_; } - bool present() const { return !this->optional() || base_.content(); } + bool present() const { return !this->optional() || base_.present(); } private: cref_type base_; From 673fa54df1c20877a5056028b85050241371e2f9 Mon Sep 17 00:00:00 2001 From: Roberto Martin Fantini Date: Fri, 28 Feb 2025 18:01:52 +0100 Subject: [PATCH 3/4] More tests in the group encoder --- tests/CMakeLists.txt | 4 +++ tests/group_encoder_decoder.cpp | 42 ++++++++++++++++++++++++++++++ tests/group_encoder_decoder_v2.cpp | 42 ++++++++++++++++++++++++++++++ tests/simple13.xml | 32 +++++++++++++++++++++++ 4 files changed, 120 insertions(+) create mode 100644 tests/group_encoder_decoder.cpp create mode 100644 tests/group_encoder_decoder_v2.cpp create mode 100644 tests/simple13.xml diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 3f652e20..e98473c1 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -21,6 +21,7 @@ FASTTYPEGEN_TARGET(simple_types9 simple9.xml) FASTTYPEGEN_TARGET(simple_types10 simple10.xml) FASTTYPEGEN_TARGET(simple_types11 simple11.xml) FASTTYPEGEN_TARGET(simple_types12 simple12.xml) +FASTTYPEGEN_TARGET(simple_types13 simple13.xml) FASTTYPEGEN_TARGET(test_types1 test1.xml test2.xml) @@ -41,6 +42,8 @@ add_executable (mfast_test encoder_decoder_test.cpp encoder_decoder_test_v2.cpp field_comparator_test.cpp + group_encoder_decoder_v2.cpp + group_encoder_decoder.cpp coder_test.cpp value_storage_test.cpp ${FASTTYPEGEN_test_types1_OUTPUTS} @@ -60,6 +63,7 @@ add_executable (mfast_test ${FASTTYPEGEN_simple_types10_OUTPUTS} ${FASTTYPEGEN_simple_types11_OUTPUTS} ${FASTTYPEGEN_simple_types12_OUTPUTS} + ${FASTTYPEGEN_simple_types13_OUTPUTS} fast_type_gen_test.cpp dictionary_builder_test.cpp json_test.cpp diff --git a/tests/group_encoder_decoder.cpp b/tests/group_encoder_decoder.cpp new file mode 100644 index 00000000..00d19977 --- /dev/null +++ b/tests/group_encoder_decoder.cpp @@ -0,0 +1,42 @@ +#include "catch.hpp" +#include + +#include "fast_test_coding_case.hpp" +#include "byte_stream.h" + +#include "simple13.h" + +using namespace test::coding; + +TEST_CASE("only field without group encoder/decoder","[field_without_group_encoder_decoder]") +{ + fast_test_coding_case test_case; + + simple13::Test_1 test_1; + simple13::Test_1_mref test_1_mref = test_1.mref(); + + test_1_mref.set_field_1_1().as(30); + test_1_mref.set_field_1_2().as(40); + + REQUIRE(test_case.encoding(test_1.cref(),"\xC0\x81\x9F\xA8",true)); + REQUIRE(test_case.decoding("\xC0\x81\x9F\xA8",test_1.cref(),true)); +} + +TEST_CASE("field with group encoder/decoder","[field_without_group_encoder_decoder]") +{ + fast_test_coding_case test_case; + + simple13::Test_1 test_1; + simple13::Test_1_mref test_1_mref = test_1.mref(); + + test_1_mref.set_field_1_1().as(30); + test_1_mref.set_field_1_2().as(40); + + auto group_1 = test_1_mref.set_group_1(); + group_1.set_field_1_4().as(10); + + REQUIRE(test_case.encoding(test_1.cref(),"\xE0\x81\x9F\xA8\x80\x8B",true)); + REQUIRE(test_case.decoding("\xE0\x81\x9F\xA8\x80\x8B",test_1.cref(),true)); +} + + diff --git a/tests/group_encoder_decoder_v2.cpp b/tests/group_encoder_decoder_v2.cpp new file mode 100644 index 00000000..1c5a624e --- /dev/null +++ b/tests/group_encoder_decoder_v2.cpp @@ -0,0 +1,42 @@ +#include "catch.hpp" +#include + +#include "fast_test_coding_case_v2.hpp" +#include "byte_stream.h" + +#include "simple13.h" + +using namespace test::coding; + +TEST_CASE("only field without group encoder_v2/decoder_v2","[field_without_group_encoder_v2_decoder_v2]") +{ + fast_test_coding_case_v2 test_case; + + simple13::Test_1 test_1; + simple13::Test_1_mref test_1_mref = test_1.mref(); + + test_1_mref.set_field_1_1().as(30); + test_1_mref.set_field_1_2().as(40); + + REQUIRE(test_case.encoding(test_1.cref(),"\xC0\x81\x9F\xA8",true)); + REQUIRE(test_case.decoding("\xC0\x81\x9F\xA8",test_1.cref(),true)); +} + +TEST_CASE("field with group encoder_v2/decoder_v2","[field_without_group_encoder_v2_decoder_v2]") +{ + fast_test_coding_case_v2 test_case; + + simple13::Test_1 test_1; + simple13::Test_1_mref test_1_mref = test_1.mref(); + + test_1_mref.set_field_1_1().as(30); + test_1_mref.set_field_1_2().as(40); + + auto group_1 = test_1_mref.set_group_1(); + group_1.set_field_1_4().as(10); + + REQUIRE(test_case.encoding(test_1.cref(),"\xE0\x81\x9F\xA8\x80\x8B",true)); + REQUIRE(test_case.decoding("\xE0\x81\x9F\xA8\x80\x8B",test_1.cref(),true)); +} + + diff --git a/tests/simple13.xml b/tests/simple13.xml new file mode 100644 index 00000000..8f155d5b --- /dev/null +++ b/tests/simple13.xml @@ -0,0 +1,32 @@ + + + + + + From 3809d547d428acc53677a490bf08965f446a2abe Mon Sep 17 00:00:00 2001 From: Roberto Martin Fantini Date: Mon, 3 Mar 2025 19:28:39 +0100 Subject: [PATCH 4/4] Add code changes to support the optional group for the encoder V2 --- src/fast_type_gen/inl_gen.cpp | 44 ++++++++++++++++++++++++++++------- src/mfast/ext_ref.h | 8 ++++++- src/mfast/value_storage.h | 1 + 3 files changed, 43 insertions(+), 10 deletions(-) diff --git a/src/fast_type_gen/inl_gen.cpp b/src/fast_type_gen/inl_gen.cpp index 1f4452d0..6fabeae5 100644 --- a/src/fast_type_gen/inl_gen.cpp +++ b/src/fast_type_gen/inl_gen.cpp @@ -25,11 +25,13 @@ std::string get_properties_type(const Instruction *inst) { using namespace mfast; struct ext_cref_type_getter : mfast::field_instruction_visitor { std::stringstream out_; + bool is_group_type_ = false; public: ext_cref_type_getter() {} std::string get() { return out_.str(); } + bool group_type() { return is_group_type_; } virtual void visit(const int32_field_instruction *inst, void *) override { out_ << "ext_cref"; + is_group_type_ = true; } virtual void visit(const sequence_field_instruction *inst, void *) override; @@ -132,6 +135,12 @@ std::string get_ext_cref_type(const field_instruction *inst) { return getter.get(); } +bool is_group_type(const field_instruction *inst) { + ext_cref_type_getter getter; + inst->accept(getter, nullptr); + return getter.group_type(); +} + void ext_cref_type_getter::visit(const sequence_field_instruction *inst, void *) { const uint32_field_instruction *length_inst = inst->length_instruction(); @@ -473,9 +482,17 @@ void inl_gen::visit(const mfast::group_field_instruction *inst, void *pIndex) { for (std::size_t i = 0; i < inst->subinstructions().size(); ++i) { const field_instruction *subinst = inst->subinstructions()[i]; - ; - out_ << " visitor.visit(" << get_ext_cref_type(subinst) << " ((*this)[" - << i << "]) );\n"; + + if (is_group_type(subinst) && subinst->optional()) + { + out_ << " {\n" + << " " << get_ext_cref_type(subinst) << " ext_cref_group((*this)[" << i << "]);\n" + << " ext_cref_group.set_group_present(this->field_storage(" << i << ")->is_present());\n" + << " visitor.visit(ext_cref_group);\n" + << " }\n"; + } + else + out_ << " visitor.visit(" << get_ext_cref_type(subinst) << " ((*this)[" << i << "]) );\n"; } out_ << "}\n\n"; @@ -508,6 +525,7 @@ void inl_gen::visit(const mfast::group_field_instruction *inst, void *pIndex) { for (std::size_t i = 0; i < inst->subinstructions().size(); ++i) { const field_instruction *subinst = inst->subinstructions()[i]; + out_ << " visitor.visit(" << get_ext_mref_type(subinst) << " ((*this)[" << i << "]) );\n"; } @@ -662,7 +680,7 @@ void inl_gen::visit(const mfast::sequence_field_instruction *inst, for (std::size_t i = 0; i < inst->subinstructions().size(); ++i) { const field_instruction *subinst = inst->subinstructions()[i]; - ; + out_ << " visitor.visit(" << get_ext_cref_type(subinst) << " ((*this)[" << i << "]) );\n"; } @@ -677,7 +695,7 @@ void inl_gen::visit(const mfast::sequence_field_instruction *inst, for (std::size_t i = 0; i < inst->subinstructions().size(); ++i) { const field_instruction *subinst = inst->subinstructions()[i]; - ; + out_ << " visitor.visit(" << get_ext_mref_type(subinst) << " ((*this)[" << i << "]) );\n"; } @@ -791,9 +809,17 @@ void inl_gen::visit(const mfast::template_instruction *inst, void *) { for (std::size_t i = 0; i < inst->subinstructions().size(); ++i) { const field_instruction *subinst = inst->subinstructions()[i]; - ; - out_ << " visitor.visit(" << get_ext_cref_type(subinst) << " ((*this)[" - << i << "]) );\n"; + + if (is_group_type(subinst) && subinst->optional()) + { + out_ << " {\n" + << " " << get_ext_cref_type(subinst) << " ext_cref_group((*this)[" << i << "]);\n" + << " ext_cref_group.set_group_present(this->field_storage(" << i << ")->is_present());\n" + << " visitor.visit(ext_cref_group);\n" + << " }\n"; + } + else + out_ << " visitor.visit(" << get_ext_cref_type(subinst) << " ((*this)[" << i << "]) );\n"; } out_ << "}\n\n"; @@ -842,7 +868,7 @@ void inl_gen::visit(const mfast::template_instruction *inst, void *) { for (std::size_t i = 0; i < inst->subinstructions().size(); ++i) { const field_instruction *subinst = inst->subinstructions()[i]; - ; + out_ << " visitor.visit(" << get_ext_mref_type(subinst) << " ((*this)[" << i << "]) );\n"; } diff --git a/src/mfast/ext_ref.h b/src/mfast/ext_ref.h index 9ecb4a90..9181e533 100644 --- a/src/mfast/ext_ref.h +++ b/src/mfast/ext_ref.h @@ -194,10 +194,13 @@ class ext_cref explicit ext_cref(const field_cref &base) : base_(base) {} explicit ext_cref(const aggregate_cref &base) : base_(base) {} cref_type get() const { return base_; } - bool present() const { return !this->optional() || base_.present(); } + bool present() const { return !this->optional() || group_present_; } + + void set_group_present(bool present) { group_present_ = present; } private: cref_type base_; + bool group_present_ = true; }; template @@ -212,8 +215,11 @@ class ext_cref cref_type get() const { return cref_type(aggregate_cref(base_)[0]); } bool present() const { return !this->optional() || base_.present(); } + void set_group_present(bool present) { group_present_ = present; } + private: field_cref base_; + bool group_present_ = true; }; /////////////////////////////////////////////////////////////// diff --git a/src/mfast/value_storage.h b/src/mfast/value_storage.h index 4831cb42..8522336a 100644 --- a/src/mfast/value_storage.h +++ b/src/mfast/value_storage.h @@ -107,6 +107,7 @@ union MFAST_EXPORT value_storage { void defined(bool v) { of_array.defined_bit_ = v; } bool is_empty() const { return of_array.len_ == 0; } void present(bool p) { of_array.len_ = p; } + bool is_present() const { return of_group.present_ == 1; } uint32_t array_length() const { return of_array.len_ == 0 ? 0 : of_array.len_ - 1; };