skiros2 icon indicating copy to clipboard operation
skiros2 copied to clipboard

Fix: Enable automatic skill selection if grounding fails

Open matthias-mayr opened this issue 1 year ago • 2 comments

Previously the implementation relied on the "label" property. However, this property is set to the concrete instance, even if the compound skill did not specify an instance.

To explain this a bit more: I have a use-case in which I have a skill that runs in a loop for different work pieces. In one part of the procedure, a skill needs to be automatically chosen depending on preconditions on the material properties. So it is used without specifying an implementation:

self.skill("MyFlexibleSkill", ""),

In the first iteration, it instantiates the correct implementation, but at a later point we might encounter a part that needs the other implementation & the grounding fails here: https://github.com/RVMI/skiros2/blob/04353faa3f405ab970eb49915ce114354f9f5f94/skiros2_skill/src/skiros2_skill/core/skill_utils.py#L265-L272

This is fine, but when entering tryOther, a check is performed whether the label is set to be a specific skill:

https://github.com/RVMI/skiros2/blob/04353faa3f405ab970eb49915ce114354f9f5f94/skiros2_skill/src/skiros2_skill/core/skill_utils.py#L235-L240

Unfortunately this label is set to the currently set instance:

https://github.com/RVMI/skiros2/blob/04353faa3f405ab970eb49915ce114354f9f5f94/skiros2_skill/src/skiros2_skill/core/skill.py#L424-L430

Besides the entertaining comment, the _label property of the skill is still the original empty string and is not affected by the instance that was set before So this PR proposes to use this instead.

matthias-mayr avatar Jun 30 '23 12:06 matthias-mayr

I would rather fix it to check cut the link to an instance (has_instance) as a first thing in the tryOther function

frvd avatar Jul 03 '23 14:07 frvd

That was also my first attempt, but it lead to some error. I can check what at it and look a bit more at it.

matthias-mayr avatar Jul 03 '23 15:07 matthias-mayr