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

added model for utv3 E575 with quantec and caterpillar with iontec. #58

Open
wants to merge 30 commits into
base: master
Choose a base branch
from

Conversation

ravirathnam
Copy link

@ravirathnam ravirathnam commented Jun 27, 2024

Before submitting this PR into master please make sure:

If you added a new robot model:

  • you extended the table of verified data in README.md with the new model
  • you extended the CMakeLists.txt of the appropriate moveit configuration package with the new model
  • you added a test_<robot_model>.launch.py and after launching the robot was visible in rviz
  • you added a <robot_model>_joint_limits.yaml file in the config directory (to provide moveit support)

If you modified an already existing robot model:

  • you checked and optionally updated the table of verified data in README.md with the changes
  • you have run the test_<robot_model>.launch.py and the robot was visible in rviz

Short description of the change

Ravi Kulan Rathnam and others added 30 commits July 25, 2023 18:12
forward_controller not yet working
… a configuration.

created dummy forward_contrller for testing.
# Conflicts:
#	README.md
only launching Robot nodes.
remapped platform messages to separate namespace
# Conflicts:
#	README.md
publishing transform between platform and robot
# Conflicts:
#	README.md
Comment on lines +4 to +43
if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
add_compile_options(-Wall -Wextra -Wpedantic)
endif()

# find dependencies
find_package(ament_cmake REQUIRED)
# uncomment the following section in order to fill in
# further dependencies manually.

install(DIRECTORY
launch
DESTINATION share/${PROJECT_NAME}/
)
install(DIRECTORY
config
DESTINATION share/${PROJECT_NAME}/
)
install(DIRECTORY
urdf
DESTINATION share/${PROJECT_NAME}/
)

install(DIRECTORY
meshes
DESTINATION share/${PROJECT_NAME}/
)
## COMPILE

if(BUILD_TESTING)
find_package(ament_lint_auto REQUIRED)
# the following line skips the linter which checks for copyrights
# comment the line when a copyright and license is added to all source files
set(ament_cmake_copyright_FOUND TRUE)
# the following line skips cpplint (only works in a git repo)
# comment the line when this package is in a git repo and when
# a copyright and license is added to all source files
set(ament_cmake_cpplint_FOUND TRUE)
ament_lint_auto_find_test_dependencies()
endif()
ament_package()
Copy link
Member

Choose a reason for hiding this comment

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

  • compiler options not necessary for support packages
  • install rules can be combined
  • testing should not contain the formatting rules
Suggested change
if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
add_compile_options(-Wall -Wextra -Wpedantic)
endif()
# find dependencies
find_package(ament_cmake REQUIRED)
# uncomment the following section in order to fill in
# further dependencies manually.
install(DIRECTORY
launch
DESTINATION share/${PROJECT_NAME}/
)
install(DIRECTORY
config
DESTINATION share/${PROJECT_NAME}/
)
install(DIRECTORY
urdf
DESTINATION share/${PROJECT_NAME}/
)
install(DIRECTORY
meshes
DESTINATION share/${PROJECT_NAME}/
)
## COMPILE
if(BUILD_TESTING)
find_package(ament_lint_auto REQUIRED)
# the following line skips the linter which checks for copyrights
# comment the line when a copyright and license is added to all source files
set(ament_cmake_copyright_FOUND TRUE)
# the following line skips cpplint (only works in a git repo)
# comment the line when this package is in a git repo and when
# a copyright and license is added to all source files
set(ament_cmake_cpplint_FOUND TRUE)
ament_lint_auto_find_test_dependencies()
endif()
ament_package()
find_package(ament_cmake REQUIRED)
find_package(urdf REQUIRED)
find_package(xacro REQUIRED)
install(DIRECTORY launch meshes urdf config
DESTINATION share/${PROJECT_NAME})
if(BUILD_TESTING)
endif()
ament_package()

Copy link
Member

Choose a reason for hiding this comment

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

this file should be in the drivers repo according to our conventions (this repo should only host the robot model and a simple launch file for rviz visualization)

Copy link
Member

Choose a reason for hiding this comment

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

this file should be in the drivers repo according to our conventions (this repo should only host the robot model and a simple launch file for rviz visualization)

Copy link
Member

Choose a reason for hiding this comment

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

this file should be in the drivers repo according to our conventions (this repo should only host the robot model and a simple launch file for rviz visualization)

Copy link
Member

Choose a reason for hiding this comment

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

this file should be in the examples folder of the drivers repo, as this combines two drivers, which is an example and not a base functionality

Copy link
Member

Choose a reason for hiding this comment

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

this file should be in the drivers repo according to our conventions (this repo should only host the robot model and a simple launch file for rviz visualization)

<test_depend>ament_lint_auto</test_depend>
<test_depend>ament_lint_common</test_depend>
<exec_depend>ros2launch</exec_depend>
<exec_depend>controller_manager</exec_depend>
Copy link
Member

Choose a reason for hiding this comment

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

should not depend on controller manager, this is only the model

Copy link
Member

Choose a reason for hiding this comment

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

this is also a combination of two models and should be in drivers/examples folder

Comment on lines +24 to +26
<param name ="host">127.0.0.1</param>
<param name="port">24000</param>
<param name="agv_type">omnimove</param>
Copy link
Member

Choose a reason for hiding this comment

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

the ip parameter should be configurable using the robot_manager node and should not contain hard-coded values

<visual>
<origin xyz="0 0 0" rpy="0 0 0" />
<geometry>
<mesh filename="package://kuka_omnimove_e575_support/meshes/caterpillar_simplified.dae" />
Copy link
Member

Choose a reason for hiding this comment

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

in all other packages, we have separate folders for visual and collision meshes, to make things more straighforward. It would be nice if you could also create these 2 folders here

Copy link
Member

Choose a reason for hiding this comment

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

please add a launch file that only loads the model in rviz named test_omnimove.launch.py

Copy link
Member

Choose a reason for hiding this comment

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

I am a little bit confused, is this caterpillar also a KUKA Omnimove E575?

Copy link
Member

Choose a reason for hiding this comment

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

link names should be uniform, e.g. base_link.dae and base_link.stl

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.

2 participants