Skip to content
This repository has been archived by the owner on Jun 21, 2023. It is now read-only.

Node name in user data #276

Merged
merged 2 commits into from
Mar 30, 2018
Merged

Node name in user data #276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dc5bb6a
add node name to user data
dirk-thomas Dec 7, 2017
74e28fb
get participant name from user data first
dirk-thomas Jan 8, 2018
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
17 changes: 17 additions & 0 deletions rmw_connext_shared_cpp/src/node.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,23 @@ create_node(
// This String_dup is not matched with a String_free because DDS appears to
// free this automatically.
participant_qos.participant_name.name = DDS::String_dup(name);
// since the participant name is not part of the DDS spec
// the node name is also set in the user_data
DDS_Long name_length = static_cast<DDS_Long>(strlen(name));
const char prefix[6] = "name=";
bool success = participant_qos.user_data.value.length(name_length + sizeof(prefix));
if (!success) {
RMW_SET_ERROR_MSG("failed to resize participant user_data");
return NULL;
}
memcpy(participant_qos.user_data.value.get_contiguous_buffer(), prefix, sizeof(prefix) - 1);
{
for (DDS_Long i = 0; i < name_length; ++i) {
participant_qos.user_data.value[sizeof(prefix) - 1 + i] = name[i];
}
participant_qos.user_data.value[sizeof(prefix) - 1 + name_length] = ';';
}

// forces local traffic to be sent over loopback,
// even if a more efficient transport (such as shared memory) is installed
// (in which case traffic will be sent over both transports)
Expand Down
30 changes: 25 additions & 5 deletions rmw_connext_shared_cpp/src/node_names.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,15 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#include <string>
#include <vector>

#include "rcutils/logging_macros.h"
#include "rcutils/strdup.h"

#include "rmw/convert_rcutils_ret_to_rmw_ret.h"
#include "rmw/error_handling.h"
#include "rmw/impl/cpp/key_value.hpp"
#include "rmw/sanity_checks.h"

#include "rmw_connext_shared_cpp/ndds_include.hpp"
Expand Down Expand Up @@ -67,15 +71,31 @@ get_node_names(
return RMW_RET_BAD_ALLOC;
}


for (auto i = 1; i < length; ++i) {
DDS::ParticipantBuiltinTopicData pbtd;
auto dds_ret = participant->get_discovered_participant_data(pbtd, handles[i - 1]);
const char * name = pbtd.participant_name.name;
if (!name || dds_ret != DDS_RETCODE_OK) {
name = "(no name)";
std::string name;
if (DDS_RETCODE_OK == dds_ret) {
auto data = static_cast<unsigned char *>(pbtd.user_data.value.get_contiguous_buffer());
std::vector<uint8_t> kv(data, data + pbtd.user_data.value.length());
auto map = rmw::impl::cpp::parse_key_value(kv);
auto found = map.find("name");
if (found != map.end()) {
name = std::string(found->second.begin(), found->second.end());
}
if (name.empty()) {
// use participant name if no name was found in the user data
if (pbtd.participant_name.name) {
name = pbtd.participant_name.name;
}
}
}
if (name.empty()) {
// ignore discovered participants without a name
node_names->data[i] = nullptr;
continue;
Copy link
Member

Choose a reason for hiding this comment

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

I don't think continueing here is a good idea, higher layers are probably assuming that the node_names map will return valid strings (nothing in the rcl_get_node_names documentation suggests otherwise). FWICT rclpy, for example, is assuming null-terminated strings are stored at each index: https://github.com/ros2/rclpy/blob/fbd80a27342d56b32047ecb7f2ad2e44e9d027b3/rclpy/src/rclpy/_rclpy.c#L2323

Copy link
Member Author

Choose a reason for hiding this comment

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

The line you refenced should be fine if NULL is passed in as far as I understand the API docs:

If u is NULL, this function behaves like PyUnicode_FromUnicode() with the buffer set to NULL. This usage is deprecated in favor of PyUnicode_New().

Copy link
Member

Choose a reason for hiding this comment

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

Looks like you're referencing the documentation for PyUnicode_FromStringAndSize, PyUnicode_FromString says:

Create a Unicode object from a UTF-8 encoded null-terminated char buffer u.

The error (when it doesn't segfault) is: UnicodeDecodeError: 'utf-8' codec can't decode byte 0xf0 in position 0: invalid continuation byte

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right, I stopped at the first match and didn't check the exact function name 😞

}
node_names->data[i] = rcutils_strdup(name, allocator);
node_names->data[i] = rcutils_strdup(name.c_str(), allocator);
if (!node_names->data[i]) {
RMW_SET_ERROR_MSG("could not allocate memory for node name")
rcutils_ret = rcutils_string_array_fini(node_names);
Expand Down