From 9ad7ff666854c75bdd36305764b88f1e3da9ecad Mon Sep 17 00:00:00 2001 From: Kerstin Keller Date: Tue, 26 Nov 2024 14:28:39 +0100 Subject: [PATCH 1/2] [bugfix] registration logic to throw away samples fixed so it doesn't throw away things if host_group_name is set. --- .../registration_attribute_builder.cpp | 11 +--- .../attributes/registration_attributes.h | 1 + .../attributes/sample_applier_attributes.h | 1 + .../sample_applier_attribute_builder.cpp | 1 + .../ecal_registration_sample_applier.cpp | 52 ++++++++++++------- .../ecal_registration_sample_applier.h | 3 ++ 6 files changed, 42 insertions(+), 27 deletions(-) diff --git a/ecal/core/src/config/builder/registration_attribute_builder.cpp b/ecal/core/src/config/builder/registration_attribute_builder.cpp index d907047429..45d1032d41 100644 --- a/ecal/core/src/config/builder/registration_attribute_builder.cpp +++ b/ecal/core/src/config/builder/registration_attribute_builder.cpp @@ -30,15 +30,8 @@ namespace eCAL attr.refresh = reg_config_.registration_refresh; attr.network_enabled = reg_config_.network_enabled; attr.loopback = reg_config_.loopback; - - if (reg_config_.host_group_name.empty()) - { - attr.host_group_name = eCAL::Process::GetHostName(); - } - else - { - attr.host_group_name = reg_config_.host_group_name; - } + attr.host_name = eCAL::Process::GetHostName(); + attr.host_group_name = reg_config_.host_group_name; attr.process_id = process_id_; diff --git a/ecal/core/src/registration/config/attributes/registration_attributes.h b/ecal/core/src/registration/config/attributes/registration_attributes.h index f19db2b330..1262647659 100644 --- a/ecal/core/src/registration/config/attributes/registration_attributes.h +++ b/ecal/core/src/registration/config/attributes/registration_attributes.h @@ -57,6 +57,7 @@ namespace eCAL bool shm_enabled; bool udp_enabled; unsigned int refresh; + std::string host_name; std::string host_group_name; int process_id; diff --git a/ecal/core/src/registration/config/attributes/sample_applier_attributes.h b/ecal/core/src/registration/config/attributes/sample_applier_attributes.h index efec45de90..25750dbdb2 100644 --- a/ecal/core/src/registration/config/attributes/sample_applier_attributes.h +++ b/ecal/core/src/registration/config/attributes/sample_applier_attributes.h @@ -32,6 +32,7 @@ namespace eCAL bool network_enabled; bool loopback; std::string host_group_name; + std::string host_name; int process_id; }; } diff --git a/ecal/core/src/registration/config/builder/sample_applier_attribute_builder.cpp b/ecal/core/src/registration/config/builder/sample_applier_attribute_builder.cpp index cab74eef50..596d4aeb33 100644 --- a/ecal/core/src/registration/config/builder/sample_applier_attribute_builder.cpp +++ b/ecal/core/src/registration/config/builder/sample_applier_attribute_builder.cpp @@ -31,6 +31,7 @@ namespace eCAL sample_applier_attr.network_enabled = attr_.network_enabled; sample_applier_attr.loopback = attr_.loopback; + sample_applier_attr.host_name = attr_.host_name; sample_applier_attr.host_group_name = attr_.host_group_name; sample_applier_attr.process_id = attr_.process_id; diff --git a/ecal/core/src/registration/ecal_registration_sample_applier.cpp b/ecal/core/src/registration/ecal_registration_sample_applier.cpp index 0db73c2030..af9c240faa 100644 --- a/ecal/core/src/registration/ecal_registration_sample_applier.cpp +++ b/ecal/core/src/registration/ecal_registration_sample_applier.cpp @@ -57,15 +57,42 @@ namespace eCAL bool CSampleApplier::IsHostGroupMember(const Registration::Sample& sample_) const { - std::string host_group_name; - const std::string host_name = sample_.identifier.host_name; + // When are we in the same hostgroup? + // Either we are on the same host + // Or the hgname attribute of the sample are identical + + if (IsSameHost(sample_)) + return true; + + if (IsSameHostGroup(sample_)) + return true; + + return false; + } + + bool CSampleApplier::IsSameProcess(const Registration::Sample& sample_) const + { + // is this actually sufficient? should we also check host_name? + const int32_t pid = sample_.identifier.process_id; + return pid == m_attributes.process_id; + } + + bool CSampleApplier::IsSameHost(const Registration::Sample& sample_) const + { + const std::string& sample_host_name = sample_.identifier.host_name; + return (sample_host_name == m_attributes.host_name); + } + + bool CSampleApplier::IsSameHostGroup(const Registration::Sample& sample_) const + { + std::string sample_host_group_name; switch (sample_.cmd_type) { case bct_reg_publisher: case bct_unreg_publisher: case bct_reg_subscriber: case bct_unreg_subscriber: - host_group_name = sample_.topic.hgname; + sample_host_group_name = sample_.topic.hgname; break; case bct_reg_service: case bct_unreg_service: @@ -79,25 +106,14 @@ namespace eCAL break; } - const std::string& sample_host_group_name = host_group_name.empty() ? host_name : host_group_name; - - if (sample_host_group_name.empty() || m_attributes.host_group_name.empty()) - return false; - if (sample_host_group_name != m_attributes.host_group_name) - return false; - - return true; - } - - bool CSampleApplier::IsSameProcess(const Registration::Sample& sample_) const - { - // is this actually sufficient? should we also check host_name? - const int32_t pid = sample_.identifier.process_id; - return pid == m_attributes.process_id; + return (sample_host_group_name == m_attributes.host_group_name); } bool CSampleApplier::AcceptRegistrationSample(const Registration::Sample& sample_) { + // Under wich circumstances do we apply samples, so we can filter ahead of time + // otherwise we could apply them to + // check if the sample is from the same host group if (IsHostGroupMember(sample_)) { diff --git a/ecal/core/src/registration/ecal_registration_sample_applier.h b/ecal/core/src/registration/ecal_registration_sample_applier.h index 7b313de4a7..5b340340d7 100644 --- a/ecal/core/src/registration/ecal_registration_sample_applier.h +++ b/ecal/core/src/registration/ecal_registration_sample_applier.h @@ -55,6 +55,9 @@ namespace eCAL private: bool IsSameProcess(const Registration::Sample& sample_) const; + bool IsSameHost(const Registration::Sample& sample_) const; + bool IsSameHostGroup(const Registration::Sample& sample_) const; + bool IsHostGroupMember(const eCAL::Registration::Sample& sample_) const; bool AcceptRegistrationSample(const Registration::Sample& sample_); From 29d6e47143b6801f58d8f9b8508073f7f4f086d0 Mon Sep 17 00:00:00 2001 From: Kerstin Keller Date: Tue, 26 Nov 2024 15:33:45 +0100 Subject: [PATCH 2/2] Run Tests in different configuration combinations. --- .../src/registration_getclients.cpp | 94 ++++++++++-------- .../src/registration_getpublisherids.cpp | 8 ++ .../src/registration_getservices.cpp | 95 +++++++++++-------- .../src/registration_getsubscriberids.cpp | 8 ++ 4 files changed, 129 insertions(+), 76 deletions(-) diff --git a/ecal/tests/cpp/registration_test_public/src/registration_getclients.cpp b/ecal/tests/cpp/registration_test_public/src/registration_getclients.cpp index 8c3a09b067..41befcd8b4 100644 --- a/ecal/tests/cpp/registration_test_public/src/registration_getclients.cpp +++ b/ecal/tests/cpp/registration_test_public/src/registration_getclients.cpp @@ -25,14 +25,33 @@ enum { CMN_MONITORING_TIMEOUT_MS = (5000 + 100), CMN_REGISTRATION_REFRESH_MS = (1000) }; -TEST(core_cpp_registration_public, ClientExpiration) + +// struct to hold the test parameters +struct ClientsTestParams { - // initialize eCAL API - eCAL::Initialize(0, nullptr, "core_cpp_registration_public"); + eCAL::Configuration configuration; +}; - // enable loop back communication in the same process - eCAL::Util::EnableLoopback(true); +// test class that accepts TestParams as a parameter +class ClientsTestFixture : public ::testing::TestWithParam +{ +protected: + void SetUp() override + { + // set configuration from the test parameters + auto params = GetParam(); + eCAL::Initialize(params.configuration, "core_cpp_registration_public", eCAL::Init::All); + eCAL::Util::EnableLoopback(true); + } + void TearDown() override + { + // clean up + eCAL::Finalize(); + } +}; +TEST_P(ClientsTestFixture, ClientExpiration) +{ std::map client_info_map; // create simple client and let it expire @@ -83,19 +102,10 @@ TEST(core_cpp_registration_public, ClientExpiration) // check size EXPECT_EQ(client_info_map.size(), 0); - - // finalize eCAL API - eCAL::Finalize(); } -TEST(core_cpp_registration_public, ClientEqualQualities) +TEST_P(ClientsTestFixture, ClientEqualQualities) { - // initialize eCAL API - eCAL::Initialize(0, nullptr, "core_cpp_registration_public"); - - // enable loop back communication in the same process - eCAL::Util::EnableLoopback(true); - std::map client_info_map; // create 2 clients with the same quality of data type information @@ -181,19 +191,10 @@ TEST(core_cpp_registration_public, ClientEqualQualities) // check size EXPECT_EQ(client_info_map.size(), 0); - - // finalize eCAL API - eCAL::Finalize(); } -TEST(core_cpp_registration_public, ClientDifferentQualities) +TEST_P(ClientsTestFixture, ClientDifferentQualities) { - // initialize eCAL API - eCAL::Initialize(0, nullptr, "core_cpp_registration_public"); - - // enable loop back communication in the same process - eCAL::Util::EnableLoopback(true); - std::map client_info_map; // create 2 clients with different qualities of data type information @@ -258,19 +259,10 @@ TEST(core_cpp_registration_public, ClientDifferentQualities) // check size EXPECT_EQ(client_info_map.size(), 0); - - // finalize eCAL API - eCAL::Finalize(); } -TEST(core_cpp_registration_public, GetClientIDs) +TEST_P(ClientsTestFixture, GetClientIDs) { - // initialize eCAL API - eCAL::Initialize(0, nullptr, "core_cpp_registration_public"); - - // enable loop back communication in the same process - eCAL::Util::EnableLoopback(true); - // create simple client { // create client @@ -296,7 +288,33 @@ TEST(core_cpp_registration_public, GetClientIDs) EXPECT_EQ(service_method_info, info.info); } } - - // finalize eCAL API - eCAL::Finalize(); } + +INSTANTIATE_TEST_SUITE_P( + core_cpp_registration_public_clients, + ClientsTestFixture, + ::testing::Values( + ClientsTestParams{ []() { + // shm + eCAL::Configuration config; + config.registration.layer.shm.enable = true; + config.registration.layer.udp.enable = false; + return config; + }() }, + ClientsTestParams{ []() { + // shm + host group name + eCAL::Configuration config; + config.registration.layer.shm.enable = true; + config.registration.layer.udp.enable = false; + config.registration.host_group_name = "abc"; + return config; + }() }, + ClientsTestParams{ []() { + // udp + eCAL::Configuration config; + config.registration.layer.shm.enable = false; + config.registration.layer.udp.enable = true; + return config; + }() } + ) +); \ No newline at end of file diff --git a/ecal/tests/cpp/registration_test_public/src/registration_getpublisherids.cpp b/ecal/tests/cpp/registration_test_public/src/registration_getpublisherids.cpp index 18b5fa6146..3243d01dd7 100644 --- a/ecal/tests/cpp/registration_test_public/src/registration_getpublisherids.cpp +++ b/ecal/tests/cpp/registration_test_public/src/registration_getpublisherids.cpp @@ -161,6 +161,14 @@ INSTANTIATE_TEST_SUITE_P( config.registration.layer.udp.enable = false; return config; }() }, + TestParams{ 10, []() { + // shm + host group name + eCAL::Configuration config; + config.registration.layer.shm.enable = true; + config.registration.layer.udp.enable = false; + config.registration.host_group_name = "abc"; + return config; + }() }, TestParams{ 10, []() { // udp eCAL::Configuration config; diff --git a/ecal/tests/cpp/registration_test_public/src/registration_getservices.cpp b/ecal/tests/cpp/registration_test_public/src/registration_getservices.cpp index 195053c40c..2dbb9b4f3d 100644 --- a/ecal/tests/cpp/registration_test_public/src/registration_getservices.cpp +++ b/ecal/tests/cpp/registration_test_public/src/registration_getservices.cpp @@ -26,14 +26,33 @@ enum { CMN_REGISTRATION_REFRESH_MS = (1000) }; -TEST(core_cpp_registration_public, ServiceExpiration) +// struct to hold the test parameters +struct ServicesTestParams { - // initialize eCAL API - eCAL::Initialize(0, nullptr, "core_cpp_registration_public"); + eCAL::Configuration configuration; +}; - // enable loop back communication in the same process - eCAL::Util::EnableLoopback(true); +// test class that accepts TestParams as a parameter +class ServicesTestFixture : public ::testing::TestWithParam +{ +protected: + void SetUp() override + { + // set configuration from the test parameters + auto params = GetParam(); + eCAL::Initialize(params.configuration, "core_cpp_registration_public", eCAL::Init::All); + eCAL::Util::EnableLoopback(true); + } + void TearDown() override + { + // clean up + eCAL::Finalize(); + } +}; + +TEST_P(ServicesTestFixture, ServiceExpiration) +{ std::map service_info_map; // create simple service and let it expire @@ -80,19 +99,10 @@ TEST(core_cpp_registration_public, ServiceExpiration) // check size EXPECT_EQ(service_info_map.size(), 0); - - // finalize eCAL API - eCAL::Finalize(); } -TEST(core_cpp_registration_public, ServiceEqualQualities) +TEST_P(ServicesTestFixture, ServiceEqualQualities) { - // initialize eCAL API - eCAL::Initialize(0, nullptr, "core_cpp_registration_public"); - - // enable loop back communication in the same process - eCAL::Util::EnableLoopback(true); - std::map service_info_map; // create 2 services with the same quality of data type information @@ -170,19 +180,10 @@ TEST(core_cpp_registration_public, ServiceEqualQualities) // check size EXPECT_EQ(service_info_map.size(), 0); - - // finalize eCAL API - eCAL::Finalize(); } -TEST(core_cpp_registration_public, ServiceDifferentQualities) +TEST_P(ServicesTestFixture, ServiceDifferentQualities) { - // initialize eCAL API - eCAL::Initialize(0, nullptr, "core_cpp_registration_public"); - - // enable loop back communication in the same process - eCAL::Util::EnableLoopback(true); - std::map service_info_map; // create 2 services with different qualities of data type information @@ -239,19 +240,10 @@ TEST(core_cpp_registration_public, ServiceDifferentQualities) // check size EXPECT_EQ(service_info_map.size(), 0); - - // finalize eCAL API - eCAL::Finalize(); } -TEST(core_cpp_registration_public, GetServiceIDs) +TEST_P(ServicesTestFixture, GetServiceIDs) { - // initialize eCAL API - eCAL::Initialize(0, nullptr, "core_cpp_registration_public"); - - // enable loop back communication in the same process - eCAL::Util::EnableLoopback(true); - // create simple server { // create server @@ -285,7 +277,34 @@ TEST(core_cpp_registration_public, GetServiceIDs) EXPECT_EQ(service_method_info, info.info); } } - - // finalize eCAL API - eCAL::Finalize(); } + + +INSTANTIATE_TEST_SUITE_P( + core_cpp_registration_public_services, + ServicesTestFixture, + ::testing::Values( + ServicesTestParams{[]() { + // shm + eCAL::Configuration config; + config.registration.layer.shm.enable = true; + config.registration.layer.udp.enable = false; + return config; + }() }, + ServicesTestParams{ []() { + // shm + host group name + eCAL::Configuration config; + config.registration.layer.shm.enable = true; + config.registration.layer.udp.enable = false; + config.registration.host_group_name = "abc"; + return config; + }() }, + ServicesTestParams{[]() { + // udp + eCAL::Configuration config; + config.registration.layer.shm.enable = false; + config.registration.layer.udp.enable = true; + return config; + }() } + ) +); \ No newline at end of file diff --git a/ecal/tests/cpp/registration_test_public/src/registration_getsubscriberids.cpp b/ecal/tests/cpp/registration_test_public/src/registration_getsubscriberids.cpp index 28138fcbc0..ef73cc481b 100644 --- a/ecal/tests/cpp/registration_test_public/src/registration_getsubscriberids.cpp +++ b/ecal/tests/cpp/registration_test_public/src/registration_getsubscriberids.cpp @@ -161,6 +161,14 @@ INSTANTIATE_TEST_SUITE_P( config.registration.layer.udp.enable = false; return config; }() }, + TestParams{10, []() { + // shm + host group name + eCAL::Configuration config; + config.registration.layer.shm.enable = true; + config.registration.layer.udp.enable = false; + config.registration.host_group_name = "abc"; + return config; + }() }, TestParams{ 10, []() { // udp eCAL::Configuration config;