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

Use same FP limits for TrackedVehicle to avoid self-moving #2651

Open
wants to merge 6 commits into
base: gz-sim9
Choose a base branch
from

Conversation

ntfshard
Copy link

🦟 Bug fix

Fixes #2506

Summary

Using the same constants for checking against zero here to significantly reduce cases when system behaves in unexpected way.
Still for a solid solution we should consider a refactoring a whole algorithm.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@github-actions github-actions bot added the 🏛️ ionic Gazebo Ionic label Oct 14, 2024
@peci1
Copy link
Contributor

peci1 commented Oct 14, 2024

LGTM.

I guess the constants could even be a bit higher unless you simulate mm-sized robots. The constants you suggest mean that any motion command above 0.000001 m/s or 0.000001 rad/s is considered non-straight motion. I guess no reasonably sized robot would react to a micrometer/sec command. On the other hand, I don't see anything really problematic practically with this constant.

@peci1
Copy link
Contributor

peci1 commented Oct 14, 2024

Could you please post the result of a test where you instruct the robot with e.g. 2e-6 linear and angular velocities?

@ntfshard
Copy link
Author

ntfshard commented Oct 14, 2024

I guess I can do it tomorrow, it's too late here, sorry. Maybe @iandresolares can provide, if he tested it

And I guess this constant change is not related to any physical properties of a robot, it's just about math properties of this equations, as mentioned in initial msg

@ntfshard
Copy link
Author

Could you please post the result of a test where you instruct the robot with e.g. 2e-6 linear and angular velocities?

Yes, sure. It just standing still
ubuntu22-screen0.webm

@peci1
Copy link
Contributor

peci1 commented Oct 15, 2024

Great, thanks! Now I'm more sure this PR will not have any bad side effects.

@azeey
Copy link
Contributor

azeey commented Nov 5, 2024

@osrf-jenkins run tests please

@ntfshard
Copy link
Author

ntfshard commented Nov 25, 2024

Can we have some conclusion about this hotfix? I mean it still affect us and we still need a backport to previous version (gz-sim7) tbh

Copy link

codecov bot commented Nov 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (gz-sim9@af92e29). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff             @@
##             gz-sim9    #2651   +/-   ##
==========================================
  Coverage           ?   68.78%           
==========================================
  Files              ?      341           
  Lines              ?    33053           
  Branches           ?        0           
==========================================
  Hits               ?    22735           
  Misses             ?    10318           
  Partials           ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏛️ ionic Gazebo Ionic
Projects
Status: Inbox
Development

Successfully merging this pull request may close these issues.

TrackedVehicle system does unexpected movement on low turn commands.
3 participants