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

Bump Spot SDK from 4.0.2 to 4.1.0 #495

Merged
merged 12 commits into from
Oct 17, 2024
Merged

Bump Spot SDK from 4.0.2 to 4.1.0 #495

merged 12 commits into from
Oct 17, 2024

Conversation

mhidalgo-bdai
Copy link
Collaborator

Change Overview

Precisely what the title says. This patch updates both C++ and Python SDKs, as well as bosdyn_msgs for ROS 2 integration.

Testing Done

Relying on CI and downstream tests.

@mhidalgo-bdai mhidalgo-bdai self-assigned this Oct 1, 2024
@mhidalgo-bdai
Copy link
Collaborator Author

Argh, the bosdyn_msgs bundle install assumes wget is available 🤦

@khughes-bdai khughes-bdai mentioned this pull request Oct 1, 2024
Merged
@khughes-bdai Graphite App
Copy link
Collaborator

I just put up this PR in spot_wrapper bdaiinstitute/spot_wrapper#143 so that these two repos can stay in sync -- the install_spot_ros2.sh script here points to spot_wrapper's requirements.txt, which could cause issues for external users

khughes-bdai added a commit to bdaiinstitute/spot_wrapper that referenced this pull request Oct 1, 2024
@khughes-bdai
[N/A] bump requirements.txt for 4.1.0 (#143)
5c01ea7 
Counterpart to bdaiinstitute/spot_ros2#495 that updates this repo for spot-sdk release 4.1.0.

Additionally, stops running CI tests for Python 3.8 and 3.9 as this isn't supported by this release of the spot-sdk and updates the README to reflect this.
@khughes-bdai Graphite App
Copy link
Collaborator

minor request, but can we update the README as a part of this PR to reflect this new sdk version?

@khughes-bdai khughes-bdai mentioned this pull request Oct 2, 2024
Closed
@mhidalgo-bdai
Copy link
Collaborator Author

mhidalgo-bdai commented Oct 8, 2024 •
edited
Loading

This is getting harder than expected. Spot C++ SDK 4.1.0 libraries seem to depend (yet don't declare the dependency) on a Protobuf version that is newer than the one that is available in Ubuntu 22.04. Static linkage might still work if there were no API breaks between Protobuf versions, but dynamic linkage won't. CI fails (and I can reproduce) to dynamically load libbosdyn_api.so on missing google::protobuf::Message::CopyWithSourceCheck symbols.

We should report this upstream. Edit: wait a minute, we build these debians. @amessing-bdai how are we doing it?

@mhidalgo-bdai
Bump spot-sdk from 4.0.2 to 4.1.0
7db5cc2 
Signed-off-by: Michel Hidalgo <[email protected]>
@mhidalgo-bdai
Update spot_wrapper from bde75c2 to 5c01ea7
98501d7 
Signed-off-by: Michel Hidalgo <[email protected]>
@mhidalgo-bdai
Parametrize Spot SDK version
fc3c575 
Signed-off-by: Michel Hidalgo <[email protected]>
@mhidalgo-bdai
Drop support for Python 3.8 and 3.9
ef09c20 
Signed-off-by: Michel Hidalgo <[email protected]>
@mhidalgo-bdai
Bump bosdyn_*msgs patch release version
868613b 
Signed-off-by: Michel Hidalgo <[email protected]>
@mhidalgo-bdai
Bump bosdyn_*msgs patch release version again
c2c6e24 
Signed-off-by: Michel Hidalgo <[email protected]>
@mhidalgo-bdai
Use Spot C++ SDK shared libraries
6476e48 
Signed-off-by: Michel Hidalgo <[email protected]>
@mhidalgo-bdai
Bump bosdyn_*msgs patch release version
4494cba 
Signed-off-by: Michel Hidalgo <[email protected]>
@mhidalgo-bdai
Update README.md
a0b7802 
Signed-off-by: Michel Hidalgo <[email protected]>
@mhidalgo-bdai mhidalgo-bdai force-pushed the mhidalgo-bdai/bump-spot-sdk branch from 4b5c0a0 to a0b7802 Compare October 17, 2024 17:46
@mhidalgo-bdai
Copy link
Collaborator Author

@tcappellari-bdai @khughes-bdai FYI 6ad3c15. The OS distributed version of Protobuf (3.12.4) does not have protobuf::RepeatedField<T>::Assign().

@coveralls
Copy link

coveralls commented Oct 17, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 11391342201

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 51.043%
Totals Coverage Status
Change from base Build 11373703153: 0.04%
Covered Lines: 1933
Relevant Lines: 3787
💛 - Coveralls

khughes-bdai
khughes-bdai reviewed Oct 17, 2024
View reviewed changes
spot_driver/CMakeLists.txt
@@ -85,7 +86,7 @@ target_include_directories(spot_api PUBLIC
$<INSTALL_INTERFACE:include>
)

target_link_libraries(spot_api PUBLIC bosdyn::bosdyn_client_static)
target_link_libraries(spot_api PUBLIC bosdyn::bosdyn_client)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this same change also needed for the ros2 control package?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch. Done in 93d3b30.

khughes-bdai
khughes-bdai reviewed Oct 17, 2024
View reviewed changes
spot_driver/CMakeLists.txt
Comment on lines -3 to -4
list(APPEND CMAKE_PREFIX_PATH /opt/spot-cpp-sdk)

Copy link
Collaborator

Choose a reason for hiding this comment

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

this line is also in the spot_ros2_control cmake and probably could also be removed if it's not needed anymore

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done in 93d3b30.

khughes-bdai
khughes-bdai approved these changes Oct 17, 2024
View reviewed changes
Copy link
Collaborator

@khughes-bdai khughes-bdai left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM! manually ran install script to successfully upgrade packages on my laptop, and successfully ran tests on robot in both spot_driver and spot_ros2_control

@mhidalgo-bdai Graphite App
Copy link
Collaborator Author

mhidalgo-bdai commented Oct 17, 2024 •
edited
Loading

Merge activity

  • Oct 17, 5:22 PM EDT: A user started a stack merge that includes this pull request via Graphite.
  • Oct 17, 5:22 PM EDT: A user merged this pull request with Graphite.

@mhidalgo-bdai mhidalgo-bdai merged commit 6e8af7a into main Oct 17, 2024
3 checks passed
@mhidalgo-bdai mhidalgo-bdai deleted the mhidalgo-bdai/bump-spot-sdk branch October 17, 2024 21:23
@mhidalgo-bdai mhidalgo-bdai mentioned this pull request Oct 18, 2024
Merged
1 task
AngelRodriguez8008 added a commit to AngelRodriguez8008/spot_ros2 that referenced this pull request Oct 26, 2024
@AngelRodriguez8008
Merge branch 'main' into dev/angels_changes
507bc66 
* main: (23 commits)
  [dependabot] Bump spot_wrapper from `61c2574` to `76e522b` (bdaiinstitute#517)
  [SW-1483] Reorganize ROS 2 control packages (bdaiinstitute#511)
  [dependabot] Bump ros_utilities from `06077a2` to `23a79c9` (bdaiinstitute#513)
  [dependabot] Bump spot_wrapper from `f410423` to `61c2574` (bdaiinstitute#514)
  [dependabot] Bump spot_wrapper from `5c01ea7` to `f410423` (bdaiinstitute#512)
  Bump Spot SDK from `4.0.2` to `4.1.0` (bdaiinstitute#495)
  [MAPLE-743] Add hand posing non blocking option (bdaiinstitute#510)
  [SW-1481] Fix namespacing with spot ros2 control examples  (bdaiinstitute#505)
  [dependabot] Bump ros_utilities from `6ed50d7` to `06077a2` (bdaiinstitute#503)
  [MAPLE-743] Add Cartesian arm pose commands (bdaiinstitute#501)
  [SW-1465] Fix image stitching for greyscale images (bdaiinstitute#499)
  [SW-1391] controller that lets you specify position, velocity, and load at the same time  (bdaiinstitute#493)
  [dependabot] Bump ros_utilities from `e4f6138` to `6ed50d7` (bdaiinstitute#498)
  [dependabot] Bump ros_utilities from `2dc3c6b` to `e4f6138` (bdaiinstitute#496)
  [dependabot] Bump spot_wrapper from `2eec3ab` to `bde75c2` (bdaiinstitute#491)
  Updated navigateTo interface (bdaiinstitute#487)
  [MAPLE-663] Spot Gripper Angle Service (bdaiinstitute#488)
  [dependabot] Bump ros_utilities from `2d3e031` to `2dc3c6b` (bdaiinstitute#490)
  [dependabot] Bump ros_utilities from `e93bfc9` to `2d3e031` (bdaiinstitute#489)
  [SW-1394] add option for odom/fiducial/camera publishing from ros2 control launchfile (bdaiinstitute#485)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amessing-bdai amessing-bdai amessing-bdai left review comments

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

@tcappellari-bdai tcappellari-bdai tcappellari-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

@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

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

Assignees

@mhidalgo-bdai mhidalgo-bdai

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mhidalgo-bdai @khughes-bdai @coveralls @amessing-bdai @tcappellari-bdai