Skip to content

Added Omnimove external control #152

Open
ravirathnam wants to merge 58 commits into
kroshu:masterfrom
ravirathnam:ungarn_merge
Open

Added Omnimove external control #152
ravirathnam wants to merge 58 commits into
kroshu:masterfrom
ravirathnam:ungarn_merge

Conversation

@ravirathnam

Copy link
Copy Markdown

No description provided.

ravirathnam and others added 30 commits July 25, 2023 18:42
sending stop command when commanded velocity not changing.
# Conflicts:
#	.gitignore
allow movement of lift with platform for caterpillar.removed caterpillarlift command
increase rotation speed in caterpillar tests
@ravirathnam
ravirathnam requested a review from VX792 as a code owner April 8, 2024 12:46
@sonarqubecloud

sonarqubecloud Bot commented Apr 9, 2024

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Comment thread omnimove_external_control/include/omnimove/external_control_message.hpp Outdated
Comment thread omnimove_external_control/CMakeLists.txt Outdated
Comment thread examples/kuka_rsi_robot_tests/test/test_copyright.py Outdated
Comment thread omnimove_external_control/package.xml Outdated
Comment thread omnimove_external_control/package.xml Outdated
Comment thread omnimove_external_control/include/omnimove/omnimove_external_control.hpp Outdated
Comment thread examples/kuka_rsi_robot_tests/package.xml

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this example is almost the same code as the iontec test, would be better to combined these with some parameters

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the best would be to use a parameter file with the position values. Evertyhing else could be in one python file that would be valid for all KUKA robots (not only using RSI)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

So, for now, i've made one file with the robot_type as a parameter, and changing position according to type. But i think i'll change it to have the parameters coming from outside

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

movements are fully configured in a parameter file, fully extendible

@Svastits Svastits Jul 1, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

how do you start the example? Would not be a launch file necessary to "give" the config files to the node?

Comment thread examples/kuka_rsi_robot_tests/kuka_rsi_robot_tests/quantec_test.py Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what is the startup sequence for this driver?
all of our other drivers have a robot_manager node with a lifecycle interface, where activation starting the actual real-time communication. It would be nice to also add this component here, as the launch sequence is non-deterministic in timing which could lead to severe issues in my opinion

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I use the standard ros robot_manager node. So i didn't need to reinvent my own. I guess it becomes clear with the changes in kuka_robot_descriptions repo.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What do you mean by the standard robot_manager node? This node was implemented by us and is part of the common non-real time interface for all drivers
The reasoning for needing this addtional node can be found in the wiki
Please add this robot_manager node to your driver as well, and move the config and launch files to this package too, as is the convention in this repo

@Svastits

Copy link
Copy Markdown
Member

please add a short doc somewhere about setup and launch process

Comment thread omnimove_external_control/include/omnimove/external_control_message.hpp Outdated
Comment thread omnimove_external_control/include/omnimove/external_control_message.hpp Outdated
Comment thread omnimove_external_control/src/omnimove_external_control.cpp Outdated
Comment thread omnimove_external_control/src/external_control_message.cpp
@ravirathnam
ravirathnam requested a review from kovacsge11 as a code owner June 4, 2024 12:33
ravirathnam and others added 13 commits June 4, 2024 14:35
Co-authored-by: Áron Svastits <49677296+Svastits@users.noreply.github.com>
removed unused variables,
removed unneeded test files.
renamed variable
…t with configured robot_type

Added boost dependency directly in CMakefiles
simplified if statements to remove code duplication.
removed commented out code
@ravirathnam
ravirathnam requested a review from Svastits June 27, 2024 09:32
<version>0.0.1</version>
<description>A ROS2 hardware interface for use with KUKA UTV3 vehicles</description>
<maintainer email="ravi.rathnam@kuka.com">Ravi Rathnam</maintainer>
<license>Apache 2.0</license>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use SPDX license identifiers.

Suggested change
<license>Apache 2.0</license>
<license>Apache-2.0</license>

@sonarqubecloud

sonarqubecloud Bot commented Jul 1, 2024

Copy link
Copy Markdown

@Svastits

Svastits commented Jul 1, 2024

Copy link
Copy Markdown
Member

Another thing that is missing is the controller for calculating the offset from the starting position that could update the transforms from world to base_link to enable visualization and planning

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