Plan-repair (3/3): goal_manager_node with private topics and abstracted flow#29
Plan-repair (3/3): goal_manager_node with private topics and abstracted flow#29JY-HIM4U wants to merge 2 commits into
Conversation
…ed flow goal_manager improvements: - Private topics (~/commanded_goal, ~/planner_goal, ~/plan_visited_pois, ~/executor_pause, ~/executor_resume). Concrete wiring is done by roslaunch remap, not by ROS parameters. - commanded_goal_cb structured as pause -> decide -> resume_or_replan -> log; the domain-specific "is the new goal already covered" check is isolated in _goal_already_satisfied, making the extension points visible.
GoldenZephyr
left a comment
GoldenZephyr
left a comment
There was a problem hiding this comment.
Reasonable overall, but see my comment about extracting information from the goal string.
| _VISIT_PREDICATE_RE = re.compile(r"\(visited-(?:place|object|poi)\s+([^\s\)]+)\)") | ||
|
|
||
|
|
||
| def extract_visit_targets(pddl_goal: str) -> Set[str]: |
There was a problem hiding this comment.
My understanding is that this function is being used to detect which places the plan needs to visit, to check if we need to replan. I don't think this regex is sufficient for that. There are two problems: 1) The goal string might have something like (not (visited-place p1)) , so p1 is in the goal string but shouldn't be considered a place we want to visit, 2) The goal can have disjunctions, so if we have (or (visited p1) (visited p2)) we can't assume that both p1 and p2 should be visited. The goal string is a nested tree structure so we shouldn't expect to be able to parse it with a regular expression.
There was a problem hiding this comment.
Thanks for pointing out this issue. I edited!
2 participants
goal_manager improvements:
Private topics (~/commanded_goal, ~/planner_goal, ~/plan_visited_pois, ~/executor_pause, ~/executor_resume).
commanded_goal_cb structured as pause -> decide -> resume_or_replan -> log; the domain-specific "is the new goal already covered" check is isolated in _goal_already_satisfied, making the extension points visible.