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

Fix spam of logs on failed hardware component initialization #1719

Conversation

saikishor
Copy link
Member

When the hardware component fails, it continues to stay in the UNKNOWN lifecycle state, if the state is other than the UNCONFIGURED or FINALIZED the return_type is ERROR as default and then replaced by the outcome from read and write method. So, returning ERROR, makes it continuously spam with the following logs

RCLCPP_ERROR(
get_logger(),
"Deactivating following hardware components as their read cycle resulted in an error: [ %s]",
failed_hardware_string.c_str());

RCLCPP_ERROR(
get_logger(),
"Deactivating following hardware components as their write cycle resulted in an error: [ "
"%s]",
failed_hardware_string.c_str());

[ros2_control_node-2] [INFO] [1724675998.045345227] [controller_manager.resource_manager]: Loaded hardware 'ros2_control_tiago_system' from plugin 'robot_control/RobotControl'
[ros2_control_node-2] [INFO] [1724675998.045399502] [controller_manager.resource_manager]: Initialize hardware 'ros2_control_tiago_system' 
[ros2_control_node-2] [ERROR] [1724675999.154326093] [controller_manager.resource_manager]: Exception occurred while initializing hardware 'ros2_control_tiago_system': std::bad_alloc
[ros2_control_node-2] [WARN] [1724675999.158483059] [controller_manager.resource_manager]: System hardware component 'ros2_control_tiago_system' from plugin 'robot_control/RobotControl' failed to initialize.
[ros2_control_node-2] [ERROR] [1724675999.162434111] [controller_manager]: Deactivating following hardware components as their read cycle resulted in an error: [ ros2_control_tiago_system ]
[ros2_control_node-2] [ERROR] [1724675999.162489926] [controller_manager]: Deactivating following hardware components as their write cycle resulted in an error: [ ros2_control_tiago_system ]
[ros2_control_node-2] [ERROR] [1724675999.172426457] [controller_manager]: Deactivating following hardware components as their read cycle resulted in an error: [ ros2_control_tiago_system ]
[ros2_control_node-2] [ERROR] [1724675999.172483207] [controller_manager]: Deactivating following hardware components as their write cycle resulted in an error: [ ros2_control_tiago_system ]
[ros2_control_node-2] [ERROR] [1724675999.182431039] [controller_manager]: Deactivating following hardware components as their read cycle resulted in an error: [ ros2_control_tiago_system ]
[ros2_control_node-2] [ERROR] [1724675999.182491207] [controller_manager]: Deactivating following hardware components as their write cycle resulted in an error: [ ros2_control_tiago_system ]
[ros2_control_node-2] [ERROR] [1724675999.192426473] [controller_manager]: Deactivating following hardware components as their read cycle resulted in an error: [ ros2_control_tiago_system ]
[ros2_control_node-2] [ERROR] [1724675999.192488350] [controller_manager]: Deactivating following hardware components as their write cycle resulted in an error: [ ros2_control_tiago_system ]
[ros2_control_node-2] [ERROR] [1724675999.202426093] [controller_manager]: Deactivating following hardware components as their read cycle resulted in an error: [ ros2_control_tiago_system ]
[ros2_control_node-2] [ERROR] [1724675999.202480027] [controller_manager]: Deactivating following hardware components as their write cycle resulted in an error: [ ros2_control_tiago_system ]
[ros2_control_node-2] [ERROR] [1724675999.212425084] [controller_manager]: Deactivating following hardware components as their read cycle resulted in an error: [ ros2_control_tiago_system ]

This PR proposes fix for the above issue

Copy link

codecov bot commented Aug 26, 2024

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Project coverage is 86.75%. Comparing base (d200408) to head (80126ab).
Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
hardware_interface/src/actuator.cpp 0.00% 0 Missing and 2 partials ⚠️
hardware_interface/src/system.cpp 0.00% 0 Missing and 2 partials ⚠️
hardware_interface/src/sensor.cpp 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1719      +/-   ##
==========================================
- Coverage   86.78%   86.75%   -0.04%     
==========================================
  Files         116      116              
  Lines       10693    10682      -11     
  Branches      978      983       +5     
==========================================
- Hits         9280     9267      -13     
+ Misses       1061     1059       -2     
- Partials      352      356       +4     
Flag Coverage Δ
unittests 86.75% <0.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
hardware_interface/src/sensor.cpp 69.36% <0.00%> (-1.43%) ⬇️
hardware_interface/src/actuator.cpp 70.99% <0.00%> (-2.35%) ⬇️
hardware_interface/src/system.cpp 70.99% <0.00%> (-2.35%) ⬇️

... and 1 file with indirect coverage changes

Comment on lines 249 to 252
if (
impl_->get_lifecycle_state().id() == lifecycle_msgs::msg::State::PRIMARY_STATE_UNKNOWN ||
impl_->get_lifecycle_state().id() == lifecycle_msgs::msg::State::PRIMARY_STATE_UNCONFIGURED ||
impl_->get_lifecycle_state().id() == lifecycle_msgs::msg::State::PRIMARY_STATE_FINALIZED)
Copy link
Member

Choose a reason for hiding this comment

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

Might be a good idea to roll this into a method.

constexpr bool lifecycleStateRequiresNoAction(const lifecycle_msgs::msg::State::_id_type state)
{
    return state == lifecycle_msgs::msg::State::PRIMARY_STATE_UNKNOWN ||
           state == lifecycle_msgs::msg::State::PRIMARY_STATE_UNCONFIGURED ||
           state == lifecycle_msgs::msg::State::PRIMARY_STATE_FINALIZED;
}

if (lifecycleStateRequiresNoAction(impl_->get_lifecycle_state().id()))
    return return_type::OK;

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. I like the idea

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@saikishor saikishor force-pushed the fix/failed/hw/initialization/logs branch from 1bab232 to fd1a50a Compare August 26, 2024 16:51
@saikishor saikishor force-pushed the fix/failed/hw/initialization/logs branch from 167e1b2 to 30fb971 Compare August 27, 2024 13:29
Copy link
Member

@bmagyar bmagyar left a comment

Choose a reason for hiding this comment

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

thank you!

@bmagyar bmagyar merged commit 8552dcf into ros-controls:master Aug 30, 2024
19 checks passed
@bmagyar bmagyar deleted the fix/failed/hw/initialization/logs branch August 30, 2024 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants