ros-navigation / navigation2

ROS 2 Navigation Framework and System
https://nav2.org/
Other
2.3k stars 1.2k forks source link

WIP: Add a BT plugin to check valid forward motion with an ML model (ChatGPT specifically) #4483

Closed AndyZe closed 1 week ago

AndyZe commented 1 week ago

Basic Info

| ------ | ----------- | | Ticket(s) this addresses | n/a | | Primary OS tested on | Ubuntu | | Robotic platform tested on | RealSense D415 | | Does this PR contain AI generated software? | No |


Description of contribution in a few bullet points

IMG_20240624_164854061

Description of documentation updates required from your changes


Future work that may be required in bullet points

For Maintainers:

mergify[bot] commented 1 week ago

@AndyZe, all pull requests must be targeted towards the main development branch. Once merged into main, it is possible to backport to @humble, but it must be in main to have these changes reflected into new distributions.

AndyZe commented 1 week ago

@AndyZe, all pull requests must be targeted towards the main development branch

I'll port to main when it's complete. BTW what ROS2 distro is main on, now?

ajtudela commented 1 week ago

Hi @AndyZe,

I'm not sury what exactly do you want to accomplish with this PR.

There is already a IsPathValidCondition here: https://github.com/ros-navigation/navigation2/blob/main/nav2_behavior_tree/include/nav2_behavior_tree/plugins/condition/is_path_valid_condition.hpp

Could you elaborate more? Thanks

AndyZe commented 1 week ago

Sure, the is_path_valid plugin only checks for collisions. For the example of this cord, it wouldn't be detected as a collision because it's so low to the ground, yet it's still a dangerous obstacle.

There are lots of other examples where this would be useful too, like a mud puddle. (Note that the PR is still in progress.)

SteveMacenski commented 1 week ago

This looks like its unimplemented, not sure what is actionable about this PR at the moment. None of the things in your description are true about the software, its all TODO. I suggest closing this until there's something to review.

I'll port to main when it's complete. BTW what ROS2 distro is main on, now?

All contributions need to be in main first and then backported to make sure all new distributions have all new features / fixes. main is rolling and probably still compatible with jazzy since that's only recently come out.

Sure, the is_path_valid plugin only checks for collisions. For the example of this cord, it wouldn't be detected as a collision because it's so low to the ground, yet it's still a dangerous obstacle.

The server called under the hood to the is path valid can use GPT, but I'm unclear why a new BT node is required. Are you going to change the API for calling the GPT service and thus need a different interface definiton? If not, you can simply use the existing BT node pointed to your proper server. I also don't see the point in the new BT node unless there's more implementation details in mind.

AndyZe commented 1 week ago

Here it says IsPathValid only checks collisions. Is that server something that's exposed to modify easily?

http://aigrantli.com:881/configuration/packages/bt-plugins/conditions/IsPathValid.html#ispathvalid)

AndyZe commented 1 week ago

Alright, i'll go ahead and close the PR and open an issue to discuss this. And continue working on the PR in the meantime.

SteveMacenski commented 1 week ago

First off, please point to our actual docs, I don't know what that website is :wink: https://docs.nav2.org/configuration/packages/bt-plugins/conditions/IsPathValid.html?highlight=ispathvalid

But we can update that description to be more generic. We intended to have it be more than just for critical collisions like in https://github.com/ros-navigation/navigation2/tree/is_path_valid_cost_change for triggering updates when opportunistic or necessary rather than regularly.

AndyZe commented 1 week ago

Thanks for the, umm, very rapid and negative but enlightening feedback :+1: