RSDK-13405: Allow for motion plan requests to express a goal with leeway.#5747
RSDK-13405: Allow for motion plan requests to express a goal with leeway.#5747dgottlieb wants to merge 6 commits intoviamrobotics:mainfrom
Conversation
viambot
added
the
safe to test
dgottlieb
force-pushed
the
RSDK-13405
branch
from
7290635 to
6e34c4d
Compare
viambot
added
safe to test
dgottlieb
force-pushed
the
RSDK-13405
branch
from
6e34c4d to
db6a8e9
Compare
viambot
added
safe to test
motionplan/metrics.go
Outdated
| // these leeways describe a cloud where arriving at any destination within that cloud are considered | ||
| // equivalent. | ||
| // | ||
| // All of the leeways affect the algorithm independently. None of the leeways will be scaled based |
There was a problem hiding this comment.
I'm not sure this is useful enough to keep in here. I did at least want to provide a more rigorous description in the PR and scale back (or add more detail) as we like.
motionplan/metrics.go
Outdated
| // goal pose. Any candidate pose with an Z inside the leeway will be considered equivalent. | ||
| Z [2]float64 `json:"z"` | ||
|
|
||
| // The orientation values are unitless. As above, each of the OX, OY and OZ are broken into a |
There was a problem hiding this comment.
This was my interpretation of an analogous treatment of orientations. No idea if this is useful at all. It's not obvious to me that this is better than simply a distance.
Also (so long as orientation.viam.dev is to be trusted). Asking for a goal of OX/OY/OZ of 0,0,1 and then saying OX can be -0.2 -> 0.2 creates a problem. In the 0.2 case, the teapot tips forward (spout tipping down a bit) compared to the goal. But in the OX: -0.2 case, while the orientation vector moves just a small bit (the teapot "tips backwards"), the spout is 180d. Facing downward on the opposite side as opposed to a little upward.
Because the spout is what matters (in the case of pouring), I think a surprise theta flip would lead to bugs. I expect this is all math we can do under the hood. But then savvy users that know about the negative flipping when constructing goal poses might get hung up on how to use OX/OY/OZ leeways.
Just surfacing my thoughts/investigation on what leeway means for orientation vectors.
Availability
Quality
Performance
The above data was generated by running scenes defined in the
|
|
|
||
| // GoalCloud can describe a cloud of candidate poses that are considered equivalent to the goal | ||
| // pose for a motion plan. The default value allows no leeway in any dimension. | ||
| GoalCloud motionplan.GoalCloud `json:"goal_cloud"` |
There was a problem hiding this comment.
I think this should be on PlanRequest, not options.
Options feels low level to me.
I almost want to put this one very goal at some point, so would rather have it closer.
There was a problem hiding this comment.
I made (the easiest) change to put this on each goal object. In a way that's obviously not backwards breaking. Just so you can see.
Read: it's a new member of PoseInFrame. And I filled in lines that highlight there'd be some changes to the api repo. If you think living on PoseInFrame looks good, we're basically done, I can make/roll out the mechnical API changes.
But if you want it elsewhere, we'll need to brainstorm ways to put it in a nicer spot with an appropriate (if any) amount of backwards breaking.
motionplan/metrics.go
Outdated
|
|
||
| // Theta represents the [negative leeway, positive leeway] in an objects rotation around its | ||
| // orientation axis in the unit of degrees. | ||
| Theta [2]float64 `json:"theta"` |
There was a problem hiding this comment.
This is ok, but insufficient as we discussed today.
This one is especially bad because 0 and 360 are the same
The rest don't have that property
There was a problem hiding this comment.
I was probably too distracted/too much time has passed where I lost context here. I agree 0-360 are the same. And I expect -180 and +180 are also the same (even though I try to use them to make the wrist rotate "the right way").
Was this "ok for now but we'll need to make forward-compatible changes"? Or was this "needs a change to now to avoid a future backwards compatibility problem"?
I removed the [2]float64 in favor of a single float64, but did nothing else here.
motionplan/metrics.go
Outdated
| // a cup. There may be some freedom with respect to the exact orientation and theta of the | ||
| // gripper. But a scenario where a legal, but severe orientation leeway might work with a smaller | ||
| // difference in the candidate's theta, but not necessarily a larger one. | ||
| type GoalCloud struct { |
There was a problem hiding this comment.
i guess my big question is do we really want different min and max
i
There was a problem hiding this comment.
I've removed them -- was easier to put them in and get validation it was onerous than to omit them and get negative acknowledgment that we didn't want more fine-grained.
viambot
added
safe to test
viambot
added
safe to test
|
i like the api |
viambot
added
safe to test
3 participants
No description provided.