From c1bf170d706277fd34264d4e9b4f0afd5a19807a Mon Sep 17 00:00:00 2001 From: Minggang Wang Date: Wed, 10 Jan 2024 11:11:35 +0800 Subject: [PATCH] Use rcl functions to grab the service name (#953) This patch implements: 1. Leverage rcl_client_get_service_name() to get the service name for client. 2. Leverage rcl_service_get_service_name() to get the service name for service. The unit tests get updated accordingly. Fix: #952 --- lib/client.js | 2 +- lib/service.js | 3 +-- src/rcl_bindings.cpp | 33 ++++++++++++++++++++++----------- test/test-node-oo.js | 4 ++-- test/test-node.js | 4 ++-- test/test-remapping.js | 4 ++-- 6 files changed, 30 insertions(+), 20 deletions(-) diff --git a/lib/client.js b/lib/client.js index e85df39d..188af6d0 100644 --- a/lib/client.js +++ b/lib/client.js @@ -128,7 +128,7 @@ class Client extends Entity { * @type {string} */ get serviceName() { - return this._serviceName; + return rclnodejs.getClientServiceName(this._handle); } /** diff --git a/lib/service.js b/lib/service.js index 91960f2f..9508f63b 100644 --- a/lib/service.js +++ b/lib/service.js @@ -112,8 +112,7 @@ class Service extends Entity { * @type {string} */ get serviceName() { - const fullServiceName = rclnodejs.getServiceName(this._handle); // returns /my_node/myservice - return fullServiceName.split('/').pop(); + return rclnodejs.getServiceServiceName(this._handle); } /** diff --git a/src/rcl_bindings.cpp b/src/rcl_bindings.cpp index cee69318..3a84d233 100644 --- a/src/rcl_bindings.cpp +++ b/src/rcl_bindings.cpp @@ -1107,16 +1107,6 @@ NAN_METHOD(RclTakeRequest) { info.GetReturnValue().Set(Nan::Undefined()); } -NAN_METHOD(GetServiceName) { - rcl_service_t* service = reinterpret_cast( - RclHandle::Unwrap( - Nan::To(info[0]).ToLocalChecked()) - ->ptr()); - - const char* name = rcl_service_get_service_name(service); - info.GetReturnValue().Set(Nan::New(name).ToLocalChecked()); -} - NAN_METHOD(SendResponse) { rcl_service_t* service = reinterpret_cast( RclHandle::Unwrap( @@ -2017,6 +2007,26 @@ NAN_METHOD(RclTakeRaw) { "Failed to deallocate message buffer"); } +NAN_METHOD(GetClientServiceName) { + rcl_client_t* client = reinterpret_cast( + RclHandle::Unwrap( + Nan::To(info[0]).ToLocalChecked()) + ->ptr()); + + const char* service_name = rcl_client_get_service_name(client); + info.GetReturnValue().Set(Nan::New(service_name).ToLocalChecked()); +} + +NAN_METHOD(GetServiceServiceName) { + rcl_service_t* service = reinterpret_cast( + RclHandle::Unwrap( + Nan::To(info[0]).ToLocalChecked()) + ->ptr()); + + const char* service_name = rcl_service_get_service_name(service); + info.GetReturnValue().Set(Nan::New(service_name).ToLocalChecked()); +} + std::vector binding_methods = { {"init", Init}, {"createNode", CreateNode}, @@ -2055,7 +2065,6 @@ std::vector binding_methods = { {"rclTakeResponse", RclTakeResponse}, {"sendRequest", SendRequest}, {"createService", CreateService}, - {"getServiceName", GetServiceName}, {"rclTakeRequest", RclTakeRequest}, {"sendResponse", SendResponse}, {"shutdown", Shutdown}, @@ -2088,6 +2097,8 @@ std::vector binding_methods = { {"serviceServerIsAvailable", ServiceServerIsAvailable}, {"publishRawMessage", PublishRawMessage}, {"rclTakeRaw", RclTakeRaw}, + {"getClientServiceName", GetClientServiceName}, + {"getServiceServiceName", GetServiceServiceName}, {"", nullptr} #if ROS_VERSION > 2205 // 2205 == Humble , diff --git a/test/test-node-oo.js b/test/test-node-oo.js index 86e5a921..46abee44 100644 --- a/test/test-node-oo.js +++ b/test/test-node-oo.js @@ -456,14 +456,14 @@ describe('topic & serviceName getter/setter', function () { it('client: serviceName property getter', function () { var node = new rclnodejs.Node('client', '/servicename_getter'); var client = node.createClient(AddTwoInts, 'add_two_ints'); - assert.deepStrictEqual(client.serviceName, 'add_two_ints'); + assert.deepStrictEqual(client.serviceName, '/servicename_getter/add_two_ints'); node.destroy(); }); it('service: topic property getter', function () { var node = new rclnodejs.Node('service', '/servicename_getter'); var service = node.createService(AddTwoInts, 'add_two_ints', (req) => {}); - assert.deepStrictEqual(service.serviceName, 'add_two_ints'); + assert.deepStrictEqual(service.serviceName, '/servicename_getter/add_two_ints'); node.destroy(); }); }); diff --git a/test/test-node.js b/test/test-node.js index 93d01af0..3bf980eb 100644 --- a/test/test-node.js +++ b/test/test-node.js @@ -457,14 +457,14 @@ describe('topic & serviceName getter/setter', function () { it('client: serviceName property getter', function () { var node = rclnodejs.createNode('client', '/servicename_getter'); var client = node.createClient(AddTwoInts, 'add_two_ints'); - assert.deepStrictEqual(client.serviceName, 'add_two_ints'); + assert.deepStrictEqual(client.serviceName, '/servicename_getter/add_two_ints'); node.destroy(); }); it('service: topic property getter', function () { var node = rclnodejs.createNode('service', '/servicename_getter'); var service = node.createService(AddTwoInts, 'add_two_ints', (req) => {}); - assert.deepStrictEqual(service.serviceName, 'add_two_ints'); + assert.deepStrictEqual(service.serviceName, '/servicename_getter/add_two_ints'); node.destroy(); }); }); diff --git a/test/test-remapping.js b/test/test-remapping.js index 8a5247bb..94061830 100644 --- a/test/test-remapping.js +++ b/test/test-remapping.js @@ -66,7 +66,7 @@ describe('rcl node remapping', function () { (request) => {} ); - assert.equal(service.serviceName, 'foo_service'); + assert.equal(service.serviceName, '/foo_service'); }); it('remap node, namespace, topic and servicename', async function () { @@ -95,6 +95,6 @@ describe('rcl node remapping', function () { assert.equal(node.name(), 'foo_node'); assert.equal(node.namespace(), '/foo_ns'); assert.equal(publisher.topic, '/foo_ns/foo_topic'); - assert.equal(service.serviceName, 'foo_service'); + assert.equal(service.serviceName, '/foo_ns/foo_service'); }); });