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

Add SDFormat model plugins #609

Merged
merged 18 commits into from
Jan 11, 2024
Merged

Add SDFormat model plugins #609

merged 18 commits into from
Jan 11, 2024

Conversation

jhanca-robotecai
Copy link
Contributor

@jhanca-robotecai jhanca-robotecai commented Nov 13, 2023 •
edited
Loading

The code was tested using modified ROSBot urdf files. The only modification was adding the plugin itself based on libgazebo_ros_skid_steer_drive docs and libgazebo_ros_diff_drive docs. Please find the robots attached.

The attached robots test_models.zip require one change after import: setting "Use motor" toggle to true in all used wheels. This part will be fixed in the next PR.

Please note, that ROS topic names are not parsed at this point, therefore the robots can be controlled via /base_link/default_topic topic. This will be fixed in another PR (topic names are not parsed in Sensor importer either).

@jhanca-robotecai jhanca-robotecai marked this pull request as ready for review November 23, 2023 09:34
@jhanca-robotecai jhanca-robotecai requested review from a team as code owners November 23, 2023 09:34
@jhanca-robotecai jhanca-robotecai mentioned this pull request Nov 27, 2023
Merged
@jhanca-robotecai
Copy link
Contributor Author

The current version requires changes in o3de, please see o3de/o3de#17124

It works with both articulations and joints. Note, that the torque in case of articulations is too high and the imported samples fly away. This can be changed e.g. by modifying the weight of a base_link.

adamdbrw
adamdbrw reviewed Dec 6, 2023
View reviewed changes
Copy link
Contributor

@adamdbrw adamdbrw left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Good work! I would like this feature to be merged once documentation PR (at least a draft) is submitted about hooks and their registration mechanism. This would help to review the PR.

The documentation can be based on RFC text.

Gems/ROS2/Code/Include/ROS2/RobotImporter/SDFormatModelPluginImporterHook.h Show resolved Hide resolved
Gems/ROS2/Code/Source/RobotImporter/ROS2RobotImporterEditorSystemComponent.cpp Show resolved Hide resolved
Gems/ROS2/Code/Source/RobotImporter/SDFormat/Hooks/ROS2SkidSteeringModelHook.cpp Outdated Show resolved Hide resolved
Gems/ROS2/Code/Source/RobotImporter/SDFormat/Hooks/ROS2SkidSteeringModelHook.cpp Outdated Show resolved Hide resolved
Gems/ROS2/Code/Source/RobotImporter/SDFormat/Hooks/ROS2SkidSteeringModelHook.cpp Outdated Show resolved Hide resolved
Gems/ROS2/Code/Source/RobotImporter/SDFormat/Hooks/ROS2SkidSteeringModelHook.cpp Outdated Show resolved Hide resolved
Gems/ROS2/Code/Source/RobotImporter/SDFormat/Hooks/ROS2SkidSteeringModelHook.cpp Outdated Show resolved Hide resolved
Gems/ROS2/Code/Source/RobotImporter/SDFormat/Hooks/ROS2SkidSteeringModelHook.cpp Outdated Show resolved Hide resolved
Gems/ROS2/Code/Source/RobotImporter/SDFormat/Hooks/ROS2SkidSteeringModelHook.cpp Show resolved Hide resolved
Gems/ROS2/Code/Source/RobotImporter/SDFormat/ROS2SDFormatHooksUtils.h Outdated Show resolved Hide resolved
@michalpelka michalpelka requested a review from a team December 7, 2023 14:08
adamdbrw
adamdbrw reviewed Dec 20, 2023
View reviewed changes
Copy link
Contributor

@adamdbrw adamdbrw left a comment

Choose a reason for hiding this comment

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

Valuable contribution, some comments (rather minor).

Gems/ROS2/Code/Source/RobotImporter/SDFormat/Hooks/ROS2SkidSteeringModelHook.cpp Outdated Show resolved Hide resolved
Gems/ROS2/Code/Source/RobotImporter/SDFormat/Hooks/ROS2SkidSteeringModelHook.cpp Outdated Show resolved Hide resolved
Gems/ROS2/Code/Source/RobotImporter/SDFormat/Hooks/ROS2SkidSteeringModelHook.cpp Outdated Show resolved Hide resolved
Gems/ROS2/Code/Source/VehicleDynamics/ModelLimits/SkidSteeringModelLimits.cpp Outdated Show resolved Hide resolved
@jhanca-robotecai jhanca-robotecai mentioned this pull request Dec 20, 2023
Merged
@michalpelka
Copy link
Contributor

