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

TrackedVehicle system does unexpected movement on low turn commands. #2506

Open
ValeriiKoshelev opened this issue Aug 1, 2024 · 19 comments · May be fixed by #2651
Open

TrackedVehicle system does unexpected movement on low turn commands. #2506

ValeriiKoshelev opened this issue Aug 1, 2024 · 19 comments · May be fixed by #2651
Labels
bug Something isn't working help wanted We accept pull requests!

Comments

@ValeriiKoshelev
Copy link

Environment

  • OS Version: Ubuntu 22.04
  • Source or binary build? Source, gz-sim7.

Description

If you send command with zero linear component and small (<0.1) angular component, vehicle behaves in unexpected way.

Expected behavior: no movement or slow turn in place.
Observed behavior: slow linear motion.

Probably something wrong with edge case here: https://github.com/gazebosim/gz-sim/blame/007aa87cd0007c2bbe2887e79b3679070d7bc806/src/systems/tracked_vehicle/TrackedVehicle.cc#L689
Code looks same in gz-sim8, so probably reproducible in newer versions.

Steps to reproduce

Commands executed:

gz sim -r tracked_vehicle_simple.sdf
gz topic -t /model/simple_tracked/cmd_vel -m gz.msgs.Twist -p "linear: {x: 0.0}, angular: {z: 0.05}"

Output

@ValeriiKoshelev ValeriiKoshelev added the bug Something isn't working label Aug 1, 2024
@azeey
Copy link
Contributor

azeey commented Aug 6, 2024

Yeah, it seems like it could be an issue with how that condition is handled on line 689. Would you be able to work on a fix?

cc @peci1

@azeey azeey added the help wanted We accept pull requests! label Aug 6, 2024
@peci1
Copy link
Contributor

peci1 commented Aug 11, 2024

Yeah, it seems to be caused by the hardcoded threshold. The threshold is there to prevent division by zero when computing turning radius. But I don't see any reason why it couldn't have a lower value. Not sure if it makes sense to turn the thresholds into configurable values. Probably just changing the hardcoded values to something like 1e-4 would suffice. This would, of course, be a breaking change, but I don't think it would really affect anyone.

@azeey not sure what the current policy is, but feel free to add me as a codeowner for the files added in the tracked vehicles PR.

@ntfshard
Copy link

Should we prepare this PR?

@peci1
Copy link
Contributor

peci1 commented Aug 26, 2024

You definitely can do that. I'm away this week.

@iandresolares
Copy link

Hello,
has there been any updates on this topic? I am using Gazebo Sim, version 8.6.0, and faced this same issue. Would lowering this threshold to a smaller value like 0.0001 will do it? Or it would brake something else. I am thinking on installing from source and modifying the plugin for now.

@ntfshard
Copy link

ntfshard commented Oct 14, 2024

Hi. I have a branch with fix, but I didn't check it properly yet. And I also want to think about this question, but maybe it can be done in a separate way
(and also faced with some build problems)

@iandresolares
Copy link

I see, I tried to build gazebo harmonic from source in order to modify the plugin but I get errors when running:
vcs import < collection-harmonic.yaml

do you know if it is possible to overwrite the plugin installed with binaries so I can build only the plugin to test it with the modification?

Thanks!

@ntfshard
Copy link

I see, I tried to build gazebo harmonic from source in order to modify the plugin but I get errors when running: vcs import < collection-harmonic.yaml

do you know if it is possible to overwrite the plugin installed with binaries so I can build only the plugin to test it with the modification?

Thanks!

vcs is just an automation to do a several git clone commands. You can overwrite anything on your Linux if you have enough permissions. Plugin is just a so file. Also some configurations can be done with env.variables https://gazebosim.org/api/sim/8/server_config.html
Also maybe a --install-base option for a build command colcon build can help, but I didn't tried
I use docker, but it's a long story

@iandresolares
Copy link

iandresolares commented Oct 14, 2024

Hello,
I have been able to run gz sim from source and modify the plugin as you did, it seems like it does fix the issue of lateral linear motion.
So far I have not encountered any other issues related to this change, if I do I will update here.

