-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fleet adapter and PerformAction tutorial #105
Conversation
Signed-off-by: Xi Yu Oh <[email protected]>
Signed-off-by: Xi Yu Oh <[email protected]>
Signed-off-by: Xi Yu Oh <[email protected]>
Signed-off-by: Xi Yu Oh <[email protected]>
Signed-off-by: Xi Yu Oh <[email protected]>
Signed-off-by: Xi Yu Oh <[email protected]>
Signed-off-by: Xiyu Oh <[email protected]>
Signed-off-by: Xiyu Oh <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went through the tutorials. The perform action stuff is fine. The fleet adapter tutorial has few minor typos/issues with it.
## 1. Pre-requisites | ||
|
||
### Fetch dependencies | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a note about installing RMF. Link it to some section.
- Fleet `config.yaml` file | ||
- Navigation graph | ||
|
||
### a. Install ROS 2 and sRMF |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
### a. Install ROS 2 and sRMF | |
### a. Install ROS 2 and RMF |
Move this to the top with pre-requisites. I don't think there is a need to repeat ourselves
|
||
Since the `rmf_demos_fleet_adapter` uses REST API as an interface between the fleet adapter and robot fleet manager, we will need to install the required dependencies to use FastAPI. | ||
```bash | ||
pip3 install fastapi uvicorn |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading the whole tutorial, I'm a bit confused why we need this. Sure FastAPI and Uvicorn are useful for creating fleet managers. But do we use it in the fleet adapter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, added an explanation to explicitly mention that this step is only required to get the demos working.
|
||
### Fetch dependencies | ||
|
||
Since the `rmf_demos_fleet_adapter` uses REST API as an interface between the fleet adapter and robot fleet manager, we will need to install the required dependencies to use FastAPI. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth explaining the difference between a robot fleet manager and a fleet adapter.
i imagine the fastapi dependency is for the fleet manager and not fleet adapter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The explanation is available in the previous section of the ROS 2 Book.
) | ||
``` | ||
|
||
Then, in our `main` function, we at the computed transforms to our fleet configuration. The EasyFullControl fleet adapter will process these transforms and send out navigation commands in the robot's coordinates accordingly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then, in our `main` function, we at the computed transforms to our fleet configuration. The EasyFullControl fleet adapter will process these transforms and send out navigation commands in the robot's coordinates accordingly. | |
Then, in our `main` function, we add the computed transforms from our fleet configuration. The EasyFullControl fleet adapter will process these transforms and send out navigation commands in the robot's coordinates accordingly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected the first typo, but the second edit is not a typo, fleet configuration is an object and we are adding the transforms to it. Modified it to FleetConfiguration
for clarity.
We have defined a helper function to compute the transforms between RMF and the robot's coordinates. In the event your robot operates in the same coordinates as RMF (e.g. in simulation), you won't need this portion of the code. | ||
|
||
```python | ||
def compute_transforms(level, coords, node=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
explain the role of level
and why we are not using it. Also explain the role of node
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
### c. Initialize the robot API and set up RobotAdapter | ||
|
||
The `config.yaml` should include any connection credentials we'd need to connect to our robot or robot fleet manager. We parse this to the `RobotAPI` to easily interact between RMF and the robot's API. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm kind of surprised we are doing this. I always thought credentials should be seperate from configs so that they don't get checked in to a public git version control repository. I guess on private networks its fine. However, in a tutorial I would advise against recommending storing credentials in a config file. Or put in a warning asking a user to do a threat assessment first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only for this example, added a note on that in aadb22d
|
||
Notice that `execute_action(~)` does not have any implemented code in the fleet adapter template. This callback is designed to be flexible and caters to custom performable actions that may not be availble under the tasks offered in RMF. You can learn how to design and compose your own actions and execute them from the fleet adapter in the [PerformAction tutorial](./integration_fleets_action_tutorial.md) section. | ||
|
||
```python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this code block about?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is just to add all the callbacks at one go, added in aadb22d
Signed-off-by: Xiyu Oh <[email protected]>
This PR updates the existing fleet adapter tutorial based on the Easy API introduced in open-rmf/rmf_ros2#235. It also adds a tutorial for integrating custom performable actions using the Easy API.
This should be merged after open-rmf/fleet_adapter_template#22 as the template should be provided as a base for readers to work off from, and the links will direct readers to the
main
branch.