Added Footprint Collision Checker (Simple Commander API) #3280

afifswaidan · 2022-11-13T09:54:10Z

Basic Info

Info	Please fill out this column
Ticket(s) this addresses	#3151
Primary OS tested on	Ubuntu 22.04.01
Robotic platform tested on	None

Description of contribution in a few bullet points

Added Footprint Collision Checker (Simple Commander API)
Added one missing function in costmap_2d.py, it was needed in the footprint collision checker

For Maintainers:

Check that any new parameters added are updated in navigation.ros.org
Check that any significant change is added to the migration guide
Check that any new features OR changes to existing behaviors are reflected in the tuning guide
Check that any new functions have Doxygen added
Check that any new features have test coverage
Check that any new plugins is added to the plugins page
If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists

mergify · 2022-11-13T09:54:44Z

@afifswaidan, please properly fill in PR template in the future. @SteveMacenski, use this instead.

Check that any new parameters added are updated in navigation.ros.org
Check that any significant change is added to the migration guide
Check that any new features OR changes to existing behaviors are reflected in the tuning guide
Check that any new functions have Doxygen added
Check that any new features have test coverage
Check that any new plugins is added to the plugins page
If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists

afifswaidan · 2022-11-13T09:55:20Z

@SteveMacenski This is the initial commit, I still need to add documentation, docstrings and test cases. But I wanted to take your opinion if there are any specific test cases that I should implement?
Thanks!

SteveMacenski

LGTM for a first run, just a little cleanup and should be able to merge in soon!

nav2_simple_commander/nav2_simple_commander/footprint_collision_checker.py

nav2_simple_commander/nav2_simple_commander/costmap_2d.py

nav2_simple_commander/nav2_simple_commander/footprint_collision_checker.py

afifswaidan · 2022-11-19T14:47:55Z

I added the doc strings, migrated to Polygon and Point32 messages, and renamed the validity function.
Once you check them let me know if I need to modify anything. After that i will implement the test cases and commit them.
Thanks! :)

SteveMacenski · 2022-11-20T19:28:15Z

Looked it all over and resolved the comments solved + added more recommendations!

SteveMacenski · 2022-12-01T00:35:02Z

any update? I would like this in before Christmas to button up that ticket for 2022 😄

afifswaidan · 2022-12-01T06:59:42Z

any update? I would like this in before Christmas to button up that ticket for 2022 😄

I was sick for the past week so I couldn't work or contribute. I already implemented some test cases, I'll continue the rest and commit on the weekend as usual. Is that okay? 😊
I believe we can finish it before Christmas for sure.

SteveMacenski · 2022-12-01T19:21:11Z

Awesome, totally understand. Feel better! I appreciate the help :-)

SteveMacenski · 2022-12-13T21:22:42Z

Awesome!

i think the last thing was the verified worldtomap None return for 1 call!

afifswaidan · 2022-12-13T21:26:49Z

Awesome!

i think the last thing was the verified worldtomap None return for 1 call!

This was already implemented a few commits back. Now the function returns None in case of invalid inputs, else, it returns the [X,Y] coordinates from world coordinates to map coordinates.

EDIT: Also I added this explanation in the worldToMapValidated function's docstring in the costmap_2d.py file.
Let me know what you think when you check it out! :)

SteveMacenski · 2022-12-14T00:13:59Z

We talked about changing that to worldToMapValidated -> [None, None] so that you only called the function 1 time and you check x, y for being none.

afifswaidan · 2022-12-14T00:16:05Z

We talked about changing that to worldToMapValidated -> [None, None] so that you only called the function 1 time and you check x, y for being none.

Ah yes correct, I modified the function but still kept calling it more than one time.
My bad. It is an easy fix, I will fix it and commit ASAP. I'll try my best to do it tomorrow or at least before the weekend.
Thanks for pointing it out again.

SteveMacenski · 2022-12-14T19:36:52Z

nav2_simple_commander/test/test_footprint_collision_checker.py

+LETHAL_OBSTACLE = 254
+
+
+class TestFootprintCollisionChecker(unittest.TestCase):


Last thing : the unit tests should have some that have non-zero costs in them to make sure things are working properly. If you make a 100x100 map with a 10x10 block of cost in the middle (or something) and then run these same tests to make sure we're picking things up correctly.

