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

Fix missing import and error in tf_broadcaster #197

Merged
merged 1 commit into from
Sep 10, 2024

Conversation

J-Schaefer
Copy link
Contributor

Ran into an error here. Since the property projection_namespace is now of type ExecutionType it cannot be concatenated with a string anymore. So we can take the name for that.

@@ -78,8 +79,8 @@ def _publish_pose(self, child_frame_id: str, pose: Pose, static=False) -> None:
frame_id = pose.frame
if frame_id != child_frame_id:
tf_stamped = pose.to_transform(child_frame_id)
tf_stamped.frame = self.projection_namespace + "/" + tf_stamped.frame
tf_stamped.child_frame_id = self.projection_namespace + "/" + tf_stamped.child_frame_id
tf_stamped.frame = self.projection_namespace.name + "/" + tf_stamped.frame
Copy link
Collaborator

Choose a reason for hiding this comment

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

isnt os.path.join nicer here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really the scope of this PR, since this is only fixing a bug, but tried it nevertheless. os.path.join behaves differently depending on the platform. No problem if we are always on Linux with this. I could add "/".join(...) though.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Bug fixes are also required to be as clean as possible.
The question was more If for another os than Linux ghe rostopic still has "/" as seperator or If eg on the seperator will be ""

Copy link
Collaborator

Choose a reason for hiding this comment

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

*Backslash

@Tigul Tigul merged commit f0c8dfa into cram2:dev Sep 10, 2024
1 of 2 checks passed
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.

3 participants