RSDK-10371 provide more helpful error messages when IK fails to produce a solution #4904
Conversation
viambot
added
the
safe to test
| // If the returned bool is true, the constraint is satisfied and the segment is valid. | ||
| type SegmentConstraint func(*ik.Segment) bool | ||
| // If the returned error is nil, the constraint is satisfied and the segment is valid. | ||
| type SegmentConstraint func(*ik.Segment) error |
There was a problem hiding this comment.
Weren't we going to remove these? Since we're making a breaking change anyway it's a great time
There was a problem hiding this comment.
That change takes this from a couple hours worth of change to a lot more than that. Its going to take me some time to ramp on the TP space stuff to not mess that change up
There was a problem hiding this comment.
What are the chances we're going to do more 2d base stuff in the next ~year?
If that's very low, then sure, I agree.
If it's higher, then now/this change might be a good time to ramp you up before I go on leave
There was a problem hiding this comment.
How about this. I will timebox this change to no more than a few hours and if I cant do it then we will merge this?
raybjork
changed the title
viambot
added
safe to test
| } | ||
| cs := cg.collisions(collisionBufferMM) | ||
| if len(cs) != 0 { | ||
| // we could choose to amalgamate all the collisions into one error but its probably saner not to and choose just the first |
There was a problem hiding this comment.
If we're not combining all, could we sort so that for a given collision scenario you always get the same pair back?
There was a problem hiding this comment.
This is going to run a fair number of times considering it will be hit every time an iteration fails for any reason. I thought about doing that but I dont think its worth the performance hit. I also think the stochasticity is slightly good because it shows all the possibilities
This came up because I got really frustrated with the error messages when plans fail because of IK reasons. Before it simply reads something like (this output is gotten by running
TestArmObstacleSolve)and with this change it will read
which I think is more helpful.
In order to do this I had to change the
Constrainttypes to returnerrorinstead of bool, and this change rippled out more than I would have expected.