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.

Sign up for GitHub

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

Jump to bottom

Node name in user data #182

Merged
merged 2 commits into from
Mar 30, 2018
Merged

Node name in user data #182

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
4ba2761
add node name to user data
dirk-thomas Dec 7, 2017
252bee5
add participant listener
dirk-thomas Dec 15, 2017
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
63 changes: 63 additions & 0 deletions rmw_fastrtps_cpp/include/rmw_fastrtps_cpp/custom_participant_info.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,82 @@
#ifndef RMW_FASTRTPS_CPP__CUSTOM_PARTICIPANT_INFO_HPP_
#define RMW_FASTRTPS_CPP__CUSTOM_PARTICIPANT_INFO_HPP_

#include <map>
#include <string>
#include <vector>

#include "fastrtps/attributes/ParticipantAttributes.h"
#include "fastrtps/participant/Participant.h"
#include "fastrtps/participant/ParticipantListener.h"

#include "rmw/impl/cpp/key_value.hpp"
#include "rmw/rmw.h"

#include "rmw_fastrtps_cpp/reader_info.hpp"
#include "rmw_fastrtps_cpp/writer_info.hpp"

class ParticipantListener;

typedef struct CustomParticipantInfo
{
eprosima::fastrtps::Participant * participant;
::ParticipantListener * listener;
ReaderInfo * secondarySubListener;
WriterInfo * secondaryPubListener;
rmw_guard_condition_t * graph_guard_condition;
} CustomParticipantInfo;

class ParticipantListener : public eprosima::fastrtps::ParticipantListener
{
public:
void onParticipantDiscovery(Participant *, ParticipantDiscoveryInfo info) override
{
if (
info.rtps.m_status != DISCOVERED_RTPSPARTICIPANT &&
info.rtps.m_status != REMOVED_RTPSPARTICIPANT &&
info.rtps.m_status != DROPPED_RTPSPARTICIPANT)
{
return;
}

if (DISCOVERED_RTPSPARTICIPANT == info.rtps.m_status) {
// ignore already known GUIDs
if (discovered_names.find(info.rtps.m_guid) == discovered_names.end()) {
auto map = rmw::impl::cpp::parse_key_value(info.rtps.m_userData);
auto found = map.find("name");
std::string 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
name = info.rtps.m_RTPSParticipantName;
}
// ignore discovered participants without a name
if (!name.empty()) {
discovered_names[info.rtps.m_guid] = name;
}
}
} else {
auto it = discovered_names.find(info.rtps.m_guid);
// only consider known GUIDs
if (it != discovered_names.end()) {
discovered_names.erase(it);
}
}
}

std::vector<std::string> get_discovered_names() const
{
std::vector<std::string> names(discovered_names.size());
size_t i = 0;
for (auto it : discovered_names) {
names[i++] = it.second;
}
return names;
}

std::map<GUID_t, std::string> discovered_names;
};

#endif // RMW_FASTRTPS_CPP__CUSTOM_PARTICIPANT_INFO_HPP_
29 changes: 26 additions & 3 deletions rmw_fastrtps_cpp/src/rmw_node.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,23 @@ create_node(
}

// Declare everything before beginning to create things.
::ParticipantListener * listener = nullptr;
Participant * participant = nullptr;
rmw_guard_condition_t * graph_guard_condition = nullptr;
CustomParticipantInfo * node_impl = nullptr;
rmw_node_t * node_handle = nullptr;
ReaderInfo * tnat_1 = nullptr;
WriterInfo * tnat_2 = nullptr;
std::pair<StatefulReader *, StatefulReader *> edp_readers;

Participant * participant = Domain::createParticipant(participantAttrs);
try {
listener = new ::ParticipantListener();
} catch (std::bad_alloc &) {
RMW_SET_ERROR_MSG("failed to allocate participant listener");
goto fail;
}

