eCAL 6 API Review

[KerstinKeller]

Context

We need to review API for eCAL 6.

Proposal

eCAL Core API

ecal_util.h
Remove:

• EnableLoopback (set by Config) [core] Remove all instances of EnableLoopback as it can be set via … #1833
• PubShareType (set by Config) [core] Remove PubShareType / PubShareDescription (set via publisher c… #1834
• -> Remove also from Config / yaml ... @KerstinKeller
• PubShareDescription (set by Config) [core] Remove PubShareType / PubShareDescription (set via publisher c… #1834
• -> Remove also from Config / yaml ... @KerstinKeller
• SplitCombinedTopicType @KerstinKeller
• CombinedTopicEncodingAndType @KerstinKeller
• - Remove from Player ([play] Separate Encoding and Type in Player application. #1836)
• ShutdownCore ->remove, and ShutdownProcesses will shut down all processes @rex-schilasky

Review again: @Peguen

• ECAL_API std::string GeteCALConfigPath();
• ECAL_API std::string GeteCALUserSettingsPath();
• ECAL_API std::string GeteCALLogPath();
• ECAL_API std::string GeteCALActiveIniFile();

ecal_types.h @rex-schilasky

• SServiceId -> SServiceMethodId -> review carefully.
• STopicId -> ID part should probably be private (e.g. not accessible by user) @KerstinKeller

ecal_tlayer.h @KerstinKeller

• Remove: eSendMode, STLayer, [core] remove eSendMode and STLayer as configuration is now handled d… #1845

ecal_publisher.h / ecal_subscriber.h

• bool AddEventCallback(eCAL_Subscriber_Event type_, SubEventCallbackT callback_); -> Review (@rex-schilasky )
• Data members should be private

ecal_registration.h

• Remove DescQualityFlags from functions
  (to review again @KerstinKeller @rex-schilasky )

ecal_process.h @hannemn

• Remove ECAL_API int AddRegistrationCallback(enum eCAL_Registration_Event event_, const RegistrationCallbackT& callback_);
• Remove ECAL_API int RemRegistrationCallback(enum eCAL_Registration_Event event_);

eCAL API Redesign: @rex-schilasky

• Client:
  • New API on master
  • Implement GetServiceID
  • Provide/Use DataTypeInformation proto field on registration layer
  • Provide new MethodCallbackT type using SServiceMethodInformation instead MethodCallbackT = std::function<int(const std::string& method_, const std::string& req_type_, const std::string& resp_type_, const std::string& request_, std::string& response_)>
  • Test downward compatibility to eCAL5
• Server:
  • New API on master
  • Implement GetServiceID
  • Provide/Use DataTypeInformation proto field on registration layer
  • Change SServiceResponse to use SServiceMethodId instead single elements
  • Remove Call variant: bool CallWithResponse(const std::string& method_name_, const std::string& request_, int timeout_, ServiceResponseVecT& service_response_vec_) const; ?
  • Test downward compatibility to eCAL5
• Publisher:
  • New API on master
  • Check API of all CMsgPubXY variants
  • WriterAttr -> PublisherAttr
  • Write -> Send
  • Implement event tests (v5 + v6)
  • Test downward compatibility to eCAL5
• Subscriber:
  • New API on master
  • Check API of all CMsgSubXY variants
  • ReaderAttr -> SubscriberAttr
  • Read -> ???
  • Implement event tests (v5 + v6)
  • Test downward compatibility to eCAL5

Deprecate / Review: @KerstinKeller

• GetXXX (ProcessId, ProcessName, ...)
• eCAL::Process::StartProcess / StopProcess->** Move outside of eCAL?
• GetHostGroupName -> Move to internal function.

ecal_monitoring.h @hannemn

• GetMonitoring -> return bool instead of int.

ecal_log.h @Peguen

• Set log level via configuration
• Remove Get / Set functions of log level / filter
• Remove ECAL_API void Log(const std::string& msg_); (user should always specify level with each logging operation)

ecal_deprecate.h

• Remove eCAL 5 deprecation warnings [core] add ecal 6 deprecation warnings. Remove last deprecated function. #1844
• Add eCAL 6 deprecation warning (and deprecate SplitCombinedTopicType / old service api, ...) [core] add ecal 6 deprecation warnings. Remove last deprecated function. #1844

ecal_core.h @hannemn

• ECAL_API const char* GetVersionString();
• struct eCALVersion {int major = ECAL_VERSION_MAJOR; int minor = ECAL_VERSION_MINOR; int ECAL_VERSION_PATCH};
• eCALVersion GetVersion();

Initialize: @hannemn

• Remove versions with command line arguments
• Add "default" version
  ECAL_API int Initialize(int argc_ = 0, char **argv_ = nullptr, const char *unit_name_ = nullptr, unsigned int components_ = Init::Default);
• Add Initialize(); version without config.
• std::string for Unitname.
• ECAL_API int SetUnitName(const char *unit_name_); -> std::string
• IsInitialized, make 2 functions, one without argument, one with component argument

@rex-schilasky

• SEntityId.entityId should not be a string!

ecal_configuration.h @Peguen

• make some functions ecal internal, which have no "use" to users:
• ECAL_API TransportLayer::Configuration& GetTransportLayerConfiguration ();
• Move all helperfunctions to internal config header
• remove namespace Experimental` in config

Make all API functions internal

• config/transport_layer.h -> same type for local / network config
• host_group_name -> shm_transport_domain
• protocol_v0 -> remove from configuration. and remove from code. service protocol 0 removed #1846
• -> remove whole header file @rex-schilasky
• -> remove dump config....

dynamic.h

• add old signature for AddCallback.

@hannemn
ecal_XXX.h and ecal_XXX_cimpl.h

• change return type of C++ API to bool (true on success)
• align int return type of C API to corresponding C++ methods (zero on success)

eCAL Msg API

msg/publisher.h / msg/subscriber.h @rex-schilasky

• Subscribers are still using old school callbacks. Provide new callbacks
• Provide CMessagePublisher similar to CMessageSubscriber to add Serialization specific specific types a lot easier.

canproto

• Review Publisher API, so that Send takes a builder instead of no arguments. (see also Using eCAL with capnproto #1843)

No response

[DownerCase]

STopicId -> ID part should probably be private (e.g. not accessible by user)

Could we keep at least the entity_id part available? It's useful, for example, to have a registration callback save the entity ID and later use that ID to find the correct publisher/subscriber from the result of GetMonitoring. Basically, some way to uniquely identify eCAL entities is useful. (This applies to the server/clients too)

[KerstinKeller]

Yes, the id should be there, and we would support e.g. comparing for equality, but I'd like to keep the actual ID private, as it's only a pseudo random number and has no meaning to the user.

[DownerCase]

Yes, the id should be there, and we would support e.g. comparing for equality, but I'd like to keep the actual ID private, as it's only a pseudo random number and has no meaning to the user.

Sounds perfect, thanks.

[DownerCase]

Yes, the id should be there, and we would support e.g. comparing for equality, but I'd like to keep the actual ID private, as it's only a pseudo random number and has no meaning to the user.

Actually, I thought about it, and this isn't great, especially when considering non-C++ usage.

So for context I'm working on some Go bindings and applications and I store the Id selected Id to filter out the correct topic when updating the screen. The problem with your proposal to hide the value of the ID is that the filtering must be performed in C++. Additionally, it means having to keep the underlying C++ object alive for the duration I need it; which I solved for the publisher/subscribers but its needless complexity.

As you say, the absolute value of the ID doesn't inherently mean anything except "this is the unique identifier for this eCAL entity". So to me, there's no harm in being able to get the value, its not like one can do anything bad with it anyway; it just needs to be clear it is only to be effectively used as a UUID for that entity.
Exposing a serialized value (string, uint64, etc.) is of great general benefit and ease-of-use, not just for language bindings.

If you don't want to directly expose the backing mechanism for the IDs then I ask for a function to map it to a primitive type like a uint64 or string; at least then you can change the implementation should the need arrive.

Hope that's generally agreeable. 😃