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.

Sign up for GitHub

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

Jump to bottom

Kabirk/add simple subscriber #580

Merged
merged 13 commits into from
Feb 20, 2025
Merged

Kabirk/add simple subscriber #580

merged 13 commits into from
Feb 20, 2025

Conversation

kabirkedia
Copy link
Contributor

@kabirkedia kabirkedia commented Feb 14, 2025 •
edited
Loading

Change Overview

This PR addresses issue #569 and provides an example for controlling Spot via a ROS 2 subscriber.

I initially faced challenges using a subscriber to control Spot while integrating a learning-based MPPI model. To make this process easier for others, I’ve added an example demonstrating how to use a topic to send movement commands to Spot.
How It Works

The code subscribes to a topic that publishes target poses.
It sends the first received pose as an action to Spot.
While Spot is executing the action, it buffers any additional incoming poses.
Once Spot returns a successful feedback, it checks the buffer and sends the next pose in the queue.
This cycle repeats, ensuring smooth and continuous motion.

Additionally, I have fixed broken documentation links that referenced bdai_ros2_wrappers instead of synchros2.

Testing Done

✅ Tested with the real Spot multiple times.

(Thank you @khughes-bdai for the prompt help)

Please create a checklist of tests you plan to do and check off the ones that have been completed successfully. Ensure that ROS 2 tests use domain_coordinator to prevent port conflicts. Further guidance for testing can be found on the ros utilities wiki.
This has been tested with the real spot multiple times.

@kabirkedia kabirkedia force-pushed the kabirk/add_simple_subscriber branch from e178f24 to cad1aee Compare February 14, 2025 19:50
khughes-bdai
khughes-bdai reviewed Feb 14, 2025
View reviewed changes
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.

Thank you for the PR!

@khughes-bdai Graphite App
Copy link
Collaborator

Also, to ensure the linting check passes, can you run pre-commit install then pre-commit run --all at the repo's root, then commit those changes?

@kabirkedia kabirkedia force-pushed the kabirk/add_simple_subscriber branch 2 times, most recently from fc563c8 to 493af1e Compare February 19, 2025 23:26
@kabirkedia
Copy link
Contributor Author

Hey @khughes-bdai I made all the changes you requested! Please review them and let me know if some other changes are needed

@kabirkedia kabirkedia force-pushed the kabirk/add_simple_subscriber branch from 560eac6 to 0d33cbe Compare February 20, 2025 16:05
@kabirkedia
Copy link
Contributor Author

Hello I made the changes you requested @khughes-bdai .
Sorry for being so careless with my code! Thank you for being so patient!

@kabirkedia
Copy link
Contributor Author

pre-commit is failing but I am not sure why!

@khughes-bdai Graphite App
Copy link
Collaborator

super small but it looks like you are missing a newline at the end of the docs file -- if you run the pre-commit command again locally it should fix it!

@kabirkedia
Copy link
Contributor Author

Fixed NVM

kabirkedia and others added 11 commits February 20, 2025 11:11
@kabirkedia
Add an example for simple subscriber
77f4508 
Signed-off-by: kabirkedia <[email protected]>
@kabirkedia
add changes to setup.py
ecea9c7 
Signed-off-by: kabirkedia <[email protected]>
@kabirkedia
no debug statement
b6397c2 
Signed-off-by: kabirkedia <[email protected]>
@kabirkedia
preconfigure lint changes
c2f811c 
Signed-off-by: kabirkedia <[email protected]>
@tcappellari-bdai @kabirkedia
[SW-957] make decorator for trigger services (bdaiinstitute#578)
1e95e38 
## Change Overview

Made a descriptor class for trigger services to reduce boilerplate + copy/paste code in trigger service handlers

## Testing Done

Tested all trigger services on robot. Found a bug with estop handling within `spot_wrapper` (will make a new ticket to track this but out of scope for this PR)

Please create a checklist of tests you plan to do and check off the ones that have been completed successfully. Ensure that ROS 2 tests use `domain_coordinator` to prevent port conflicts. Further guidance for testing can be found on the [ros utilities wiki](https://github.com/bdaiinstitute/ros_utilities/wiki/Testing-guidelines).

Signed-off-by: kabirkedia <[email protected]>
@dependabot @kabirkedia
[dependabot] Bump spot_wrapper from 3555ca1 to d96cfb8 (bdaiinsti…
85c00bc 
…tute#582)

Bumps [spot_wrapper](https://github.com/bdaiinstitute/spot_wrapper) from `3555ca1` to `d96cfb8`.
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/bdaiinstitute/spot_wrapper/commit/d96cfb8e66cad5791fed8f40dc03bdb83ac3720f"><code>d96cfb8</code></a> Fix false trajectory success when blocked by obstacle (<a href="https://redirect.github.com/bdaiinstitute/spot_wrapper/issues/156">#156</a>)</li>
<li>See full diff in <a href="https://github.com/bdaiinstitute/spot_wrapper/compare/3555ca137677ed265a91c85e22d4505ebea3bc14...d96cfb8e66cad5791fed8f40dc03bdb83ac3720f">compare view</a></li>
</ul>
</details>
<br />

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

</details>

Signed-off-by: kabirkedia <[email protected]>
@gaddison-bdai @tcappellari-bdai @kabirkedia
Fix false trajectory success when blocked by obstacle (bdaiinstitute#579
49c1ad5 
)

Co-authored-by: Tiffany Cappellari <[email protected]>
Signed-off-by: kabirkedia <[email protected]>
@kabirkedia
restructure to use classes and use cli arg
42fcf4c 
Signed-off-by: kabirkedia <[email protected]>
@kabirkedia
fix linting
4c75ade 
Signed-off-by: kabirkedia <[email protected]>
@kabirkedia
changes in documentation and make code more professional
95a0a4f 
Signed-off-by: kabirkedia <[email protected]>
@kabirkedia
fix linting
66f80d9 
Signed-off-by: kabirkedia <[email protected]>
@kabirkedia kabirkedia force-pushed the kabirk/add_simple_subscriber branch from f2b568c to 66f80d9 Compare February 20, 2025 16:11
khughes-bdai
khughes-bdai approved these changes Feb 20, 2025
View reviewed changes
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.

LGTM!

@tcappellari-bdai tcappellari-bdai merged commit 702c2fa into bdaiinstitute:main Feb 20, 2025
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tcappellari-bdai tcappellari-bdai tcappellari-bdai approved these changes

@khughes-bdai khughes-bdai khughes-bdai approved these changes

@METEORITENMAX METEORITENMAX Awaiting requested review from METEORITENMAX METEORITENMAX is a code owner

@jbarry-bdai jbarry-bdai Awaiting requested review from jbarry-bdai jbarry-bdai is a code owner

@amessing-bdai amessing-bdai Awaiting requested review from amessing-bdai amessing-bdai is a code owner

@mpickett-bdai mpickett-bdai Awaiting requested review from mpickett-bdai mpickett-bdai is a code owner

@bhung-bdai bhung-bdai Awaiting requested review from bhung-bdai bhung-bdai is a code owner

@mhidalgo-bdai mhidalgo-bdai Awaiting requested review from mhidalgo-bdai

@gaddison-bdai gaddison-bdai Awaiting requested review from gaddison-bdai

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kabirkedia @khughes-bdai @tcappellari-bdai @gaddison-bdai