Then this is good to merge! Thanks!

SteveMacenski · 2022-12-14T19:37:39Z

nav2_simple_commander.test.test_footprint_collision_checker.TestFootprintCollisionChecker failed

test/test_footprint_collision_checker.py:67: in test_worldToMapValidated
    self.assertEqual(fcc_.worldToMapValidated(0, 5), None)
E   AssertionError: (None, None) != None

Also a linting error

AssertionError: Found 1 code style errors / warnings:
  ./nav2_simple_commander/footprint_collision_checker.py:70:1: W293 blank line contains whitespace
assert 1 == 0
test/test_flake8.py:23: in test_flake8
    assert rc == 0, \
E   AssertionError: Found 1 code style errors / warnings:
E     ./nav2_simple_commander/footprint_collision_checker.py:70:1: W293 blank line contains whitespace
E   assert 1 == 0

SteveMacenski · 2022-12-15T00:31:47Z

@afifswaidan I forgot - can you add docs https://navigation.ros.org/commander_api/index.html about this feature?

SteveMacenski · 2022-12-15T00:32:41Z

https://discourse.ros.org/t/nav2-costmap-collision-checking-python-api-added-to-nav2-simple-commander/28766

afifswaidan · 2022-12-15T07:51:20Z

@afifswaidan I forgot - can you add docs https://navigation.ros.org/commander_api/index.html about this feature?

Sure thing!
I have 2 question, I should add the documentation for this collision checker and for the line iterator that I did previously right?

My 2nd question is, its my first time of adding/modifying a docs website, how do you usually do it for ros2 nav2 docs? Sorry if its a basic question but I'd like to know how and do it!

Thanks :)

afifswaidan · 2022-12-15T07:53:17Z

https://discourse.ros.org/t/nav2-costmap-collision-checking-python-api-added-to-nav2-simple-commander/28766

Thank you so much for the credits!
I must say that contributing under your supervision/assistance was a pleasant and beneficial experience for me.

Please ping me whenever there are some pending issues that needs contributors, I've been working with ROS Navigation since 2017 and I hope I can be a good help for the development of ROS.

SteveMacenski · 2022-12-15T18:15:57Z

https://github.com/ros-planning/navigation.ros.org/blob/master/commander_api/index.rst Basically just modify this file under the Costmap API section to mention the collision checker and provide a link to it

SteveMacenski · 2022-12-15T18:17:57Z

Something that would be very helpful if you were interested is #3205. The idea is that if you create a while true loop, you may notice that the controller can run much faster than if you throttle it down to a set rate. The project is to try to find a way so that we can leverage more of that speed. It might be timer we use isn't working as we'd like, context switching of threads, I mis-diagnosed the issue when I was developing something else, etc.

afifswaidan · 2022-12-15T19:30:02Z

https://github.com/ros-planning/navigation.ros.org/blob/master/commander_api/index.rst Basically just modify this file under the Costmap API section to mention the collision checker and provide a link to it

Okay sure I'll do that either tomorrow or on Saturday.
Thanks for the help!

afifswaidan · 2022-12-15T19:31:23Z

Something that would be very helpful if you were interested is #3205. The idea is that if you create a while true loop, you may notice that the controller can run much faster than if you throttle it down to a set rate. The project is to try to find a way so that we can leverage more of that speed. It might be timer we use isn't working as we'd like, context switching of threads, I mis-diagnosed the issue when I was developing something else, etc.

Okay sure, I can look into it.
But can you give me some details about the timeline?
When is it expected to be finished?
Because next week I might not be fully available.

Thanks for telling me about it as well.

SteveMacenski · 2022-12-16T00:21:29Z

I don't expect much to get done from tomorrow to the end of the year from anyone 😆

When you have time and interest :-) If this doesn't interest you, there are plenty of things in the issue tracker and would very much appreciate contributions where you would have interest!

afifswaidan · 2022-12-16T07:34:06Z

I don't expect much to get done from tomorrow to the end of the year from anyone 😆

When you have time and interest :-) If this doesn't interest you, there are plenty of things in the issue tracker and would very much appreciate contributions where you would have interest!

Yeah exactly its the holidays and end of year season 😆

Okay I'll check it out and let you know. I'll check the other issues as well.

afifswaidan · 2022-12-26T17:18:26Z

Hello Steve,
I wish you a Merry Christmas!

I will be looking into all the stuff you told me about later in the first week of January 2023.
I noticed something that I wanted to ask you about.
In our last commit, a wrong email address was included in the details of the commit, can this be fixed somehow?
My email address is [email protected]
My github username is afifswaidan
Some other account is linked to the commit I don't actually know why.
Thanks in advance!

SteveMacenski · 2022-12-27T06:05:31Z

Unfortunately, that’s to do with your git configurations and not something I can control. Perhaps its possible in Git to manually manipulate that, but it would be wrong (and possibly problematic legally) for me to do that as a representative of a company.

What we can do instead though is revert the commit so its out of the git history and you can resubmit a new PR under your preferred address. Its mostly problematic for me as a 3rd party to reassign the commit. For you, its fine if its more properly representative.

afifswaidan · 2022-12-27T08:49:24Z

Unfortunately, that’s to do with your git configurations and not something I can control. Perhaps its possible in Git to manually manipulate that, but it would be wrong (and possibly problematic legally) for me to do that as a representative of a company.

What we can do instead though is revert the commit so its out of the git history and you can resubmit a new PR under your preferred address. Its mostly problematic for me as a 3rd party to reassign the commit. For you, its fine if its more properly representative.

Thanks for for the quick reply.
Of course I don't want us to do anything that is potentially problematic.
So if I understood correctly, you revert back the repo, and I submit a new PR with corrected git configs?

afifswaidan · 2023-01-06T13:34:42Z

https://github.com/ros-planning/navigation.ros.org/blob/master/commander_api/index.rst Basically just modify this file under the Costmap API section to mention the collision checker and provide a link to it

I modified the file based on previous APIs (costmap and commander)
Let me know if anything else should be added.

…ion#3280) * Added Line Iterator * Updated Line Iterator to a new iteration method * Added the resolution as a parameter/ fixed linting * Added the resolution as a parameter/ fixed linting * Added unittests for the line iterator * Added unittests based on "unittest" package * Fixed __init__.py and rephrased some docstrings * Fixed linting errors * Fixed Linting Errors * Added some unittests and removed some methods * Dummy commit for CircleCI Issue * Initial commit for python collision checker * Initial commit for footprint_collision_checker * Added docstrings and renamed the validity function * implemented worldToMapValidated and added tests * fixed wrong imports * Fixed test cases linting errors * Fixed one line comment error * Fixed docstring colon error * Fixed docstring dashed line error * Added return None to keep consistent docstrings * Changed worldToMapValidated to 1 call only * Fixed linting error and added none-zero cost test case * Fixed linting errors * Fixed linting errors Co-authored-by: Afif Swaidan <[email protected]>

Afif-Swaidan and others added 20 commits September 17, 2022 16:38

Added Line Iterator

a824708

Merge branch 'ros-planning:main' into main

5db702e

Updated Line Iterator to a new iteration method

a1767f2

Added the resolution as a parameter/ fixed linting

a9c6d6d

Added the resolution as a parameter/ fixed linting

9b79757

Merge branch 'ros-planning:main' into main

94934dd

Merge branch 'ros-planning:main' into main

9ca4db6

Added unittests for the line iterator

c050299

Merge branch 'ros-planning:main' into main

02f726d

Added unittests based on "unittest" package

9df1b62

Fixed __init__.py and rephrased some docstrings

6731c6b

Fixed linting errors

d35d26d

Fixed Linting Errors

b8d0858

Merge branch 'ros-planning:main' into main

dc438c4

Merge branch 'ros-planning:main' into main

9d3992b

Added some unittests and removed some methods

f9d48a2

Dummy commit for CircleCI Issue

a571cde

Merge branch 'ros-planning:main' into main

63ed3c9

Initial commit for python collision checker

f06c5eb

Initial commit for footprint_collision_checker

743144d

SteveMacenski requested changes Nov 14, 2022

View reviewed changes

Added docstrings and renamed the validity function

a8d252d

Merge branch 'ros-planning:main' into main

8171237

afifswaidan added 2 commits December 13, 2022 20:46

Merge branch 'ros-planning:main' into main

2007b28

Added return None to keep consistent docstrings

bd93142

Changed worldToMapValidated to 1 call only

