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

Absolem: Convert to SDF 1.7 #851

Draft
wants to merge 51 commits into
base: master
Choose a base branch
from

Conversation

peci1
Copy link
Collaborator

@peci1 peci1 commented Mar 30, 2021

This is not a model submission, but technical maintenance of our model submitted for Cave circuit. We will however build our new sensor config submission on top of this PR for the reasons explained in this description.

We're preparing new sensor configs of Absolem that correspond to a brand new hardware revision of the robot. So we returned back to the base Absolem package and found out that ign sdf -p tool started producing very different SDFs from the Blueprint version. It emits SDF 1.7 instead of 1.6, makes use of the frame semantics, and most importantly - it removes all <pose> elements from <sensor> tags that were created by the fixed joint lumping algorithm. However, as Absolem is using the Xacro->SDF process to update the model file, we would like to upgrade the model.sdf of sensor configs 1 and 2 to exactly correspond to the output of the new version of ign sdf -p tool - because the new sensor configs will use this same process.

This PR is a 100% no-change from both the ROS and Gazebo points of view. Do not get overwhelmed by the number of commits this PR wants to merge. I created so many of them to actually ease the review process. 90% of them are just commits moving definition of a joint from position after its link to position before its link. This is one of the biggest changes between Blueprint's and Dome's ign sdf -p. Unfortunately, Git cannot nicely recognize this kind of transformation when done all at once, so I broke it down joint-by-joint to separate commits, so that you can easily check that all that happened was moving one block of XML a little higher. All of the other commits are topic-grouped and each of them should be easily reviewable as a no-change. Get 51 no-changes together and you get this PR :)

After the PR is reviewed, I can squash th commits into a more reasonable number.

The target of this PR is that running update_robot_sdf results in exactly the same file as the one commited to the repository.

It is actually not needed to change the models on Fuel as nothing has really changed. This PR is meant for easing future maintenance (hopefully e.g. implementing the proper tracked motion model).

peci1 added 30 commits March 29, 2021 19:32
… it doesn't matter as it only rotates a symmetrical cube visual.
No geometrical change (verified visually).
peci1 added 21 commits March 29, 2021 20:28
After this commit, the model.sdf can be exactly recreated by the update_robot_sdf script.

Until gazebosim/sdformat#525 gets merged, `ign sdf -p` does not account for the reduced joint transform when outputting sensors attached to lumped frames. But it copies `<sensor><pose>` verbatim, so I took the old values from SDF and put them temporarily in `<sensor><pose>` where they specify relative position of the sensor to base_link where they get lumped.

Printing of the resulting SDF memory model results in flipping one Euler notation, but it's a 100% equivalent transform. Here's a proof by Ignition Math:

Quaterniond initialized by (-2.19645, -1.5707963267948966, 0) has:
 - coefs: -0.629608 -0.321859 -0.629608 0.321859
 - Euler: -2.19645 -1.5708 0

Quaterniond initialized by (3.1415926535897931, -1.5707963267948966, 0.945143) has:
 - coefs: 0.629608 0.321859 0.629608 -0.321859
 - Euler: -2.19645 -1.5708 0

This proves that the one changed line in model.sdf means no practical change.
<link name="laser">
<pose>0.2502 0 0.1407 -3.1415926535897931 -0 0</pose>
Copy link
Contributor

Choose a reason for hiding this comment

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

relative_to, and frame semantics in general, is not supported in web visualization. Can you revert these changes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Outch... but all our models are based on the output of ign sdf -p, which in Ignition Dome, automatically produces these...

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh. that makes sense. Please hold on this, while I consider options.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd be really glad for not having to change this back :) On the other hand, as all of our robots' SDFs are generated from Xacro, you could also take a virtual machine with Blueprint, convert the files to SDF there, and upload the result to Fuel... But it sounds like it could hit us back at some point

Copy link
Contributor

Choose a reason for hiding this comment

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

Updating gzweb to support 1.7 will take some effort, but it is on our plate. In the meantime, I've changed absolem to include these changes while using version 1.6 of sdf in #875. Can you take a look at the PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

SDF 1.6 alternative in #877.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is there an issue to track SDF 1.7 support in gzweb?

@peci1
Copy link
Collaborator Author

peci1 commented Apr 4, 2021

I converted this PR to draft as it is superseded by #877 until gzweb supports SDF 1.7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants