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

Add option to continuously transform visuals in odometry plugin #1686

Open
wants to merge 3 commits into
base: noetic-devel
Choose a base branch
from

Conversation

AndreasR30
Copy link
Contributor

Hello,

I made a new pull request, which is essentially the same as #1631, which was merged but then reverted as it broke the visualization of other users. But now the changes are less invasive and do not affect old visualizations as long the option is unchecked (the default). The new option triggers the same behavior as the activated frame_locked flag in the Marker Message.

…r.t. fixed frame

This option is particularly useful for messages, whose frame is moving w.r.t. fixed frame.
…values in odometry message without getting numerical issues

These large positional values can occur when the odometry message comes from a GNSS receiver where GPS positions are converted into UTM coordinates.
@Martin-Oehler
Copy link

This PR makes sense when you want to view the history of measurements as a time-independent path instead of time-specific measurements. In this case, the default behavior is not very useful, because it does not allow you to compare trajectories published in different frames:

without_patch

With this PR, the history is re-transformed with the latest transform, allowing the comparison of trajectories.

with_patch

In principal, I support merging this PR, but like mentioned in the similar #1687, I would suggest changing the naming of the option to something like "Ignore timestamp" or "Use most recent timestamp" to be consistent with the Map Display ("Use timestamp", does the opposite).

Also, in my opinion, the changes regarding ref poses for high values should be split into a separate PR.

Best regards,
Martin

@AndreasR30
Copy link
Contributor Author

This PR makes sense when you want to view the history of measurements as a time-independent path instead of time-specific measurements. In this case, the default behavior is not very useful, because it does not allow you to compare trajectories published in different frames:

I am happy to read that you see the advantages of this option for odometry display.

In principal, I support merging this PR, but like mentioned in the similar #1687, I would suggest changing the naming of the option to something like "Ignore timestamp" or "Use most recent timestamp" to be consistent with the Map Display ("Use timestamp", does the opposite).

I can change the naming in order to be consistent. @rhaschke, what do you think?

Also, in my opinion, the changes regarding ref poses for high values should be split into a separate PR.

I already split it into a separate commit. Why would you split it into a separate PR? Are you not fully convinced about this feature, yet? @rhaschke, do you also think it is useful to split it into a separate PR?

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