Template class for CMessageSubscriber (instead of CMsgSubscriber) #1582
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| { | ||
| class CDynamicSubscriber | ||
| class CapnprotoDynamicDeserializer |
There was a problem hiding this comment.
warning: constructor does not initialize these fields: m_msg_builder_, m_schema_map, m_loader [cppcoreguidelines-pro-type-member-init]
ecal/core/include/ecal/msg/capnproto/dynamic.h:81:
- capnp::MallocMessageBuilder m_msg_builder_;
- std::map<SDataTypeInformation, capnp::Schema> m_schema_map;
- capnp::SchemaLoader m_loader;
+ capnp::MallocMessageBuilder m_msg_builder_{};
+ std::map<SDataTypeInformation, capnp::Schema> m_schema_map{};
+ capnp::SchemaLoader m_loader{};| { | ||
| // we leave topic info empty | ||
| SDataTypeInformation topic_info; |
There was a problem hiding this comment.
warning: variable 'topic_info' is not initialized [cppcoreguidelines-init-variables]
| SDataTypeInformation topic_info; | |
| SDataTypeInformation topic_info = 0; |
| // This function is NOT threadsafe!!! | ||
| // what about the lifetime of the objects? | ||
| // It's totally unclear to me :/ | ||
| bool Deserialize(typename capnp::DynamicStruct::Reader& msg_, const void* buffer_, size_t size_, const SDataTypeInformation& datatype_info_) |
There was a problem hiding this comment.
warning: method 'Deserialize' can be made static [readability-convert-member-functions-to-static]
| bool Deserialize(typename capnp::DynamicStruct::Reader& msg_, const void* buffer_, size_t size_, const SDataTypeInformation& datatype_info_) | |
| static bool Deserialize(typename capnp::DynamicStruct::Reader& msg_, const void* buffer_, size_t size_, const SDataTypeInformation& datatype_info_) |
| msg_callback = callback_; | ||
| return subscriber.AddReceiveCallback(std::bind(&CDynamicSubscriber::OnReceive, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, std::placeholders::_4, std::placeholders::_5)); | ||
| } | ||
| capnp::Schema schema = GetSchema(datatype_info_); |
There was a problem hiding this comment.
warning: variable 'schema' is not initialized [cppcoreguidelines-init-variables]
| capnp::Schema schema = GetSchema(datatype_info_); | |
| capnp::Schema schema = 0 = GetSchema(datatype_info_); |
| **/ | ||
| bool Receive(T& msg_, long long* time_ = nullptr, int rcv_timeout_ = 0) | ||
| { | ||
| std::string rec_buf; |
There was a problem hiding this comment.
warning: variable 'rec_buf' is not initialized [cppcoreguidelines-init-variables]
| std::string rec_buf; | |
| std::string rec_buf = 0; |
| bool Receive(T& msg_, long long* time_ = nullptr, int rcv_timeout_ = 0) | ||
| { | ||
| std::string rec_buf; | ||
| bool success = CSubscriber::ReceiveBuffer(rec_buf, time_, rcv_timeout_); |
There was a problem hiding this comment.
warning: variable 'success' is not initialized [cppcoreguidelines-init-variables]
| bool success = CSubscriber::ReceiveBuffer(rec_buf, time_, rcv_timeout_); | |
| bool success = false = CSubscriber::ReceiveBuffer(rec_buf, time_, rcv_timeout_); |
| * @param clock_ Message writer clock. | ||
| * @param id_ Message id. | ||
| **/ | ||
| typedef std::function<void(const char* topic_name_, const T& msg_, long long time_, long long clock_, long long id_)> MsgReceiveCallbackT; |
There was a problem hiding this comment.
warning: use 'using' instead of 'typedef' [modernize-use-using]
| typedef std::function<void(const char* topic_name_, const T& msg_, long long time_, long long clock_, long long id_)> MsgReceiveCallbackT; | |
| using MsgReceiveCallbackT = std::function<void (const char *, const T &, long long, long long, long long)>; |
| **/ | ||
| bool RemReceiveCallback() | ||
| { | ||
| bool ret = CSubscriber::RemReceiveCallback(); |
There was a problem hiding this comment.
warning: variable 'ret' is not initialized [cppcoreguidelines-init-variables]
| bool ret = CSubscriber::RemReceiveCallback(); | |
| bool ret = false = CSubscriber::RemReceiveCallback(); |
| @@ -88,9 +88,9 @@ int main(int argc, char **argv) | |||
| while (eCAL::Ok()) | |||
| { | |||
| // receive content | |||
| if (sub.Receive(nullptr, 0)) | |||
| AddressBook::Reader reader; | |||
There was a problem hiding this comment.
warning: variable 'reader' is not initialized [cppcoreguidelines-init-variables]
| AddressBook::Reader reader; | |
| AddressBook::Reader reader = 0; |
KerstinKeller
force-pushed
the
feature/message-subscriber
branch
from
53d67cb
to
c050e9e
Compare
| @@ -261,7 +261,7 @@ class TreeMessageVisitor : public MessageVisitor | |||
|
|
|||
| } | |||
|
|
|||
| void PopulateProtoTree(ftxui::TreeNode &root, google::protobuf::Message *message, const std::shared_ptr<StyleSheet> style) | |||
| void PopulateProtoTree(ftxui::TreeNode &root, const std::shared_ptr<google::protobuf::Message>& message, const std::shared_ptr<StyleSheet> style) | |||
There was a problem hiding this comment.
warning: function 'PopulateProtoTree' defined in a header file; function definitions in header files can lead to ODR violations [misc-definitions-in-headers]
void PopulateProtoTree(ftxui::TreeNode &root, const std::shared_ptr<google::protobuf::Message>& message, const std::shared_ptr<StyleSheet> style)
^Additional context
app/mon/mon_tui/src/tui/view/message_visualization/proto_tree.hpp:263: make as 'inline'
void PopulateProtoTree(ftxui::TreeNode &root, const std::shared_ptr<google::protobuf::Message>& message, const std::shared_ptr<StyleSheet> style)
^| @@ -261,7 +261,7 @@ | |||
|
|
|||
| } | |||
|
|
|||
| void PopulateProtoTree(ftxui::TreeNode &root, google::protobuf::Message *message, const std::shared_ptr<StyleSheet> style) | |||
| void PopulateProtoTree(ftxui::TreeNode &root, const std::shared_ptr<google::protobuf::Message>& message, const std::shared_ptr<StyleSheet> style) | |||
There was a problem hiding this comment.
warning: the const qualified parameter 'style' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]
| void PopulateProtoTree(ftxui::TreeNode &root, const std::shared_ptr<google::protobuf::Message>& message, const std::shared_ptr<StyleSheet> style) | |
| void PopulateProtoTree(ftxui::TreeNode &root, const std::shared_ptr<google::protobuf::Message>& message, const std::shared_ptr<StyleSheet>& style) |
| @@ -272,7 +272,7 @@ | |||
| } | |||
| } | |||
|
|
|||
| ftxui::TreeNodePtr ProtoTree(google::protobuf::Message *message, const std::shared_ptr<StyleSheet> style) | |||
| ftxui::TreeNodePtr ProtoTree(const std::shared_ptr<google::protobuf::Message>& message, const std::shared_ptr<StyleSheet> style) | |||
There was a problem hiding this comment.
warning: function 'ProtoTree' defined in a header file; function definitions in header files can lead to ODR violations [misc-definitions-in-headers]
ftxui::TreeNodePtr ProtoTree(const std::shared_ptr<google::protobuf::Message>& message, const std::shared_ptr<StyleSheet> style)
^Additional context
app/mon/mon_tui/src/tui/view/message_visualization/proto_tree.hpp:274: make as 'inline'
ftxui::TreeNodePtr ProtoTree(const std::shared_ptr<google::protobuf::Message>& message, const std::shared_ptr<StyleSheet> style)
^| @@ -272,7 +272,7 @@ | |||
| } | |||
| } | |||
|
|
|||
| ftxui::TreeNodePtr ProtoTree(google::protobuf::Message *message, const std::shared_ptr<StyleSheet> style) | |||
| ftxui::TreeNodePtr ProtoTree(const std::shared_ptr<google::protobuf::Message>& message, const std::shared_ptr<StyleSheet> style) | |||
There was a problem hiding this comment.
warning: the const qualified parameter 'style' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]
| ftxui::TreeNodePtr ProtoTree(const std::shared_ptr<google::protobuf::Message>& message, const std::shared_ptr<StyleSheet> style) | |
| ftxui::TreeNodePtr ProtoTree(const std::shared_ptr<google::protobuf::Message>& message, const std::shared_ptr<StyleSheet>& style) |
| { | ||
| class CDynamicSubscriber | ||
| class CapnprotoDynamicDeserializer |
There was a problem hiding this comment.
warning: constructor does not initialize these fields: m_msg_builder_, m_schema_map, m_loader [cppcoreguidelines-pro-type-member-init]
ecal/core/include/ecal/msg/capnproto/dynamic.h:77:
- capnp::MallocMessageBuilder m_msg_builder_;
- std::map<SDataTypeInformation, capnp::Schema> m_schema_map;
- capnp::SchemaLoader m_loader;
+ capnp::MallocMessageBuilder m_msg_builder_{};
+ std::map<SDataTypeInformation, capnp::Schema> m_schema_map{};
+ capnp::SchemaLoader m_loader{};| , m_topic_name(topic_name_) | ||
| , m_deserializer() | ||
| { | ||
| CSubscriber::Create(topic_name_); |
There was a problem hiding this comment.
warning: variable 'topic_name_' is not initialized [cppcoreguidelines-init-variables]
| CSubscriber::Create(topic_name_); | |
| CSubscriber::Create(topic_name_ = 0); |
| /** | ||
| * @brief Move Constructor | ||
| **/ | ||
| CDynamicMessageSubscriber(CDynamicMessageSubscriber&& rhs) |
There was a problem hiding this comment.
warning: constructor does not initialize these fields: m_error_callback [cppcoreguidelines-pro-type-member-init]
CDynamicMessageSubscriber(CDynamicMessageSubscriber&& rhs)
^| /** | ||
| * @brief Move Constructor | ||
| **/ | ||
| CDynamicMessageSubscriber(CDynamicMessageSubscriber&& rhs) |
There was a problem hiding this comment.
warning: move constructors should be marked noexcept [performance-noexcept-move-constructor]
ecal/core/include/ecal/msg/dynamic.h:112:
- : CSubscriber(std::move(rhs))
+ noexcept : CSubscriber(std::move(rhs))| // the callback bound to the CSubscriber belongs to rhs, bind to this callback instead | ||
| CSubscriber::RemReceiveCallback(); | ||
| auto callback = std::bind(&CDynamicMessageSubscriber::ReceiveCallback, this, std::placeholders::_1, std::placeholders::_2); | ||
| CSubscriber::AddReceiveCallback(callback); |
There was a problem hiding this comment.
warning: variable 'callback' is not initialized [cppcoreguidelines-init-variables]
| CSubscriber::AddReceiveCallback(callback); | |
| CSubscriber::AddReceiveCallback(callback = 0); |
| // Do we want to call error callbacks on receive? Probably not! std::expected wouuld be a good thing to return the reason why things went wrong. | ||
| std::optional<T> Receive(long long* time_ = nullptr, int rcv_timeout_ = 0) | ||
| { | ||
| std::string rec_buf; |
There was a problem hiding this comment.
warning: variable 'rec_buf' of type 'std::string' (aka 'basic_string') can be declared 'const' [misc-const-correctness]
| std::string rec_buf; | |
| std::string const rec_buf; |
| @@ -58,7 +58,7 @@ class ProtoDynSubscriberTest : public ::testing::Test { | |||
| pub.Send(p); | |||
| } | |||
|
|
|||
| void OnPerson(const char*, const google::protobuf::Message&, long long) | |||
| void OnPerson(const char*, const std::shared_ptr<google::protobuf::Message>&, long long) | |||
There was a problem hiding this comment.
warning: all parameters should be named in a function [readability-named-parameter]
| void OnPerson(const char*, const std::shared_ptr<google::protobuf::Message>&, long long) | |
| void OnPerson(const char* /*unused*/, const std::shared_ptr<google::protobuf::Message>& /*unused*/, long long /*unused*/) |
| /** | ||
| * @brief Move Constructor | ||
| **/ | ||
| CDynamicMessageSubscriber(CDynamicMessageSubscriber&& rhs) |
There was a problem hiding this comment.
warning: constructor does not initialize these fields: m_cb_callback, m_error_callback [cppcoreguidelines-pro-type-member-init]
CDynamicMessageSubscriber(CDynamicMessageSubscriber&& rhs)
^| , m_cb_callback(std::move(rhs.m_cb_callback)) | ||
| , m_deserializer(std::move(rhs.m_deserializer)) | ||
| { | ||
| bool has_callback = (m_cb_callback != nullptr); |
There was a problem hiding this comment.
warning: variable 'has_callback' is not initialized [cppcoreguidelines-init-variables]
| bool has_callback = (m_cb_callback != nullptr); | |
| bool has_callback = false = (m_cb_callback != nullptr); |
| private: | ||
| void ReceiveCallback(const char* topic_name_, const struct eCAL::SReceiveCallbackData* data_) | ||
| { | ||
| MsgReceiveCallbackT fn_callback = nullptr; |
There was a problem hiding this comment.
warning: variable 'fn_callback' is not initialized [cppcoreguidelines-init-variables]
| MsgReceiveCallbackT fn_callback = nullptr; | |
| MsgReceiveCallbackT fn_callback = 0 = nullptr; |
|
|
||
| void CallErrorCallback(const std::string& message) | ||
| { | ||
| ErrorCallbackT error_callback = nullptr; |
There was a problem hiding this comment.
warning: variable 'error_callback' is not initialized [cppcoreguidelines-init-variables]
| ErrorCallbackT error_callback = nullptr; | |
| ErrorCallbackT error_callback = 0 = nullptr; |
…o inheritance required. For CProtoDynSubscriber implies API change.
KerstinKeller
force-pushed
the
feature/message-subscriber
branch
from
b16e400
to
9154c9f
Compare
| { | ||
| class CDynamicSubscriber | ||
| class CapnprotoDynamicDeserializer |
There was a problem hiding this comment.
warning: constructor does not initialize these fields: m_msg_builder, m_schema_map, m_loader [cppcoreguidelines-pro-type-member-init]
ecal/core/include/ecal/msg/capnproto/dynamic.h:77:
- capnp::MallocMessageBuilder m_msg_builder;
- std::map<SDataTypeInformation, capnp::Schema> m_schema_map;
- capnp::SchemaLoader m_loader;
+ capnp::MallocMessageBuilder m_msg_builder{};
+ std::map<SDataTypeInformation, capnp::Schema> m_schema_map{};
+ capnp::SchemaLoader m_loader{};| { | ||
| if (!initialized) | ||
| capnp::Schema schema = GetSchema(datatype_info_); | ||
| capnp::DynamicStruct::Builder root_builder = m_msg_builder.getRoot<capnp::DynamicStruct>(schema.asStruct()); |
There was a problem hiding this comment.
warning: variable 'root_builder' is not initialized [cppcoreguidelines-init-variables]
| capnp::DynamicStruct::Builder root_builder = m_msg_builder.getRoot<capnp::DynamicStruct>(schema.asStruct()); | |
| capnp::DynamicStruct::Builder root_builder = 0 = m_msg_builder.getRoot<capnp::DynamicStruct>(schema.asStruct()); |
| **/ | ||
| class CBuilderSubscriber : public CMsgSubscriber<capnp::MallocMessageBuilder> | ||
| template <typename T> | ||
| class CapnprotoDeserializer |
There was a problem hiding this comment.
warning: constructor does not initialize these fields: m_msg_builder [cppcoreguidelines-pro-type-member-init]
ecal/core/include/ecal/msg/capnproto/subscriber.h:69:
- capnp::MallocMessageBuilder m_msg_builder;
+ capnp::MallocMessageBuilder m_msg_builder{};| /** | ||
| * @brief Constructor. | ||
| **/ | ||
| CDynamicMessageSubscriber() : CSubscriber() |
There was a problem hiding this comment.
warning: constructor does not initialize these fields: m_cb_callback, m_error_callback [cppcoreguidelines-pro-type-member-init]
ecal/core/include/ecal/msg/dynamic.h:319:
- MsgReceiveCallbackT m_cb_callback;
+ MsgReceiveCallbackT m_cb_callback{};ecal/core/include/ecal/msg/dynamic.h:321:
- ErrorCallbackT m_error_callback;
+ ErrorCallbackT m_error_callback{};| /** | ||
| * @brief Move Constructor | ||
| **/ | ||
| CDynamicMessageSubscriber(CDynamicMessageSubscriber&& rhs) |
There was a problem hiding this comment.
warning: move constructors should be marked noexcept [performance-noexcept-move-constructor]
ecal/core/include/ecal/msg/dynamic.h:111:
- : CSubscriber(std::move(rhs))
+ noexcept : CSubscriber(std::move(rhs))| /** | ||
| * @brief Constructor. | ||
| **/ | ||
| CMessageSubscriber() : CSubscriber() |
There was a problem hiding this comment.
warning: use '= default' to define a trivial default constructor [modernize-use-equals-default]
ecal/core/include/ecal/msg/subscriber.h:249:
- {
- }
+ = default;| /** | ||
| * @brief Move Constructor | ||
| **/ | ||
| CMessageSubscriber(CMessageSubscriber&& rhs) |
There was a problem hiding this comment.
warning: move constructors should be marked noexcept [performance-noexcept-move-constructor]
ecal/core/include/ecal/msg/subscriber.h:283:
- : CSubscriber(std::move(rhs))
+ noexcept : CSubscriber(std::move(rhs))| /** | ||
| * @brief Move assignment | ||
| **/ | ||
| CMessageSubscriber& operator=(CMessageSubscriber&& rhs) |
There was a problem hiding this comment.
warning: move assignment operators should be marked noexcept [performance-noexcept-move-constructor]
ecal/core/include/ecal/msg/subscriber.h:302:
- {
+ noexcept {
No inheritance required Dynamic Subscribers (capnproto) not yet working. First need DatatypeInformation available in callback.
First PR to greatly simplify message subscribers.