Skip to content

Commit

Permalink
Merge branch 'ros-controls:master' into issue-695
Browse files Browse the repository at this point in the history
  • Loading branch information
bailaC authored Dec 20, 2023
2 parents 0260d1e + 89c8658 commit f069bdd
Show file tree
Hide file tree
Showing 36 changed files with 290 additions and 60 deletions.
1 change: 1 addition & 0 deletions .clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ DerivePointerAlignment: false
PointerAlignment: Middle
ReflowComments: true
IncludeBlocks: Preserve
InsertBraces: true
...
2 changes: 1 addition & 1 deletion .github/ISSUE_TEMPLATE/good-first-issue.md
Original file line number Diff line number Diff line change
Expand Up @@ -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!**
14 changes: 14 additions & 0 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
28 changes: 10 additions & 18 deletions .github/reviewer-lottery.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,28 +13,20 @@ groups:
- name: reviewers
reviewers: 5
usernames:
- rosterloh
- progtologist
- aprotyas
- arne48
- bijoua29
- christophfroehlich
- DasRoteSkelett
- sgmurray
- harderthan
- jaron-l
- malapatiravi
- 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
2 changes: 1 addition & 1 deletion .github/workflows/ci-coverage-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ jobs:
file: ros_ws/lcov/total_coverage.info
flags: unittests
name: codecov-umbrella
- uses: actions/upload-artifact@v3.1.3
- uses: actions/upload-artifact@v4.0.0
with:
name: colcon-logs-ubuntu-22.04-coverage-rolling
path: ros_ws/log
2 changes: 1 addition & 1 deletion .github/workflows/ci-format.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/reusable-ros-tooling-source-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ jobs:
https://raw.githubusercontent.com/ros2/ros2/${{ inputs.ros2_repo_branch }}/ros2.repos
https://raw.githubusercontent.com/${{ github.repository }}/${{ github.sha }}/ros2_control.${{ inputs.ros_distro }}.repos?token=${{ secrets.GITHUB_TOKEN }}
colcon-mixin-repository: https://raw.githubusercontent.com/colcon/colcon-mixin-repository/master/index.yaml
- uses: actions/upload-artifact@v3.1.3
- uses: actions/upload-artifact@v4.0.0
with:
name: colcon-logs-ubuntu-22.04
path: ros_ws/log
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 3 additions & 0 deletions controller_interface/CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
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>`_)
Expand Down
2 changes: 1 addition & 1 deletion controller_interface/package.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>4.1.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>
Expand Down
8 changes: 8 additions & 0 deletions controller_manager/CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@
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>`_)
Expand Down
1 change: 1 addition & 0 deletions controller_manager/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ if(BUILD_TESTING)
test_chainable_controller
ros2_control_test_assets::ros2_control_test_assets
)
set_tests_properties(test_controller_manager_srvs PROPERTIES TIMEOUT 120)
ament_target_dependencies(test_controller_manager_srvs
controller_manager_msgs
)
Expand Down
4 changes: 3 additions & 1 deletion controller_manager/doc/controller_chaining.rst
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ One can also think of it as an actual chain, you can not add a chain link or bre
Debugging outputs
----------------------------

Flag ``unavailable`` on reference interface does not provide much information about anything at the moment. So don't get confused by it. The reason we have it are internal implementation reasons irelevant for the usage.
- The reference interfaces are ``unavailable`` and ``unclaimed``, when the controller exporting them is in inactive state
- The reference interfaces are ``available`` and ``unclaimed``, when the controller exporting them is in an active state but is not in chained mode with any other controller (The controllers gets its references from the subscriber)
- The reference interfaces are ``available`` and ``claimed``, when the controller exporting them is in active state and also in chained mode with other controllers (The controller gets its references from the controllers it is chained with)


Closing remarks
Expand Down
2 changes: 1 addition & 1 deletion controller_manager/package.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>4.1.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>
Expand Down
58 changes: 38 additions & 20 deletions controller_manager/src/controller_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,9 @@ std::vector<std::string> get_following_controller_names(
}
// If the controller is not configured, return empty
if (!(is_controller_active(controller_it->c) || is_controller_inactive(controller_it->c)))
{
return following_controllers;
}
const auto cmd_itfs = controller_it->c->command_interface_configuration().names;
for (const auto & itf : cmd_itfs)
{
Expand Down Expand Up @@ -210,7 +212,10 @@ std::vector<std::string> get_preceding_controller_names(
for (const auto & ctrl : controllers)
{
// If the controller is not configured, then continue
if (!(is_controller_active(ctrl.c) || is_controller_inactive(ctrl.c))) continue;
if (!(is_controller_active(ctrl.c) || is_controller_inactive(ctrl.c)))
{
continue;
}
auto cmd_itfs = ctrl.c->command_interface_configuration().names;
for (const auto & itf : cmd_itfs)
{
Expand Down Expand Up @@ -1381,6 +1386,11 @@ void ControllerManager::deactivate_controllers(
{
const auto new_state = controller->get_node()->deactivate();
controller->release_interfaces();
// if it is a chainable controller, make the reference interfaces unavailable on deactivation
if (controller->is_chainable())
{
resource_manager_->make_controller_reference_interfaces_unavailable(request);
}
if (new_state.id() != lifecycle_msgs::msg::State::PRIMARY_STATE_INACTIVE)
{
RCLCPP_ERROR(
Expand Down Expand Up @@ -1414,18 +1424,10 @@ void ControllerManager::switch_chained_mode(
auto controller = found_it->c;
if (!is_controller_active(*controller))
{
if (controller->set_chained_mode(to_chained_mode))
{
if (to_chained_mode)
{
resource_manager_->make_controller_reference_interfaces_available(request);
}
else
{
resource_manager_->make_controller_reference_interfaces_unavailable(request);
}
}
else
// if it is a chainable controller, make the reference interfaces available on preactivation
// (This is needed when you activate a couple of chainable controller altogether)
resource_manager_->make_controller_reference_interfaces_available(request);
if (!controller->set_chained_mode(to_chained_mode))
{
RCLCPP_ERROR(
get_logger(),
Expand Down Expand Up @@ -1562,6 +1564,12 @@ void ControllerManager::activate_controllers(
lifecycle_msgs::msg::State::PRIMARY_STATE_ACTIVE);
}

// if it is a chainable controller, make the reference interfaces available on activation
if (controller->is_chainable())
{
resource_manager_->make_controller_reference_interfaces_available(request);
}

if (controller->is_async())
{
async_controller_threads_.at(controller_name)->activate();
Expand Down Expand Up @@ -2443,7 +2451,10 @@ bool ControllerManager::controller_sorting(
if (!((is_controller_active(ctrl_a.c) || is_controller_inactive(ctrl_a.c)) &&
(is_controller_active(ctrl_b.c) || is_controller_inactive(ctrl_b.c))))
{
if (is_controller_active(ctrl_a.c) || is_controller_inactive(ctrl_a.c)) return true;
if (is_controller_active(ctrl_a.c) || is_controller_inactive(ctrl_a.c))
{
return true;
}
return false;
}

Expand All @@ -2454,10 +2465,9 @@ bool ControllerManager::controller_sorting(
// The case of the controllers that don't have any command interfaces. For instance,
// joint_state_broadcaster
// If the controller b is also under the same condition, then maintain their initial order
if (ctrl_b.c->command_interface_configuration().names.empty() || !ctrl_b.c->is_chainable())
return false;
else
return true;
const auto command_interfaces_exist =
!ctrl_b.c->command_interface_configuration().names.empty();
return ctrl_b.c->is_chainable() && command_interfaces_exist;
}
else if (ctrl_b.c->command_interface_configuration().names.empty() || !ctrl_b.c->is_chainable())
{
Expand All @@ -2467,12 +2477,17 @@ bool ControllerManager::controller_sorting(
else
{
auto following_ctrls = get_following_controller_names(ctrl_a.info.name, controllers);
if (following_ctrls.empty()) return false;
if (following_ctrls.empty())
{
return false;
}
// If the ctrl_b is any of the following controllers of ctrl_a, then place ctrl_a before ctrl_b
if (
std::find(following_ctrls.begin(), following_ctrls.end(), ctrl_b.info.name) !=
following_ctrls.end())
{
return true;
}
else
{
auto ctrl_a_preceding_ctrls = get_preceding_controller_names(ctrl_a.info.name, controllers);
Expand Down Expand Up @@ -2505,7 +2520,10 @@ bool ControllerManager::controller_sorting(

// If there is no common parent, then they belong to 2 different sets
auto following_ctrls_b = get_following_controller_names(ctrl_b.info.name, controllers);
if (following_ctrls_b.empty()) return true;
if (following_ctrls_b.empty())
{
return true;
}
auto find_first_element = [&](const auto & controllers_list) -> size_t
{
auto it = std::find_if(
Expand Down
Loading

0 comments on commit f069bdd

Please sign in to comment.