Raghumanimehta/485 removing ompl path state constructor#491
Merged
Conversation
raghumanimehta
requested review from
FireBoyAJ24 and
jamenkaye
as code owners
SPDonaghy
reviewed
Feb 17, 2025
Contributor
SPDonaghy
left a comment
SPDonaghy
left a comment
There was a problem hiding this comment.
Great work! I especially appreciate your organized coding style.
I think the main thing is there might some confusion about converting from HelperLatLon to XY types before passing the state space boundary to the solver.
Hopefully its clear what I mean there, but if you have any confusion about it or questions please let me know by responding in the comment thread for this review.
I know its tough to get going with this code, OMPL is a lot. Keep up the great work excited to get this wrapped up and get you onto your next task :)
SPDonaghy
approved these changes
Feb 20, 2025
Contributor
SPDonaghy
left a comment
SPDonaghy
left a comment
There was a problem hiding this comment.
Great Work! Good to merge.
SPDonaghy
deleted the
raghumanimehta/485-removing_OMPLPathState_constructor
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
OMPLPathState class has been removed. Some of its fields have been moved as local variables to
_init_simple_setupinompl_path.pysince only this method used those fields. The rest of the fields could be derived fromlocal_path_stateand have not been changed. Logic to calculatestate_domainandstate_rangeinompl_path.pyhas been added.Verification