Skip to content

Commit

Permalink
aligh with rcl that a rosout publisher of a node might not exist (#1196)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
Chen Lihui authored Nov 15, 2023
1 parent 6cd6fd1 commit f909278
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 13 deletions.
3 changes: 1 addition & 2 deletions rclpy/rclpy/impl/rcutils_logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
15 changes: 10 additions & 5 deletions rclpy/src/rclpy/_rclpy_logging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}
}

Expand All @@ -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();
}
}

Expand Down
13 changes: 7 additions & 6 deletions rclpy/test/test_rosout_subscription.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__':
Expand Down

0 comments on commit f909278

Please sign in to comment.