[ros2]: Sync with master branch #367
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Keep the previous logic around for Generator stages. Note that this only makes sense for *pure* Generators and not for MonitoringGenerator, because for the latter we would expect monitored solutions to be passed individually (similar to pruning).
give an elaborate reason for an empty overload that doesn't call the parent.
Note that while this ensures other stages outside the Fallbacks container can compute as well, it does not solve the problem internally. A new incoming state will only ever be considered once the current stage cannot compute any more. We have no way of telling a child to compute for *a specific state* for now. So once we copied a state to its interface we have to let it compute until all possibilities are exhausted to detect whether or not it could generate a solution for it. If we wouldn't do so, there were no way of knowing when to fall back to the next child as long as the stage can still compute on *any* copied solution.
Setting up a demo for
Fallbacks({CartesianPath,PTP,RRTConnect})
I found the logic did not work as expected yet.
- process last job spec as well
- ignore failures when looking for a solution
- add more debug output
Both, failed and pruned states might get re-enabled later! This also required rework (simplification) of the sorting function for pending pairs.
- Centrally distinguish between have owner() or not in InterfaceState::updatePriority() - Have a separate updateStatus() method to just update the pruning status - Split Interface::updatePriority() into a method taking the InterfaceState* and one taking an Interface::iterator (for efficiency) - Early return in container.cpp's updateStatePrios()
- Switch directions: FORWARD <-> BACKWARD to make the function reusable for status propagation. - We need to ignore the source state when looking for opposite states of the target state. Thus add both, source and target state arguments.
... to avoid explicit template initialization
... to better indicate that such a state can be immediately re-enabled.
This also requires to drop the assertion in SerialContainer::onNewSolution() that new solutions will have enabled start+end states (a CONNECT stage's solution might not).
As only the InterfaceState* variant is actually called, we can drop the splitting introduced for performance reasons in 29d1e44
Not only propagate updates along solution paths, but also bridge the gap of a `Connecting` stage. - If a state becomes enabled, re-enable opposite `ARMED` states as well. - If a state becomes pruned, also prune opposite states if they don't have alternatives. - Make sure that we don't run into a recursive update loop by disabling notify() callbacks.
Also remove unused pushInterface(dir)
to allow propagating status updates only if the STATUS actually changed.
Moving to next child generator only in compute() requires an extra call to canCompute() to notice the failure of the next generator(s).
... factoring out functionality shared between FallbacksPrivateGenerator and FallbacksPrivatePropagator to switch to next child in nextJob().
rename method to emphasize that state updates are propagated to all children
Implement Fallbacks behavior for children of type Connecting. All other connect-like children are currently infeasible to handle, because we cannot forward a single job, i.e. a pair (from, to) to the next child, but only individual states. However, passing states, will cause creation of undesired state pairs as jobs in subsequent children.
Check collisions for FixedState's scene and report failure if needed. Optionally, disable the check via the property ignore_collisions=true.
If two Connect stages are sequenced, both sides can become ARMED. However, that means that the wave of PRUNED status updates, shouldn't overwrite a present ARMED state. Added unit test.
Codecov Report
@@ Coverage Diff @@
## ros2 #367 +/- ##
==========================================
+ Coverage 38.85% 42.03% +3.19%
==========================================
Files 78 78
Lines 7520 7767 +247
==========================================
+ Hits 2921 3264 +343
+ Misses 4599 4503 -96
Continue to review full report at Codecov.
|
henningkayser
approved these changes
May 31, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please fast-forward merge