Get hashing for inverted lookups, but incur
structural overhead.
Whether this is worth it depends on the number of mapped interface states
and the number of pruning/reactivation requests.
Start and End are already used for an entirely different concept,
so if anyone ever wants to read this code, we should use new terms instead.
Because the source state is the disabled state that *failed* to extend,
triggering the whole subtree to be disabled, I went for the new terms
DISABLED and DISABLED_FAILED.
The key to pruning in the Connecting stage was the following:
- Don't remove states during pruning, but only disable them.
They might become re-enabled due to further input.
- Distinguish START and END sides of a disabled solution tree to break their symmetry.
The START side from where we started disabling, can be re-enabled by a new partner state in
Connecting, the END side must not. This was important as, otherwise, the states would simply
get re-enabled immediately. The END side only gets re-enabled if the START side actually
connects the whole solution branch.
If a stage fails to find a solution, this often implies that further planning
on the open end(s) of connected InterfaceStates is not needed anymore.
Thus the InterfaceStates along all connected solution paths will be marked as disabled.
They are not removed from the pending state lists though, because they might get
reactivated by solutions found in future.
To this end, we introduced the method ContainerBase::onNewFailure().
To allow pruning, we need to enable and disable InterfaceStates to be considered for further
planning. In the past, we just indicated the disabled status with infinite costs.
However, because we might need to re-enable states (with previous state),
we need to separate these two concepts.
This is somewhat cumbersome because of the additional internal/external
layer introduced through the container.
But it's still better than leaving this unverified.
We do use ROS in the background. But there is no need for a public export dependency on it.
This patch also resolves the following catkin_lint issue:
moveit_task_constructor_core: CMakeLists.txt(17): error: package 'roscpp' must be in CATKIN_DEPENDS in catkin_package()
* unify usage of pimpl()
* fix StatePair constructors
* improve/add comments
* test_container: reset MOCK_ID for each test to facilitate identification of stages
These methods were introduced to temporarily set meaningful states for cost calculation
w/o connecting the solution to these temporary states (to solve a chicken-egg problem).
This commit provides TmpInterfaceStateProvider as an alternative approach to this problem.
This essentially reverts 53c0964618.
requested in review.
Without support for custom aggregators, which we dropped again
after finding more flaws with it, I agree that this is the nicer solution.
On the downside, it converts the interfaces from copyable objects
to another round of shared_ptrs.
I added shortcuts for lambda costs to keep support for
`stage->setCostTerm([](auto&& s){ return 42; })`
without the additional
`stage->setCostTerm(LambdaCostTerm{ [](auto&& s){ return 42; } } )`
This reverts commit dc7ce9bdfac97eb468f5a850adcb27cc118b5fd7.
It turns out multiple places in SerialContainer's cost inference
expect 0.0 as the neutral element (which is why std::min and multiply did not work).
While these additional issues can be fixed, it would make the interface much less elegant.
We should consider adding it back if an actual use-case is there to discuss.
In theory this can be done with a PassThrough with a modifying transform,
but this specific mapping is intrinsic to the definition of Clearance as a cost.