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.

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

ros2node info: add action category #345

Merged
merged 4 commits into from
Oct 3, 2019

Conversation

claireyywang
Copy link
Contributor

Signed-off-by: claireyywang clairewang@openrobotics.org

Resolves #344

Adds separate action category to ros2 node info. Output looks like

clairewang@karr:~/ros2_ws/src/ros2/ros2cli/ros2node$ ros2 node info /fibonacci_action_server /fibonacci_action_server
  Subscribers:

  Publishers:
    /fibonacci/_action/feedback: action_tutorials_interfaces/action/Fibonacci_FeedbackMessage
    /fibonacci/_action/status: action_msgs/msg/GoalStatusArray
    /parameter_events: rcl_interfaces/msg/ParameterEvent
    /rosout: rcl_interfaces/msg/Log
  Services:
    /fibonacci/_action/cancel_goal: action_msgs/srv/CancelGoal
    /fibonacci/_action/get_result: action_tutorials_interfaces/action/Fibonacci_GetResult
    /fibonacci/_action/send_goal: action_tutorials_interfaces/action/Fibonacci_SendGoal
    /fibonacci_action_server/describe_parameters: rcl_interfaces/srv/DescribeParameters
    /fibonacci_action_server/get_parameter_types: rcl_interfaces/srv/GetParameterTypes
    /fibonacci_action_server/get_parameters: rcl_interfaces/srv/GetParameters
    /fibonacci_action_server/list_parameters: rcl_interfaces/srv/ListParameters
    /fibonacci_action_server/set_parameters: rcl_interfaces/srv/SetParameters
    /fibonacci_action_server/set_parameters_atomically: rcl_interfaces/srv/SetParametersAtomically
  Actions:
    /fibonacci: action_tutorials_interfaces/action/Fibonacci

Should I also modify Services category? It seems to include more than what's mentioned in #344

Signed-off-by: claireyywang <clairewang@openrobotics.org>
@claireyywang claireyywang added the in review Waiting for review (Kanban column) label Oct 2, 2019
@claireyywang claireyywang self-assigned this Oct 2, 2019
Signed-off-by: claireyywang <clairewang@openrobotics.org>
@maryaB-osr
Copy link
Contributor

Should I also modify Services category? It seems to include more than what's mentioned in #344

Yup, sorry I didn't really explain that part in the initial issue. All the results with /_action/ in the name are supposed to be hidden (leading underscore = hidden namespace)

The output should be like this comment. Also, did nothing come up under Subscribers:? There was a result under that category before

@claireyywang claireyywang added in progress Actively being worked on (Kanban column) and removed in review Waiting for review (Kanban column) labels Oct 2, 2019
@mjcarroll
Copy link
Member

I would propose adding --include-hidden much like:

parser.add_argument(

@mjcarroll
Copy link
Member

Also, did nothing come up under Subscribers:? There was a result under that category before

I believe because this is the Fibonacci example, there wouldn't be any subscribers, maybe turtlesim would be more representative now that an action has been included.

jacobperron
jacobperron previously approved these changes Oct 3, 2019
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely an improvement 👍

I think it would be even better if instead of a single list of actions, we made two lists for action clients and action servers, e.g.

  Subscribers:

  Publishers:
    /fibonacci/_action/feedback: action_tutorials_interfaces/action/Fibonacci_FeedbackMessage
    /fibonacci/_action/status: action_msgs/msg/GoalStatusArray
    /parameter_events: rcl_interfaces/msg/ParameterEvent
    /rosout: rcl_interfaces/msg/Log
  Services:
    /fibonacci/_action/cancel_goal: action_msgs/srv/CancelGoal
    /fibonacci/_action/get_result: action_tutorials_interfaces/action/Fibonacci_GetResult
    /fibonacci/_action/send_goal: action_tutorials_interfaces/action/Fibonacci_SendGoal
    /fibonacci_action_server/describe_parameters: rcl_interfaces/srv/DescribeParameters
    /fibonacci_action_server/get_parameter_types: rcl_interfaces/srv/GetParameterTypes
    /fibonacci_action_server/get_parameters: rcl_interfaces/srv/GetParameters
    /fibonacci_action_server/list_parameters: rcl_interfaces/srv/ListParameters
    /fibonacci_action_server/set_parameters: rcl_interfaces/srv/SetParameters
    /fibonacci_action_server/set_parameters_atomically: rcl_interfaces/srv/SetParametersAtomically
  Actions clients:

  Action servers:
    /fibonacci: action_tutorials_interfaces/action/Fibonacci

It should be possible with the action graph functions: get_action_client_names_and_types_by_node and get_action_server_names_and_types_by_node.

The same could be done for services, i.e. splitting it into two lists for distinguishing clients from servers (but I would worry about services in a separate PR).

@jacobperron
Copy link
Member

I think that hiding the hidden services and topics related to actions can be addressed in a follow-up PR (or this one if you like), since it's kind of a separate issue.

@claireyywang
Copy link
Contributor Author

I think that hiding the hidden services and topics related to actions can be addressed in a follow-up PR (or this one if you like), since it's kind of a separate issue.

I'll split up the action into client and server, then make another PR hiding the hidden services and splitting service into client and server.

Signed-off-by: claireyywang <clairewang@openrobotics.org>
@claireyywang claireyywang added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Oct 3, 2019
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nitpicks about indentation, otherwise LGTM with green CI

def get_action_server_info(*, node, remote_node_name):
remote_node = parse_node_name(remote_node_name)
names_and_types = get_action_server_names_and_types_by_node(
node, remote_node.name, remote_node.namespace)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: Though PEP 8 states that 4-space indentation is optional for continuation lines, I think it is preferred when possible (e.g. when it doesn't lead to visual ambiguity). I think it would also look more consistent with our Python code elsewhere if this were indented 4 spaces.

Same for other instances below.

def get_action_client_info(*, node, remote_node_name):
remote_node = parse_node_name(remote_node_name)
names_and_types = get_action_client_names_and_types_by_node(
node, remote_node.name, remote_node.namespace)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: use 4-space indentation

@@ -52,5 +54,13 @@ def main(self, *, args):
services = get_service_info(node=node, remote_node_name=args.node_name)
print(' Services:')
print_names_and_types(services)
actions_servers = get_action_server_info(
node=node, remote_node_name=args.node_name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: use 4-space indentation

print(' Actions Servers:')
print_names_and_types(actions_servers)
actions_clients = get_action_client_info(
node=node, remote_node_name=args.node_name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: use 4-space indentation

Signed-off-by: claireyywang <clairewang@openrobotics.org>
@claireyywang
Copy link
Contributor Author

claireyywang commented Oct 3, 2019

Linux Build Status
Linux-aaaarch64 Build Status
OSX Build Status
Windows Build Status

@claireyywang claireyywang merged commit 4384453 into master Oct 3, 2019
@delete-merged-branch delete-merged-branch bot deleted the claire/ros2node-add-action-category branch October 3, 2019 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in review Waiting for review (Kanban column)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Give actions their own category in ros2 node info results
4 participants