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

Fixes flaky RobotState test #3105

Merged
merged 2 commits into from
Nov 16, 2024

Conversation

TSNoble
Copy link
Contributor

@TSNoble TSNoble commented Nov 15, 2024

Description

Fixes #3103 by ensuring that the dirty_link_transforms_ member is cleared before querying the RobotState

See #3102 for more details on the cause.

TLDR:

Setting the build type to Release causes tests to silently fail, whereas Debug does not (some tests rely on assert calls, which are enabled / disabled based on the presence of the NDBUG macro variable, which is set by CMake depending on the build type).

See the SonarScan pipeline call to ros-industrial/industrial_ci@master, which sets DCMAKE_BUILD_TYPE to Debug (and where the tests fail), and the same call in the rolling-ci pipeline (where the tests pass), which sets the same variable to Release.

I've also added a comment (and demonstration of the fix, at least for this test) locally.

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference
  • Document API changes relevant to the user in the MIGRATION.md notes
  • Create tests, which fail without this PR reference
  • Include a screenshot if changing a GUI
  • While waiting for someone to review your request, please help review another open pull request to support the maintainers

@TSNoble
Copy link
Contributor Author

TSNoble commented Nov 15, 2024

Without the changes (Release) -- Test passes:

root@d1aa334f4187:~/ws_moveit# colcon build --cmake-args -DBUILD_TESTING=ON -DCMAKE_BUILD_TYPE=Release --packages-up-to moveit_core
...
root@d1aa334f4187:~/ws_moveit# cd build/moveit_core/robot_state/ && ./test_robot_state --gtest_filter="*rigid*"
Note: Google Test filter = *rigid*
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from OneRobot
[ RUN      ] OneRobot.rigidlyConnectedParent
Error:   Group 'base_with_bad_subgroups' has unsatisfied subgroups
         at line 304 in /root/ws_moveit/src/srdfdom/src/model.cpp
