Conversation
…rams to stabilice the filter in summit
Signed-off-by: Francisco Martín Rico <fmrico@gmail.com>
Fix segfault in some cases and reduce extrapolation to the future
Added default path for route if not specified
New version of fusion localizer tested in real robot and simulator
Signed-off-by: Francisco Martín Rico <fmrico@gmail.com>
Signed-off-by: Francisco Martín Rico <fmrico@gmail.com>
Update plugins to new sensors API
Signed-off-by: Francisco Martín Rico <fmrico@gmail.com>
Signed-off-by: Francisco Martín Rico <fmrico@gmail.com>
There was a problem hiding this comment.
Pull request overview
Normalizes RViz2 /initialpose handling across multiple EasyNav localizers and adds functional tests to validate parameter-seeded initialization and runtime pose resets.
Changes:
- Added
/initialposesubscriptions and callbacks to AMCL-based localizers and to GPS/Fusion localizers. - Added/updated GTest-based functional tests for AMCL (simple/navmap/costmap), GPS, and Fusion localizers.
- Updated Fusion UKF set-pose behavior to make
/initialposeupdates deterministic.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| maps_managers/easynav_simple_maps_manager/src/easynav_simple_maps_manager/SimpleMapsManager.cpp | Removes debug std::cout noise during updates. |
| localizers/easynav_simple_localizer/tests/simple_localizer_tests.cpp | Replaces previous tests with functional /initialpose behavior test. |
| localizers/easynav_simple_localizer/src/easynav_simple_localizer/AMCLLocalizer.cpp | Adds /initialpose subscription + particle reinitialization callback; safer callback group fallback. |
| localizers/easynav_simple_localizer/include/easynav_simple_localizer/AMCLLocalizer.hpp | Declares /initialpose subscriber and callback. |
| localizers/easynav_simple_localizer/CMakeLists.txt | Enables building tests subdirectory. |
| localizers/easynav_navmap_localizer/tests/navmap_localizer_tests.cpp | New functional test validating seed pose + /initialpose update. |
| localizers/easynav_navmap_localizer/tests/costmap_localizer_tests.cpp | Removes older tests (dynamic update/map/service) in favor of functional pattern. |
| localizers/easynav_navmap_localizer/tests/CMakeLists.txt | Renames/rewires gtest target to new navmap test file. |
| localizers/easynav_navmap_localizer/src/easynav_navmap_localizer/AMCLLocalizer.cpp | Adds /initialpose subscription + callback; safer callback group fallback. |
| localizers/easynav_navmap_localizer/include/easynav_navmap_localizer/AMCLLocalizer.hpp | Declares /initialpose subscriber and callback. |
| localizers/easynav_navmap_localizer/CMakeLists.txt | Enables building tests subdirectory. |
| localizers/easynav_gps_localizer/tests/gps_localizer_tests.cpp | New functional test validating parameter seed + /initialpose alignment behavior. |
| localizers/easynav_gps_localizer/tests/CMakeLists.txt | Adds gtest target for GPS localizer functional test. |
| localizers/easynav_gps_localizer/src/easynav_gps_localizer/GpsLocalizer.cpp | Adds /initialpose subscription, optional parameter seed, and origin alignment on first valid fix. |
| localizers/easynav_gps_localizer/include/easynav_gps_localizer/GpsLocalizer.hpp | Adds /initialpose subscriber + pending init pose storage. |
| localizers/easynav_gps_localizer/CMakeLists.txt | Enables building tests subdirectory. |
| localizers/easynav_fusion_localizer/tests/fusion_localizer_tests.cpp | New functional test validating parameter seed + /initialpose update reflected in output odometry. |
| localizers/easynav_fusion_localizer/tests/CMakeLists.txt | Adds gtest target for Fusion localizer functional test. |
| localizers/easynav_fusion_localizer/src/easynav_fusion_localizer/ukf_wrapper.cpp | Makes set-pose deterministic and initializes filter only when needed. |
| localizers/easynav_fusion_localizer/src/easynav_fusion_localizer/FusionLocalizer.cpp | Adds /initialpose subscription + callback and optional parameter-based seed pose application. |
| localizers/easynav_fusion_localizer/include/easynav_fusion_localizer/FusionLocalizer.hpp | Declares /initialpose subscriber and callback. |
| localizers/easynav_fusion_localizer/CMakeLists.txt | Enables building tests subdirectory. |
| localizers/easynav_costmap_localizer/tests/costmap_localizer_tests.cpp | Updates test suite to functional seed + /initialpose reset pattern. |
| localizers/easynav_costmap_localizer/CMakeLists.txt | Enables building tests subdirectory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| double init_x = 0.0; | ||
| double init_y = 0.0; | ||
| double init_yaw = 0.0; | ||
| localizer_node->get_parameter(plugin_name + ".initial_pose.x", init_x); | ||
| localizer_node->get_parameter(plugin_name + ".initial_pose.y", init_y); | ||
| localizer_node->get_parameter(plugin_name + ".initial_pose.yaw", init_yaw); | ||
|
|
||
| if (std::abs(init_x) > 1e-12 || std::abs(init_y) > 1e-12 || std::abs(init_yaw) > 1e-12) { | ||
| auto init_pose = std::make_shared<geometry_msgs::msg::PoseWithCovarianceStamped>(); |
There was a problem hiding this comment.
FusionLocalizer.cpp now uses std::abs(...) for the initial pose parameter check, but the file doesn’t include <cmath>. Please include <cmath> explicitly to avoid relying on transitive includes / accidental overload resolution.
| #include "easynav_common/YTSession.hpp" | ||
|
|
||
| #include "tf2_geometry_msgs/tf2_geometry_msgs.hpp" | ||
| #include "tf2/LinearMath/Quaternion.hpp" | ||
|
|
There was a problem hiding this comment.
FusionLocalizer.cpp now directly includes/uses tf2_geometry_msgs (e.g., tf2::toMsg for seeding and /initialpose forwarding). Please ensure localizers/easynav_fusion_localizer/CMakeLists.txt declares tf2/tf2_geometry_msgs as direct dependencies (via find_package(...) and linking/exporting the targets), rather than relying on transitive dependencies from robot_localization.
| if (std::abs(init_x) > 1e-12 || std::abs(init_y) > 1e-12 || std::abs(init_yaw) > 1e-12) { | ||
| geometry_msgs::msg::PoseWithCovarianceStamped init_pose; | ||
| init_pose.header.stamp = node->now(); | ||
| const auto & tf_info = RTTFBuffer::getInstance()->get_tf_info(); | ||
| init_pose.header.frame_id = tf_info.map_frame; | ||
| init_pose.pose.pose.position.x = init_x; | ||
| init_pose.pose.pose.position.y = init_y; | ||
| init_pose.pose.pose.position.z = 0.0; | ||
| tf2::Quaternion q; | ||
| q.setRPY(0.0, 0.0, init_yaw); | ||
| init_pose.pose.pose.orientation = tf2::toMsg(q); | ||
| init_pose.pose.covariance.fill(0.0); | ||
| pending_init_pose_ = init_pose; | ||
| } |
There was a problem hiding this comment.
GpsLocalizer.cpp now uses std::abs(...) (e.g., for the non-zero initial pose check) but this translation unit doesn’t include <cmath>. Please include <cmath> explicitly (or otherwise ensure the correct overloads are declared) to avoid relying on transitive includes / potential build failures on different toolchains.
| #include "sensor_msgs/msg/nav_sat_fix.hpp" | ||
| #include "geometry_msgs/msg/pose_with_covariance_stamped.hpp" | ||
| #include "easynav_core/LocalizerMethodBase.hpp" | ||
| #include "tf2_ros/static_transform_broadcaster.hpp" |
There was a problem hiding this comment.
GpsLocalizer now directly depends on geometry_msgs (/initialpose message type) and tf2_geometry_msgs (tf2::toMsg), but localizers/easynav_gps_localizer/CMakeLists.txt does not declare those dependencies via find_package(...) / exported targets. Please add the missing dependencies and link/export them so this header remains consumable without relying on transitive includes.
| #include "tf2_ros/static_transform_broadcaster.hpp" | |
| #include "tf2_ros/static_transform_broadcaster.hpp" | |
| #include "tf2_geometry_msgs/tf2_geometry_msgs.hpp" |
…easynav features. Initial pose now changes initial utm pose to change robot position in the map
PR: Normalize
/initialposeacross localizers + functional tests (AMCL/GPS/Fusion)Context / motivation
Within
easynav_plugins/localizers, plugins differed in how they consumed the standard/initialposetopic (RViz-style initial pose). This made integration harder and prevented consistent validation that localizers react correctly to:/initialpose.What changed
1) Consistent
/initialposesubscription in all localizersAll plugins under
src/easynav_plugins/localizerswere reviewed and normalized so that:/initialpose(geometry_msgs/msg/PoseWithCovarianceStamped).Localizers covered:
easynav_costmap_localizereasynav_simple_localizereasynav_navmap_localizereasynav_gps_localizereasynav_fusion_localizer2) Functional tests to validate end-to-end behavior
Tests were migrated/re-structured into a consistent functional pattern (GTest) to cover:
NavState)./initialpose.Updated/added tests:
easynav_costmap_localizer/tests/*easynav_simple_localizer/tests/*easynav_navmap_localizer/tests/*easynav_gps_localizer/tests/gps_localizer_tests.cppeasynav_fusion_localizer/tests/fusion_localizer_tests.cpp3) Parameter-based initial pose seed in GPS and Fusion
To enable deterministic initialization (and make it testable), the following parameters are now read:
<plugin>.initial_pose.x<plugin>.initial_pose.y<plugin>.initial_pose.yawImplemented in:
easynav_gps_localizer(preloads the initial pose)easynav_fusion_localizer(preloads / applies the initial pose to the filter(s))4) Fix:
/initialposemust be reflected deterministically in Fusion (UKF)An issue was found where Fusion would receive
/initialpose, but the output (NavState[robot_pose]) would not reliably update due to how the UKF set-pose path was handled.robot_localization::UkfWrapper::setPoseCallback()was updated so that “set pose”:processMeasurement()if the filter was not initialized yet.This makes
/initialposeimpact Fusion’s published pose immediately and enables robust functional tests.Please, check it @midemig
Compatibility notes
Checklist
/initialpose.SubscriptionOptions(no explicit callback group)./initialposeupdates./initialposedeterministically inrobot_pose.