35c67ad

SteveMacenski reviewed Dec 14, 2022

View reviewed changes

afifswaidan added 3 commits December 14, 2022 21:31

Fixed linting error and added none-zero cost test case

41e30f4

Fixed linting errors

6b7498f

Fixed linting errors

1010460

SteveMacenski approved these changes Dec 15, 2022

View reviewed changes

SteveMacenski merged commit 06e3d29 into ros-navigation:main Dec 15, 2022

afifswaidan mentioned this pull request Dec 27, 2022

Copyrights+Email Correction #3339

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Footprint Collision Checker (Simple Commander API) #3280

Added Footprint Collision Checker (Simple Commander API) #3280

afifswaidan commented Nov 13, 2022

mergify bot commented Nov 13, 2022

afifswaidan commented Nov 13, 2022

SteveMacenski left a comment

afifswaidan commented Nov 19, 2022

SteveMacenski commented Nov 20, 2022

SteveMacenski commented Dec 1, 2022 •

edited

Loading

afifswaidan commented Dec 1, 2022

SteveMacenski commented Dec 1, 2022

SteveMacenski commented Dec 13, 2022

afifswaidan commented Dec 13, 2022 •

edited

Loading

SteveMacenski commented Dec 14, 2022

afifswaidan commented Dec 14, 2022

SteveMacenski Dec 14, 2022

SteveMacenski commented Dec 14, 2022

SteveMacenski commented Dec 15, 2022

SteveMacenski commented Dec 15, 2022

afifswaidan commented Dec 15, 2022

afifswaidan commented Dec 15, 2022

SteveMacenski commented Dec 15, 2022

SteveMacenski commented Dec 15, 2022

afifswaidan commented Dec 15, 2022 •

edited

Loading

afifswaidan commented Dec 15, 2022

SteveMacenski commented Dec 16, 2022

afifswaidan commented Dec 16, 2022 •

edited

Loading

afifswaidan commented Dec 26, 2022 •

edited

Loading

SteveMacenski commented Dec 27, 2022

afifswaidan commented Dec 27, 2022

afifswaidan commented Jan 6, 2023

		LETHAL_OBSTACLE = 254


		class TestFootprintCollisionChecker(unittest.TestCase):

Added Footprint Collision Checker (Simple Commander API) #3280

Added Footprint Collision Checker (Simple Commander API) #3280

Conversation

afifswaidan commented Nov 13, 2022

Basic Info

Description of contribution in a few bullet points

For Maintainers:

mergify bot commented Nov 13, 2022

afifswaidan commented Nov 13, 2022

SteveMacenski left a comment

Choose a reason for hiding this comment

afifswaidan commented Nov 19, 2022

SteveMacenski commented Nov 20, 2022

SteveMacenski commented Dec 1, 2022 • edited Loading

afifswaidan commented Dec 1, 2022

SteveMacenski commented Dec 1, 2022

SteveMacenski commented Dec 13, 2022

afifswaidan commented Dec 13, 2022 • edited Loading

SteveMacenski commented Dec 14, 2022

afifswaidan commented Dec 14, 2022

SteveMacenski Dec 14, 2022

Choose a reason for hiding this comment

SteveMacenski commented Dec 14, 2022

SteveMacenski commented Dec 15, 2022

SteveMacenski commented Dec 15, 2022

afifswaidan commented Dec 15, 2022

afifswaidan commented Dec 15, 2022

SteveMacenski commented Dec 15, 2022

SteveMacenski commented Dec 15, 2022

afifswaidan commented Dec 15, 2022 • edited Loading

afifswaidan commented Dec 15, 2022

SteveMacenski commented Dec 16, 2022

afifswaidan commented Dec 16, 2022 • edited Loading

afifswaidan commented Dec 26, 2022 • edited Loading

SteveMacenski commented Dec 27, 2022

afifswaidan commented Dec 27, 2022

afifswaidan commented Jan 6, 2023

SteveMacenski commented Dec 1, 2022 •

edited

Loading

afifswaidan commented Dec 13, 2022 •

edited

Loading

afifswaidan commented Dec 15, 2022 •

edited

Loading

afifswaidan commented Dec 16, 2022 •

edited

Loading

afifswaidan commented Dec 26, 2022 •

edited

Loading