Skip to content

Conversation

@cmower
Copy link
Contributor

@cmower cmower commented Apr 5, 2022

  • update __init__ and check_collision in RRT class to include a robot radius
  • during animation, if a robot radius is given then it is drawn

What does this implement?

This simply adds an optional robot radius feature for the RRT/RRTStar path planners.

Additional information

Examples updated to highlight feature.

CheckList

  • Did you add an unittest for your new example or defect fix? The radius feature is optional so should not break anything. I added an additional test in test_rrt_star.py
  • Did you add documents for your new example?
  • All CIs are green? (You can check it after submitting)

cmower added 2 commits April 5, 2022 20:10
* update __init__ and check_collision to include radius
* during animation, if a robot radius is given then it is drawn
@lgtm-com
Copy link

lgtm-com bot commented Apr 5, 2022

This pull request introduces 1 alert when merging 53c23a4 into 7ac2a17 - view on LGTM.com

new alerts:

  • 1 for Wrong number of arguments in a call

@AtsushiSakai
Copy link
Owner

@cmower Thank you for PR. It looks good improvement. Could you please fix CI errors? Other RRT related scripts are using the changed codes.

@cmower
Copy link
Contributor Author

cmower commented Apr 9, 2022

@AtsushiSakai thanks, looks like all CI checks pass now, let me know if you need any other changes.

Copy link
Owner

@AtsushiSakai AtsushiSakai left a comment

Choose a reason for hiding this comment

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

@cmower Thank you. I have some comments. I will merge this PR after addressing these. PTAL.

Copy link
Owner

@AtsushiSakai AtsushiSakai left a comment

Choose a reason for hiding this comment

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

Sorry. This is final comment.

@cmower
Copy link
Contributor Author

cmower commented Apr 18, 2022

@AtsushiSakai no worries, assuming CI goes green, then should be ok to merge?

Copy link
Owner

@AtsushiSakai AtsushiSakai left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!!

@AtsushiSakai AtsushiSakai merged commit b53fdf7 into AtsushiSakai:master Apr 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants