Skip to content

Commit

Permalink
Leverage rcl functions to get topic of publisher/subscription (#951)
Browse files Browse the repository at this point in the history
This patch implements:

1. Return the result of rcl_publisher_get_topic_name() directly, instead
of splitting it by "/" and returns the last part.

2. Use rcl_subscription_get_topic_name() to get the topic of a
subscription.

Meanwhile, this patch updates the following tests:

- test/test-node-oo.js
- test/test-node.js
- test/test-remapping.js

Fix: #949, #950
  • Loading branch information
minggangw authored Jan 8, 2024
1 parent dc7f54b commit 625b016
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 11 deletions.
3 changes: 1 addition & 2 deletions lib/publisher.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ class Publisher extends Entity {
* @type {string}
*/
get topic() {
const fullTopic = rclnodejs.getTopic(this._handle); // returns /my_node/mytopic
return fullTopic.split('/').pop();
return rclnodejs.getPublisherTopic(this._handle);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion lib/subscription.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class Subscription extends Entity {
* @type {string}
*/
get topic() {
return this._topic;
return rclnodejs.getSubscriptionTopic(this._handle);
}

/**
Expand Down
15 changes: 13 additions & 2 deletions src/rcl_bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -942,7 +942,7 @@ NAN_METHOD(Publish) {
info.GetReturnValue().Set(Nan::Undefined());
}

NAN_METHOD(GetTopic) {
NAN_METHOD(GetPublisherTopic) {
rcl_publisher_t* publisher = reinterpret_cast<rcl_publisher_t*>(
RclHandle::Unwrap<RclHandle>(
Nan::To<v8::Object>(info[0]).ToLocalChecked())
Expand All @@ -952,6 +952,16 @@ NAN_METHOD(GetTopic) {
info.GetReturnValue().Set(Nan::New(topic).ToLocalChecked());
}

NAN_METHOD(GetSubscriptionTopic) {
rcl_subscription_t* subscription = reinterpret_cast<rcl_subscription_t*>(
RclHandle::Unwrap<RclHandle>(
Nan::To<v8::Object>(info[0]).ToLocalChecked())
->ptr());

const char* topic = rcl_subscription_get_topic_name(subscription);
info.GetReturnValue().Set(Nan::New(topic).ToLocalChecked());
}

NAN_METHOD(CreateClient) {
v8::Local<v8::Context> currentContent = Nan::GetCurrentContext();
RclHandle* node_handle = RclHandle::Unwrap<RclHandle>(
Expand Down Expand Up @@ -2039,7 +2049,8 @@ std::vector<BindingMethod> binding_methods = {
{"clearContentFilter", ClearContentFilter},
{"createPublisher", CreatePublisher},
{"publish", Publish},
{"getTopic", GetTopic},
{"getPublisherTopic", GetPublisherTopic},
{"getSubscriptionTopic", GetSubscriptionTopic},
{"createClient", CreateClient},
{"rclTakeResponse", RclTakeResponse},
{"sendRequest", SendRequest},
Expand Down
4 changes: 2 additions & 2 deletions test/test-node-oo.js
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ describe('topic & serviceName getter/setter', function () {
it('publisher: topic property getter', function () {
var node = new rclnodejs.Node('publisher', '/topic_getter');
var publisher = node.createPublisher(RclString, 'chatter');
assert.deepStrictEqual(publisher.topic, 'chatter');
assert.deepStrictEqual(publisher.topic, '/topic_getter/chatter');
node.destroy();
});

Expand All @@ -449,7 +449,7 @@ describe('topic & serviceName getter/setter', function () {
'chatter',
(msg) => {}
);
assert.deepStrictEqual(subscription.topic, 'chatter');
assert.deepStrictEqual(subscription.topic, '/topic_getter/chatter');
node.destroy();
});

Expand Down
4 changes: 2 additions & 2 deletions test/test-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ describe('topic & serviceName getter/setter', function () {
it('publisher: topic property getter', function () {
var node = rclnodejs.createNode('publisher', '/topic_getter');
var publisher = node.createPublisher(RclString, 'chatter');
assert.deepStrictEqual(publisher.topic, 'chatter');
assert.deepStrictEqual(publisher.topic, '/topic_getter/chatter');
node.destroy();
});

Expand All @@ -450,7 +450,7 @@ describe('topic & serviceName getter/setter', function () {
'chatter',
(msg) => {}
);
assert.deepStrictEqual(subscription.topic, 'chatter');
assert.deepStrictEqual(subscription.topic, '/topic_getter/chatter');
node.destroy();
});

Expand Down
4 changes: 2 additions & 2 deletions test/test-remapping.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ describe('rcl node remapping', function () {
node = rclnodejs.createNode('my_node');
let publisher = node.createPublisher('std_msgs/msg/String', 'my_topic');

assert.equal(publisher.topic, 'foo_topic');
assert.equal(publisher.topic, '/foo_topic');
});

it('remap service name', async function () {
Expand Down Expand Up @@ -94,7 +94,7 @@ describe('rcl node remapping', function () {

assert.equal(node.name(), 'foo_node');
assert.equal(node.namespace(), '/foo_ns');
assert.equal(publisher.topic, 'foo_topic');
assert.equal(publisher.topic, '/foo_ns/foo_topic');
assert.equal(service.serviceName, 'foo_service');
});
});

0 comments on commit 625b016

Please sign in to comment.