Skip to content
This repository was archived by the owner on Oct 7, 2021. It is now read-only.

Combine package name with message namespace in type support struct #268

Merged
merged 3 commits into from
May 8, 2019
Merged

Combine package name with message namespace in type support struct #268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
201a45d
Combine package name with message namespace in type support struct
jacobperron May 6, 2019
5a8f95f
Update demangle logic for services
jacobperron May 7, 2019
b28bdfd
fix lint
jacobperron May 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 21 additions & 16 deletions rmw_opensplice_cpp/src/demangle.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#include <string>
#include <algorithm>
#include <regex>
#include <string>
#include <vector>

#include "rcutils/logging_macros.h"
Expand All @@ -36,19 +37,21 @@ _demangle_if_ros_topic(const std::string & topic_name)
std::string
_demangle_if_ros_type(const std::string & dds_type_string)
{
std::string substring = "::msg::dds_::";
std::string substring = "dds_::";
size_t substring_position = dds_type_string.find(substring);
if (
dds_type_string[dds_type_string.size() - 1] == '_' &&
substring_position != std::string::npos)
dds_type_string[dds_type_string.size() - 1] != '_' ||
substring_position == std::string::npos)
{
std::string pkg = dds_type_string.substr(0, substring_position);
size_t start = substring_position + substring.size();
std::string type_name = dds_type_string.substr(start, dds_type_string.length() - 1 - start);
return pkg + "/" + type_name;
// not a ROS type
return dds_type_string;
}
// not a ROS type
return dds_type_string;

std::string type_namespace = dds_type_string.substr(0, substring_position);
type_namespace = std::regex_replace(type_namespace, std::regex("::"), "/");
size_t start = substring_position + substring.size();
std::string type_name = dds_type_string.substr(start, dds_type_string.length() - 1 - start);
return type_namespace + type_name;
Copy link
Member

Choose a reason for hiding this comment

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

Inverting the logic makes the diff more difficult to review. What was the reason to do so?

Copy link
Member Author

Choose a reason for hiding this comment

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

Consistency. The logic now matches the other rmw implementations. I can revert it or break it into two commits if you prefer.

Copy link
Member

Choose a reason for hiding this comment

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

No need it - the change looks good. I just wanted to check.

}

std::string
Expand Down Expand Up @@ -108,7 +111,7 @@ _demangle_service_from_topic(const std::string & topic_name)
std::string
_demangle_service_type_only(const std::string & dds_type_name)
{
std::string ns_substring = "::srv::dds_::";
std::string ns_substring = "dds_::";
size_t ns_substring_position = dds_type_name.find(ns_substring);
if (ns_substring_position == std::string::npos) {
// not a ROS service type
Expand All @@ -125,7 +128,7 @@ _demangle_service_type_only(const std::string & dds_type_name)
if (suffix_position != std::string::npos) {
if (dds_type_name.length() - suffix_position - suffix.length() != 0) {
RCUTILS_LOG_WARN_NAMED("rmw_opensplice_cpp",
"service type contains '::srv::dds_::' and a suffix, but not at the end"
"service type contains 'dds_::' and a suffix, but not at the end"
", report this: '%s'", dds_type_name.c_str());
continue;
}
Expand All @@ -135,13 +138,15 @@ _demangle_service_type_only(const std::string & dds_type_name)
}
if (suffix_position == std::string::npos) {
RCUTILS_LOG_WARN_NAMED("rmw_opensplice_cpp",
"service type contains '::srv::dds_::' but does not have a suffix"
"service type contains 'dds_::' but does not have a suffix"
", report this: '%s'", dds_type_name.c_str());
return "";
}
// everything checks out, reformat it from '<pkg>::srv::dds_::<type><suffix>' to '<pkg>/<type>'
std::string pkg = dds_type_name.substr(0, ns_substring_position);
// everything checks out, reformat it from '[namespace::]dds_::<type><suffix>'
// to '[namespace/]<type>'
std::string type_namespace = dds_type_name.substr(0, ns_substring_position);
type_namespace = std::regex_replace(type_namespace, std::regex("::"), "/");
size_t start = ns_substring_position + ns_substring.length();
std::string type_name = dds_type_name.substr(start, suffix_position - start);
return pkg + "/" + type_name;
return type_namespace + type_name;
}
2 changes: 1 addition & 1 deletion rmw_opensplice_cpp/src/rmw_publisher.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ rmw_create_publisher(
RMW_SET_ERROR_MSG("callbacks handle is null");
return NULL;
}
std::string type_name = create_type_name(callbacks, "msg");
std::string type_name = create_type_name(callbacks);

const char * error_string = callbacks->register_type(participant, type_name.c_str());
if (error_string) {
Expand Down
2 changes: 1 addition & 1 deletion rmw_opensplice_cpp/src/rmw_subscription.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ rmw_create_subscription(
RMW_SET_ERROR_MSG("callbacks handle is null");
return NULL;
}
std::string type_name = create_type_name(callbacks, "msg");
std::string type_name = create_type_name(callbacks);

const char * error_string = callbacks->register_type(participant, type_name.c_str());
if (error_string) {
Expand Down
7 changes: 2 additions & 5 deletions rmw_opensplice_cpp/src/types.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,9 @@
#include "namespace_prefix.hpp"

std::string
create_type_name(
const message_type_support_callbacks_t * callbacks,
const std::string & sep)
create_type_name(const message_type_support_callbacks_t * callbacks)
{
return std::string(callbacks->package_name) +
"::" + sep + "::dds_::" + callbacks->message_name + "_";
return std::string(callbacks->message_namespace) + "::dds_::" + callbacks->message_name + "_";
}

CustomDataReaderListener::CustomDataReaderListener()
Expand Down
4 changes: 1 addition & 3 deletions rmw_opensplice_cpp/src/types.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,7 @@

RMW_LOCAL
std::string
create_type_name(
const message_type_support_callbacks_t * callbacks,
const std::string & sep);
create_type_name(const message_type_support_callbacks_t * callbacks);

// The extern "C" here enforces that overloading is not used.
extern "C"
Expand Down