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

Unify ROSbots links with rosbot_description #6250

Closed
wants to merge 47 commits into from

Conversation

delihus
Copy link
Contributor

@delihus delihus commented Jun 23, 2023

Description
Unify ROSbots link with https://github.com/husarion/rosbot_ros.
Related Issues
cyberbotics/webots_ros2#770

delihus and others added 30 commits November 21, 2022 19:57
Co-authored-by: ad-daniel <44834743+ad-daniel@users.noreply.github.com>
Co-authored-by: ad-daniel <44834743+ad-daniel@users.noreply.github.com>
Co-authored-by: ad-daniel <44834743+ad-daniel@users.noreply.github.com>
Co-authored-by: ad-daniel <44834743+ad-daniel@users.noreply.github.com>
Co-authored-by: ad-daniel <44834743+ad-daniel@users.noreply.github.com>
Co-authored-by: ad-daniel <44834743+ad-daniel@users.noreply.github.com>
@delihus delihus requested a review from a team as a code owner June 23, 2023 08:41
@delihus delihus added the test sources Start the sources test on all platforms label Jun 23, 2023
@ygoumaz
Copy link
Contributor

ygoumaz commented Jun 23, 2023

Just to get a little more context, is there any benefit to apply these changes? Because we are introducing some less clear and redundant names (namely with the link keyword) for the nodes of the robot.

@delihus
Copy link
Contributor Author

delihus commented Jun 23, 2023

Just to get a little more context, is there any benefit to apply these changes? Because we are introducing some less clear and redundant names (namely with the link keyword) for the nodes of the robot.

We switched to publish robot description from .proto file here cyberbotics/webots_ros2#770. It means that the links names are the same as in the .proto file.

If there is an user who want to test the software on the simulation and then switch to the real robot nothing has to be changed, any link name, topic and so on.

I think it is good practice to simulate the real robot.

@ygoumaz
Copy link
Contributor

ygoumaz commented Jun 23, 2023

I see the point and I totally agree that we should be able to match the Webots node names with link names of the real robot. However I am not sure changing the names in the PROTO file directly is really the best and cleanest solution. I would instead implement a new parameter in the webots_ros2_driver package (WebotsController ROS2 node) to pass a dictionary of key-node-name->value-link-name. This way it would be possible to define for necessary nodes the corresponding value of the real robot and have exactly the same topic names. Do you think this solution could work for your use case?

This would allow the change to be made only on the ROS2 side and when transferring the PROTO to URDF for the state publisher.

@delihus
Copy link
Contributor Author

delihus commented Jun 23, 2023

Okay! It sounds good especially, taking into account all remappings I made here https://github.com/husarion/webots_ros2/blob/develop-husarion/webots_ros2_husarion/launch/rosbot_launch.py#L72. Please inform me about updates of WebotsController.

@delihus delihus closed this Jun 23, 2023
@ygoumaz
Copy link
Contributor

ygoumaz commented Jun 23, 2023

I will implement it start of next week. Meanwhile I will also check the status of cyberbotics/webots_ros2#770 and try to fix the tests if you are still blocked. It is not mandatory to merge the PR before the release of R2023b. We can also release a new version of webots_ros2 later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test sources Start the sources test on all platforms
Development

Successfully merging this pull request may close these issues.

2 participants