[WARN] [1731709187.657265541] [moveit_2277077567]: exception thrown while creating node for logging: failed to create guard condition: context argument is null, at ./src/rcl/guard_condition.c:65
[WARN] [1731709187.657291808] [moveit_2277077567]: if rclcpp::init was not called, messages from this logger may be missing from /rosout
[INFO] [1731709187.657325479] [moveit_2277077567.moveit.core.robot_model]: Loading robot model 'one_robot'...
[ERROR] [1731709187.657558759] [moveit_2277077567.moveit.core.robot_state]: Unable to find link for frame 'no_object'
[ERROR] [1731709187.657571642] [moveit_2277077567.moveit.core.robot_state]: Unable to find link for frame 'object/no_subframe'
[ERROR] [1731709187.657574262] [moveit_2277077567.moveit.core.robot_state]: Unable to find link for frame ''
[ERROR] [1731709187.657584565] [moveit_2277077567.moveit.core.robot_state]: Unable to find link for frame 'object/'
[ERROR] [1731709187.657595636] [moveit_2277077567.moveit.core.robot_state]: Unable to find link for frame '/'
[       OK ] OneRobot.rigidlyConnectedParent (6 ms)
[----------] 1 test from OneRobot (6 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (6 ms total)
[  PASSED  ] 1 test.

Without the changes (Debug) -- Test fails:

root@d1aa334f4187:~/ws_moveit# colcon build --cmake-args -DBUILD_TESTING=ON -DCMAKE_BUILD_TYPE=Debug --packages-up-to moveit_core
...
root@d1aa334f4187:~/ws_moveit# cd build/moveit_core/robot_state/ && ./test_robot_state --gtest_filter="*rigid*"
Note: Google Test filter = *rigid*
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from OneRobot
[ RUN      ] OneRobot.rigidlyConnectedParent
Error:   Group 'base_with_bad_subgroups' has unsatisfied subgroups
         at line 304 in /root/ws_moveit/src/srdfdom/src/model.cpp
[WARN] [1731708234.797945875] [moveit_2006049342]: exception thrown while creating node for logging: failed to create guard condition: context argument is null, at ./src/rcl/guard_condition.c:65
[WARN] [1731708234.797975187] [moveit_2006049342]: if rclcpp::init was not called, messages from this logger may be missing from /rosout
[INFO] [1731708234.798013942] [moveit_2006049342.moveit.core.robot_model]: Loading robot model 'one_robot'...
[WARN] [1731708234.800753550] [moveit_2006049342.moveit.core.robot_state]: Returning dirty link transforms
test_robot_state: /root/ws_moveit/src/moveit2/moveit_core/robot_state/src/robot_state.cpp:1309: const Eigen::Isometry3d& moveit::core::RobotState::getFrameInfo(const std::string&, const moveit::core::LinkModel*&, bool&) const: Assertion `checkLinkTransforms()' failed.
Aborted

With the changes (Debug) -- Test passes:

root@d1aa334f4187:~/ws_moveit# colcon build --cmake-args -DBUILD_TESTING=ON -DCMAKE_BUILD_TYPE=Debug --packages-up-to moveit_core
...
root@d1aa334f4187:~/ws_moveit# cd build/moveit_core/robot_state/ && ./test_robot_state --gtest_filter="*rigid*"
Note: Google Test filter = *rigid*
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from OneRobot
[ RUN      ] OneRobot.rigidlyConnectedParent
Error:   Group 'base_with_bad_subgroups' has unsatisfied subgroups
         at line 304 in /root/ws_moveit/src/srdfdom/src/model.cpp
[WARN] [1731708144.887481051] [moveit_3787422709]: exception thrown while creating node for logging: failed to create guard condition: context argument is null, at ./src/rcl/guard_condition.c:65
[WARN] [1731708144.887508333] [moveit_3787422709]: if rclcpp::init was not called, messages from this logger may be missing from /rosout
[INFO] [1731708144.887537809] [moveit_3787422709.moveit.core.robot_model]: Loading robot model 'one_robot'...
[ERROR] [1731708144.890794401] [moveit_3787422709.moveit.core.robot_state]: Unable to find link for frame 'no_object'
[ERROR] [1731708144.890822982] [moveit_3787422709.moveit.core.robot_state]: Unable to find link for frame 'object/no_subframe'
[ERROR] [1731708144.890838105] [moveit_3787422709.moveit.core.robot_state]: Unable to find link for frame ''
[ERROR] [1731708144.890841557] [moveit_3787422709.moveit.core.robot_state]: Unable to find link for frame 'object/'
[ERROR] [1731708144.890853957] [moveit_3787422709.moveit.core.robot_state]: Unable to find link for frame '/'
[       OK ] OneRobot.rigidlyConnectedParent (9 ms)
[----------] 1 test from OneRobot (9 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (9 ms total)
[  PASSED  ] 1 test.

Copy link
Contributor

@sea-bass sea-bass left a comment

Choose a reason for hiding this comment

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

Nice find!

@sea-bass
Copy link
Contributor

Does this need backport?

@TSNoble
Copy link
Contributor Author

TSNoble commented Nov 16, 2024

@sea-bass Yeah, that'd be great, thanks!

@sea-bass sea-bass added backport-humble Mergify label that triggers a PR backport to Humble backport-iron Mergify label that triggers a PR backport to Iron labels Nov 16, 2024
@TSNoble
Copy link
Contributor Author

TSNoble commented Nov 16, 2024

@sea-bass Out of curiosity, are main PRs automatically merged into jazzy too? This one in particular feels like it'll affect all versions

@sea-bass
Copy link
Contributor

@sea-bass Out of curiosity, are main PRs automatically merged into jazzy too? This one in particular feels like it'll affect all versions

There is no jazzy branch right now, since it's compatible with main.

What will happen is that, at some point, there will likely be a breaking change in Rolling that prompts us to cut a different jazzy branch that diverges from main. But until then, one and the same!

@sea-bass sea-bass added this pull request to the merge queue Nov 16, 2024
Merged via the queue into moveit:main with commit f20265a Nov 16, 2024
11 checks passed
mergify bot pushed a commit that referenced this pull request Nov 16, 2024
* Fixes flaky RobotState test

* Removes redundant line

(cherry picked from commit f20265a)

# Conflicts:
#	moveit_core/robot_state/test/robot_state_test.cpp
mergify bot pushed a commit that referenced this pull request Nov 16, 2024
* Fixes flaky RobotState test

* Removes redundant line

(cherry picked from commit f20265a)
@TSNoble TSNoble deleted the bugfix/fix-flaky-robot-state-test branch November 16, 2024 09:35
sea-bass added a commit that referenced this pull request Nov 16, 2024
* Fixes flaky RobotState test (#3105)

* Fixes flaky RobotState test

* Removes redundant line

(cherry picked from commit f20265a)

# Conflicts:
#	moveit_core/robot_state/test/robot_state_test.cpp

* Update robot_state_test.cpp

---------

Co-authored-by: Tom Noble <53005340+TSNoble@users.noreply.github.com>
Co-authored-by: Sebastian Castro <4603398+sea-bass@users.noreply.github.com>
sea-bass added a commit that referenced this pull request Nov 16, 2024
* Fixes flaky RobotState test

* Removes redundant line

(cherry picked from commit f20265a)

Co-authored-by: Tom Noble <53005340+TSNoble@users.noreply.github.com>
Co-authored-by: Sebastian Castro <4603398+sea-bass@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-humble Mergify label that triggers a PR backport to Humble backport-iron Mergify label that triggers a PR backport to Iron
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OneRobot.rigidlyAttachedParent test fails in jobs
2 participants