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 the controller sorting bug when the interfaces are not configured (fixes #1164) #1165

Merged

Conversation

saikishor
Copy link
Member

This PR:
Updates the tests to throw the exception on_configure as usual and then fixes with the appropriate condition checks to prevent the case mentioned in the #1164

Copy link

codecov bot commented Nov 13, 2023

Codecov Report

Merging #1165 (155969d) into master (0863acd) will decrease coverage by 0.01%.
The diff coverage is 28.57%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1165      +/-   ##
==========================================
- Coverage   47.65%   47.64%   -0.01%     
==========================================
  Files          40       40              
  Lines        3435     3442       +7     
  Branches     1860     1865       +5     
==========================================
+ Hits         1637     1640       +3     
  Misses        480      480              
- Partials     1318     1322       +4     
Flag Coverage Δ
unittests 47.64% <28.57%> (-0.01%) ⬇️

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

Files Coverage Δ
controller_manager/src/controller_manager.cpp 38.72% <28.57%> (+0.02%) ⬆️

@bmagyar bmagyar added backport-humble This label should be used by maintainers only! Label triggers PR backport to ROS2 humble. backport-iron labels Nov 13, 2023
Copy link
Contributor

@fmauch fmauch left a comment

Choose a reason for hiding this comment

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

Looks good and I tested this locally as mentioned in #1164 on Humble. As expected, this fixes the crash there.

On rolling I could not test things, as I ran into another issue there, that I first have to look at.

@saikishor
Copy link
Member Author

Thank you for letting us know @fmauch

@fmauch
Copy link
Contributor

fmauch commented Nov 14, 2023

@bmagyar Since the Humble sync is around the corner: Could we get this merged to Humble with priority? I posted a comment on the sync announcement about this.

@bmagyar bmagyar merged commit 75e7efd into ros-controls:master Nov 14, 2023
13 checks passed
mergify bot pushed a commit that referenced this pull request Nov 14, 2023
mergify bot pushed a commit that referenced this pull request Nov 14, 2023
bmagyar pushed a commit that referenced this pull request Nov 14, 2023
…fixes #1164) (#1165) (#1166)

(cherry picked from commit 75e7efd)

Co-authored-by: Sai Kishor Kothakota <[email protected]>
bmagyar pushed a commit that referenced this pull request Nov 14, 2023
…fixes #1164) (#1165) (#1167)

(cherry picked from commit 75e7efd)

Co-authored-by: Sai Kishor Kothakota <[email protected]>
@saikishor saikishor deleted the fix_controller_sorting_on_configure_bug branch August 17, 2024 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-humble This label should be used by maintainers only! Label triggers PR backport to ROS2 humble.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants