[core] separate unytyped msg client #2028

rex-schilasky · 2025-02-14T10:17:53Z

Description

This pull request introduces several renaming and reorganization changes to improve clarity and consistency within the protobuf client interfaces:

Renaming Changes:

Typed Clients:
- eCAL::protobuf::CServiceClientTyped → eCAL::protobuf::CServiceClient
- eCAL::protobuf::CClientInstanceTyped → eCAL::protobuf::CClientInstance
Untyped Clients:
- eCAL::protobuf::CServiceClient → eCAL::protobuf::CServiceClientUntyped
- eCAL::protobuf::CClientInstance → eCAL::protobuf::CClientInstanceUntyped

Reorganization:

The untyped versions have been moved into dedicated header files:
- client_untyped.h
- client_untyped_instance.h

Additional Improvements:

Updated and harmonized the comments and documentation in all four files.
The untyped response versions are retained for now to simplify migration and reduce testing effort for current applications and samples based on untyped responses.

These changes provide a clearer separation between typed and untyped client interfaces, easing future maintenance and migration efforts.

CClientInstanceTyped renamed to CClientInstance CCServiceClientTyped renamed to CServiceClient

… easier migration adapted comments

KerstinKeller

That's better!

github-actions

clang-tidy made some suggestions

github-actions · 2025-02-14T10:39:39Z

app/rec/rec_server_cli/src/commands/command.h


-        static eCAL::rec::Error CallRemoteEcalrecService(const std::shared_ptr<eCAL::protobuf::CServiceClient<eCAL::pb::rec_server::EcalRecServerService>>& remote_ecalsys_service
+        static eCAL::rec::Error CallRemoteEcalrecService(const std::shared_ptr<eCAL::protobuf::CServiceClientUntyped<eCAL::pb::rec_server::EcalRecServerService>>& remote_ecalsys_service


warning: function 'eCAL::rec_cli::command::Command::CallRemoteEcalrecService' has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]

static eCAL::rec::Error CallRemoteEcalrecService(const std::shared_ptr<eCAL::protobuf::CServiceClientUntyped<eCAL::pb::rec_server::EcalRecServerService>>& remote_ecalsys_service ^

Additional context

app/rec/rec_server_cli/src/commands/command.cpp:75: the definition seen here

eCAL::rec::Error Command::CallRemoteEcalrecService(const std::shared_ptr<eCAL::protobuf::CServiceClientUntyped<eCAL::pb::rec_server::EcalRecServerService>>& remote_ecalrec_service ^

app/rec/rec_server_cli/src/commands/command.h:63: differing parameters are named here: ('remote_ecalsys_service'), in definition: ('remote_ecalrec_service')

static eCAL::rec::Error CallRemoteEcalrecService(const std::shared_ptr<eCAL::protobuf::CServiceClientUntyped<eCAL::pb::rec_server::EcalRecServerService>>& remote_ecalsys_service ^

github-actions · 2025-02-14T10:39:39Z

app/rec/rec_server_cli/src/ecal_rec_server_cli.cpp

@@ -119,7 +119,7 @@ bool IsBuiltInFtpServerBusy(bool print_status = false);

 // Rec Sever instance and rec_server_service. We will only use one of those, depending on the remote-control setting
 std::shared_ptr<eCAL::rec_server::RecServer>                                                rec_server_instance;
-std::shared_ptr<eCAL::protobuf::CServiceClient<eCAL::pb::rec_server::EcalRecServerService>> remote_rec_server_service;
+std::shared_ptr<eCAL::protobuf::CServiceClientUntyped<eCAL::pb::rec_server::EcalRecServerService>> remote_rec_server_service;


warning: variable 'remote_rec_server_service' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

std::shared_ptr<eCAL::protobuf::CServiceClientUntyped<eCAL::pb::rec_server::EcalRecServerService>> remote_rec_server_service; ^

github-actions · 2025-02-14T10:39:40Z

app/rec/rec_tests/rec_rpc_tests/src/external_ecal_rec.h

@@ -22,7 +22,7 @@
 #pragma warning(push)
 #pragma warning(disable: 4100 4127 4146 4505 4800 4189 4592) // disable proto warnings 
 #endif
-#include <ecal/msg/protobuf/client.h>
+#include <ecal/msg/protobuf/client_untyped.h>


warning: 'ecal/msg/protobuf/client_untyped.h' file not found [clang-diagnostic-error]

#include <ecal/msg/protobuf/client_untyped.h> ^

github-actions · 2025-02-14T10:39:40Z

samples/cpp/services/ecalplayer_client/src/ecalplayer_client.cpp

@@ -92,7 +92,7 @@ int main()
  eCAL::Initialize("ecalplayer client");

  // create player service client
-  eCAL::protobuf::CServiceClient<eCAL::pb::play::EcalPlayService> player_service;
+  eCAL::protobuf::CServiceClientUntyped<eCAL::pb::play::EcalPlayService> player_service;


warning: variable 'player_service' of type 'eCAL::protobuf::CServiceClientUntypedeCAL::pb::play::EcalPlayService' can be declared 'const' [misc-const-correctness]

Suggested change

eCAL::protobuf::CServiceClientUntyped<eCAL::pb::play::EcalPlayService> player_service;

eCAL::protobuf::CServiceClientUntyped<eCAL::pb::play::EcalPlayService> const player_service;

github-actions · 2025-02-14T10:39:40Z

samples/cpp/services/ecalsys_client/src/ecalsys_client.cpp

@@ -69,7 +69,7 @@ int main()
  eCAL::Initialize("ecalsys client");

  // create player service client
-  eCAL::protobuf::CServiceClient<eCAL::pb::sys::Service> sys_service;
+  eCAL::protobuf::CServiceClientUntyped<eCAL::pb::sys::Service> sys_service;


warning: variable 'sys_service' of type 'eCAL::protobuf::CServiceClientUntypedeCAL::pb::sys::Service' can be declared 'const' [misc-const-correctness]

Suggested change

eCAL::protobuf::CServiceClientUntyped<eCAL::pb::sys::Service> sys_service;

eCAL::protobuf::CServiceClientUntyped<eCAL::pb::sys::Service> const sys_service;

github-actions · 2025-02-14T10:39:41Z

serialization/protobuf/protobuf/include/ecal/msg/protobuf/client_untyped.h

+
+#pragma once
+
+#include <ecal/service/client.h>


warning: 'ecal/service/client.h' file not found [clang-diagnostic-error]

^

github-actions · 2025-02-14T10:39:41Z

serialization/protobuf/protobuf/include/ecal/msg/protobuf/client_untyped_instance.h

+
+#pragma once
+
+#include <ecal/service/client_instance.h>


warning: 'ecal/service/client_instance.h' file not found [clang-diagnostic-error]

#include <ecal/service/client_instance.h> ^

github-actions · 2025-02-14T10:39:41Z

serialization/protobuf/protobuf/include/ecal/msg/protobuf/client_untyped_instance.h

+    {
+    public:
+      // Constructors
+      CClientInstanceUntyped(eCAL::CClientInstance&& base_instance_) noexcept


warning: constructor does not initialize these fields: m_instance [cppcoreguidelines-pro-type-member-init]

serialization/protobuf/protobuf/include/ecal/msg/protobuf/client_untyped_instance.h:153:

- eCAL::CClientInstance m_instance; + eCAL::CClientInstance m_instance{};

github-actions · 2025-02-14T10:39:41Z

serialization/protobuf/protobuf/include/ecal/msg/protobuf/client_untyped_instance.h

+    {
+    public:
+      // Constructors
+      CClientInstanceUntyped(eCAL::CClientInstance&& base_instance_) noexcept


warning: rvalue reference parameter 'base_instance_' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]

CClientInstanceUntyped(eCAL::CClientInstance&& base_instance_) noexcept ^

github-actions · 2025-02-14T10:39:41Z

serialization/protobuf/protobuf/include/ecal/msg/protobuf/client_untyped_instance.h

+      {
+      }
+
+      CClientInstanceUntyped(const SEntityId& entity_id_,


warning: constructor does not initialize these fields: m_instance [cppcoreguidelines-pro-type-member-init]

CClientInstanceUntyped(const SEntityId& entity_id_, ^

rex-schilasky added 4 commits February 13, 2025 16:07

untyped versions of protobuf CClientInstance and CServiceClient removed

a245831

CClientInstanceTyped renamed to CClientInstance CCServiceClientTyped renamed to CServiceClient

introduced again CServiceClientUntyped and CClientInstanceUntyped for…

f4b63f3

… easier migration adapted comments

adapted all apps to use CServiceClientUntyped

72e9efd

adapted all samples to use CServiceClientUntyped

cf23f1d

rex-schilasky added the cherry-pick-to-NONE Don't cherry-pick these changes label Feb 14, 2025

rex-schilasky added this to the eCAL 6.0 milestone Feb 14, 2025

KerstinKeller approved these changes Feb 14, 2025

View reviewed changes

github-actions bot reviewed Feb 14, 2025

View reviewed changes

rex-schilasky merged commit 0ecfbee into master Feb 14, 2025
27 of 28 checks passed

rex-schilasky deleted the core/separate-unytyped-msg-client branch February 14, 2025 11:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core] separate unytyped msg client #2028

[core] separate unytyped msg client #2028

rex-schilasky commented Feb 14, 2025

KerstinKeller left a comment

github-actions bot left a comment

github-actions bot Feb 14, 2025

github-actions bot Feb 14, 2025

github-actions bot Feb 14, 2025

github-actions bot Feb 14, 2025

github-actions bot Feb 14, 2025

github-actions bot Feb 14, 2025

github-actions bot Feb 14, 2025

github-actions bot Feb 14, 2025

github-actions bot Feb 14, 2025

github-actions bot Feb 14, 2025


		static eCAL::rec::Error CallRemoteEcalrecService(const std::shared_ptr<eCAL::protobuf::CServiceClient<eCAL::pb::rec_server::EcalRecServerService>>& remote_ecalsys_service
		static eCAL::rec::Error CallRemoteEcalrecService(const std::shared_ptr<eCAL::protobuf::CServiceClientUntyped<eCAL::pb::rec_server::EcalRecServerService>>& remote_ecalsys_service

	eCAL::protobuf::CServiceClientUntyped<eCAL::pb::play::EcalPlayService> player_service;
	eCAL::protobuf::CServiceClientUntyped<eCAL::pb::play::EcalPlayService> const player_service;

[core] separate unytyped msg client #2028

[core] separate unytyped msg client #2028

Conversation

rex-schilasky commented Feb 14, 2025

Description

KerstinKeller left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Feb 14, 2025

Choose a reason for hiding this comment

github-actions bot Feb 14, 2025

Choose a reason for hiding this comment

github-actions bot Feb 14, 2025

Choose a reason for hiding this comment

github-actions bot Feb 14, 2025

Choose a reason for hiding this comment

github-actions bot Feb 14, 2025

Choose a reason for hiding this comment

github-actions bot Feb 14, 2025

Choose a reason for hiding this comment

github-actions bot Feb 14, 2025

Choose a reason for hiding this comment

github-actions bot Feb 14, 2025

Choose a reason for hiding this comment

github-actions bot Feb 14, 2025

Choose a reason for hiding this comment

github-actions bot Feb 14, 2025

Choose a reason for hiding this comment