Tested against development.

@jhanca-robotecai
Copy link
Contributor Author

Rebased and force-pushed on top of development to rerun and fix Jenkins output.

@jhanca-robotecai
add constructors with params; rename HooksUtils
543f8cd 
Signed-off-by: Jan Hanca <[email protected]>
@jhanca-robotecai
add skid steering components with hardcoded params
5b337f5 
Signed-off-by: Jan Hanca <[email protected]>
@jhanca-robotecai
clean up ROS2RobotControlComponent in RobotImporter
b4af2af 
Signed-off-by: Jan Hanca <[email protected]>
@jhanca-robotecai
add parser; first working version
b590db8 
Signed-off-by: Jan Hanca <[email protected]>
@jhanca-robotecai
update attribute enumerator
9376e0c 
Signed-off-by: Jan Hanca <[email protected]>
@jhanca-robotecai
enable motor in wheels' joints
e26d6cc 
Signed-off-by: Jan Hanca <[email protected]>
@jhanca-robotecai
refactor hook, revert changes in articulation link creation
7ddf7fc 
Signed-off-by: Jan Hanca <[email protected]>
@jhanca-robotecai
fix IsWheelHeuristic, clang-format
e146f73 
Signed-off-by: Jan Hanca <[email protected]>
@jhanca-robotecai
move SkidSteering parser to utils for Ackermann
302715d 
Signed-off-by: Jan Hanca <[email protected]>
@jhanca-robotecai
code review changes
1641c6b 
Signed-off-by: Jan Hanca <[email protected]>
@jhanca-robotecai
code review: change namespace, revert clang-format changes
4e7b9e2 
Signed-off-by: Jan Hanca <[email protected]>
@jhanca-robotecai
code review: refactor HooksUtils::EnableMotor()
07a3f12 
Signed-off-by: Jan Hanca <[email protected]>
@jhanca-robotecai
code review: add more comments
5abe31f 
Signed-off-by: Jan Hanca <[email protected]>
@jhanca-robotecai
fix CopyHooksCallback to return always true
31681e5 
Signed-off-by: Jan Hanca <[email protected]>
@jhanca-robotecai
code review: rename methods; remove obsolete
979d457 
Signed-off-by: Jan Hanca <[email protected]>
@jhanca-robotecai
Copy link
Contributor Author

Rebased and force-pushed again after #641 was merged

@michalpelka michalpelka merged commit 3f98b47 into o3de:development Jan 11, 2024
2 checks passed
@jhanca-robotecai jhanca-robotecai deleted the jh/model_hooks branch January 11, 2024 11:42
@jhanca-robotecai jhanca-robotecai mentioned this pull request Jan 11, 2024
Merged
jhanca-robotecai added a commit to jhanca-robotecai/o3de-extras that referenced this pull request Jan 11, 2024
@jhanca-robotecai
code review fixes from o3de#609
aa61df2 
Signed-off-by: Jan Hanca <[email protected]>
michalpelka pushed a commit that referenced this pull request Jan 15, 2024
@jhanca-robotecai
Fix SkidSteeringDriveModel (#642)
d964488 
* Fix SkidSteeringDriveModel
* code review: fix names and comments
* code review fixes from #609
* fix Skid import hook after changes in Skid model

---------

Signed-off-by: Jan Hanca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spham-amzn spham-amzn spham-amzn approved these changes

@lemonade-dm lemonade-dm lemonade-dm left review comments

@adamdbrw adamdbrw adamdbrw approved these changes

@michalpelka michalpelka michalpelka approved these changes

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

Successfully merging this pull request may close these issues.

RobotImporter incorrectly adding Robot Control component to non-robot imports
5 participants
@jhanca-robotecai @michalpelka @adamdbrw @lemonade-dm @spham-amzn