-
Notifications
You must be signed in to change notification settings - Fork 70
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
New EffBox task map. #624
New EffBox task map. #624
Conversation
d68ddc7
to
484bb4e
Compare
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.
Thanks, a few minor including a twice committed file above then good to go
exotations/exotica_core_task_maps/include/exotica_core_task_maps/eff_box_constraint.h
Outdated
Show resolved
Hide resolved
exotations/exotica_core_task_maps/include/exotica_core_task_maps/eff_box_constraint.h
Outdated
Show resolved
Hide resolved
exotica_examples/resources/configs/example_ik_eff_box_constraint.xml
Outdated
Show resolved
Hide resolved
exotica_examples/resources/configs/example_ik_eff_box_constraint.xml
Outdated
Show resolved
Hide resolved
{ | ||
if (phi.rows() != TaskSpaceDim()) ThrowNamed("Wrong size of phi!"); | ||
|
||
Eigen::VectorXd e; |
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.
This allocated at every iteration. This is not great. Also you could make line 40 and 41 be one. Directly operate on Phi
} | ||
} | ||
|
||
const Eigen::VectorXd EffBox::GetLowerLimit() |
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.
This is the other kind of const... I meant EffBox::GetLowerLimit() const
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.
Missing:
- Unit test
- issue to extend to cost, especially now that it's renamed
I think all should be good to merge after travis ci goes green, builds and runs successfully as expected locally. |
{ | ||
pub_markers_ = Server::Advertise<visualization_msgs::MarkerArray>("eff_box_objects", 1, true); | ||
visualization_msgs::Marker md; // delete previous markers | ||
md.action = visualization_msgs::Marker::DELETEALL; |
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.
This won't compile on older versions, make it = 3; and add a comment it corresponds to deleteall
* eff_upper -> eff_upper_ and similar for eff_lower * New functions GetLowerLimit and GetUpperLimit, both exposed to python
* EffBoxConstraint -> EffBox * Frame -> FrameWithBoxLimits
* EffBoxConstraint -> EFfBox * added const as per review * updated code to allow a different box constraint to be defined for every end-effector * new FrameWithBoxLimits initializer to allow box constraint for every end-effector * added todo * code formatting
* push_back -> emplace_back + reserve in instantiate * added assign scene function * use GetRootFrameName rather than assume world_frame
I'm not sure what the issue is with second Travis CI checks.. @wxmerkt any ideas? |
The issue was one of the four tests had a transient issue. But our pre-release tests are now timing out (build times are too long). Likely need to ask for an extension in Travis... |
[task_maps] New task map.
[core] Initializer support for Eigen::Vector2d.
[examples] New example for new task map.