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

Fix Ackermann world topic name, change Ambulance to AmbulanceSlotcar, use robot spawning for test_ackman map #86

Closed
wants to merge 3 commits into from

Conversation

luca-della-vedova
Copy link
Member

Bug fix

Fixed bug

Depends on rmf_simulation #49, change the topic name for the ackermann test world to follow the RMF standard, as well as add a bit of documentation for users to launch it.

@luca-della-vedova luca-della-vedova changed the title Fix/ackermann topic name Fix Ackermann world topic name Sep 7, 2021
ddengster
ddengster previously approved these changes Sep 7, 2021
Copy link
Contributor

@ddengster ddengster left a comment

Choose a reason for hiding this comment

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

Thanks for the changes!

@ddengster ddengster changed the title Fix Ackermann world topic name Fix Ackermann world topic name, change Ambulance to AmbulanceSlotcar, use robot spawning for test_ackman map Sep 8, 2021
Copy link
Member

@Yadunund Yadunund left a comment

Choose a reason for hiding this comment

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

I find the name AmbulanceSlotcar to be a bit strange since other robot models in rmf_demos_assets also include the slotcar plugin but are simply called TinyRobot, DeliveryRobot etc. Also, it seems that the top level folder was renamed to AmbulanceSlotcar but the model name in model.sdf and mode.config is ambulance which is a bit confusing.

I understand the need to rename the model as fuel may contain other Ambulance models which don't have the plugin included. My recommendation would be to rename to EmergencyVehicle or HospitalVan.


#### Ackermann test Scenario:

This scenario has a simple demo to show trajectory following for ackermann steering vehicles (such as ambulances). The feature is currently only supported in Ignition, to test the feature:
Copy link
Member

Choose a reason for hiding this comment

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

@luca-della-vedova what makes this only available in ign?

Copy link
Member

Choose a reason for hiding this comment

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

Looks like it is related to the plugin, running the simulation in rmf_demos_gz shows that it fails to find the necessary joints,

[gzserver-14] [INFO] [1631084275.040217309] [slotcar_ambulance23]: Initialising slotcar for ambulance23
[gzserver-14] [ERROR] [1631084275.040260706] [slotcar_ambulance23]: Could not find tire for [joint_tire_left]
[gzserver-14] [ERROR] [1631084275.040271843] [slotcar_ambulance23]: Could not find tire for [joint_tire_right]

Copy link
Member Author

@luca-della-vedova luca-della-vedova Sep 8, 2021

Choose a reason for hiding this comment

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

Good question, in the original implementation of slotcar there was a lot of code specific to the simulator, I believe this might be fixed with the latest rmf_simulation refactor, however Gazebo is hanging on startup on my side so not sure what is happening there.

Edit: Saw @aaronchongth reply below, I believe that is one of the reasons the specific demo wouldn't work in Gazebo, however if the model happened to have wheels instead of being a big box that should be fixed, that might require some work though, especially for this narrow use case demo scenario.

Copy link
Member

@aaronchongth aaronchongth left a comment

Choose a reason for hiding this comment

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

Thanks for updating!

The names of the model AmbulanceSlotcar in model.sdf and model.config are not matching, as well as the names of the .obj and .mtl file, which causes a clean build to fail.

[ign-14] [GUI] [Err] [SystemPaths.cc:377] Unable to find file with URI [model://Ambulance/meshes/ambulance.obj]
[ign-14] [GUI] [Err] [SystemPaths.cc:467] Could not resolve file [model://Ambulance/meshes/ambulance.obj]
[ign-14] [GUI] [Err] [MeshManager.cc:172] Unable to find file[model://Ambulance/meshes/ambulance.obj]
[ign-14] [GUI] [Err] [MeshDescriptor.cc:56] Mesh manager can't find mesh named [model://Ambulance/meshes/ambulance.obj]

There is currently an issue regarding model downloading that is being tracked, open-rmf/rmf_traffic_editor#386. In the meantime, could you try a clean build, cleaning up ~/.gazebo/models such that there is only sun and ground_plane left, and deleting everything in ~/.ignition/fuel/fuel.ignitionrobotics.org/openrobotics?


#### Ackermann test Scenario:

This scenario has a simple demo to show trajectory following for ackermann steering vehicles (such as ambulances). The feature is currently only supported in Ignition, to test the feature:
Copy link
Member

Choose a reason for hiding this comment

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

Looks like it is related to the plugin, running the simulation in rmf_demos_gz shows that it fails to find the necessary joints,

[gzserver-14] [INFO] [1631084275.040217309] [slotcar_ambulance23]: Initialising slotcar for ambulance23
[gzserver-14] [ERROR] [1631084275.040260706] [slotcar_ambulance23]: Could not find tire for [joint_tire_left]
[gzserver-14] [ERROR] [1631084275.040271843] [slotcar_ambulance23]: Could not find tire for [joint_tire_right]

@Yadunund
Copy link
Member

Yadunund commented Sep 8, 2021

Also just noticed within the test_ackman.launch.xml, the <arg name="use_tpe" value="$(var use_tpe)"/> is included within the common.launch.xml include. I think it should be under <include file="$(find-pkg-share rmf_demos_ign)/simulation.launch.xml">.

Also in my personal opinion, it seems like a lot of the work to support ackermann vehicles is not fully there yet. I think it would be best to remove the test_ackman map and Ambulance model from main. We can work on this in a separate branch. Then when everything is finalized (including a proper map for demonstrating ackermann steer vehicles and updated Ambulance model with wheels), we can merge these components into main and release as a patch.

@luca-della-vedova
Copy link
Member Author

Also in my personal opinion, it seems like a lot of the work to support ackermann vehicles is not fully there yet. I think it would be best to remove the test_ackman map and Ambulance model from main. We can work on this in a separate branch. Then when everything is finalized (including a proper map for demonstrating ackermann steer vehicles and updated Ambulance model with wheels), we can merge these components into main and release as a patch.

Closing in favor of #88 removing the test world.

@luca-della-vedova luca-della-vedova deleted the fix/ackermann_topic_name branch September 8, 2021 08:39
@ddengster ddengster restored the fix/ackermann_topic_name branch September 8, 2021 10:08
@Yadunund Yadunund deleted the fix/ackermann_topic_name branch January 16, 2024 08:13
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.

4 participants