Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core] Move eCAL::protobuf::CDynamicJSONSubscriber #1605

Merged
merged 1 commit into from
May 21, 2024

Conversation

KerstinKeller
Copy link
Contributor

based on CDynamicMessageSubscriber

Description

eCAL::protobuf::CDynamicJSON Subscriber is now based on CDynamicMessageSubscriber, which greatly simplifies code.

API break: the message callback API is now changed, which should be fine for eCAL 6.

@KerstinKeller KerstinKeller added the cherry-pick-to-NONE Don't cherry-pick these changes label May 17, 2024
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

@@ -26,13 +26,15 @@

#include <ecal/ecal.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

#include <ecal/ecal.h>
         ^

class ProtobufDynamicJSONDeserializer
{
public:
std::string Deserialize(const void* buffer_, size_t size_, const SDataTypeInformation& datatype_info_)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: method 'Deserialize' can be made static [readability-convert-member-functions-to-static]

Suggested change
std::string Deserialize(const void* buffer_, size_t size_, const SDataTypeInformation& datatype_info_)
static std::string Deserialize(const void* buffer_, size_t size_, const SDataTypeInformation& datatype_info_)

m_msg_callback = callback_;
return true;
}
std::string binary_input;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'binary_input' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::string binary_input;
std::string binary_input = 0;

}
std::string binary_input;
binary_input.assign(static_cast<const char*>(buffer_), static_cast<size_t>(size_));
std::string json_output;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'json_output' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::string json_output;
std::string json_output = 0;

return true;
}
private:
std::shared_ptr<google::protobuf::util::TypeResolver> GetTypeResolver(const SDataTypeInformation& datatype_info_)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: method 'GetTypeResolver' can be made static [readability-convert-member-functions-to-static]

Suggested change
std::shared_ptr<google::protobuf::util::TypeResolver> GetTypeResolver(const SDataTypeInformation& datatype_info_)
static std::shared_ptr<google::protobuf::util::TypeResolver> GetTypeResolver(const SDataTypeInformation& datatype_info_)

m_topic_type_full = "/" + m_topic_type_full;
std::shared_ptr<google::protobuf::util::TypeResolver> CreateTypeResolver(const SDataTypeInformation& datatype_info_)
{
std::string unqualified_topic_type = GetUnqualifiedTopicType(datatype_info_);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'unqualified_topic_type' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::string unqualified_topic_type = GetUnqualifiedTopicType(datatype_info_);
std::string unqualified_topic_type = 0 = GetUnqualifiedTopicType(datatype_info_);


google::protobuf::FileDescriptorSet proto_desc;
proto_desc.ParseFromString(topic_desc);
std::string error_s;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'error_s' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::string error_s;
std::string error_s = 0;


if (resolver == nullptr)
{
std::stringstream s;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 's' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::stringstream s;
std::stringstream s = 0;


if (m_topic_type.empty())
std::string GetQualifiedTopicType(const SDataTypeInformation& data_type_info_)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: method 'GetQualifiedTopicType' can be made static [readability-convert-member-functions-to-static]

Suggested change
std::string GetQualifiedTopicType(const SDataTypeInformation& data_type_info_)
static std::string GetQualifiedTopicType(const SDataTypeInformation& data_type_info_)

// get topic description
m_topic_desc = topic_info.descriptor;
if (m_topic_desc.empty())
std::string GetUnqualifiedTopicType(const SDataTypeInformation& data_type_info_)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: method 'GetUnqualifiedTopicType' can be made static [readability-convert-member-functions-to-static]

Suggested change
std::string GetUnqualifiedTopicType(const SDataTypeInformation& data_type_info_)
static std::string GetUnqualifiedTopicType(const SDataTypeInformation& data_type_info_)

Copy link
Contributor

@rex-schilasky rex-schilasky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@KerstinKeller KerstinKeller merged commit 0bdb60c into master May 21, 2024
16 of 20 checks passed
@KerstinKeller KerstinKeller deleted the feature/proto-dyn-json-sub branch May 28, 2024 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-to-NONE Don't cherry-pick these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants