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

Extract publishing mechanism to a new function #246

Merged

Conversation

nachovizzo
Copy link
Collaborator

@nachovizzo nachovizzo commented Oct 19, 2023

Just opening this to review this a bit easier.

The whole idea of this is to reduce the reading burden when reasoning/inspecting the main logic. Publishing is something no we don't really care :)

Like this, when I look at the main function of the server, I can really see what's going on:
image

@nachovizzo
Copy link
Collaborator Author

@benemer happy to revert this, I'm merging this to move forward a bit ;)

@nachovizzo nachovizzo merged commit 70d972d into nacho/fix_ros_tf_tree_usage Oct 19, 2023
@nachovizzo nachovizzo deleted the nacho/fix_ros_tf_usage_clean branch October 19, 2023 21:37
Copy link
Member

@benemer benemer left a comment

Choose a reason for hiding this comment

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

Approved post-merge 🙃

tizianoGuadagnino added a commit that referenced this pull request Oct 27, 2023
* Fix TF tree usage in ROS 2 wrapper

Signed-off-by: Ignacio Vizzo <ignacio@dexory.com>

* Transform the pose instead of the pointcloud

* remove include

* Remove default base_frame parameter value

This will make KISS-ICP work ego-centric as default

* Remove unused variable

* Do not reuse pose_msg.pose

* I'm not ever sure why we did that in first instance

* Be more explicit about the stamps and frame_id of the headers

* Extract publishing mechanism to a new function (#246)

* Convert class member function to more useful utility

And fix a small bug, the order was of the transformation before was the
opposite, and therefore we were obtaining base2cloud. Since we multiply
by both sides we can't really see the difference, but it was
conceptually wrong.

* Bring back debugging clouds to ros driver

* re-arange logic on when and when not to publish clouds

* fix lofic

* Update rviz2

* Loosing my mind already with this debugging info

* Backport tf multisensor fix (#245)

* Build system changes for tf fix

* Modify params for tf fix

* Add ROS 1 tf fixes similar to ROS 2

* Update rviz config

* Remove unused debug publishers

* Remove unnecessary smart pointers

* Update ROS 1 to match ROS 2 changes

* Fix style

* Remove sophus from build system

Fixing now the CI is a big pain

* Remove unnecessary alias

---------

Signed-off-by: Ignacio Vizzo <ignacio@dexory.com>
Co-authored-by: Tim Player <tim@overland.ai>
Co-authored-by: raw_t <37455909+tizianoGuadagnino@users.noreply.github.com>
Co-authored-by: tizianoGuadagnino <frevo93@gmail.com>
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