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: Fix static transforms and joint states #543

Closed
wants to merge 5 commits into from

Conversation

peci1
Copy link
Collaborator

@peci1 peci1 commented Aug 11, 2020

Commit 1ae9358 mistakenly removed a robot_state_publisher that handled publication of static transforms. This PR returns it back.

It also returns back joints left_track_j and right_track_j into the published joints states, since these joints are expected to be moving in the future.

@peci1
Copy link
Collaborator Author

peci1 commented Aug 11, 2020

This is how RViz displays the robot with the current master branch:

Snímek obrazovky pořízený 2020-08-11 03-35-49

@peci1 peci1 mentioned this pull request Aug 11, 2020
@acschang
Copy link
Contributor

These new features will not be incorporated into the SubT Virtual Testbed prior to Cave Circuit. We will keep this PR open for consideration after Cave Circuit.

@peci1
Copy link
Collaborator Author

peci1 commented Aug 20, 2020

At least the part of this PR that fixes the static transforms is not a new feature but a bug fix. It is not only about missing links in rviz, but alsou about the inability to correctly transform e.g. the realsense cloud to base_link.

@acschang
Copy link
Contributor

At least the part of this PR that fixes the static transforms is not a new feature but a bug fix. It is not only about missing links in rviz, but alsou about the inability to correctly transform e.g. the realsense cloud to base_link.

The simulated realsense cloud is published in the alpha/base_link/camera frame which there exists a transform to alpha/base_link.

@peci1
Copy link
Collaborator Author

peci1 commented Aug 26, 2020

Ah, you're right, the camera link is published by Gazebo relative to base_link. But for example our algorithms make use of the laser_base frame, which is a convenience frame that denotes the place where the lidar gimbal connects to the robot body. This frame is missing without this fix.

@nkoenig
Copy link
Contributor

nkoenig commented Jan 13, 2021

The bounding box for this model is

Min[-0.608707 -0.296315 -0.188594] Max[0.608707 0.296317 0.615689]

@osrf-jenkins
Copy link

Build finished. No test results found.

@peci1
Copy link
Collaborator Author

peci1 commented Feb 10, 2021

I updated this PR with master and it's ready for review.

@nkoenig nkoenig self-requested a review March 1, 2021 21:05
@peci1
Copy link
Collaborator Author

peci1 commented Mar 3, 2021

Another merge with master (no conflict).

@peci1
Copy link
Collaborator Author

peci1 commented Mar 16, 2021

@nkoenig Friendly ping. I'd like to submit new sensor configs of Absolem, but it'd be more convenient if these changes were already merged. This PR has been open for 7 months now...

@iche033
Copy link
Contributor

iche033 commented Mar 16, 2021

The changes look fine and I see the new frames published after adding robot_state_publisher.

Just want to check if preserving fixed joints when converting urdf to sdf is an option? That'll let ignition's pose publisher system publish the frames but will likely make the model.sdf diff larger

<gazebo reference="base_to_link">
  <disableFixedJointLumping>true</disableFixedJointLumping> 
  <preserveFixedJoint>true</preserveFixedJoint>
</gazebo>

I'll let @nkoenig make the final call since he removed the publisher and joint states - not sure if there is a specific reason.

@peci1
Copy link
Collaborator Author

peci1 commented Mar 16, 2021

if preserving fixed joints when converting urdf to sdf is an option

Yes, that would also work. I've discovered that while preparing our new model and am using it there. What about merging this PR now and I would utilize preserving of fixed joints in a next batch of absolem updates?

# Conflicts:
#	submitted_models/ctu_cras_norlab_absolem_sensor_config_1/launch/vehicle_topics.launch
@peci1
Copy link
Collaborator Author

peci1 commented Apr 10, 2021

I updated this PR to resolve conflicts with master. We're still interesting in merging this in. The robot_state_publisher approach has been approved in #867.

@peci1
Copy link
Collaborator Author

peci1 commented May 19, 2021

The robot_state_publisher was returned back in #859 and we can live without the track joints in joint states, so I'm closing this PR.

@peci1 peci1 closed this May 19, 2021
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.

5 participants