RobotLabLTH / skiros2

A skill-based platform for ROS v.2
Other
163 stars 20 forks source link

Fix: Enable automatic skill selection if grounding fails #72

Open matthias-mayr opened 1 year ago

matthias-mayr commented 1 year ago

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.

frvd commented 1 year ago

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

matthias-mayr commented 1 year ago

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.