Code format: planner #167
Code format: planner #167
Conversation
| void interpolate( | ||
| const ::ompl::base::State* _from, | ||
| const ::ompl::base::State* _to, | ||
| const double _t, |
There was a problem hiding this comment.
Unnecessary const. It's a bummer that clang-format can't fix this. :(
| const double& _maxDistBtwValidityChecks); | ||
| MotionValidator( | ||
| const ::ompl::base::SpaceInformationPtr& _si, | ||
| const double& _maxDistBtwValidityChecks); |
There was a problem hiding this comment.
Nit: There's no good reason to take a double by const &.
| @@ -79,11 +83,12 @@ trajectory::InterpolatedPtr planOMPL( | |||
| /// valid bounds defined on the StateSpace | |||
| /// \param _maxPlanTime The maximum time to allow the planner to search for a | |||
| /// solution | |||
| /// \param _maxDistanceBtwValidityChecks The maximum distance (under dmetric) between | |||
| /// \param _maxDistanceBtwValidityChecks The maximum distance (under dmetric) | |||
| /// between | |||
| /// validity checking two successive points on a tree extension | |||
| @@ -121,30 +129,35 @@ trajectory::InterpolatedPtr planOMPL( | |||
| /// solution | |||
| /// \param _maxExtensionDistance The maximum distance to extend the tree on | |||
| /// a single extension | |||
| /// \param _maxDistanceBtwProjections The maximum distance (under dmetric) between | |||
| /// \param _maxDistanceBtwProjections The maximum distance (under dmetric) | |||
| /// between | |||
| /// projecting and validity checking two successive points on a tree extension | |||
src/planner/ompl/CRRT.cpp
Outdated
| Planner::declareParam<double>( | ||
| "goal_bias", this, &CRRT::setGoalBias, &CRRT::getGoalBias, "0.:.05:1."); | ||
| Planner::declareParam<double>( | ||
| "proj_res", |
There was a problem hiding this comment.
Nit: Unrelated to this PR, rename this to projection_resolution. We're not charged by the byte. 😬
src/planner/ompl/CRRT.cpp
Outdated
| mStartTree->list(motions); | ||
| for (unsigned int i = 0; i < motions.size(); ++i) { | ||
| if (motions[i]->state) { | ||
| for (unsigned int i = 0; i < motions.size(); ++i) |
There was a problem hiding this comment.
Nit: size_t instead of unsigned int?
| goal, | ||
| false, | ||
| bestdist, | ||
| foundgoal); |
There was a problem hiding this comment.
I know. I strongly prefer placing either all be on the same line or will have one line each for a function declaration's and definition's parameters. However, for a function call's arguments, I still debate myself which style is better. I started to slightly lean towards not to so (allowing multi items on a line) though. After looking at the diff, I prefer taking one line per item (when single line is impossible), and I (want to) believe this function takes too many parameters. 👿
Here is the diff.
There was a problem hiding this comment.
I deeply understand your point. Well, we have // clang-format on [off] option at least. 😓
src/planner/ompl/CRRT.cpp
Outdated
| ::ompl::geometric::PathGeometric *path = | ||
| new ::ompl::geometric::PathGeometric(si_); | ||
| ::ompl::geometric::PathGeometric* path | ||
| = new ::ompl::geometric::PathGeometric(si_); |
There was a problem hiding this comment.
This will leak memory if constructing the path raises an exception. We should create the PathPtr (or - if necessary - a shared_ptr<PathGeometric> here, instead of several lines below.
src/planner/ompl/CRRTConnect.cpp
Outdated
| mGoalTree->list(motions); | ||
| for (unsigned int i = 0; i < motions.size(); ++i) { | ||
| for (unsigned int i = 0; i < motions.size(); ++i) |
| void GeometricStateSpace::interpolate( | ||
| const ::ompl::base::State* _from, | ||
| const ::ompl::base::State* _to, | ||
| const double _t, |
| } | ||
| } | ||
| } | ||
| } // ompl |
No description provided.