Odometry topic for the track controller system

[jrutgeer]

🎉 New feature

Closes #2016
Requires gazebosim/gz-msgs#352

Summary

This PR adds odometry output to the track controller system, publishing position and instantaneous velocity. That way a conveyor with an encoder and/or resolver for respectively position and velocity feedback can be simulated.

The default topic is /model/<model name>/link/<link name>/odometry.
An alternative can be chosen by specifying the odometry_topic parameter of the plugin in the SDF.
The publication frequency is set through parameter odometry_publish_frequency.

Test it

Run the conveyor demo: gz sim conveyor.sdf
Echo the odometry messages: gz topic -e -t /model/conveyor/link/base_link/odometry

[jrutgeer]

Ok I tried to update this PR to use an odometry message instead of the kinematic state as suggested here, but obviously I must have missed some git wizzardry command as suddenly 55 files are marked as changed.

What I did is the following:

• Make my local changes,
• Added upstream remote:
  • git remote add upstream git@github.com:gazebosim/gz-sim.git
  • git fetch upstream
• Commit local changes:
  • git add examples/worlds/conveyor.sdf
  • git add src/systems/track_controller/TrackController.cc
  • git commit -m "Changed to odometry publisher"
• Rebase local branch so my changes would be relative to the current state (correct?):
  • git rebase upstream/gz-sim7
• Push changes:
  • git push --> complains that I should do a git pull first
  • git pull -->complains I should do git config pull.rebase false or .rebase true or .ff only
  • git config pull.rebase true
  • git pull
  • git push

At that point I didn't check if it already had the "55 files changed", but since the DCO was no OK I did what it said, i.e.:

• git rebase HEAD~11 --signoff
• git push --force-with-lease origin gz-sim7