participant = Domain::createParticipant(participantAttrs, listener);
if (!participant) {
RMW_SET_ERROR_MSG("create_node() could not create participant");
return nullptr;
Expand All @@ -98,6 +107,7 @@ create_node(
}
node_handle->implementation_identifier = eprosima_fastrtps_identifier;
node_impl->participant = participant;
node_impl->listener = listener;
node_impl->graph_guard_condition = graph_guard_condition;
node_handle->data = node_impl;

Expand Down Expand Up @@ -160,6 +170,7 @@ create_node(
"failed to destroy guard condition during error handling")
}
}
rmw_free(listener);
if (participant) {
Domain::removeParticipant(participant);
}
Expand Down Expand Up @@ -217,7 +228,17 @@ rmw_create_node(
Domain::getDefaultParticipantAttributes(participantAttrs);

participantAttrs.rtps.builtin.domainId = static_cast<uint32_t>(domain_id);
// since the participant name is not part of the DDS spec
participantAttrs.rtps.setName(name);
// the node name is also set in the user_data
Copy link
Member

Choose a reason for hiding this comment

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

Should this go next to the previous comment line ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so since the previous line sets the name on the participant and only the following lines set it in the user data.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry that was unclear, I meant the other way around:

  • we set the name in participant name
  • then we say "// since the participant name is not part of the DDS spec the node name is also set in the user_data"
  • the we set it in user_data

Does that make sense?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it is fine as it is. Please feel free to update it if you think its clearer.

Copy link
Member Author

Choose a reason for hiding this comment

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

Currently the first comment makes clear that setting the participant name is not covered by the spec - right when it is setting that attribute.

Copy link
Member

Choose a reason for hiding this comment

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

fair enough, let's leave it as is then

size_t name_length = strlen(name);
const char prefix[6] = "name=";
participantAttrs.rtps.userData.resize(name_length + sizeof(prefix));
memcpy(participantAttrs.rtps.userData.data(), prefix, sizeof(prefix) - 1);
for (size_t i = 0; i < name_length; ++i) {
participantAttrs.rtps.userData[sizeof(prefix) - 1 + i] = name[i];
}
participantAttrs.rtps.userData[sizeof(prefix) - 1 + name_length] = ';';

if (security_options->security_root_path) {
// if security_root_path provided, try to find the key and certificate files
Expand Down Expand Up @@ -307,10 +328,12 @@ rmw_destroy_node(rmw_node_t * node)
result_ret = RMW_RET_ERROR;
}

delete impl;

Domain::removeParticipant(participant);

delete impl->listener;
impl->listener = nullptr;
delete impl;
Copy link
Member

Choose a reason for hiding this comment

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

would impl = nullptr; make sense here ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since impl is a local variable I don't think it would make sense.


return result_ret;
}

Expand Down
13 changes: 8 additions & 5 deletions rmw_fastrtps_cpp/src/rmw_node_names.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,18 +51,21 @@ rmw_get_node_names(
}

auto impl = static_cast<CustomParticipantInfo *>(node->data);
Participant * participant = impl->participant;
auto participant_names = impl->listener->get_discovered_names();

auto participant_names = participant->getParticipantNames();
rcutils_allocator_t allocator = rcutils_get_default_allocator();
rcutils_ret_t rcutils_ret =
rcutils_string_array_init(node_names, participant_names.size(), &allocator);
rcutils_string_array_init(node_names, participant_names.size() + 1, &allocator);
if (rcutils_ret != RCUTILS_RET_OK) {
RMW_SET_ERROR_MSG(rcutils_get_error_string_safe())
return rmw_convert_rcutils_ret_to_rmw_ret(rcutils_ret);
}
for (size_t i = 0; i < participant_names.size(); ++i) {
node_names->data[i] = rcutils_strdup(participant_names[i].c_str(), allocator);
for (size_t i = 0; i < participant_names.size() + 1; ++i) {
if (0 == i) {
node_names->data[i] = rcutils_strdup(node->name, allocator);
} else {
node_names->data[i] = rcutils_strdup(participant_names[i - 1].c_str(), allocator);
}
if (!node_names->data[i]) {
RMW_SET_ERROR_MSG("failed to allocate memory for node name")
rcutils_ret = rcutils_string_array_fini(node_names);
Expand Down