Remove unnecessary exposed utility function from ROS API #289
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Since with this new changes PublishOdometry is the only member that requieres to know the user specified target-frame, it is not necesary to handle all this bits. This makes the implementation cleaner, easier to read and reduces the lines of code. Now RegisterFrame is a simple callback as few months ago one can read in seconds.
This reverts commit 424ee90.
With this change this function is not exposed (which was never the intention to) to the header. This way we can also "hide" this into a private unnamed namesapces and benefit from inlining the simple function into the translation unit
This reverts commit 23cd7ef.
With this change this function is not exposed (which was never the intention to) to the header. This way we can also "hide" this into a private unnamed namesapces and benefit from inlining the simple function into the translation unit
This reverts commit d1dcb48.
nachovizzo
added a commit
that referenced
this pull request
Mar 1, 2024
benemer
previously approved these changes
Mar 3, 2024
tizianoGuadagnino
previously approved these changes
Mar 18, 2024
…nto nacho/make_utility_function_private
|
Fix the merge confict ;) |
Base automatically changed from
nacho/refactor_register_frame
to
AUTO-2217_revise_tf_tree_clouds
March 18, 2024 09:54
nachovizzo
changed the base branch from
AUTO-2217_revise_tf_tree_clouds
to
main
nachovizzo
dismissed stale reviews from tizianoGuadagnino and benemer
The base branch was changed.
nachovizzo
changed the base branch from
main
to
AUTO-2217_revise_tf_tree_clouds
…y_function_private
tizianoGuadagnino
added a commit
that referenced
this pull request
Mar 25, 2024
* Fix this madness Simplify implementation by debugging in an ego-centric way always. Since the KISS-ICP internal map is on the global coordinates, fetch the last ego-centric pose and apply it to the map. Seen from the cloud_frame_id (which is the sensor frame) everything should always work in terms of visualization, no matter the multi-sensor setup. * Now is safe to disable this by default * Simplify, borrow the header from the input pointcloud msg This actually makes the visualization closer to the Python visualizer * Disable path/odom visualization by default In the case where we are not computing the poses in an egocentric world (base_frame != "") and we are not publishing to the TF tree, then the visualization wouldn't make sense. Therefore, disable it by default * Changed my mind If someone doesn't have that particular frame defined, then the visualization won't work. Leave this default * Move responsability of handling tf frames out of Registration (#288) * Move responsability of handling tf frames out of Registration Since with this new changes PublishOdometry is the only member that requieres to know the user specified target-frame, it is not necesary to handle all this bits. This makes the implementation cleaner, easier to read and reduces the lines of code. Now RegisterFrame is a simple callback as few months ago one can read in seconds. * typo * Easier to read * We need this for LookupTransform * Remove unused variable * Revert "Remove unused variable" This reverts commit 424ee90. * Remove unnecessary check * Remove unnecessary exposed utility function from ROS API With this change this function is not exposed (which was never the intention to) to the header. This way we can also "hide" this into a private unnamed namesapces and benefit from inlining the simple function into the translation unit * Revert "Remove unnecessary exposed utility function from ROS API" This reverts commit 23cd7ef. * Revert "Remove unnecessary check" This reverts commit d1dcb48. * merge conflicts :0 * Remove unnecessary exposed utility function from ROS API (#289) * Move responsability of handling tf frames out of Registration Since with this new changes PublishOdometry is the only member that requieres to know the user specified target-frame, it is not necesary to handle all this bits. This makes the implementation cleaner, easier to read and reduces the lines of code. Now RegisterFrame is a simple callback as few months ago one can read in seconds. * typo * Easier to read * We need this for LookupTransform * Remove unused variable * Revert "Remove unused variable" This reverts commit 424ee90. * Remove unnecessary check * Remove unnecessary exposed utility function from ROS API With this change this function is not exposed (which was never the intention to) to the header. This way we can also "hide" this into a private unnamed namesapces and benefit from inlining the simple function into the translation unit * Revert "Remove unnecessary exposed utility function from ROS API" This reverts commit 23cd7ef. * Remove unnecessary exposed utility function from ROS API With this change this function is not exposed (which was never the intention to) to the header. This way we can also "hide" this into a private unnamed namesapces and benefit from inlining the simple function into the translation unit * Revert "Remove unnecessary check" This reverts commit d1dcb48. --------- Co-authored-by: tizianoGuadagnino <[email protected]> * too many merges * Merge Rviz and Python colors * Just make the default construction more clear --------- Co-authored-by: tizianoGuadagnino <[email protected]>
tizianoGuadagnino
added a commit
to 02alexander/kiss-icp
that referenced
this pull request
Mar 25, 2024
* Fix this madness Simplify implementation by debugging in an ego-centric way always. Since the KISS-ICP internal map is on the global coordinates, fetch the last ego-centric pose and apply it to the map. Seen from the cloud_frame_id (which is the sensor frame) everything should always work in terms of visualization, no matter the multi-sensor setup. * Now is safe to disable this by default * Simplify, borrow the header from the input pointcloud msg This actually makes the visualization closer to the Python visualizer * Disable path/odom visualization by default In the case where we are not computing the poses in an egocentric world (base_frame != "") and we are not publishing to the TF tree, then the visualization wouldn't make sense. Therefore, disable it by default * Changed my mind If someone doesn't have that particular frame defined, then the visualization won't work. Leave this default * Move responsability of handling tf frames out of Registration (PRBonn#288) * Move responsability of handling tf frames out of Registration Since with this new changes PublishOdometry is the only member that requieres to know the user specified target-frame, it is not necesary to handle all this bits. This makes the implementation cleaner, easier to read and reduces the lines of code. Now RegisterFrame is a simple callback as few months ago one can read in seconds. * typo * Easier to read * We need this for LookupTransform * Remove unused variable * Revert "Remove unused variable" This reverts commit 424ee90. * Remove unnecessary check * Remove unnecessary exposed utility function from ROS API With this change this function is not exposed (which was never the intention to) to the header. This way we can also "hide" this into a private unnamed namesapces and benefit from inlining the simple function into the translation unit * Revert "Remove unnecessary exposed utility function from ROS API" This reverts commit 23cd7ef. * Revert "Remove unnecessary check" This reverts commit d1dcb48. * merge conflicts :0 * Remove unnecessary exposed utility function from ROS API (PRBonn#289) * Move responsability of handling tf frames out of Registration Since with this new changes PublishOdometry is the only member that requieres to know the user specified target-frame, it is not necesary to handle all this bits. This makes the implementation cleaner, easier to read and reduces the lines of code. Now RegisterFrame is a simple callback as few months ago one can read in seconds. * typo * Easier to read * We need this for LookupTransform * Remove unused variable * Revert "Remove unused variable" This reverts commit 424ee90. * Remove unnecessary check * Remove unnecessary exposed utility function from ROS API With this change this function is not exposed (which was never the intention to) to the header. This way we can also "hide" this into a private unnamed namesapces and benefit from inlining the simple function into the translation unit * Revert "Remove unnecessary exposed utility function from ROS API" This reverts commit 23cd7ef. * Remove unnecessary exposed utility function from ROS API With this change this function is not exposed (which was never the intention to) to the header. This way we can also "hide" this into a private unnamed namesapces and benefit from inlining the simple function into the translation unit * Revert "Remove unnecessary check" This reverts commit d1dcb48. --------- Co-authored-by: tizianoGuadagnino <[email protected]> * too many merges * Merge Rviz and Python colors * Just make the default construction more clear --------- Co-authored-by: tizianoGuadagnino <[email protected]>
tizianoGuadagnino
added a commit
that referenced
this pull request
Mar 25, 2024
* Fix this madness Simplify implementation by debugging in an ego-centric way always. Since the KISS-ICP internal map is on the global coordinates, fetch the last ego-centric pose and apply it to the map. Seen from the cloud_frame_id (which is the sensor frame) everything should always work in terms of visualization, no matter the multi-sensor setup. * Now is safe to disable this by default * Simplify, borrow the header from the input pointcloud msg This actually makes the visualization closer to the Python visualizer * Disable path/odom visualization by default In the case where we are not computing the poses in an egocentric world (base_frame != "") and we are not publishing to the TF tree, then the visualization wouldn't make sense. Therefore, disable it by default * Changed my mind If someone doesn't have that particular frame defined, then the visualization won't work. Leave this default * Move responsability of handling tf frames out of Registration (#288) * Move responsability of handling tf frames out of Registration Since with this new changes PublishOdometry is the only member that requieres to know the user specified target-frame, it is not necesary to handle all this bits. This makes the implementation cleaner, easier to read and reduces the lines of code. Now RegisterFrame is a simple callback as few months ago one can read in seconds. * typo * Easier to read * We need this for LookupTransform * Remove unused variable * Revert "Remove unused variable" This reverts commit 424ee90. * Remove unnecessary check * Remove unnecessary exposed utility function from ROS API With this change this function is not exposed (which was never the intention to) to the header. This way we can also "hide" this into a private unnamed namesapces and benefit from inlining the simple function into the translation unit * Revert "Remove unnecessary exposed utility function from ROS API" This reverts commit 23cd7ef. * Revert "Remove unnecessary check" This reverts commit d1dcb48. * merge conflicts :0 * Remove unnecessary exposed utility function from ROS API (#289) * Move responsability of handling tf frames out of Registration Since with this new changes PublishOdometry is the only member that requieres to know the user specified target-frame, it is not necesary to handle all this bits. This makes the implementation cleaner, easier to read and reduces the lines of code. Now RegisterFrame is a simple callback as few months ago one can read in seconds. * typo * Easier to read * We need this for LookupTransform * Remove unused variable * Revert "Remove unused variable" This reverts commit 424ee90. * Remove unnecessary check * Remove unnecessary exposed utility function from ROS API With this change this function is not exposed (which was never the intention to) to the header. This way we can also "hide" this into a private unnamed namesapces and benefit from inlining the simple function into the translation unit * Revert "Remove unnecessary exposed utility function from ROS API" This reverts commit 23cd7ef. * Remove unnecessary exposed utility function from ROS API With this change this function is not exposed (which was never the intention to) to the header. This way we can also "hide" this into a private unnamed namesapces and benefit from inlining the simple function into the translation unit * Revert "Remove unnecessary check" This reverts commit d1dcb48. --------- Co-authored-by: tizianoGuadagnino <[email protected]> * too many merges * Merge Rviz and Python colors * Just make the default construction more clear --------- Co-authored-by: tizianoGuadagnino <[email protected]>
tizianoGuadagnino
added a commit
that referenced
this pull request
Mar 25, 2024
* Fix bug where point would match with random point * Use closest neightbor distance * Switch order * Refactor ROS parametrization (#282) * Move ros params from launch file to YAML * Reuse arguments for debug clouds * Rename odometry_node to kiss_icp_node odometry is way to generic * Rename node agin * Voxel size is optional paramter * Reads better like this * New parameter proposal * Add odometry covariance to ros node (#283) * Add fixed covariance to odometry msg * Add default value just in case * pre-commit * Expose number of icp iterations in ros (#292) * Expose registration params to ROS node * More merge conflicts * Default to multithread * Revert "Refactor ROS parametrization (#282)" This reverts commit 7b8b095. * Fix and improve ROS visualization (#285) * Fix this madness Simplify implementation by debugging in an ego-centric way always. Since the KISS-ICP internal map is on the global coordinates, fetch the last ego-centric pose and apply it to the map. Seen from the cloud_frame_id (which is the sensor frame) everything should always work in terms of visualization, no matter the multi-sensor setup. * Now is safe to disable this by default * Simplify, borrow the header from the input pointcloud msg This actually makes the visualization closer to the Python visualizer * Disable path/odom visualization by default In the case where we are not computing the poses in an egocentric world (base_frame != "") and we are not publishing to the TF tree, then the visualization wouldn't make sense. Therefore, disable it by default * Changed my mind If someone doesn't have that particular frame defined, then the visualization won't work. Leave this default * Move responsability of handling tf frames out of Registration (#288) * Move responsability of handling tf frames out of Registration Since with this new changes PublishOdometry is the only member that requieres to know the user specified target-frame, it is not necesary to handle all this bits. This makes the implementation cleaner, easier to read and reduces the lines of code. Now RegisterFrame is a simple callback as few months ago one can read in seconds. * typo * Easier to read * We need this for LookupTransform * Remove unused variable * Revert "Remove unused variable" This reverts commit 424ee90. * Remove unnecessary check * Remove unnecessary exposed utility function from ROS API With this change this function is not exposed (which was never the intention to) to the header. This way we can also "hide" this into a private unnamed namesapces and benefit from inlining the simple function into the translation unit * Revert "Remove unnecessary exposed utility function from ROS API" This reverts commit 23cd7ef. * Revert "Remove unnecessary check" This reverts commit d1dcb48. * merge conflicts :0 * Remove unnecessary exposed utility function from ROS API (#289) * Move responsability of handling tf frames out of Registration Since with this new changes PublishOdometry is the only member that requieres to know the user specified target-frame, it is not necesary to handle all this bits. This makes the implementation cleaner, easier to read and reduces the lines of code. Now RegisterFrame is a simple callback as few months ago one can read in seconds. * typo * Easier to read * We need this for LookupTransform * Remove unused variable * Revert "Remove unused variable" This reverts commit 424ee90. * Remove unnecessary check * Remove unnecessary exposed utility function from ROS API With this change this function is not exposed (which was never the intention to) to the header. This way we can also "hide" this into a private unnamed namesapces and benefit from inlining the simple function into the translation unit * Revert "Remove unnecessary exposed utility function from ROS API" This reverts commit 23cd7ef. * Remove unnecessary exposed utility function from ROS API With this change this function is not exposed (which was never the intention to) to the header. This way we can also "hide" this into a private unnamed namesapces and benefit from inlining the simple function into the translation unit * Revert "Remove unnecessary check" This reverts commit d1dcb48. --------- Co-authored-by: tizianoGuadagnino <[email protected]> * too many merges * Merge Rviz and Python colors * Just make the default construction more clear --------- Co-authored-by: tizianoGuadagnino <[email protected]> * Revert "Fix and improve ROS visualization (#285)" This reverts commit 20797de. --------- Co-authored-by: tizianoGuadagnino <[email protected]> Co-authored-by: Benedikt Mersch <[email protected]> Co-authored-by: Ignacio Vizzo <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove unnecessary exposed utility function from ROS API
With this change, this function is not exposed (which was never the
intention to) to the header. This way, we can "hide" this into private unnamed namespaces and benefit from inlining the simple function into the translation unit.