Merge branch 'master' into fix_multiple_cleanup_on_unconfigured_compo…

…nents
ros-controls · Dec 12, 2023 · 5ad0199 · 5ad0199
2 parents 63b725d + 3313796
commit 5ad0199
Show file tree

Hide file tree

Showing 51 changed files with 803 additions and 128 deletions.
diff --git a/.clang-format b/.clang-format
@@ -12,4 +12,5 @@ DerivePointerAlignment: false
 PointerAlignment: Middle
 ReflowComments: true
 IncludeBlocks: Preserve
+InsertBraces: true
 ...
diff --git a/.github/ISSUE_TEMPLATE/good-first-issue.md b/.github/ISSUE_TEMPLATE/good-first-issue.md
@@ -55,6 +55,6 @@ Don’t hesitate to ask questions or to get help if you feel like you are gettin
 Furthermore, you find helpful resources here:
 * [ROS2 Control Contribution Guide](https://control.ros.org/master/doc/contributing/contributing.html)
 * [ROS2 Tutorials](https://docs.ros.org/en/rolling/Tutorials.html)
-* [ROS Answers](https://answers.ros.org/questions/)
+* [Robotics Stack Exchange](https://robotics.stackexchange.com)
 
 **Good luck with your first issue!**
diff --git a/.github/dependabot.yml b/.github/dependabot.yml
@@ -11,3 +11,17 @@ updates:
     directory: "/"
     schedule:
       interval: "weekly"
+  - package-ecosystem: "github-actions"
+    # Workflow files stored in the
+    # default location of `.github/workflows`
+    directory: "/"
+    schedule:
+      interval: "weekly"
+    target-branch: "humble"
+  - package-ecosystem: "github-actions"
+    # Workflow files stored in the
+    # default location of `.github/workflows`
+    directory: "/"
+    schedule:
+      interval: "weekly"
+    target-branch: "iron"
diff --git a/.github/reviewer-lottery.yml b/.github/reviewer-lottery.yml
@@ -13,28 +13,21 @@ groups:
   - name: reviewers
     reviewers: 5
     usernames:
-      - rosterloh
-      - progtologist
+      - aprotyas
       - arne48
+      - bijoua29
       - christophfroehlich
       - DasRoteSkelett
-      - sgmurray
-      - harderthan
-      - jaron-l
-      - malapatiravi
+      - duringhof
       - erickisos
-      - sachinkum0009
-      - qiayuanliao
-      - homalozoa
-      - anfemosa
-      - jackcenter
-      - VX792
-      - mhubii
+      - fmauch
+      - jaron-l
       - livanov93
-      - aprotyas
+      - mcbed
+      - moriarty
+      - olivier-stasse
       - peterdavidfagan
-      - duringhof
+      - progtologist
+      - saikishor
       - VanshGehlot
-      - bijoua29
-      - LukasMacha97
-      - mcbed
+      - VX792
diff --git a/.github/workflows/ci-coverage-build.yml b/.github/workflows/ci-coverage-build.yml
@@ -17,7 +17,7 @@ jobs:
     env:
       ROS_DISTRO: rolling
     steps:
-      - uses: ros-tooling/[email protected].0
+      - uses: ros-tooling/[email protected].1
         with:
           required-ros-distributions: ${{ env.ROS_DISTRO }}
       - uses: actions/checkout@v4

diff --git a/.github/workflows/ci-format.yml b/.github/workflows/ci-format.yml
@@ -13,7 +13,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v4
-    - uses: actions/setup-python@v4.7.1
+    - uses: actions/setup-python@v5.0.0
       with:
         python-version: '3.10'
     - name: Install system hooks

diff --git a/.github/workflows/ci-ros-lint.yml b/.github/workflows/ci-ros-lint.yml
@@ -14,7 +14,7 @@ jobs:
       AMENT_CPPCHECK_ALLOW_SLOW_VERSIONS: true
     steps:
     - uses: actions/checkout@v4
-    - uses: ros-tooling/[email protected].0
+    - uses: ros-tooling/[email protected].1
     - uses: ros-tooling/[email protected]
       with:
         distribution: rolling
@@ -38,7 +38,7 @@ jobs:
           linter: [cpplint]
     steps:
     - uses: actions/checkout@v4
-    - uses: ros-tooling/[email protected].0
+    - uses: ros-tooling/[email protected].1
     - uses: ros-tooling/[email protected]
       with:
         distribution: rolling

diff --git a/.github/workflows/reusable-ros-tooling-source-build.yml b/.github/workflows/reusable-ros-tooling-source-build.yml
@@ -26,7 +26,7 @@ jobs:
     strategy:
       fail-fast: false
     steps:
-      - uses: ros-tooling/[email protected].0
+      - uses: ros-tooling/[email protected].1
         with:
           required-ros-distributions: ${{ inputs.ros_distro }}
       - uses: actions/checkout@v4

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -62,7 +62,7 @@ repos:
 
   # CPP hooks
   - repo: https://github.com/pre-commit/mirrors-clang-format
-    rev: v14.0.6
+    rev: v15.0.6
     hooks:
       - id: clang-format
 

diff --git a/controller_interface/CHANGELOG.rst b/controller_interface/CHANGELOG.rst
@@ -2,6 +2,21 @@
 Changelog for package controller_interface
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+4.2.0 (2023-12-12)
+------------------
+
+4.1.0 (2023-11-30)
+------------------
+* Add few warning compiler options to error (`#1181 <https://github.com/ros-controls/ros2_control/issues/1181>`_)
+* [ControllerInterface] Avoid warning about conversion from `int64_t` to `unsigned int` (`#1173 <https://github.com/ros-controls/ros2_control/issues/1173>`_)
+* Contributors: Dr. Denis, Sai Kishor Kothakota
+
+4.0.0 (2023-11-21)
+------------------
+* Pass controller manager update rate on the init of the controller interface  (`#1141 <https://github.com/ros-controls/ros2_control/issues/1141>`_)
+* Pass URDF to controllers on init (`#1088 <https://github.com/ros-controls/ros2_control/issues/1088>`_)
+* Contributors: Bence Magyar, Sai Kishor Kothakota
+
 3.21.0 (2023-11-06)
 -------------------
 

diff --git a/controller_interface/CMakeLists.txt b/controller_interface/CMakeLists.txt
@@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.16)
 project(controller_interface LANGUAGES CXX)
 
 if(CMAKE_CXX_COMPILER_ID MATCHES "(GNU|Clang)")
-  add_compile_options(-Wall -Wextra -Wconversion)
+  add_compile_options(-Wall -Wextra -Werror=conversion -Werror=unused-but-set-variable -Werror=return-type -Werror=shadow)
 endif()
 
 set(THIS_PACKAGE_INCLUDE_DEPENDS

diff --git a/controller_interface/include/controller_interface/controller_interface_base.hpp b/controller_interface/include/controller_interface/controller_interface_base.hpp
@@ -114,7 +114,7 @@ class ControllerInterfaceBase : public rclcpp_lifecycle::node_interfaces::Lifecy
 
   CONTROLLER_INTERFACE_PUBLIC
   virtual return_type init(
-    const std::string & controller_name, const std::string & urdf,
+    const std::string & controller_name, const std::string & urdf, unsigned int cm_update_rate,
     const std::string & namespace_ = "",
     const rclcpp::NodeOptions & node_options = rclcpp::NodeOptions().enable_logger_service(true));
 

diff --git a/controller_interface/package.xml b/controller_interface/package.xml
@@ -2,7 +2,7 @@
 <?xml-model href="http://download.ros.org/schema/package_format2.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
 <package format="2">
   <name>controller_interface</name>
-  <version>3.21.0</version>
+  <version>4.2.0</version>
   <description>Description of controller_interface</description>
   <maintainer email="[email protected]">Bence Magyar</maintainer>
   <maintainer email="[email protected]">Denis Štogl</maintainer>

diff --git a/controller_interface/src/controller_interface_base.cpp b/controller_interface/src/controller_interface_base.cpp
@@ -25,16 +25,16 @@
 namespace controller_interface
 {
 return_type ControllerInterfaceBase::init(
-  const std::string & controller_name, const std::string & urdf, const std::string & namespace_,
-  const rclcpp::NodeOptions & node_options)
+  const std::string & controller_name, const std::string & urdf, unsigned int cm_update_rate,
+  const std::string & namespace_, const rclcpp::NodeOptions & node_options)
 {
   node_ = std::make_shared<rclcpp_lifecycle::LifecycleNode>(
     controller_name, namespace_, node_options, false);  // disable LifecycleNode service interfaces
   urdf_ = urdf;
 
   try
   {
-    auto_declare<int>("update_rate", 0);
+    auto_declare<int>("update_rate", cm_update_rate);
     auto_declare<bool>("is_async", false);
   }
   catch (const std::exception & e)
@@ -85,7 +85,7 @@ const rclcpp_lifecycle::State & ControllerInterfaceBase::configure()
   // Other solution is to add check into the LifecycleNode if a transition is valid to trigger
   if (get_state().id() == lifecycle_msgs::msg::State::PRIMARY_STATE_UNCONFIGURED)
   {
-    update_rate_ = get_node()->get_parameter("update_rate").as_int();
+    update_rate_ = static_cast<unsigned int>(get_node()->get_parameter("update_rate").as_int());
     is_async_ = get_node()->get_parameter("is_async").as_bool();
   }
 

diff --git a/controller_interface/test/test_chainable_controller_interface.cpp b/controller_interface/test/test_chainable_controller_interface.cpp
@@ -21,7 +21,7 @@ TEST_F(ChainableControllerInterfaceTest, default_returns)
   TestableChainableControllerInterface controller;
 
   // initialize, create node
-  ASSERT_EQ(controller.init(TEST_CONTROLLER_NAME, ""), controller_interface::return_type::OK);
+  ASSERT_EQ(controller.init(TEST_CONTROLLER_NAME, "", 50.0), controller_interface::return_type::OK);
   ASSERT_NO_THROW(controller.get_node());
 
   EXPECT_TRUE(controller.is_chainable());
@@ -33,7 +33,7 @@ TEST_F(ChainableControllerInterfaceTest, export_reference_interfaces)
   TestableChainableControllerInterface controller;
 
   // initialize, create node
-  ASSERT_EQ(controller.init(TEST_CONTROLLER_NAME, ""), controller_interface::return_type::OK);
+  ASSERT_EQ(controller.init(TEST_CONTROLLER_NAME, "", 50.0), controller_interface::return_type::OK);
   ASSERT_NO_THROW(controller.get_node());
 
   auto reference_interfaces = controller.export_reference_interfaces();
@@ -50,7 +50,7 @@ TEST_F(ChainableControllerInterfaceTest, reference_interfaces_storage_not_correc
   TestableChainableControllerInterface controller;
 
   // initialize, create node
-  ASSERT_EQ(controller.init(TEST_CONTROLLER_NAME, ""), controller_interface::return_type::OK);
+  ASSERT_EQ(controller.init(TEST_CONTROLLER_NAME, "", 50.0), controller_interface::return_type::OK);
   ASSERT_NO_THROW(controller.get_node());
 
   // expect empty return because storage is not resized
@@ -64,7 +64,7 @@ TEST_F(ChainableControllerInterfaceTest, reference_interfaces_prefix_is_not_node
   TestableChainableControllerInterface controller;
 
   // initialize, create node
-  ASSERT_EQ(controller.init(TEST_CONTROLLER_NAME, ""), controller_interface::return_type::OK);
+  ASSERT_EQ(controller.init(TEST_CONTROLLER_NAME, "", 50.0), controller_interface::return_type::OK);
   ASSERT_NO_THROW(controller.get_node());
 
   controller.set_name_prefix_of_reference_interfaces("some_not_correct_interface_prefix");
@@ -79,7 +79,7 @@ TEST_F(ChainableControllerInterfaceTest, setting_chained_mode)
   TestableChainableControllerInterface controller;
 
   // initialize, create node
-  ASSERT_EQ(controller.init(TEST_CONTROLLER_NAME, ""), controller_interface::return_type::OK);
+  ASSERT_EQ(controller.init(TEST_CONTROLLER_NAME, "", 50.0), controller_interface::return_type::OK);
   ASSERT_NO_THROW(controller.get_node());
 
   auto reference_interfaces = controller.export_reference_interfaces();
@@ -126,7 +126,7 @@ TEST_F(ChainableControllerInterfaceTest, test_update_logic)
   TestableChainableControllerInterface controller;
 
   // initialize, create node
-  ASSERT_EQ(controller.init(TEST_CONTROLLER_NAME, ""), controller_interface::return_type::OK);
+  ASSERT_EQ(controller.init(TEST_CONTROLLER_NAME, "", 50.0), controller_interface::return_type::OK);
   ASSERT_NO_THROW(controller.get_node());
 
   EXPECT_FALSE(controller.is_in_chained_mode());

diff --git a/controller_interface/test/test_controller_interface.cpp b/controller_interface/test/test_controller_interface.cpp
@@ -38,15 +38,15 @@ TEST(TestableControllerInterface, init)
   ASSERT_THROW(controller.get_node(), std::runtime_error);
 
   // initialize, create node
-  ASSERT_EQ(controller.init(TEST_CONTROLLER_NAME, ""), controller_interface::return_type::OK);
+  ASSERT_EQ(controller.init(TEST_CONTROLLER_NAME, "", 10.0), controller_interface::return_type::OK);
   ASSERT_NO_THROW(controller.get_node());
 
   // update_rate is set to default 0
   ASSERT_EQ(controller.get_update_rate(), 0u);
 
-  // Even after configure is 0
+  // Even after configure is 10
   controller.configure();
-  ASSERT_EQ(controller.get_update_rate(), 0u);
+  ASSERT_EQ(controller.get_update_rate(), 10u);
 
   rclcpp::shutdown();
 }
@@ -60,7 +60,7 @@ TEST(TestableControllerInterface, setting_update_rate_in_configure)
 
   TestableControllerInterface controller;
   // initialize, create node
-  ASSERT_EQ(controller.init(TEST_CONTROLLER_NAME, ""), controller_interface::return_type::OK);
+  ASSERT_EQ(controller.init(TEST_CONTROLLER_NAME, "", 1.0), controller_interface::return_type::OK);
 
   // initialize executor to be able to get parameter update
   auto executor =
@@ -122,7 +122,8 @@ TEST(TestableControllerInterfaceInitError, init_with_error)
   TestableControllerInterfaceInitError controller;
 
   // initialize, create node
-  ASSERT_EQ(controller.init(TEST_CONTROLLER_NAME, ""), controller_interface::return_type::ERROR);
+  ASSERT_EQ(
+    controller.init(TEST_CONTROLLER_NAME, "", 100.0), controller_interface::return_type::ERROR);
 
   rclcpp::shutdown();
 }
@@ -136,7 +137,8 @@ TEST(TestableControllerInterfaceInitFailure, init_with_failure)
   TestableControllerInterfaceInitFailure controller;
 
   // initialize, create node
-  ASSERT_EQ(controller.init(TEST_CONTROLLER_NAME, ""), controller_interface::return_type::ERROR);
+  ASSERT_EQ(
+    controller.init(TEST_CONTROLLER_NAME, "", 50.0), controller_interface::return_type::ERROR);
 
   rclcpp::shutdown();
 }

diff --git a/controller_interface/test/test_controller_with_options.cpp b/controller_interface/test/test_controller_with_options.cpp
@@ -42,7 +42,7 @@ TEST(ControllerWithOption, init_with_overrides)
   rclcpp::init(argc, argv);
   // creates the controller
   FriendControllerWithOptions controller;
-  EXPECT_EQ(controller.init("controller_name", ""), controller_interface::return_type::OK);
+  EXPECT_EQ(controller.init("controller_name", "", 50.0), controller_interface::return_type::OK);
   // checks that the node options have been updated
   const auto & node_options = controller.get_node()->get_node_options();
   EXPECT_TRUE(node_options.allow_undeclared_parameters());
@@ -63,7 +63,7 @@ TEST(ControllerWithOption, init_without_overrides)
   rclcpp::init(argc, argv);
   // creates the controller
   FriendControllerWithOptions controller;
-  EXPECT_EQ(controller.init("controller_name", ""), controller_interface::return_type::ERROR);
+  EXPECT_EQ(controller.init("controller_name", "", 50.0), controller_interface::return_type::ERROR);
   // checks that the node options have been updated
   const auto & node_options = controller.get_node()->get_node_options();
   EXPECT_TRUE(node_options.allow_undeclared_parameters());

diff --git a/controller_interface/test/test_controller_with_options.hpp b/controller_interface/test/test_controller_with_options.hpp
@@ -40,14 +40,14 @@ class ControllerWithOptions : public controller_interface::ControllerInterface
   }
 
   controller_interface::return_type init(
-    const std::string & controller_name, const std::string & urdf,
+    const std::string & controller_name, const std::string & urdf, unsigned int cm_update_rate,
     const std::string & namespace_ = "",
     const rclcpp::NodeOptions & node_options =
       rclcpp::NodeOptions()
         .allow_undeclared_parameters(true)
         .automatically_declare_parameters_from_overrides(true)) override
   {
-    ControllerInterface::init(controller_name, urdf, namespace_, node_options);
+    ControllerInterface::init(controller_name, urdf, cm_update_rate, namespace_, node_options);
 
     switch (on_init())
     {

diff --git a/controller_manager/CHANGELOG.rst b/controller_manager/CHANGELOG.rst
@@ -2,6 +2,29 @@
 Changelog for package controller_manager
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+4.2.0 (2023-12-12)
+------------------
+* [CM] Linting if/else statements (`#1193 <https://github.com/ros-controls/ros2_control/issues/1193>`_)
+* Reformat with braces added (`#1209 <https://github.com/ros-controls/ros2_control/issues/1209>`_)
+* Report inactive controllers as a diagnostics ok instead of an error (`#1184 <https://github.com/ros-controls/ros2_control/issues/1184>`_)
+* Fix controller sorting issue while loading large number of controllers (`#1180 <https://github.com/ros-controls/ros2_control/issues/1180>`_)
+* Contributors: Bence Magyar, Dr. Denis, Lennart Nachtigall, Sai Kishor Kothakota
+
+4.1.0 (2023-11-30)
+------------------
+* Add few warning compiler options to error (`#1181 <https://github.com/ros-controls/ros2_control/issues/1181>`_)
+* [ControllerManager] Fix all warnings from the latets features. (`#1174 <https://github.com/ros-controls/ros2_control/issues/1174>`_)
+* Compute the actual update period for each controller (`#1140 <https://github.com/ros-controls/ros2_control/issues/1140>`_)
+* Contributors: Dr. Denis, Sai Kishor Kothakota
+
+4.0.0 (2023-11-21)
+------------------
+* Pass controller manager update rate on the init of the controller interface  (`#1141 <https://github.com/ros-controls/ros2_control/issues/1141>`_)
+* Fix the controller sorting bug when the interfaces are not configured (fixes `#1164 <https://github.com/ros-controls/ros2_control/issues/1164>`_) (`#1165 <https://github.com/ros-controls/ros2_control/issues/1165>`_)
+* Pass URDF to controllers on init (`#1088 <https://github.com/ros-controls/ros2_control/issues/1088>`_)
+* Remove deprecation warning (`#1101 <https://github.com/ros-controls/ros2_control/issues/1101>`_)
+* Contributors: Bence Magyar, Christoph Fröhlich, Sai Kishor Kothakota
+
 3.21.0 (2023-11-06)
 -------------------
 * Sort controllers while configuring instead of while activating (`#1107 <https://github.com/ros-controls/ros2_control/issues/1107>`_)

diff --git a/controller_manager/CMakeLists.txt b/controller_manager/CMakeLists.txt
@@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.16)
 project(controller_manager LANGUAGES CXX)
 
 if(CMAKE_CXX_COMPILER_ID MATCHES "(GNU|Clang)")
-  add_compile_options(-Wall -Wextra -Wconversion)
+  add_compile_options(-Wall -Wextra -Werror=conversion -Werror=unused-but-set-variable -Werror=return-type -Werror=shadow)
 endif()
 
 set(THIS_PACKAGE_INCLUDE_DEPENDS

diff --git a/controller_manager/package.xml b/controller_manager/package.xml
@@ -2,7 +2,7 @@
 <?xml-model href="http://download.ros.org/schema/package_format2.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
 <package format="2">
   <name>controller_manager</name>
-  <version>3.21.0</version>
+  <version>4.2.0</version>
   <description>Description of controller_manager</description>
   <maintainer email="[email protected]">Bence Magyar</maintainer>
   <maintainer email="[email protected]">Denis Štogl</maintainer>