Skip to content
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

rcll-central: moves robot out of the way when goal exceeds retry limit #707

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

kalrasamridhi7
Copy link
Contributor

No description provided.

@kalrasamridhi7
Copy link
Contributor Author

TODO: re-create retracted move-out-of-way goals after their completion

Copy link
Contributor

@TarikViehmann TarikViehmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job, i have a few comments

(assert (goal-retry-wait-timer (goal-id ?goal-id) (timeout-duration ?*GOAL-RETRY-TIMEOUT*) (start-time ?now)))
(assert (wm-fact (key monitoring goal-in-retry-wait-period args? goal-id ?goal-id robot ?robot)))
(modify ?gm (retries 0))
(do-for-fact ((?robot wm-fact))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the do-for fact?
looks to me as if you can just put it in the precondition.

=>
(retract ?gt ?wf)
(delayed-do-for-all-facts ((?counter wm-fact))
(and (wm-key-prefix ?counter:key (create$ monitoring goal retry robot counter))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, this should e a lhs pattern not a rhs fact query

to unblock the machine
"
(declare (salience ?*MONITORING-SALIENCE*))
(wm-fact (key monitoring goal retry robot counter args? goal ?goal-id r ?robot) (value ?counter&:(> ?counter ?*GOAL-RETRY-MAX*)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should define a clear state hen this should happen, e.g., during executability checks for the robot that is exceeding the retry limit, or during goal evaluation.
Otherwise we might break some implicit assumptions if it defines at some arbitrary point during the execution cycle.

@@ -758,6 +758,9 @@
(mode FORMULATED) (parent ?pa-id&~nil)
(priority ?p&:(> ?p 0))
)
(goal-meta (goal-id ?goal-id) (assigned-to ?robot))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This rule needs appropriate priority e.g., ?*SALIENCE-GOAL-EXECUTABLE-CHECK*, because otherwise it will be ignored on selection.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then you can also simplify this rule and remove the ?or, essentially just updating the rule during executability check

@@ -551,6 +551,13 @@
(modify ?gm (root-for-order nil))
)

;(defrule force-fail-goal
; (declare (salience (+ 2 ?*SALIENCE-GOAL-FORMULATE*)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this

;if a move-out-of-the-way has urgent high priority, reset it on goal completion
(do-for-fact ((?f wm-fact))
(wm-key-prefix ?f:key (create$ monitoring move-out-of-way high-prio))
(do-for-all-facts ((?og goal) (?ogm goal-meta))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we actually need this?
The goal is not assigned to a robot anymore after this rule, this should trigger the priority reset already (rule goal-production-change-priority-move-out-of-way)
Hence i would remove the innder do-for-all-facts

@TarikViehmann TarikViehmann force-pushed the skalra/goal-retry-counter-for-robot branch from c6d6e49 to 98ba17b Compare July 20, 2024 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants