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.

Sign up for GitHub

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

Enhanced documentation for composeMultiArrayMessage function #3270

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Enhanced documentation for composeMultiArrayMessage function #3270

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9d56081
Enhanced documentation for composeMultiArrayMessage function
attu0 Jan 25, 2025
447e529
Nitpicky wording correction
AndyZe Jan 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions moveit_ros/moveit_servo/src/utils/common.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,22 @@ void updateSlidingWindow(KinematicState& next_joint_state, std::deque<KinematicS
joint_cmd_rolling_window.push_back(next_joint_state);
}

/**
* @brief Composes a Float64MultiArray message for controllers requiring Float64MultiArray input.
*
* This function converts the given joint state into a Float64MultiArray message.
Comment on lines +259 to +261
Copy link
Contributor

Choose a reason for hiding this comment

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

these two sentences virtually say the same thing -- you probably can combine these into just a single line.

*
* @param servo_params Configuration parameters for the servo, including message type selection.
* @param joint_state The joint state to be converted into the message.
* @return A Float64MultiArray message containing the joint state data.
*
* Example:
* @code
* auto message = composeMultiArrayMessage(servo_params, joint_state);
* controller.publish(message);
Copy link
Contributor

@sea-bass sea-bass Jan 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It may not be obvious to most people that controller in this case is actually a ROS 2 publisher. Maybe call this variable controller_pub or something like that, or add a brief sentence explaining that the code snippet is publishing to a controller topic?

Also looking at other examples, it appears you may need to indent what is inside the @code / @endcode?

For example:

moveit2/moveit_ros/planning/planning_scene_monitor/include/moveit/planning_scene_monitor/planning_scene_monitor.hpp

Lines 607 to 610 in 524ca4f

* @code
* planning_scene_monitor::LockedPlanningSceneRO ls(planning_scene_monitor);
* moveit::core::RobotModelConstPtr model = ls->getRobotModel();
* @endcode

Which renders to: https://moveit.picknik.ai/main/api/html/classplanning__scene__monitor_1_1LockedPlanningSceneRO.html#details

* @endcode
*/

std_msgs::msg::Float64MultiArray composeMultiArrayMessage(const servo::Params& servo_params,
const KinematicState& joint_state)
{
Expand Down