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

[EK-25] Use bosdyn_msgs bundles #234

Merged
merged 22 commits into from
Mar 6, 2024
Merged

Conversation

mhidalgo-bdai
Copy link
Collaborator

Depends on bdaiinstitute/bosdyn_msgs#14. This patch updates this repository to work with the binary bundles that https://github.com/bdaiinstitute/bosdyn_msgs will release from now on. Changes are minimal (but changes nonetheless).

Tested manually by running examples against an Spot robot. I'm open to ideas on how to further test this.

@mhidalgo-bdai
Copy link
Collaborator Author

FTR CI will fail for now until a new bosdyn_msgs release is rolled out.

@mhidalgo-bdai
Copy link
Collaborator Author

mhidalgo-bdai commented Jan 29, 2024

@amessing-bdai CI fails for the same reasons it failed over at ros_utilities and here for the proto2ros_tests: the dependency between container image and test job is not there. The test job runs on an old image.

We can re-work CI here like we did for ros_utilities. WDYT?

@mhidalgo-bdai
Copy link
Collaborator Author

This now needs bdaiinstitute/spot_wrapper#90.

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 once the merge conflicts get resolved

@mhidalgo-bdai mhidalgo-bdai force-pushed the use-bosdyn_msgs-bundles branch from 62cebdb to 2ab53cf Compare March 1, 2024 13:58
@mhidalgo-bdai mhidalgo-bdai changed the title Use bosdyn_msgs bundles [EK-25] Use bosdyn_msgs bundles Mar 1, 2024
Signed-off-by: Michel Hidalgo <[email protected]>
Copy link

coveralls-official bot commented Mar 1, 2024

Pull Request Test Coverage Report for Build 8175805734

Details

  • 130 of 142 (91.55%) changed or added relevant lines in 5 files are covered.
  • 97 unchanged lines in 8 files lost coverage.
  • Overall coverage increased (+1.9%) to 45.925%

Changes Missing Coverage Covered Lines Changed/Added Lines %
spot_driver/src/robot_state/state_middleware_handle.cpp 0 1 0.0%
spot_driver/src/conversions/kinematic_conversions.cpp 86 97 88.66%
Files with Coverage Reduction New Missed Lines %
inverse_kinematics_example/send_inverse_kinematics_requests.py 1 0.0%
simple_arm_motion/arm_simple.py 1 0.0%
simple_walk_forward/walk_forward.py 1 0.0%
spot_wrapper/wrapper.py 1 53.05%
spot_wrapper/tests/test_wrapper.py 2 93.59%
spot_wrapper/spot_world_objects.py 6 79.41%
spot_driver/src/conversions/time.cpp 16 42.86%
spot_wrapper/testing/grpc.py 69 74.87%
Totals Coverage Status
Change from base Build 8173464182: 1.9%
Covered Lines: 2274
Relevant Lines: 4891

💛 - Coveralls

Signed-off-by: Michel Hidalgo <[email protected]>
Signed-off-by: Michel Hidalgo <[email protected]>
Signed-off-by: Michel Hidalgo <[email protected]>
@mhidalgo-bdai
Copy link
Collaborator Author

Local testing came out positive (thanks @khughes-bdai!). We are confident enough to move forward with this as-is.

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!

@mhidalgo-bdai mhidalgo-bdai merged commit 330d72a into main Mar 6, 2024
8 checks passed
@mhidalgo-bdai mhidalgo-bdai deleted the use-bosdyn_msgs-bundles branch March 6, 2024 17:58
marlow-fawn pushed a commit to marlow-fawn/spot_ros2 that referenced this pull request Aug 19, 2024
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