Add interfaces for logging service.#154
Conversation
|
this PR is a component of #ros2/rclcpp#2122 |
230553f to
f18b211
Compare
fujitatomoya
left a comment
There was a problem hiding this comment.
https://github.com/ros2/rcl_interfaces/blob/rolling/rcl_interfaces/README.md should also updated.
|
@llapx as interface, it looks good to me. just requesting update https://github.com/ros2/rcl_interfaces/blob/rolling/rcl_interfaces/README.md with additional message and service files. |
Updated, thanks. |
|
@gbiggs requesting review on this. |
|
I am not sure who to ask the review, since @gbiggs (maintainer according to pacakge.xml) not responding? CC: @ros2/dev |
|
@iuhilnehc-ynos can you review this? |
aprotyas
left a comment
There was a problem hiding this comment.
Just a couple of comments, not an exhaustive review by any means
Signed-off-by: Lei Liu <Lei.Liu.AP@sony.com>
Signed-off-by: Lei Liu <Lei.Liu.AP@sony.com>
Signed-off-by: Barry Xu <barry.xu@sony.com>
29251e0 to
fbda6c0
Compare
|
Rebase was done. |
|
CI:
|
Signed-off-by: Barry Xu <barry.xu@sony.com>
rcl_interfaces/msg/LoggerLevel.msg
Outdated
| # logger level: UNKNOWN, DEBUG, INFO, WARN, ERROR, FATAL | ||
| # 0 10 20 30 40 50 |
There was a problem hiding this comment.
| # logger level: UNKNOWN, DEBUG, INFO, WARN, ERROR, FATAL | |
| # 0 10 20 30 40 50 |
These are now specified as constants, so the comment isn't needed
Signed-off-by: Barry Xu <barry.xu@sony.com>
rcl_interfaces/README.md
Outdated
| * [SetParametersAtomically](srv/SetParametersAtomically.srv): Add or change all parameters in a list or none at all. | ||
| * [GetLoggerLevels](srv/GetLoggerLevels.srv): Get the logger levels of a specific list of nodes which are in the same process space. | ||
| * [SetLoggerLevels](srv/SetLoggerLevels.srv): Change a list of logger levels of nodes which are in the same process space. |
There was a problem hiding this comment.
| * [SetParametersAtomically](srv/SetParametersAtomically.srv): Add or change all parameters in a list or none at all. | |
| * [GetLoggerLevels](srv/GetLoggerLevels.srv): Get the logger levels of a specific list of nodes which are in the same process space. | |
| * [SetLoggerLevels](srv/SetLoggerLevels.srv): Change a list of logger levels of nodes which are in the same process space. | |
| * [SetParametersAtomically](srv/SetParametersAtomically.srv): Add or change all parameters in a list or none at all | |
| * [GetLoggerLevels](srv/GetLoggerLevels.srv): Get the logger levels of a specific list of nodes which are in the same process space | |
| * [SetLoggerLevels](srv/SetLoggerLevels.srv): Change a list of logger levels of nodes which are in the same process space |
rcl_interfaces/msg/LoggerLevel.msg
Outdated
| @@ -0,0 +1,13 @@ | |||
| # All available logger levels | |||
There was a problem hiding this comment.
We should note here that this will match what is in rcutils. Maybe something like:
| # All available logger levels | |
| # All available logger levels; these correspond to the enum in rcutils/logger.h |
rcl_interfaces/msg/LoggerLevel.msg
Outdated
| string name | ||
|
|
||
| # The logger level | ||
| uint8 level |
There was a problem hiding this comment.
Doesn't this need to be the same size as what rcutils can represent? In https://github.com/ros2/rcutils/blob/04aa9804feb46403f0058f4b089134a6985e19d3/include/rcutils/logging.h#L171-L179 , it is an enum, which I think is at least 4 bytes (uint32).
| uint8 level | |
| uint32 level |
|
@Barry-Xu-2018 i was trying to push the fix but no permission. can you check the unresolved comments? |
Signed-off-by: Barry Xu <barry.xu@sony.com>
|
I updated codes based on review comments. @fujitatomoya |
|
CI:
|
No description provided.