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

Gaddison/spot sdk examples conversion #564

Merged
merged 45 commits into from
Feb 10, 2025

Conversation

gaddison-bdai
Copy link
Collaborator

@gaddison-bdai gaddison-bdai commented Jan 28, 2025

Change Overview

Created hello_spot in spot_examples to mirror that of spot-sdk, as part of SW-1864.

Compiled and behavior qualitatively verified on hardware.

Copy link
Collaborator

@khughes-bdai khughes-bdai left a comment

Choose a reason for hiding this comment

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

overall looks great to me! Could you add to the readme with the command to run this example + a short description of what it does?
Also, to get CI to pass, in the repo root run pre-commit install then pre-commit run --all and this should fix it.
(you can also follow the steps from the dco log to fix that error: https://github.com/bdaiinstitute/spot_ros2/runs/36316801944)

@gaddison-bdai gaddison-bdai force-pushed the gaddison/spot-sdk_examples_conversion branch from 5eccf8b to 56168d8 Compare January 30, 2025 18:54
@gaddison-bdai
Copy link
Collaborator Author

gaddison-bdai commented Feb 6, 2025

Three examples - hello_spot, arm_with_body_follow, and wasd - have been added. Next step is to update README. @khughes-bdai , should I also add .md files for each new example?

@coveralls
Copy link

coveralls commented Feb 7, 2025

Pull Request Test Coverage Report for Build 13208847320

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 51.319%

Totals Coverage Status
Change from base Build 13032831138: 0.0%
Covered Lines: 1965
Relevant Lines: 3829

💛 - Coveralls

@khughes-bdai
Copy link
Collaborator

khughes-bdai commented Feb 7, 2025

Three examples - hello_spot, arm_with_body_follow, and wasd - have been added. Next step is to update README. @khughes-bdai , should I also add .md files for each new example?

Sure, i think that would be helpful to match the format from the other examples. (it doesn't have to be long, just could be a few sentences explaining what the example does, and maybe link to the original examples from spot sdk for reference?)

@gaddison-bdai
Copy link
Collaborator Author

Adding .md's for the three new examples now.

@gaddison-bdai
Copy link
Collaborator Author

Added docs (markdowns) for each of the three new behaviors here.

Copy link
Collaborator

@tcappellari-bdai tcappellari-bdai left a comment

Choose a reason for hiding this comment

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

lgtm

spot_examples/spot_examples/wasd.py Outdated Show resolved Hide resolved
spot_examples/docs/wasd.md Show resolved Hide resolved
Copy link
Collaborator

@tcappellari-bdai tcappellari-bdai left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Collaborator

@khughes-bdai khughes-bdai left a comment

Choose a reason for hiding this comment

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

🎉

Copy link
Collaborator Author

gaddison-bdai commented Feb 10, 2025

Merge activity

  • Feb 10, 11:32 AM EST: A user started a stack merge that includes this pull request via Graphite.
  • Feb 10, 11:33 AM EST: Graphite rebased this pull request as part of a merge.
  • Feb 10, 11:50 AM EST: Graphite couldn't merge this PR because it failed for an unknown reason.
  • Feb 10, 4:02 PM EST: A user started a stack merge that includes this pull request via Graphite.
  • Feb 10, 4:05 PM EST: Graphite couldn't merge this PR because it failed for an unknown reason.

gaddison-bdai and others added 26 commits February 10, 2025 16:33
….ManipulationApiRequestOneOfManipulationCmd.msg
@gaddison-bdai gaddison-bdai force-pushed the gaddison/spot-sdk_examples_conversion branch from 743dd34 to 3f7facf Compare February 10, 2025 16:33
@tcappellari-bdai tcappellari-bdai merged commit 63f0976 into main Feb 10, 2025
5 checks passed
@tcappellari-bdai tcappellari-bdai deleted the gaddison/spot-sdk_examples_conversion branch February 10, 2025 21:47
WillWu88 pushed a commit to USC-ACTLab/spot_ros2 that referenced this pull request Feb 12, 2025
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.

4 participants