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

Added joint constraint to goal regions #259

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

ell0ry
Copy link
Contributor

@ell0ry ell0ry commented Sep 13, 2021

Added methods to add joint constraints to a goal region for a MotionRequestBuilder.

@@ -37,6 +37,7 @@ core

# Compile Commands
*compile_commands.json
.cache/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this for CCLS compilation databases

* \param[in] distance The distance from the cylinder to place the regions.
* \param[in] depth The depth of boxes to create.
* \param[in] n The number of regions to create.
/** \brief Adds a set of regions to grasp a cylinder from the side. This function assumes the
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These diffs are from my clang formatter. I'm using the lab clang-format file, it just didn't like the long lines.

Copy link
Contributor

Choose a reason for hiding this comment

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

As long as it passes the check it's fine.

Copy link
Contributor

@zkingston zkingston left a comment

Choose a reason for hiding this comment

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

Small API changes but otherwise good.

* \param[in] position The joint value to constrain to.
* \param[in] tolerances A 2D vector of the upper bound and lower bound from \a position.
*/
void addJointConstraintToGoal(int goal_idx, const std::string &joint_name, float position,
Copy link
Contributor

Choose a reason for hiding this comment

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

Use std::size_t for all counting variables, and use double for all numbers.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd also move the goal index to the last argument and make it optional, defaulting to 0 for ease of use. Similarly, I'd add a default value to the tolerances of +/- epsilon.

* \param[in] goal_idx The index of the goal region to add this joint constraint to.
* \param[in] joint_constraint The MoveIt! joint constraint to add.
*/
void addJointConstraintToGoal(int goal_idx, moveit_msgs::JointConstraint joint_constraint);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the message be const &?

* \param[in] distance The distance from the cylinder to place the regions.
* \param[in] depth The depth of boxes to create.
* \param[in] n The number of regions to create.
/** \brief Adds a set of regions to grasp a cylinder from the side. This function assumes the
Copy link
Contributor

Choose a reason for hiding this comment

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

As long as it passes the check it's fine.

* \param[in] tolerances A vector of upper bound and lower bound offsets from position.
* \return The joint constraint as a MoveIt message.
*/
moveit_msgs::JointConstraint getJointConstraint(const std::string &joint_name, float position,
Copy link
Contributor

Choose a reason for hiding this comment

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

double instead of float

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