And here we are. :-(

What should I have done differently?
How can I configure the DCO by default?
How to proceed, close this PR and submit a new one?

Thanks.

[jrutgeer]

How to proceed, close this PR and submit a new one?

Given below output, I think I should do a git reset --hard HEAD@{25} followed by a git commit -S and git push, or, if that does not resolve it, a git reset --hard HEAD@{29}?

$ git reflog
827def133 (HEAD -> gz-sim7, origin/gz-sim7, origin/HEAD) HEAD@{0}: rebase (finish): returning to refs/heads/gz-sim7
827def133 (HEAD -> gz-sim7, origin/gz-sim7, origin/HEAD) HEAD@{1}: rebase (pick): Changed to odometry publisher
e1615e777 HEAD@{2}: rebase (pick): Support loading mesh by mesh name in <mesh><uri> (#2007)
7c782ff6c HEAD@{3}: rebase (pick): ComponentInspector: display PhysicsEnginePlugin (#2032)
e71af082a HEAD@{4}: rebase (pick): Send BlockOrbit false events only once from TransformControl plugin (#2030)
6dbe4daaa HEAD@{5}: rebase (pick): Categorize tutorials list (#2028)
11823d4a1 HEAD@{6}: rebase (pick): add time out to wait to avoid deadlock (#2025)
421e5b7ea HEAD@{7}: rebase (pick): Add optional binary relocatability (#1968)
e7dc89d75 HEAD@{8}: rebase (pick): Several minor fixes (#2027)
492f5cb5e HEAD@{9}: rebase (pick): Protobuf: Do not require version 3 do support Protobuf 4.23.2 (23.2) (#2006)
c2b918c96 HEAD@{10}: rebase (pick): Changed some formatting.
895ce964c HEAD@{11}: rebase (pick): Added publisher for track position and velocity.
064f69ed7 HEAD@{12}: rebase (start): checkout HEAD~11
d30b922a8 HEAD@{13}: pull (finish): returning to refs/heads/gz-sim7
d30b922a8 HEAD@{14}: pull (pick): Changed to odometry publisher
397e5a9f2 HEAD@{15}: pull (pick): Support loading mesh by mesh name in <mesh><uri> (#2007)
d5b3c8364 HEAD@{16}: pull (pick): ComponentInspector: display PhysicsEnginePlugin (#2032)
fc09e99fb HEAD@{17}: pull (pick): Send BlockOrbit false events only once from TransformControl plugin (#2030)
c4e20d3c2 HEAD@{18}: pull (pick): Categorize tutorials list (#2028)
b5eed54d6 HEAD@{19}: pull (pick): add time out to wait to avoid deadlock (#2025)
53bb5bc1a HEAD@{20}: pull (pick): Add optional binary relocatability (#1968)
02c312008 HEAD@{21}: pull (pick): Several minor fixes (#2027)
8aa45bc3f HEAD@{22}: pull (pick): Protobuf: Do not require version 3 do support Protobuf 4.23.2 (23.2) (#2006)
192d20c94 HEAD@{23}: pull (start): checkout 192d20c946fd5863f177cf8b23f95af974d78e0b
e895c7149 HEAD@{24}: rebase (finish): returning to refs/heads/gz-sim7
e895c7149 HEAD@{25}: rebase (pick): Changed to odometry publisher
a8cf56061 HEAD@{26}: rebase (pick): Changed some formatting.
08237a61a HEAD@{27}: rebase (pick): Added publisher for track position and velocity.
50a99e6c2 (upstream/gz-sim7) HEAD@{28}: rebase (start): checkout upstream/gz-sim7
3464f140d HEAD@{29}: commit: Changed to odometry publisher
192d20c94 HEAD@{30}: rebase (finish): returning to refs/heads/gz-sim7
192d20c94 HEAD@{31}: rebase (pick): Changed some formatting.
028510d46 HEAD@{32}: rebase (pick): Added publisher for track position and velocity.
064f69ed7 HEAD@{33}: rebase (start): checkout HEAD~2
935ae3a3b HEAD@{34}: checkout: moving from gz-sim7 to gz-sim7
935ae3a3b HEAD@{35}: commit: Changed some formatting.
96b1ae4b9 HEAD@{36}: commit: Added publisher for track position and velocity.
064f69ed7 HEAD@{37}: clone: from github.com:jrutgeer/gz-sim.git

[jrutgeer]

How can I configure the DCO by default?

According to this documentation, following command should configure commit signing by default:

git config --global commit.gpgsign true

As @mjcarroll points out this is not correct, -s (signoff) is not the same as -S.

[mjcarroll]

When we talk about signing for DCO, it is using -s (little s, the equivalent of the long form argument --signoff).

I believe with a new enough git, you can do (for example) git rebase --signoff HEAD~25

[jrutgeer]

I think I succeeded in reverting everything and committing only the files I changed including DCO signoff. :-)
Slowly getting there.

[arjo129]

This looks pretty good to me. The only minor knit I have is that you should document SDF parameters in the header, this allows the documentation online to be updated.

[arjo129]

Would be nice to get an integration test for this in.

[jrutgeer]

Well, I'm generally all in favour of integration tests, but in this case I don't really see what could be a good test:

• I could run the example world with different values for <odometry_publish_frequency> and <odometry_topic>, but imo. that would rather be a test for sdf::Element::Get() and validTopic, which are known to work, rather than an actual test for the plugin,
• Similarly, I could also run the example world, start the conveyor and see if the position and velocity changes, but that is rather a test of Publisher::Pub(msg) and maybe also of this but that's also rather trivial?

Unless you have a suggestion for a better test?

[azeey]

Similarly, I could also run the example world, start the conveyor and see if the position and velocity changes, but that is rather a test of Publisher::Pub(msg) and maybe also of this but that's also rather trivial?

I'd say a test showing the changes in position and velocity would be good. Might be trivial, but could catch future changes that break behavior.

[jrutgeer]

@azeey I added the test and guess what.. there was a bug. :-) I was publishing velocity instead of limitedVelocity.
So adding the test indeed proved to be valuable.

The test will currently fail if Dart 6.13 is used due to #2008. As I am running Dart 6.13 I had to change some signs to get it to run (but these are removed in the commit).
The tracked vehicle test also fails on my setup, presumably also due to Dart 6.13. As I could not easily fix it by changing some signs, and as I did not do any changes to the tracked vehicle plugin, I did not look further into this.

[jrutgeer]

@azeey I just noticed the beta tag was removed. I thought I had until code freeze (2023/08/30) to finish this?
Can the tag be added again?

[azeey]

I'll add it back in. The code freeze is when everything must be merged by. Since I didn't see any changes recently, I just assumed you didn't have time to work on it.

[jrutgeer]

Is this ready now? Some checks are not successful but it is not clear to me if this needs further action and if so, which.

[azeey]

There are a few style errors (run make codecheck). The ABI checker is also failing, but I think that's because the branch needs to be synced with gz-sim7. Can you merge from gz-sim7 and fix the style errors?

[jrutgeer]

@azeey I rebased the PR and fixed the style errors.

• ignition_gazebo-ci-pr_any-ubuntu_auto-amd64 fails on INTEGRATION_odometry_publisher and INTEGRATION_reset_sensors, which are unrelated,
• The other tests pass, although the TrackedVehicleTest is skipped due to Skipping test because physics engine does not support SetContactPropertiesCallbackFeature, so I assume CI does not have the patched Dart version installed.
• ignition_gazebo-ci-pr_any-homebrew-amd64 also fails, but "No problems were identified." so I don't know what that means.

[azeey]

ignition_gazebo-ci-pr_any-homebrew-amd64 also fails, but "No problems were identified." so I don't know what that means.

This seems like a problem with the machine. We're looking into it.

[jrutgeer]

@azeey So is there still any further action required on my side?

[azeey]

@azeey So is there still any further action required on my side?

No, this just needs an approval from @arjo129.

[arjo129]

Works great. Thanks for this awesome addition.