One other issue I have been encountering with this plugin (which is not related so this should not be the best place to refer to it) is that the center of rotation is not in the expected position (I would expect it to allign to the center of gravity of the vehicle + tracks) however, it alligns with the geometric center of the tracks (visual and collision), the links are alligned with what I though want to be the rotation axis, so not with the geometric center of the tracks. Have you find this issue also, or do you know what could be related to it?

Thank you!!

@peci1
Copy link
Contributor

peci1 commented Oct 14, 2024

One other issue I have been encountering with this plugin (which is not related so this should not be the best place to refer to it) is that the center of rotation is not in the expected position (I would expect it to allign to the center of gravity of the vehicle + tracks) however, it alligns with the geometric center of the tracks, which is not the behivour I expect, have you find this issue also, or do you know what could be related to it?

Yeah, that would be an inherent limitation of the approximate method that is used to simulate tracks. There's no way to fix this in this approach. However, you could probably work it around by creating an extra link positioned in the CoM and setting that link as <body_link>.

@iandresolares
Copy link

Oh I see, so the center of rotation will always be alligned with the base_link?

@peci1
Copy link
Contributor

peci1 commented Oct 14, 2024

Oh I see, so the center of rotation will always be alligned with the base_link?

Mostly yes. More exactly, it will rotate around whatever is set in the <body_link> tag in the plugin configuration. If the tag is not specified, it uses the model's canonical link which, in the usual case, is base_link.

@ntfshard
Copy link

Hello, I have been able to run gz sim from source and modify the plugin as you did, it seems like it does fix the issue of lateral linear motion. So far I have not encountered any other issues related to this change, if I do I will update here.

I guess in this case I'll open a PR.

@iandresolares
Copy link

Oh I see, so the center of rotation will always be alligned with the base_link?

Mostly yes. More exactly, it will rotate around whatever is set in the <body_link> tag in the plugin configuration. If the tag is not specified, it uses the model's canonical link which, in the usual case, is base_link.

Since this is out of the scope of this issue I have open a new one: Using body_link to move center of rotation in TrackedVehicle plugin. Also, I posted some days ago another issue regarding this plugin here: Change odometry publication frequence if you wanna take a look.

Thank you very much!

@ntfshard ntfshard linked a pull request Oct 14, 2024 that will close this issue
8 tasks
@ntfshard
Copy link

I created PR to gz-sim9 branch due to it's a main branch right now.
I'm not sure how it should be according to process, but after it will be merged (or can I do) how to properly port it to gz-sim8? Cherry-pick is ok or it should be something other?

@peci1
Copy link
Contributor

peci1 commented Oct 14, 2024

I created PR to gz-sim9 branch due to it's a main branch right now.
I'm not sure how it should be according to process, but after it will be merged (or can I do) how to properly port it to gz-sim8? Cherry-pick is ok or it should be something other?

That process is described here: https://community.gazebosim.org/t/gazebo-policy-update-new-backporting-policy/2880 . It should reflect what is in the docs, however, the docs still mention the old forward-porting policy. @azeey Could you have a look?

@ntfshard
Copy link

I created PR to gz-sim9 branch due to it's a main branch right now.
I'm not sure how it should be according to process, but after it will be merged (or can I do) how to properly port it to gz-sim8? Cherry-pick is ok or it should be something other?

That process is described here: https://community.gazebosim.org/t/gazebo-policy-update-new-backporting-policy/2880 . It should reflect what is in the docs, however, the docs still mention the old forward-porting policy. @azeey Could you have a look?

Yes, I saw this document, but I just want to clarify how the backport should looks like (which git mechanism I should use to do it according to a project rules: should it be cherry-pick or I can just apply same changes ad-hoc)
Also text is already slightly outdated in second sentence: New Gazebo features should target Harmonic branches. and sentence almost in the end of text

@peci1
Copy link
Contributor

peci1 commented Oct 14, 2024

Backports are done by the maintainers.

@iche033
Copy link
Contributor

iche033 commented Oct 14, 2024

however, the docs still mention the old forward-porting policy.

I created a PR to update the contribution policy doc: gazebosim/docs#525

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted We accept pull requests!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants