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.

Sign up for GitHub

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

Jump to bottom

Manipulation Components redesign & refactor #366

Merged
merged 16 commits into from
Jul 4, 2023
Merged

Manipulation Components redesign & refactor #366

merged 16 commits into from
Jul 4, 2023

Conversation

adamdbrw
Copy link
Contributor

@adamdbrw adamdbrw commented Jun 21, 2023 •
edited
Loading

Improvements to components for manipulating chains of joints and articulation links, such as in manipulator arms.
This is a substantial rework that changes how the Components depend on each other and what they do.
It also adds features such as reporting feedback to action user.
Adds error handling.
Fixes some errors or undefined situations.
Exposes interfaces allowing users to create develop their own components and controllers more easily.

The code has been tested to run by @michalpelka .

@adamdbrw adamdbrw requested review from a team as code owners June 21, 2023 14:18
@adamdbrw adamdbrw marked this pull request as draft June 21, 2023 14:19
@michalpelka michalpelka mentioned this pull request Jun 22, 2023
Merged
3 tasks
@michalpelka michalpelka mentioned this pull request Jun 26, 2023
Merged
adamdbrw
adamdbrw commented Jun 29, 2023
View reviewed changes
Gems/ROS2/Code/Source/Manipulation/JointsManipulationComponent.cpp Outdated Show resolved Hide resolved
Gems/ROS2/Code/Source/Manipulation/JointsManipulationComponent.cpp Outdated Show resolved Hide resolved
Gems/ROS2/Code/Source/Manipulation/JointsManipulationComponent.cpp Outdated Show resolved Hide resolved
Gems/ROS2/Code/Source/Manipulation/JointsManipulationComponent.cpp Outdated Show resolved Hide resolved
Gems/ROS2/Code/Source/Manipulation/JointsManipulationComponent.cpp Outdated Show resolved Hide resolved
Gems/ROS2/Code/Source/Manipulation/JointsManipulationComponent.cpp Outdated Show resolved Hide resolved
Gems/ROS2/Code/Source/Manipulation/JointsManipulationComponent.cpp Outdated Show resolved Hide resolved
Gems/ROS2/Code/Source/Manipulation/JointsManipulationComponent.cpp Outdated Show resolved Hide resolved
Gems/ROS2/Code/Source/Manipulation/JointsManipulationComponent.cpp Outdated Show resolved Hide resolved
Gems/ROS2/Code/Source/Manipulation/JointsManipulationComponent.cpp Outdated Show resolved Hide resolved
michalpelka
michalpelka approved these changes Jun 29, 2023
View reviewed changes
Copy link
Contributor

@michalpelka michalpelka left a comment

Choose a reason for hiding this comment

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

LGTM, works and tested.
Authored robot is available :
https://github.com/o3de/o3de-physics-test-scene/tree/test_366

@adamdbrw adamdbrw mentioned this pull request Jun 29, 2023
Closed
@michalpelka michalpelka mentioned this pull request Jun 29, 2023
Closed
@adamdbrw adamdbrw marked this pull request as ready for review June 29, 2023 12:39
@adamdbrw adamdbrw requested review from pawelbudziszewski and a team June 29, 2023 12:39
@adamdbrw adamdbrw mentioned this pull request Jun 29, 2023
Closed
AMZN-alexpete
AMZN-alexpete approved these changes Jun 29, 2023
View reviewed changes
Copy link
Contributor

@AMZN-alexpete AMZN-alexpete left a comment

Choose a reason for hiding this comment

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

Nice! Just some minor comments. Also, please consider bumping the gem.json version because it looks like some of the public APIs were changed and new ones introduced - here's the docs regarding version changes: https://docs.o3de.org/docs/user-guide/gems/gem-versioning/

pijaro
pijaro approved these changes Jun 30, 2023
View reviewed changes
Copy link
Contributor

@pijaro pijaro left a comment

Choose a reason for hiding this comment

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

just a few nitpicks, LGTM

Gems/ROS2/Code/Include/ROS2/Manipulation/JointsManipulationRequests.h Outdated Show resolved Hide resolved
Gems/ROS2/Code/Include/ROS2/Manipulation/JointsManipulationRequests.h Show resolved Hide resolved
michalpelka and others added 16 commits July 3, 2023 13:25
@michalpelka @adamdbrw
Added joint name field handling to URDF importer.
42a2c59 
 - It looks like a regression
 - Added for articulated mode

Signed-off-by: Michał Pełka <michal.pelka@robotec.ai>
@michalpelka @adamdbrw
Adjust manipulation components
55c344c 
 - Adding articulation controller
 - Adding MoveIt2 test

