Skip to content

Conversation

@mbpeterson70
Copy link
Collaborator

  • add option to use a static tf that is published in /tf in data_params
  • allow skipping non-overlapping submaps and self loop closures
  • make the offline rpgo result plot nicer

@mbpeterson70 mbpeterson70 marked this pull request as ready for review November 23, 2025 02:00
@mbpeterson70 mbpeterson70 requested a review from liqyn November 23, 2025 02:00
if not np.isnan(robots_nearby_mat[i, j]):
relative_yaw_angle = transform_to_xyzrpy(T_ij)[5]
submap_yaw_diff_mat[i, j] = np.abs(np.rad2deg(relative_yaw_angle))
if submap_distance > sm_io.skip_distance:
Copy link
Member

Choose a reason for hiding this comment

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

Consider moving this after the submap_descriptor calculations, so that submap descriptor similarity is still saved?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I like that, sounds good.

)
# by default looks for a static tf, but if the user wants to reference a tf that is
# theoretically static, but is published under /tf, then 'try_non_static_tf' can be set.
if param_dict.get('try_non_static_tf', False):
Copy link
Member

Choose a reason for hiding this comment

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

The naming 'try_non_static_tf' makes it sound like it would look under both /tf_static and /tf as a fallback. Maybe just get rid of the param and use a try/catch fallback instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Haha that's actually what I was doing before but I realized that PoseData.from_bag_tf is basically a super-set of any_static_tf_from_bag, so if the user is okay with the fact that they might be putting in a transform in their tree that shows up in /tf then we might as well use the PoseData.from_bag_tf. What do you think about renaming from try_non_static_tf to include_non_static_tf ?

Copy link
Member

Choose a reason for hiding this comment

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

Sure!

@mbpeterson70 mbpeterson70 merged commit 1c30300 into main Dec 3, 2025
1 check passed
@mbpeterson70 mbpeterson70 deleted the feature/utility-options branch December 3, 2025 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants