Skip to content

Conversation

Muhammad-Yazdian
Copy link
Contributor

What does this implement/fix?

  • An improvement on move_to_pose.py
  • I implemented a Robot class to organize the move to pose algorithm (see image below)
    Figure_1

Additional information

The advantage of this class is that you can define as many as robots with different specifications (name, color, controller and speed limitations)

CheckList

  • the unittest was passed.
  • The documents text added. Where should I upload the gif/image?

@lgtm-com
Copy link

lgtm-com bot commented Dec 13, 2021

This pull request introduces 1 alert when merging adaabdd into bf4e682 - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Dec 13, 2021

This pull request introduces 1 alert when merging a58dec4 into bf4e682 - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Dec 13, 2021

This pull request introduces 1 alert when merging f536c77 into bf4e682 - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Dec 13, 2021

This pull request introduces 1 alert when merging f332cbd into bf4e682 - view on LGTM.com

new alerts:

  • 1 for Unused import

@Muhammad-Yazdian
Copy link
Contributor Author

  • The Linux_CI says
    • The test for 'test_probabilistic_road_map.py::test1' failed.
    • The 'move_to_pose_robot_class.py' test passed.
    • Do we need to run all test for this PR?

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.

Thank you. This is the last review. After addressing these my comments, I will merge it.

@Muhammad-Yazdian
Copy link
Contributor Author

@AtsushiSakai I think the whole move to pose code algorithm is in a great shape right now. I rename one file; added docstring comments; and updated the documentation. To expedite the process, I have two questions:

  1. What tool would you suggest to use for removing trailing whitespaces? I'm using VS Code and it seems that the built-in 'Format Document' feature does not remove trailing whitespaces inside comment/docstring texts.
  2. Is there any way that I can preview my the doc after I modify some of its contents?

Thank you

@AtsushiSakai AtsushiSakai linked an issue Dec 25, 2021 that may be closed by this pull request
@AtsushiSakai
Copy link
Owner

AtsushiSakai commented Dec 25, 2021

What tool would you suggest to use for removing trailing whitespaces? I'm using VS Code and it seems that the built-in 'Format Document' feature does not remove trailing whitespaces inside comment/docstring texts.

I'm not using any tool to remove trailing white spaces in comment/docstring texts. I will try to find a format tool for the issue.

Is there any way that I can preview my the doc after I modify some of its contents?

Usually, you can check the latest doc in Circle CI result Ref
But, It seems that the latest CI result does not have it, maybe if you commit something, it will be shown.

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.

I think the doc needs improvement, but the next PR is fine. Let's merge this PR. Thank you for your work!!


Ref:

PathFinderController class
Copy link
Owner

Choose a reason for hiding this comment

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

[General comment] I think the rst doc should focus on the mathematics and the algorithm of the example. Documentation of codes should be in python script as docstrings or header comments. If you have time, please consider to fix the doc in the next PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AtsushiSakai Thanks for your comments. I agree to keep the rst doc only for the mathematical explanations. Also, it will be great if we can automatically generate a Reference document based on the docstrings comments. I will try to address your points in my next PR.

Merry Christmas and Happy new Year :)

@AtsushiSakai AtsushiSakai merged commit 680ecda into AtsushiSakai:master Dec 25, 2021
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.

Kp_beta gain with a negative value!
2 participants