Signed-off-by: Michał Pełka <michal.pelka@robotec.ai>
@adamdbrw
Formatting fix
16d84a1 
Signed-off-by: Adam Dabrowski <adam.dabrowski@robotec.ai>
@adamdbrw
work in progress
75ffba4 
Signed-off-by: Adam Dabrowski <adam.dabrowski@robotec.ai>
@adamdbrw
work in progress part 2
f10b5d9 
Signed-off-by: Adam Dabrowski <adam.dabrowski@robotec.ai>
@adamdbrw
final refactoring stage before compilation
c70ddf9 
Signed-off-by: Adam Dabrowski <adam.dabrowski@robotec.ai>
@adamdbrw
Cleanup of interfaces
e9ca19b 
Signed-off-by: Adam Dabrowski <adam.dabrowski@robotec.ai>
@adamdbrw
Compiles
d07ffc0 
Signed-off-by: Adam Dabrowski <adam.dabrowski@robotec.ai>
@adamdbrw
removed extra dep
e415293 
Signed-off-by: Adam Dabrowski <adam.dabrowski@robotec.ai>
@michalpelka @adamdbrw
Small adjusments and bugfixes to 366. (#372)
73424bc 
Signed-off-by: Michał Pełka <michal.pelka@robotec.ai>
@adamdbrw
Refactoring completed. Added Editor Component for Joints
56b18d1 
Signed-off-by: Adam Dabrowski <adam.dabrowski@robotec.ai>
@adamdbrw
work in progress
947149d 
Signed-off-by: Adam Dabrowski <adam.dabrowski@robotec.ai>
@michalpelka @adamdbrw
Adjusments to run it against moveit
4429e77 
Signed-off-by: Michał Pełka <michal.pelka@robotec.ai>
@adamdbrw
Apply suggestions from code review
c3df8af 
Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>
@adamdbrw
removed TODO's (replaced with GH issues)
3fd61a3 
Signed-off-by: Adam Dabrowski <adam.dabrowski@robotec.ai>
@adamdbrw @AMZN-alexpete @pijaro
Applied review-comments
f299b20 
Co-authored-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Co-authored-by: Piotr Jaroszek <piotr.jaroszek@robotec.ai>
Signed-off-by: Adam Dabrowski <adam.dabrowski@robotec.ai>
@adamdbrw
Copy link
Contributor Author

adamdbrw commented Jul 3, 2023

@michalpelka considering we are bumping up version to 2.0.0 due to API changes, perhaps it would be good to remove unnecessary parts of the MotorizedJoints API?

@adamdbrw adamdbrw mentioned this pull request Jul 3, 2023
Merged
@arturkamieniecki
Copy link
Contributor

There is a bug with stopping the execution of a plan in moveit. After clicking the execute button and then the stop button, O3DE crashes with the error:

  what():  goal_handle attempted invalid transition from state EXECUTING with event CANCELED, at ./src/rcl_action/goal_handle.c:95
Aborted (core dumped)

@arturkamieniecki arturkamieniecki mentioned this pull request Jul 4, 2023
Merged
@adamdbrw adamdbrw merged commit 6052835 into development Jul 4, 2023
1 check passed
@adamdbrw adamdbrw mentioned this pull request Jul 5, 2023
Merged
4 tasks
@lgleim lgleim mentioned this pull request Jul 6, 2023
Closed
michalpelka pushed a commit to RobotecAI/o3de-extras that referenced this pull request Jul 13, 2023
@adamdbrw @michalpelka
Manipulation Components redesign & refactor (o3de#366)
9b103fd 
Redesign of manipulation components
- Fixed dependencies and improved handling of actions.
- New interfaces.
- Major changes (breaking manipulation API).
- Moved motorized joints to a subfolder.
- Adding articulation controller
- Adding MoveIt2 test
- Cleanup of interfaces
- Added Editor Component for Joint Manipulation

Co-authored-by: Michał Pełka <michal.pelka@robotec.ai>
Co-authored-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Co-authored-by: Piotr Jaroszek <piotr.jaroszek@robotec.ai>

Signed-off-by: Adam Dąbrowski <adam.dabrowski@robotec.ai>
@adamdbrw adamdbrw mentioned this pull request Sep 29, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michalpelka michalpelka michalpelka approved these changes

@pijaro pijaro pijaro approved these changes

@AMZN-alexpete AMZN-alexpete AMZN-alexpete approved these changes

@lgleim lgleim Awaiting requested review from lgleim

@pawelbudziszewski pawelbudziszewski Awaiting requested review from pawelbudziszewski

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@adamdbrw @arturkamieniecki @michalpelka @pijaro @AMZN-alexpete