RSDK-8576 spline constraints for motion #4379
Conversation
viambot
added
the
safe to test
| @@ -54,6 +54,95 @@ func newPlanManager( | |||
| return &planManager{planner: p, frame: frame, useTPspace: len(frame.PTGSolvers()) > 0}, nil | |||
| } | |||
|
|
|||
| // PlanMultiWaypoint plans a motion through multiple waypoints, using identical constraints for each | |||
| // Unlike PlanSingleWaypoint, this does not break up individual. | |||
| func (pm *planManager) PlanMultiWaypoint(ctx context.Context, request *PlanRequest, goals []spatialmath.Pose) (Plan, error) { | |||
There was a problem hiding this comment.
Why not just change the PlanRequest destination to accept a slice? Its confusing to me how the list of goals would interact with the request destination
There was a problem hiding this comment.
Since the features we are adding via DoCommand and extra are quick shortest-path fixes rather than fully fleshed-out updates, I'd prefer not to make major structural changes to public data structures until the feature is more formalized.
I may update this as part of RSDK-8835 but prefer not to do so now.
There was a problem hiding this comment.
Ok I'm good with this then, but just want to flag this as something that's suboptimal that we should go back and fix later on once we figure out more sustainable data structures.
Maybe you could make a ticket for removing this function?
There was a problem hiding this comment.
Made RSDK-8850 as an umbrella for all of this.
| // It returns an error if the geometry type is unsupported or if points cannot be computed. | ||
| // The points returned are in order, in frame of the capsule's parent, and follow the right hand rule around the plane normal. | ||
| func CapsuleIntersectionWithPlane(g Geometry, planeNormal, planePoint r3.Vector, numPoints int) ([]r3.Vector, error) { | ||
| c, ok := g.(*capsule) |
There was a problem hiding this comment.
[nit] can use utils.AssertType
There was a problem hiding this comment.
Is there really a benefit to this? It's the same number of LoC, but now with a function call abstraction.
There was a problem hiding this comment.
I like it personally since it cuts down on needing to write similar errors in a ton of places. I've been trying to promote it where applicable for better uniformity.
The only knock on it I see is the overhead of an additional function call?
There was a problem hiding this comment.
I'm less worried about the computational overhead of the function call, and more worried about the overhead from a code readability standpoint. Instead of doing the thing in one line right there, we abstract it away in a function call to another package which, if there is unexpected behavior going on, requires way more effort to track down and check. It's a higher mental load to ensure code correctness, without a line count reduction to make up for it.
viambot
added
safe to test
Availability
Quality
Performance
The above data was generated by running scenes defined in the
|
This PR adds complex constraints for motion, allowing a series of poses to be passed such that splines, complex curves, or any other surface describable by the user can be followed at arbitrary precision.