Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

a rosout publisher of a node might not exist #1115

Merged
merged 4 commits into from
Nov 15, 2023
Merged

a rosout publisher of a node might not exist #1115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
11103d7
a rosout publisher of a node might not exist
Nov 3, 2023
6df738b
use NOT_FOUND instead of OK
Nov 6, 2023
c28fa31
keep consistant behavior with setting error message if not return OK
Nov 9, 2023
b376e58
address review
Nov 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion rcl/include/rcl/logging_rosout.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ rcl_logging_rosout_output_handler(
* \param[in] sublogger_name a sublogger name
* \return #RCL_RET_OK if the subordinate logger was created successfully, or
* \return #RCL_RET_INVALID_ARGUMENT if any arguments are invalid, or
* \return #RCL_RET_SUBLOGGER_ALREADY_EXIST if the subordinate logger already exists, or
fujitatomoya marked this conversation as resolved.
Show resolved Hide resolved
* \return #RCL_RET_NOT_FOUND if the parent logger does not exist, or
* \return #RCL_RET_BAD_ALLOC if allocating memory failed, or
* \return #RCL_RET_ERROR if an unspecified error occurs.
*/
Expand Down Expand Up @@ -242,6 +242,7 @@ rcl_logging_rosout_add_sublogger(
* \param[in] sublogger_name a sublogger name
* \return #RCL_RET_OK if the subordinate logger was finalized successfully, or
* \return #RCL_RET_INVALID_ARGUMENT if any arguments are invalid, or
* \return #RCL_RET_NOT_FOUND if the sublogger does not exist, or
* \return #RCL_RET_BAD_ALLOC if allocating memory failed, or
* \return #RCL_RET_ERROR if an unspecified error occurs.
*/
Expand Down
2 changes: 2 additions & 0 deletions rcl/include/rcl/types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ typedef rmw_ret_t rcl_ret_t;
#define RCL_RET_UNKNOWN_SUBSTITUTION 105
/// rcl_shutdown() already called return code.
#define RCL_RET_ALREADY_SHUTDOWN 106
/// Resource not found
#define RCL_RET_NOT_FOUND 107

// rcl node specific ret codes in 2XX
/// Invalid rcl_node_t given return code.
Expand Down
2 changes: 2 additions & 0 deletions rcl/src/rcl/common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ rcl_convert_rcutils_ret_to_rcl_ret(rcutils_ret_t rcutils_ret)
return RCL_RET_INVALID_ARGUMENT;
case RCUTILS_RET_NOT_INITIALIZED:
return RCL_RET_NOT_INIT;
case RCUTILS_RET_NOT_FOUND:
return RCL_RET_NOT_FOUND;
default:
return RCL_RET_ERROR;
}
Expand Down
26 changes: 16 additions & 10 deletions rcl/src/rcl/logging_rosout.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "rcl/logging_rosout.h"

#include "rcl/allocator.h"
#include "rcl/common.h"
#include "rcl/error_handling.h"
#include "rcl/node.h"
#include "rcl/publisher.h"
Expand Down Expand Up @@ -438,20 +439,23 @@ rcl_logging_rosout_add_sublogger(
rosout_map_entry_t entry;
rosout_sublogger_entry_t sublogger_entry;

RCL_CHECK_ARGUMENT_FOR_NULL(logger_name, RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(sublogger_name, RCL_RET_INVALID_ARGUMENT);
fujitatomoya marked this conversation as resolved.
Show resolved Hide resolved
rcutils_ret_t rcutils_ret = rcutils_hash_map_get(&__logger_map, &logger_name, &entry);
if (RCUTILS_RET_OK != rcutils_ret) {
if (RCUTILS_RET_NOT_FOUND == rcutils_ret) {
RCL_SET_ERROR_MSG_WITH_FORMAT_STRING("Failed to get logger entry for '%s'.", logger_name);
}
return rcl_convert_rcutils_ret_to_rcl_ret(rcutils_ret);
}

status =
_rcl_logging_rosout_get_full_sublogger_name(logger_name, sublogger_name, &full_sublogger_name);
if (RCL_RET_OK != status) {
// Error already set
return status;
}

rcutils_ret_t rcutils_ret = rcutils_hash_map_get(&__logger_map, &logger_name, &entry);
if (RCUTILS_RET_OK != rcutils_ret) {
RCL_SET_ERROR_MSG_WITH_FORMAT_STRING("The entry of logger '%s' not exist.", logger_name);
status = RCL_RET_ERROR;
goto cleanup;
}

if (rcutils_hash_map_key_exists(&__logger_map, &full_sublogger_name)) {
// To get the entry and increase the reference count
status = rcl_ret_from_rcutils_ret(
Expand Down Expand Up @@ -517,6 +521,8 @@ rcl_logging_rosout_remove_sublogger(
rcl_ret_t status = RCL_RET_OK;
char * full_sublogger_name = NULL;
rosout_sublogger_entry_t sublogger_entry;
RCL_CHECK_ARGUMENT_FOR_NULL(logger_name, RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(sublogger_name, RCL_RET_INVALID_ARGUMENT);

status =
_rcl_logging_rosout_get_full_sublogger_name(logger_name, sublogger_name, &full_sublogger_name);
Expand All @@ -525,13 +531,13 @@ rcl_logging_rosout_remove_sublogger(
return status;
}

// remove the entry from the map
if (!rcutils_hash_map_key_exists(&__logger_map, &full_sublogger_name)) {
RCL_SET_ERROR_MSG_WITH_FORMAT_STRING("Sub-logger '%s' not exist.", full_sublogger_name);
status = RCL_RET_ERROR;
status = RCL_RET_NOT_FOUND;
RCL_SET_ERROR_MSG_WITH_FORMAT_STRING("Logger for '%s' not found.", full_sublogger_name);
goto cleanup;
}

// remove the entry from the map
status = rcl_ret_from_rcutils_ret(
rcutils_hash_map_get(&__sublogger_map, &full_sublogger_name, &sublogger_entry));
if (RCL_RET_OK != status) {
Expand Down
6 changes: 3 additions & 3 deletions rcl/test/rcl/test_logging_rosout.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,9 +380,9 @@ TEST_F(
rcl_reset_error();
EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_logging_rosout_add_sublogger(logger_name, ""));
rcl_reset_error();
EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_logging_rosout_add_sublogger("", "child"));
EXPECT_EQ(RCL_RET_NOT_FOUND, rcl_logging_rosout_add_sublogger("", "child"));
rcl_reset_error();
EXPECT_EQ(RCL_RET_ERROR, rcl_logging_rosout_add_sublogger("no_exist", "child"));
EXPECT_EQ(RCL_RET_NOT_FOUND, rcl_logging_rosout_add_sublogger("no_exist", "child"));
rcl_reset_error();

EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_logging_rosout_remove_sublogger(nullptr, nullptr));
Expand All @@ -407,7 +407,7 @@ TEST_F(

EXPECT_EQ(RCL_RET_OK, rcl_logging_rosout_add_sublogger(logger_name, "child2"));
EXPECT_EQ(RCL_RET_OK, rcl_logging_rosout_remove_sublogger(logger_name, "child2"));
EXPECT_EQ(RCL_RET_ERROR, rcl_logging_rosout_remove_sublogger(logger_name, "child2"));
EXPECT_EQ(RCL_RET_NOT_FOUND, rcl_logging_rosout_remove_sublogger(logger_name, "child2"));
rcl_reset_error();
}

Expand Down