RSDK-8542 expose plans generated from builtin motion service without executing them

[raybjork]

This PR enables us to plan and execute separately using motion/builtin.DoCommand(). The major changes here involve

• grouping the arguments for Move into a single struct, similar to what we've done with MoveOnGlobe, etc. This lets us conveniently reuse this struct for a few different endpoints
• implement DoCommand with two "functions" DoPlan, DoExecute

[github-actions]

Warning your change may break code samples. If your change modifies any of the following functions please contact @viamrobotics/fleet-management. Thanks!

component	function
base	IsMoving
board	GPIOPinByName
camera	Properties
encoder	Properties
motor	IsMoving
sensor	Readings
servo	Position
arm	EndPosition
audio	MediaProperties
gantry	Lengths
gripper	IsMoving
input_controller	Controls
movement_sensor	LinearAcceleration
power_sensor	Power
pose_tracker	Poses
motion	GetPose
vision	GetProperties

[raybjork]

Previously we were accepting pointers for all of the above, but this gets a little weird when you put it into a struct since the default if unset becomes a nil. Technically keeping them as pointers wouldnt be a regression because it was always possible to pass nils into the function, so I could see the argument for minimizing change by going back to making them pointers.

[biotinker]

IMO pointers are better.

Imagine the case where a Destination is not provided. If this is a pointer, then as you say, it is nil; easy to check. If it is a PoseInFrame, then an uninitialized Destination is a real PoseInFrame with a nil underlying Pose, and an empty string for parent frame. This is much more difficult to check for, and for some structs may have overlap with actual valid, meaningful inputs.

[raybjork]

Cool I'll revert back to this then. I think it will make the code easier to read too. I take it based on your lack of other comments that you're cool with the structifying and the general direction of the PR?

[biotinker]

As far as it pertains to Move yes.

I think we'll want the DoCommand API to be much more fully featured than what you have wireframed.

[raybjork]

Yeah I agree. Do you have ideas of what you want to see in it now? I will do my best to include

[biotinker]

We will need things like being able to request motion from motionplan whose goal is a joint configuration, rather than a pose.

We will need to be able to request a motion that starts from an arbitrary set of joint positions, rather than the current ones.

[raybjork]

Oh right thats a good callout thanks

[raybjork]

I didn't add tests for these functions but can if we think thats a good idea

[nfranczak]

🚀

[biotinker]

We should probably generally validate all contents of the MoveRequest

[raybjork]

We are already, the only components of the MoveReq that are not allowed to be nil are the Destination and the componentName which are already being checked

[biotinker]

The way this is done is fine for this PR/POC, but it is increasingly worrying to me that we have such sharply diverging paths between this and how MoveOnMap/Globe are done, especially considering that the primitives are identical for all of them.

[raybjork]

Yeah I agree we've built up a lot of tech debt here. I suspect a long term solution involves decimating this service into multiple motion models. Otherwise we would need to abstract common interfaces to work across both stacks

[biotinker]

Otherwise we would need to abstract common interfaces to work across both stacks

I think this is the way. We've got a pretty solid system with the motion state package, we just need to make the update to its API to plug in to the different bits to get the functionality we want.

[biotinker]

LGTM for a prototype

[github-actions]

Availability

Scene #	viamrobotics:main	raybjork:rsdk-8542	Percent Improvement	Health
1	100%	100%	0%	➖
2	100%	100%	0%	➖
3	100%	100%	0%	➖
4	100%	100%	0%	➖
5	80%	80%	0%	➖
6	50%	50%	0%	➖
7	10%	10%	0%	➖
8	100%	100%	0%	➖
9	90%	100%	11%	✅
10	100%	100%	0%	➖
11	100%	100%	0%	➖
12	100%	100%	0%	➖
13	100%	100%	0%	➖
14	100%	100%	0%	➖
15	100%	100%	0%	➖
16	90%	90%	0%	➖
17	100%	100%	0%	➖
18	70%	70%	0%	➖

Quality

Scene #	viamrobotics:main	raybjork:rsdk-8542	Percent Improvement	Probability of Improvement	Health
1	1.31±0.00	1.31±0.00	-0%	50%	➖
2	0.91±0.00	0.91±0.00	-0%	50%	➖
3	2.44±0.01	2.44±0.01	-0%	50%	➖
4	4.86±0.92	4.44±1.24	9%	61%	➖
5	16.93±11.62	17.08±11.97	-1%	50%	➖
6	10.71±3.45	12.09±4.17	-13%	40%	➖
7	2.49±0.00	2.49±0.00	-0%	50%	➖
8	5.58±1.40	5.58±1.39	-0%	50%	➖
9	4.52±0.14	5.51±3.13	-22%	38%	➖
10	4.13±0.34	4.13±0.34	-0%	50%	➖
11	3.13±0.00	3.13±0.00	-0%	50%	➖
12	3.91±0.85	3.91±0.85	0%	50%	➖
13	904.77±13.78	904.77±13.78	-0%	50%	➖
14	2035.66±601.28	2035.67±601.28	-0%	50%	➖
15	50790.10±9415.58	50790.10±9415.58	-0%	50%	➖
16	59103.42±7272.96	59103.42±7272.96	-0%	50%	➖
17	15936.40±3302.53	15936.40±3302.53	-0%	50%	➖
18	119158.62±15173.52	118540.90±14907.49	1%	51%	➖

Performance

Scene #	viamrobotics:main	raybjork:rsdk-8542	Percent Improvement	Probability of Improvement	Health
1	0.10±0.01	0.09±0.00	2%	60%	➖
2	0.12±0.01	0.12±0.01	-6%	33%	➖
3	0.96±0.42	1.00±0.43	-5%	47%	➖
4	2.08±0.15	2.04±0.13	2%	57%	➖
5	2.57±0.62	2.51±0.58	2%	53%	➖
6	2.57±0.61	2.49±0.58	3%	54%	➖
7	2.26±0.00	2.31±0.00	-2%	0%	❌
8	0.30±0.11	0.32±0.11	-4%	47%	➖
9	5.00±0.14	4.97±0.13	1%	56%	➖
10	0.15±0.03	0.14±0.03	4%	56%	➖
11	0.11±0.01	0.11±0.01	-4%	30%	➖
12	0.13±0.01	0.13±0.01	-2%	39%	➖
13	0.08±0.01	0.08±0.01	-1%	47%	➖
14	0.84±0.30	0.82±0.29	2%	52%	➖
15	1.16±0.18	1.14±0.17	1%	53%	➖
16	2.57±0.82	2.65±0.85	-3%	47%	➖
17	1.35±0.14	1.35±0.15	-0%	50%	➖
18	4.67±0.77	4.69±0.73	-0%	49%	➖

The above data was generated by running scenes defined in the motion-testing repository
The SHA1 for viamrobotics:main is: 900738751a11aa6be03dd90a3932f38e1c35f187
The SHA1 for raybjork:rsdk-8542 is: 900738751a11aa6be03dd90a3932f38e1c35f187

• 10 samples were taken for each scene
• A timeout of 5.0 seconds was imposed for each trial