-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Panav/feat/update mppi visualization #5975
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
Changes from all commits
f996cd4
52a9346
9ff3b84
3bdcd73
377f8f6
e48334c
4020e81
496ac89
4983bcb
fcefafb
5e8c162
97e3aee
c6a41d2
48592e1
06e0211
e74ff23
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -104,28 +104,14 @@ class MPPIController : public nav2_core::Controller | |
| void setSpeedLimit(const double & speed_limit, const bool & percentage) override; | ||
|
|
||
| protected: | ||
| /** | ||
| * @brief Visualize trajectories | ||
| * @param cmd_stamp Command stamp | ||
| * @param optimal_trajectory Optimal trajectory, if already computed | ||
| */ | ||
| void visualize( | ||
| const builtin_interfaces::msg::Time & cmd_stamp, | ||
| const Eigen::ArrayXXf & optimal_trajectory); | ||
|
|
||
| std::string name_; | ||
| nav2::LifecycleNode::WeakPtr parent_; | ||
| rclcpp::Logger logger_{rclcpp::get_logger("MPPIController")}; | ||
| std::shared_ptr<nav2_costmap_2d::Costmap2DROS> costmap_ros_; | ||
| std::shared_ptr<tf2_ros::Buffer> tf_buffer_; | ||
| nav2::Publisher<nav2_msgs::msg::Trajectory>::SharedPtr opt_traj_pub_; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add this back, this is not debug. |
||
|
|
||
| std::unique_ptr<ParametersHandler> parameters_handler_; | ||
| Optimizer optimizer_; | ||
| TrajectoryVisualizer trajectory_visualizer_; | ||
|
|
||
| bool visualize_; | ||
| bool publish_optimal_trajectory_; | ||
| }; | ||
|
|
||
| } // namespace nav2_mppi_controller | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,11 +15,15 @@ | |
| #ifndef NAV2_MPPI_CONTROLLER__CRITIC_FUNCTION_HPP_ | ||
| #define NAV2_MPPI_CONTROLLER__CRITIC_FUNCTION_HPP_ | ||
|
|
||
| #include <string> | ||
| #include <memory> | ||
| #include <string> | ||
| #include <utility> | ||
|
|
||
| #include "rclcpp_lifecycle/lifecycle_node.hpp" | ||
| #include "nav2_costmap_2d/costmap_2d_ros.hpp" | ||
| #include "geometry_msgs/msg/pose_stamped.hpp" | ||
| #include "tf2_geometry_msgs/tf2_geometry_msgs.hpp" | ||
| #include "nav2_ros_common/publisher.hpp" | ||
|
|
||
| #include "nav2_mppi_controller/tools/parameters_handler.hpp" | ||
| #include "nav2_mppi_controller/critic_data.hpp" | ||
|
|
@@ -70,7 +74,9 @@ class CriticFunction | |
| ParametersHandler * param_handler) | ||
| { | ||
| parent_ = parent; | ||
| logger_ = parent_.lock()->get_logger(); | ||
| auto node = parent_.lock(); | ||
| logger_ = node->get_logger(); | ||
| clock_ = node->get_clock(); | ||
| name_ = name; | ||
| parent_name_ = parent_name; | ||
| costmap_ros_ = costmap_ros; | ||
|
|
@@ -103,6 +109,55 @@ class CriticFunction | |
| } | ||
|
|
||
| protected: | ||
| /** | ||
| * @brief Initialize a debug pose publisher for this critic | ||
| * @param topic Topic name suffix (e.g. "furthest_reached_path_point") | ||
| */ | ||
| void initDebugPosePublisher(const std::string & topic) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tonynajjar what do you think? I don't love the idea of each critic having this internally, rather the responsibility of the manager / visualizer? |
||
| { | ||
| auto getParam = parameters_handler_->getParamGetter(name_); | ||
| getParam(debug_visualizations_, "debug_visualizations", false); | ||
| if (debug_visualizations_) { | ||
| auto node = parent_.lock(); | ||
| if (node) { | ||
| // Extract short critic name from full namespaced name | ||
| // e.g. "ctrl.PathAngleCritic" -> "PathAngleCritic" | ||
| std::string short_name = name_; | ||
| auto pos = short_name.rfind('.'); | ||
| if (pos != std::string::npos) { | ||
| short_name = short_name.substr(pos + 1); | ||
| } | ||
| debug_pose_pub_ = node->create_publisher<geometry_msgs::msg::PoseStamped>( | ||
| short_name + "/" + topic, 1); | ||
| debug_pose_pub_->on_activate(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @brief Publish a debug pose if visualization is enabled and there are subscribers | ||
| * @param x X position | ||
| * @param y Y position | ||
| * @param yaw Yaw orientation | ||
| */ | ||
| void publishDebugPose(float x, float y, float yaw) | ||
| { | ||
| if (debug_visualizations_ && debug_pose_pub_ && | ||
| debug_pose_pub_->get_subscription_count() > 0) | ||
| { | ||
| auto msg = std::make_unique<geometry_msgs::msg::PoseStamped>(); | ||
| msg->header.frame_id = costmap_ros_->getGlobalFrameID(); | ||
| msg->header.stamp = clock_->now(); | ||
| msg->pose.position.x = x; | ||
| msg->pose.position.y = y; | ||
| msg->pose.position.z = 0.0; | ||
| tf2::Quaternion quat; | ||
| quat.setRPY(0.0, 0.0, yaw); | ||
| msg->pose.orientation = tf2::toMsg(quat); | ||
|
Comment on lines
+154
to
+156
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nav2 utils has a function for this |
||
| debug_pose_pub_->publish(std::move(msg)); | ||
| } | ||
| } | ||
|
|
||
| bool enabled_; | ||
| std::string name_, parent_name_; | ||
| nav2::LifecycleNode::WeakPtr parent_; | ||
|
|
@@ -111,6 +166,10 @@ class CriticFunction | |
|
|
||
| ParametersHandler * parameters_handler_; | ||
| rclcpp::Logger logger_{rclcpp::get_logger("MPPIController")}; | ||
| rclcpp::Clock::SharedPtr clock_; | ||
|
|
||
| bool debug_visualizations_{false}; | ||
| nav2::Publisher<geometry_msgs::msg::PoseStamped>::SharedPtr debug_pose_pub_; | ||
| }; | ||
|
|
||
| } // namespace mppi::critics | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,8 @@ | |
| #include <string> | ||
| #include <memory> | ||
| #include <tuple> | ||
| #include <utility> | ||
| #include <vector> | ||
|
|
||
| #include "rclcpp_lifecycle/lifecycle_node.hpp" | ||
|
|
||
|
|
@@ -118,6 +120,28 @@ class Optimizer | |
| */ | ||
| const models::ControlSequence & getOptimalControlSequence(); | ||
|
|
||
| /** | ||
| * @brief Get the costs for trajectories for visualization | ||
| * @return Costs array | ||
| */ | ||
| const Eigen::ArrayXf & getCosts() const | ||
| { | ||
| return costs_; | ||
| } | ||
|
|
||
| /** | ||
| * @brief Get the per-critic costs for trajectories for visualization | ||
| * @return Vector of (critic_name, costs) pairs | ||
| */ | ||
| const std::vector<std::pair<std::string, Eigen::ArrayXf>> & getCriticCosts() const | ||
| { | ||
| if (critics_data_.individual_critics_cost) { | ||
| return *critics_data_.individual_critics_cost; | ||
| } | ||
| static const std::vector<std::pair<std::string, Eigen::ArrayXf>> empty; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why static? |
||
| return empty; | ||
| } | ||
|
|
||
| /** | ||
| * @brief Set the maximum speed based on the speed limits callback | ||
| * @param speed_limit Limit of the speed for use | ||
|
|
@@ -290,8 +314,8 @@ class Optimizer | |
|
|
||
| CriticData critics_data_ = { | ||
| state_, generated_trajectories_, path_, goal_, | ||
| costs_, settings_.model_dt, false, nullptr, nullptr, | ||
| std::nullopt, std::nullopt}; /// Caution, keep references | ||
| costs_, std::nullopt, settings_.model_dt, false, nullptr, nullptr, | ||
| std::nullopt, std::nullopt, std::nullopt}; /// Caution, keep references | ||
|
|
||
| rclcpp::Logger logger_{rclcpp::get_logger("MPPIController")}; | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think as I mentioned to @tonynajjar I don't want there to be a ton of these
publish_XYZdebug parameters. I think there should be1. Visualize / debug or not. You then publish all or nothign. Its just too much to break out individually and you can always just check if the publisher has any subscriptions to pass population/publication if no one is listening to that one.As such, the namespace, each of the
publish_can be removed.publish_optimal_path/publish_optimal_trajectory_msg(which ever existed before, it looks like its been renamed or something) is not a debug topic, that should be removed back to the global namespace