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
RobotWebTools · Jan 10, 2024 · c1bf170 · c1bf170
1 parent e7d144f
commit c1bf170
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 20 deletions.
diff --git 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
@@ -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
@@ -1107,16 +1107,6 @@ NAN_METHOD(RclTakeRequest) {
   info.GetReturnValue().Set(Nan::Undefined());
 }
 
-NAN_METHOD(GetServiceName) {
-  rcl_service_t* service = reinterpret_cast<rcl_service_t*>(
-      RclHandle::Unwrap<RclHandle>(
-          Nan::To<v8::Object>(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<rcl_service_t*>(
       RclHandle::Unwrap<RclHandle>(
@@ -2017,6 +2007,26 @@ NAN_METHOD(RclTakeRaw) {
                            "Failed to deallocate message buffer");
 }
 
+NAN_METHOD(GetClientServiceName) {
+  rcl_client_t* client = reinterpret_cast<rcl_client_t*>(
+      RclHandle::Unwrap<RclHandle>(
+          Nan::To<v8::Object>(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<rcl_service_t*>(
+      RclHandle::Unwrap<RclHandle>(
+          Nan::To<v8::Object>(info[0]).ToLocalChecked())
+          ->ptr());
+
+  const char* service_name = rcl_service_get_service_name(service);
+  info.GetReturnValue().Set(Nan::New(service_name).ToLocalChecked());
+}
+
 std::vector<BindingMethod> binding_methods = {
     {"init", Init},
     {"createNode", CreateNode},
@@ -2055,7 +2065,6 @@ std::vector<BindingMethod> binding_methods = {
     {"rclTakeResponse", RclTakeResponse},
     {"sendRequest", SendRequest},
     {"createService", CreateService},
-    {"getServiceName", GetServiceName},
     {"rclTakeRequest", RclTakeRequest},
     {"sendResponse", SendResponse},
     {"shutdown", Shutdown},
@@ -2088,6 +2097,8 @@ std::vector<BindingMethod> 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
@@ -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
@@ -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
@@ -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');
   });
 });