Skip to content

Conversation

erh
Copy link
Member

@erh erh commented Sep 12, 2025

No description provided.

@erh erh requested a review from dgottlieb September 12, 2025 19:48
@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Sep 12, 2025
@@ -75,24 +75,16 @@ func initRRTSolutions(ctx context.Context, wp atomicWaypoint) *rrtSolution {
})
rrt.maps.optNode = &basicNode{q: goalNodes[0].Q()}

// Check for direct interpolation for the subset of IK solutions within some multiple of optimal
// Since solutions are returned ordered, we check until one is out of bounds, then skip remaining checks
canInterp := true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you find this claim was just wrong? And we were passing up on possible solutions?

Or is the calculation simply cheap and the optimization was unnecessary?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's sort of cheap.
the optimization does speed things up,
BUT most of that logic is already being run and it's duplicated here.
debating if i'm going to put it in this change or separate.

@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Sep 12, 2025
@erh erh requested a review from dgottlieb September 12, 2025 22:51
@erh
Copy link
Member Author

erh commented Sep 12, 2025

i made the change much bigger, so re-requested a look
it's pretty different now, but much cleaner.
the names all need to be cleaned up

@erh erh changed the title remove spam and clean api so you get better errors only check plans in one place in first solution loop, remove spam and clean api so you get better errors Sep 12, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Sep 12, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Sep 13, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Sep 13, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Sep 13, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Sep 13, 2025
@viambot viambot removed the safe to test This pull request is marked safe to test from a trusted zone label Sep 13, 2025
@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Sep 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to test This pull request is marked safe to test from a trusted zone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants