a rosout publisher of a node might not exist #1115
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
to return OK if add/remove sublogger if the rosout publisher not exist rather than using a new `get` function. Signed-off-by: Chen Lihui <[email protected]>
iuhilnehc-ynos
requested review from
audrow,
ivanpauno and
wjwwood
as code owners
|
CI:
|
fujitatomoya
reviewed
Nov 3, 2023
|
Thanks for looking at this fix so quickly! Just leaving a comment here that this should also be backported to Iron. |
Signed-off-by: Chen Lihui <[email protected]>
|
CI:
|
Signed-off-by: Chen Lihui <[email protected]>
|
@clalancette Could you please help review it? |
SteveMacenski
approved these changes
Nov 9, 2023
clalancette
requested changes
Nov 13, 2023
rcl/include/rcl/logging_rosout.h
Outdated
| @@ -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 | |||
| * \return #RCL_RET_NOT_FOUND if the parent logger not exists, or | |||
There was a problem hiding this comment.
Suggested change
| * \return #RCL_RET_NOT_FOUND if the parent logger not exists, or | |
| * \return #RCL_RET_NOT_FOUND if the parent logger does not exist, or |
rcl/include/rcl/logging_rosout.h
Outdated
| @@ -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 not exists, or | |||
There was a problem hiding this comment.
Suggested change
| * \return #RCL_RET_NOT_FOUND if the sublogger not exists, or | |
| * \return #RCL_RET_NOT_FOUND if the sublogger does not exist, or |
rcl/src/rcl/logging_rosout.c
Outdated
Comment on lines
445
to
449
| if (RCL_RET_OK != (status = rcl_convert_rcutils_ret_to_rcl_ret(rcutils_ret))) { | ||
| if (RCL_RET_NOT_FOUND == status) { | ||
| RCL_SET_ERROR_MSG_WITH_FORMAT_STRING("Failed to get logger entry for '%s'.", logger_name); | ||
| } | ||
| return status; |
There was a problem hiding this comment.
I think we can make this easier to read with:
Suggested change
| if (RCL_RET_OK != (status = rcl_convert_rcutils_ret_to_rcl_ret(rcutils_ret))) { | |
| if (RCL_RET_NOT_FOUND == status) { | |
| RCL_SET_ERROR_MSG_WITH_FORMAT_STRING("Failed to get logger entry for '%s'.", logger_name); | |
| } | |
| return status; | |
| if (RCUTILS_RET_OK != rcutils_ret) { | |
| if (RCUTILS_RET_NOT_FOUND == status) { | |
| 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); |
Co-authored-by: Chris Lalancette <[email protected]> Signed-off-by: Chen Lihui <[email protected]>
Thank you. Updated in b376e58. |
|
CI:
|
clalancette
approved these changes
Nov 15, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
to fix ros2/rclpy#1195