From f90927818e2f84de4dbcd0a0e3ebe3cd744e81d3 Mon Sep 17 00:00:00 2001
From: Chen Lihui <lihui.chen@sony.com>
Date: Wed, 15 Nov 2023 23:44:08 +0800
Subject: [PATCH] aligh with rcl that a rosout publisher of a node might not
 exist (#1196)

* aligh with rcl that a rosout publisher of a node might not exist

* check for RCL_RET_NOT_FOUND

* print detailed message for exception

reset error message for RCL_RET_NOT_FOUND as well

* test for a node with rosout disabled

Signed-off-by: Chen Lihui <lihui.chen@sony.com>
---
 rclpy/rclpy/impl/rcutils_logger.py     |  3 +--
 rclpy/src/rclpy/_rclpy_logging.cpp     | 15 ++++++++++-----
 rclpy/test/test_rosout_subscription.py | 13 +++++++------
 3 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/rclpy/rclpy/impl/rcutils_logger.py b/rclpy/rclpy/impl/rcutils_logger.py
index b8d999a30..c14cd793c 100644
--- a/rclpy/rclpy/impl/rcutils_logger.py
+++ b/rclpy/rclpy/impl/rcutils_logger.py
@@ -230,13 +230,12 @@ def get_child(self, name):
 
         if self.name:
             # Prepend the name of this logger
-            _rclpy.rclpy_logging_rosout_add_sublogger(self.name, name)
             fullname = self.name + _rclpy.rclpy_logging_get_separator_string() + name
         else:
             fullname = name
 
         logger = RcutilsLogger(name=fullname)
-        if self.name:
+        if self.name and _rclpy.rclpy_logging_rosout_add_sublogger(self.name, name):
             logger.logger_sublogger_namepair = (self.name, name)
         return logger
 
diff --git a/rclpy/src/rclpy/_rclpy_logging.cpp b/rclpy/src/rclpy/_rclpy_logging.cpp
index 7f95ebb43..083a51f5f 100644
--- a/rclpy/src/rclpy/_rclpy_logging.cpp
+++ b/rclpy/src/rclpy/_rclpy_logging.cpp
@@ -206,14 +206,19 @@ rclpy_logging_get_logging_directory()
 }
 
 /// Add a subordinate logger based on a logger
-void
+bool
 rclpy_logging_rosout_add_sublogger(const char * logger_name, const char * sublogger_name)
 {
   rclpy::LoggingGuard scoped_logging_guard;
   rcl_ret_t rcl_ret = rcl_logging_rosout_add_sublogger(logger_name, sublogger_name);
-  if (RCL_RET_OK != rcl_ret) {
-    rcutils_reset_error();
-    throw std::runtime_error("Failed to call rcl_logging_rosout_add_sublogger");
+  if (RCL_RET_OK == rcl_ret) {
+    return true;
+  } else if (RCL_RET_NOT_FOUND == rcl_ret) {
+    rcl_reset_error();
+    return false;
+  } else {
+    throw std::runtime_error(
+            rclpy::append_rcl_error("Failed to call rcl_logging_rosout_add_sublogger"));
   }
 }
 
@@ -225,7 +230,7 @@ rclpy_logging_rosout_remove_sublogger(const char * logger_name, const char * sub
   rcl_ret_t rcl_ret = rcl_logging_rosout_remove_sublogger(logger_name, sublogger_name);
 
   if (RCL_RET_OK != rcl_ret) {
-    rcutils_reset_error();
+    rcl_reset_error();
   }
 }
 
diff --git a/rclpy/test/test_rosout_subscription.py b/rclpy/test/test_rosout_subscription.py
index 037938cb6..f6332f973 100644
--- a/rclpy/test/test_rosout_subscription.py
+++ b/rclpy/test/test_rosout_subscription.py
@@ -114,12 +114,13 @@ def call_logger(logger):
         self.executor.spin_until_future_complete(self.fut, 3)
         self.assertTrue(self.fut.done())
 
-    def test_node_logger_not_exist(self):
-        node = rclpy.create_node('test_extra_node', context=self.context)
-        logger = node.get_logger()
-        node = None
-        with self.assertRaises(RuntimeError):
-            logger.get_child('child')
+    def test_logger_rosout_disabled_without_exception(self):
+        node = rclpy.create_node('mynode', context=self.context, enable_rosout=False)
+        try:
+            logger = node.get_logger().get_child('child')
+            logger.info('test')
+        except Exception as e:
+            self.fail(f'Not expected failure: {e}')
 
 
 if __name__ == '__main__':