Skip to content
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

Update autowalk interfaces #6

Open
wants to merge 12 commits into
base: arm
Choose a base branch
from

Conversation

sktometometo
Copy link

This PR enables to use AutoWalk Interface

  • Add UploadGraph, DownloadGraph, SetLocalizationWaypoint, SetLocalizationFiducial services and update ListGraph and NavigateTo actions.
    • So that autowalk graph can be loaded fast and robot can move along the graph without reloading it in each time.

@sktometometo
Copy link
Author

@k-okada Could you merge this for my experiments?

@@ -547,36 +551,81 @@ def bodyPoseCallback(self, data):
mobility_params.body_control.CopyFrom(body_control)
self.spot_wrapper.set_mobility_params(mobility_params)

def handle_list_graph(self, upload_path):
def handle_start_recording(self, req):
Copy link
Owner

Choose a reason for hiding this comment

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

what is the actual usecase of start/stop recording without using Tablet?

Copy link
Author

Choose a reason for hiding this comment

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

It enables to edit and extend the graph used in spot core. If we use Tablet, we have to copy the graph to tablet, edit and extend the graph with tablet and then copy it again to the SpotCore to use it.

Copy link
Author

Choose a reason for hiding this comment

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

That is very inefficient.

string navigate_to # Waypoint id string for where to go
bool initial_localization_fiducial # Tells the initializer whether to use fiducials
string initial_localization_waypoint # Waypoint id to trigger localization
string id_navigate_to # Waypoint id string for where to go
Copy link
Owner

Choose a reason for hiding this comment

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

This seems your original idea, beccause no one have noticed the necessarity of update interface https://github.com/heuristicus/spot_ros/blob/master/spot_msgs/action/NavigateTo.action

Whta it the benefit of this change? To support any new features ? https://dev.bostondynamics.com/docs/release_notes#autowalk-service ?

Copy link
Author

Choose a reason for hiding this comment

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

It does not related to any new features. Original spot-ros API limits the capability of AutoWalk API since it loads all the graph files when any navigation command is issued even if the same graph is already loaded to the robot. It extremely lengthens the time it takes to execute a single navigation command.

Copy link
Author

Choose a reason for hiding this comment

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

In fact, there are some other people use this update heuristicus/spot_ros#50

@k-okada
Copy link
Owner

k-okada commented May 25, 2023 via email

@sktometometo
Copy link
Author

That will not be merged since it diverged extremly from master branch. (No response for months and years from maintainer) It may be good to create a new PR to spot_ros and spot_wrapper repository

@sktometometo sktometometo force-pushed the PR/arm/update-autowalk-interfaces branch from 88c8c83 to 982c74b Compare November 22, 2023 00:38
@sktometometo sktometometo force-pushed the PR/arm/update-autowalk-interfaces branch from 982c74b to 0c153ae Compare November 20, 2024 03:17
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