search

ros2 / examples

Example packages for ROS 2
Apache License 2.0
714 stars 316 forks source link

Linting errors in examples #286

Closed athackst closed 4 years ago

athackst commented 4 years ago

Bug report

Required Info:

Steps to reproduce issue

The following linters show errors in this package

ament_cpplint ``` /home/runner/work/vscode_ros2_workspace/vscode_ros2_workspace/src/examples/rclcpp/multithreaded_executor/multithreaded_executor.cpp:0: No copyright message found. You should have a line: "Copyright [year] " [legal/copyright] [5] Category 'legal/copyright' errors found: 1 Total errors found: 1 ```
ament_flake8 ``` src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server.py:45 in public method `goal_callback`: D401: First line should be in imperative mood (perhaps 'Accept', not 'Accepts') src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server.py:51 in public method `cancel_callback`: D401: First line should be in imperative mood (perhaps 'Accept', not 'Accepts') src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server.py:56 in public method `execute_callback`: D401: First line should be in imperative mood (perhaps 'Execute', not 'Executes') checking src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_defer.py src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_defer.py:48 in public method `goal_callback`: D401: First line should be in imperative mood (perhaps 'Accept', not 'Accepts') src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_defer.py:53 in public method `handle_accepted_callback`: D401: First line should be in imperative mood (perhaps 'Provide', not 'Provides') src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_defer.py:59 in public method `cancel_callback`: D401: First line should be in imperative mood (perhaps 'Accept', not 'Accepts') src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_defer.py:70 in public method `execute_callback`: D401: First line should be in imperative mood (perhaps 'Execute', not 'Executes') checking src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_not_composable.py src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_not_composable.py:35 in public function `execute_callback`: D401: First line should be in imperative mood (perhaps 'Execute', not 'Executes') checking src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_queue_goals.py src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_queue_goals.py:42 in public method `goal_callback`: D401: First line should be in imperative mood (perhaps 'Accept', not 'Accepts') src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_queue_goals.py:47 in public method `cancel_callback`: D401: First line should be in imperative mood (perhaps 'Accept', not 'Accepts') src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_queue_goals.py:52 in public method `execute_callback`: D401: First line should be in imperative mood (perhaps 'Execute', not 'Executes') checking src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_single_goal.py src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_single_goal.py:49 in public method `goal_callback`: D401: First line should be in imperative mood (perhaps 'Accept', not 'Accepts') src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_single_goal.py:65 in public method `cancel_callback`: D401: First line should be in imperative mood (perhaps 'Accept', not 'Accepts') src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_single_goal.py:70 in public method `execute_callback`: D401: First line should be in imperative mood (perhaps 'Execute', not 'Executes') ```
ament_pep257 ``` src/examples/rclpy/actions/minimal_action_client/examples_rclpy_minimal_action_client/client_cancel.py:15:1: F401 'action_msgs.msg.GoalStatus' imported but unused from action_msgs.msg import GoalStatus ^ src/examples/rclpy/actions/minimal_action_client/examples_rclpy_minimal_action_client/client_cancel.py:20:1: F401 'rclpy.callback_groups.ReentrantCallbackGroup' imported but unused from rclpy.callback_groups import ReentrantCallbackGroup ^ src/examples/rclpy/actions/minimal_action_client/examples_rclpy_minimal_action_client/client_not_composable.py:21:1: F401 'rclpy.node.Node' imported but unused from rclpy.node import Node ^ src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server.py:45:1: D401 First line should be in imperative mood """Accepts or rejects a client request to begin an action.""" ^ src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server.py:51:1: D401 First line should be in imperative mood """Accepts or rejects a client request to cancel an action.""" ^ src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server.py:51:1: D401 First line should be in imperative mood """Accepts or rejects a client request to cancel an action.""" ^ src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server.py:56:1: D401 First line should be in imperative mood """Executes a goal.""" ^ src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_queue_goals.py:42:1: D401 First line should be in imperative mood """Accepts or rejects a client request to begin an action.""" ^ src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_queue_goals.py:47:1: D401 First line should be in imperative mood """Accepts or rejects a client request to cancel an action.""" ^ src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_queue_goals.py:52:1: D401 First line should be in imperative mood """Executes a goal.""" ^ src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_defer.py:48:1: D401 First line should be in imperative mood """Accepts or rejects a client request to begin an action.""" ^ src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_defer.py:53:1: D401 First line should be in imperative mood """Provides a handle to an accepted goal.""" ^ src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_defer.py:59:1: D401 First line should be in imperative mood """Accepts or rejects a client request to cancel an action.""" ^ src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_defer.py:70:1: D401 First line should be in imperative mood """Executes a goal.""" ^ src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_not_composable.py:23:1: F401 'rclpy.node.Node' imported but unused from rclpy.node import Node ^ src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_not_composable.py:35:1: D401 First line should be in imperative mood """Executes the goal.""" ^ src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_single_goal.py:49:1: D401 First line should be in imperative mood """Accepts or rejects a client request to begin an action.""" ^ src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_single_goal.py:65:1: D401 First line should be in imperative mood """Accepts or rejects a client request to cancel an action.""" ^ src/examples/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_single_goal.py:70:1: D401 First line should be in imperative mood """Executes the goal.""" ^ 14 D401 First line should be in imperative mood 4 F401 'action_msgs.msg.GoalStatus' imported but unused 55 files checked 18 errors 'D'-type errors: 14 'F'-type errors: 4 ```
ament_lint_cmake ``` src/examples/rclcpp/minimal_composition/CMakeLists.txt:30: Mismatching spaces inside () after command [whitespace/mismatch] ```

The fixes for these are pretty straight forward and I've implemented them here:

https://github.com/athackst/examples/tree/foxy https://github.com/athackst/examples/tree/eloquent https://github.com/athackst/examples/tree/dashing

Is this something of use to this repo?

clalancette commented 4 years ago

In general, yes. What I'm confused about is why these problems don't show up in CI or my local testing. @athackst did you have to enable more things in the CMakeLists.txt to see these errors?

If you do open a PR, please open one targeting the latest (master branch), and then we can consider whether it is worthwhile to backport from there. Thanks.

athackst commented 4 years ago

Ah. If you want to add them as unit tests then yes, you'll have to add more things to the CMakelists.txt file. To see the errors you just have to run the commands on the repo. I found them when I was setting up a vscode/ros2 template repo which includes linting in a GitHub action. You may want to consider doing something similar.

To fix the unit tests you'd have to add them to the minimal_action_client and minimal_action_server as well as edit some of the multithreaded_executer and minimal_composition CMakelists.txt

clalancette commented 4 years ago

Ah, I see what happened here. https://github.com/ros2/examples/commit/df1f37c833e63392bd544d1cf39ab86b7581b1b4 fixed up all of the problems you see on master; we just haven't backported that to any of the active distributions.

@athackst If it makes your life easier, please feel free to open up backport PRs for that commit. If it doesn't make too much difference to you one way or another, please close this out. Thanks for the report.

athackst commented 4 years ago

I think that df1f37c only fixes the CPP packages. I've opened a PR to address the python packages. I would like to backport it, if possible.

athackst commented 4 years ago

Opened a backport for foxy. Is that what you need? I did have to sign off again on the cherry-picks, so it might be easier on your end. I can go ahead and open the others if that looks good to you.

jacobperron commented 4 years ago

@athackst Thanks for the PRs; all are merged :bow: