Skip to content

Commit 1a23985

Browse files
committed
Validate and remap the topic name for publishers/subscriptions (#1177)
This PR introduces topic name validation and remapping for publisher and subscription endpoints. - Updated test cases in test/test-graph.js to verify topic remapping under a namespace. - Added a new C++ binding function in src/rcl_node_bindings.cpp to support topic remapping with proper memory management. - Enhanced the Node class in lib/node.js to validate and remap topic names before retrieving publisher/subscription info. Fix: #1176
1 parent 1770100 commit 1a23985

File tree

4 files changed

+149
-25
lines changed

4 files changed

+149
-25
lines changed

lib/node.js

Lines changed: 57 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ const TypeDescriptionService = require('./type_description_service.js');
4343
const Entity = require('./entity.js');
4444
const { SubscriptionEventCallbacks } = require('../lib/event_handler.js');
4545
const { PublisherEventCallbacks } = require('../lib/event_handler.js');
46+
const { validateFullTopicName } = require('./validator.js');
4647

4748
// Parameter event publisher constants
4849
const PARAMETER_EVENT_MSG_TYPE = 'rcl_interfaces/msg/ParameterEvent';
@@ -1072,27 +1073,57 @@ class Node extends rclnodejs.ShadowNode {
10721073
}
10731074

10741075
/**
1075-
* Get a list of publishers on a given topic.
1076-
* @param {string} topic - the topic name to get the publishers for.
1077-
* @param {boolean} noDemangle - if `true`, `topic_name` needs to be a valid middleware topic name,
1078-
* otherwise it should be a valid ROS topic name.
1076+
* Return a list of publishers on a given topic.
1077+
*
1078+
* The returned parameter is a list of TopicEndpointInfo objects, where each will contain
1079+
* the node name, node namespace, topic type, topic endpoint's GID, and its QoS profile.
1080+
*
1081+
* When the `no_mangle` parameter is `true`, the provided `topic` should be a valid
1082+
* topic name for the middleware (useful when combining ROS with native middleware (e.g. DDS)
1083+
* apps). When the `no_mangle` parameter is `false`, the provided `topic` should
1084+
* follow ROS topic name conventions.
1085+
*
1086+
* `topic` may be a relative, private, or fully qualified topic name.
1087+
* A relative or private topic will be expanded using this node's namespace and name.
1088+
* The queried `topic` is not remapped.
1089+
*
1090+
* @param {string} topic - The topic on which to find the publishers.
1091+
* @param {boolean} [noDemangle=false] - If `true`, `topic` needs to be a valid middleware topic
1092+
* name, otherwise it should be a valid ROS topic name. Defaults to `false`.
10791093
* @returns {Array} - list of publishers
10801094
*/
1081-
getPublishersInfoByTopic(topic, noDemangle) {
1082-
return rclnodejs.getPublishersInfoByTopic(this.handle, topic, noDemangle);
1095+
getPublishersInfoByTopic(topic, noDemangle = false) {
1096+
return rclnodejs.getPublishersInfoByTopic(
1097+
this.handle,
1098+
this._getValidatedTopic(topic, noDemangle),
1099+
noDemangle
1100+
);
10831101
}
10841102

10851103
/**
1086-
* Get a list of subscriptions on a given topic.
1087-
* @param {string} topic - the topic name to get the subscriptions for.
1088-
* @param {boolean} noDemangle - if `true`, `topic_name` needs to be a valid middleware topic name,
1089-
* otherwise it should be a valid ROS topic name.
1104+
* Return a list of subscriptions on a given topic.
1105+
*
1106+
* The returned parameter is a list of TopicEndpointInfo objects, where each will contain
1107+
* the node name, node namespace, topic type, topic endpoint's GID, and its QoS profile.
1108+
*
1109+
* When the `no_mangle` parameter is `true`, the provided `topic` should be a valid
1110+
* topic name for the middleware (useful when combining ROS with native middleware (e.g. DDS)
1111+
* apps). When the `no_mangle` parameter is `false`, the provided `topic` should
1112+
* follow ROS topic name conventions.
1113+
*
1114+
* `topic` may be a relative, private, or fully qualified topic name.
1115+
* A relative or private topic will be expanded using this node's namespace and name.
1116+
* The queried `topic` is not remapped.
1117+
*
1118+
* @param {string} topic - The topic on which to find the subscriptions.
1119+
* @param {boolean} [noDemangle=false] - If `true`, `topic` needs to be a valid middleware topic
1120+
name, otherwise it should be a valid ROS topic name. Defaults to `false`.
10901121
* @returns {Array} - list of subscriptions
10911122
*/
1092-
getSubscriptionsInfoByTopic(topic, noDemangle) {
1123+
getSubscriptionsInfoByTopic(topic, noDemangle = false) {
10931124
return rclnodejs.getSubscriptionsInfoByTopic(
10941125
this.handle,
1095-
topic,
1126+
this._getValidatedTopic(topic, noDemangle),
10961127
noDemangle
10971128
);
10981129
}
@@ -1428,7 +1459,7 @@ class Node extends rclnodejs.ShadowNode {
14281459
* Determine if a parameter descriptor exists.
14291460
*
14301461
* @param {string} name - The name of a descriptor to for.
1431-
* @return {boolean} - True if a descriptor has been declared; otherwise false.
1462+
* @return {boolean} - true if a descriptor has been declared; otherwise false.
14321463
*/
14331464
hasParameterDescriptor(name) {
14341465
return !!this.getParameterDescriptor(name);
@@ -1864,6 +1895,19 @@ class Node extends rclnodejs.ShadowNode {
18641895
this._actionServers.push(actionServer);
18651896
this.syncHandles();
18661897
}
1898+
1899+
_getValidatedTopic(topicName, noDemangle) {
1900+
if (noDemangle) {
1901+
return topicName;
1902+
}
1903+
const fqTopicName = rclnodejs.expandTopicName(
1904+
topicName,
1905+
this.name(),
1906+
this.namespace()
1907+
);
1908+
validateFullTopicName(fqTopicName);
1909+
return rclnodejs.remapTopicName(this.handle, fqTopicName);
1910+
}
18671911
}
18681912

18691913
/**

src/rcl_node_bindings.cpp

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <rcl/arguments.h>
2121
#include <rcl/error_handling.h>
2222
#include <rcl/rcl.h>
23+
#include <rcl/remap.h>
2324
#include <rcl_action/rcl_action.h>
2425
#include <rcl_yaml_param_parser/parser.h>
2526
#include <rcl_yaml_param_parser/types.h>
@@ -507,6 +508,47 @@ Napi::Value ResolveName(const Napi::CallbackInfo& info) {
507508
return Napi::String::New(env, output_cstr);
508509
}
509510

511+
Napi::Value RemapTopicName(const Napi::CallbackInfo& info) {
512+
Napi::Env env = info.Env();
513+
RclHandle* node_handle = RclHandle::Unwrap(info[0].As<Napi::Object>());
514+
rcl_node_t* node = reinterpret_cast<rcl_node_t*>(node_handle->ptr());
515+
std::string topic_name = info[1].As<Napi::String>().Utf8Value();
516+
517+
const rcl_node_options_t* node_options = rcl_node_get_options(node);
518+
if (nullptr == node_options) {
519+
Napi::Error::New(env, "failed to get node options")
520+
.ThrowAsJavaScriptException();
521+
return env.Undefined();
522+
}
523+
524+
const rcl_arguments_t* global_args = nullptr;
525+
if (node_options->use_global_arguments) {
526+
global_args = &(node->context->global_arguments);
527+
}
528+
529+
char* output_cstr = nullptr;
530+
rcl_ret_t ret = rcl_remap_topic_name(
531+
&(node_options->arguments), global_args, topic_name.c_str(),
532+
rcl_node_get_name(node), rcl_node_get_namespace(node),
533+
node_options->allocator, &output_cstr);
534+
if (RCL_RET_OK != ret) {
535+
Napi::Error::New(env, "failed to remap topic name")
536+
.ThrowAsJavaScriptException();
537+
return env.Undefined();
538+
}
539+
if (nullptr == output_cstr) {
540+
return Napi::String::New(env, topic_name);
541+
}
542+
543+
auto name_deleter = [&]() {
544+
node_options->allocator.deallocate(output_cstr,
545+
node_options->allocator.state);
546+
};
547+
RCPPUTILS_SCOPE_EXIT({ name_deleter(); });
548+
549+
return Napi::String::New(env, output_cstr);
550+
}
551+
510552
Napi::Object InitNodeBindings(Napi::Env env, Napi::Object exports) {
511553
exports.Set("getParameterOverrides",
512554
Napi::Function::New(env, GetParameterOverrides));
@@ -532,6 +574,7 @@ Napi::Object InitNodeBindings(Napi::Env env, Napi::Object exports) {
532574
exports.Set("getRMWImplementationIdentifier",
533575
Napi::Function::New(env, GetRMWImplementationIdentifier));
534576
exports.Set("resolveName", Napi::Function::New(env, ResolveName));
577+
exports.Set("remapTopicName", Napi::Function::New(env, RemapTopicName));
535578
return exports;
536579
}
537580

test/test-graph.js

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,21 +29,34 @@ describe('rclnodejs graph test suite', function () {
2929
});
3030

3131
it('Get publishers info by topic', function () {
32-
const node = rclnodejs.createNode('publisher_node');
32+
const node = rclnodejs.createNode('publisher_node', '/my_ns');
33+
assert.deepStrictEqual(
34+
0,
35+
node.getPublishersInfoByTopic('/my_ns/topic', false).length
36+
);
3337
const String = 'std_msgs/msg/String';
3438
node.createPublisher(String, 'topic');
35-
const publishers = node.getPublishersInfoByTopic('/topic', false);
39+
const publishers = node.getPublishersInfoByTopic('/my_ns/topic', false);
3640
assert.strictEqual(publishers.length, 1);
41+
assert.strictEqual(publishers[0].node_namespace, '/my_ns');
3742
assert.strictEqual(publishers[0].node_name, 'publisher_node');
3843
assert.strictEqual(publishers[0].topic_type, String);
3944
});
4045

4146
it('Get subscriptions info by topic', function () {
42-
const node = rclnodejs.createNode('subscription_node');
47+
const node = rclnodejs.createNode('subscription_node', '/my_ns');
48+
assert.deepStrictEqual(
49+
0,
50+
node.getSubscriptionsInfoByTopic('/my_ns/topic', false).length
51+
);
4352
const String = 'std_msgs/msg/String';
4453
node.createSubscription(String, 'topic', (msg) => {});
45-
const subscriptions = node.getSubscriptionsInfoByTopic('/topic', false);
54+
const subscriptions = node.getSubscriptionsInfoByTopic(
55+
'/my_ns/topic',
56+
false
57+
);
4658
assert.strictEqual(subscriptions.length, 1);
59+
assert.strictEqual(subscriptions[0].node_namespace, '/my_ns');
4760
assert.strictEqual(subscriptions[0].node_name, 'subscription_node');
4861
assert.strictEqual(subscriptions[0].topic_type, String);
4962
});

types/node.d.ts

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -750,21 +750,45 @@ declare module 'rclnodejs' {
750750
getServiceNamesAndTypes(): Array<NamesAndTypesQueryResult>;
751751

752752
/**
753-
* Get an array of publishers on a given topic.
753+
* Return a list of publishers on a given topic.
754754
*
755-
* @param topic - The name of the topic.
756-
* @param noDemangle - if `true`, `topic_name` needs to be a valid middleware topic name,
757-
* otherwise it should be a valid ROS topic name.
755+
* The returned parameter is a list of TopicEndpointInfo objects, where each will contain
756+
* the node name, node namespace, topic type, topic endpoint's GID, and its QoS profile.
757+
*
758+
* When the `no_mangle` parameter is `true`, the provided `topic` should be a valid
759+
* topic name for the middleware (useful when combining ROS with native middleware (e.g. DDS)
760+
* apps). When the `no_mangle` parameter is `false`, the provided `topic` should
761+
* follow ROS topic name conventions.
762+
*
763+
* `topic` may be a relative, private, or fully qualified topic name.
764+
* A relative or private topic will be expanded using this node's namespace and name.
765+
* The queried `topic` is not remapped.
766+
*
767+
* @param topic - The topic on which to find the publishers.
768+
* @param [noDemangle=false] - If `true`, `topic` needs to be a valid middleware topic
769+
* name, otherwise it should be a valid ROS topic name. Defaults to `false`.
758770
* @returns An array of publishers.
759771
*/
760772
getPublishersInfoByTopic(topic: string, noDemangle: boolean): Array<object>;
761773

762774
/**
763-
* Get an array of subscriptions on a given topic.
775+
* Return a list of subscriptions on a given topic.
764776
*
765-
* @param topic - The name of the topic.
766-
* @param noDemangle - if `true`, `topic_name` needs to be a valid middleware topic name,
767-
* otherwise it should be a valid ROS topic name.
777+
* The returned parameter is a list of TopicEndpointInfo objects, where each will contain
778+
* the node name, node namespace, topic type, topic endpoint's GID, and its QoS profile.
779+
*
780+
* When the `no_mangle` parameter is `true`, the provided `topic` should be a valid
781+
* topic name for the middleware (useful when combining ROS with native middleware (e.g. DDS)
782+
* apps). When the `no_mangle` parameter is `false`, the provided `topic` should
783+
* follow ROS topic name conventions.
784+
*
785+
* `topic` may be a relative, private, or fully qualified topic name.
786+
* A relative or private topic will be expanded using this node's namespace and name.
787+
* The queried `topic` is not remapped.
788+
*
789+
* @param topic - The topic on which to find the subscriptions..
790+
* @param [noDemangle=false] - If `true`, `topic` needs to be a valid middleware topic
791+
name, otherwise it should be a valid ROS topic name. Defaults to `false`.
768792
* @returns An array of subscriptions.
769793
*/
770794
getSubscriptionsInfoByTopic(

0 commit comments

Comments
 (0)