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

RSDK-1232 remove pb.JointPositions from Arm interface #4502

Merged
merged 7 commits into from
Oct 30, 2024

Conversation

raybjork
Copy link
Member

@raybjork raybjork commented Oct 29, 2024

This ticket resolves some tech debt we've been putting off for a couple years. Presently, the armpb.JointPositions protobuf struct is being used in the Arm interface. The issue with this is that we have to deal with the conversion from this type to the Input type which is the type we use throughout the motion stack in multiple places rather than once at ingestion of the structure. In order to properly make this conversion we need to use information from the arm kinematics structure to determine how to process the raw floating point numbers.

Because the arm's Kinematics endpoint might not be implemented correctly, or at all extra guardrails were implemented in this PR:

  • asking to move the arm to joint positions of a length that do not match the kinematics will now result in an error
  • implementing the model frame as nil will now produce a warning when instantiating arm clients, indicating to users that the armpb.JointPositions struct will be treated as degrees and converted to radians while inside the context of the Arm interface

Overall, these changes should make things more type safe and incentivize users to properly use the kinematics endpoint of the arm when building modules. Finally, it paves the way for a simplification of the InputEnabled interface and addition of the upcoming MoveThroughJointPositions arm method

Note: This is a breaking change to the Arm package and after merging this, Viam supported arm modules written in Go will need to be updated to reflect this change.

@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Oct 29, 2024
Copy link
Contributor

Warning your change may break code samples. If your change modifies any of the following functions please contact @viamrobotics/fleet-management. Thanks!

component function
base IsMoving
board GPIOPinByName
camera Properties
encoder Properties
motor IsMoving
sensor Readings
servo Position
arm EndPosition
audio MediaProperties
gantry Lengths
gripper IsMoving
input_controller Controls
movement_sensor LinearAcceleration
power_sensor Power
pose_tracker Poses
motion GetPose
vision GetProperties

@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Oct 29, 2024
@cheukt cheukt removed their request for review October 29, 2024 18:43
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Oct 29, 2024
Copy link
Member

@biotinker biotinker left a comment

Choose a reason for hiding this comment

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

LGTM

@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Oct 30, 2024
@raybjork raybjork changed the title RSDK-4120 remove pb.JointPositions from Arm interface RSDK-1232 remove pb.JointPositions from Arm interface Oct 30, 2024
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Oct 30, 2024
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Oct 30, 2024
Copy link
Contributor

Availability

Scene # viamrobotics:main raybjork:RSDK-4120 Percent Improvement Health
1 100% 100% 0%
2 100% 100% 0%
3 100% 100% 0%
4 100% 100% 0%
5 90% 90% 0%
6 70% 60% -14%
7 30% 30% 0%
8 100% 100% 0%
9 100% 100% 0%
10 100% 100% 0%
11 100% 100% 0%
12 100% 100% 0%
13 100% 100% 0%
14 100% 100% 0%
15 100% 100% 0%
16 100% 100% 0%
17 100% 100% 0%
18 40% 40% 0%

Quality

Scene # viamrobotics:main raybjork:RSDK-4120 Percent Improvement Probability of Improvement Health
1 1.31±0.00 1.31±0.00 -0% 50%
2 0.90±0.00 0.90±0.00 -0% 50%
3 3.49±1.60 3.49±1.60 -0% 50%
4 3.66±1.07 4.16±1.14 -14% 37%
5 14.86±6.45 14.60±6.70 2% 51%
6 14.37±5.53 15.57±4.43 -8% 43%
7 5.51±1.57 4.59±1.94 17% 64%
8 4.61±0.32 4.61±0.32 -0% 50%
9 4.41±0.44 4.41±0.44 0% 50%
10 4.43±0.46 4.40±0.45 1% 52%
11 5.54±0.72 5.55±0.72 -0% 50%
12 3.96±1.11 3.96±1.11 -0% 50%
13 904.77±13.78 904.77±13.78 -0% 50%
14 2129.60±711.35 2129.60±711.35 -0% 50%
15 50549.88±4594.49 50549.88±4594.49 -0% 50%
16 54723.04±11672.98 54723.04±11672.98 -0% 50%
17 15859.97±5452.61 15859.97±5452.61 -0% 50%
18 135263.45±33918.29 134306.11±34440.42 1% 51%

Performance

Scene # viamrobotics:main raybjork:RSDK-4120 Percent Improvement Probability of Improvement Health
1 0.08±0.00 0.09±0.00 -1% 43%
2 0.16±0.01 0.16±0.01 0% 51%
3 0.54±0.26 0.55±0.26 -0% 50%
4 1.96±0.12 1.99±0.11 -1% 43%
5 2.93±0.88 2.86±0.93 2% 52%
6 3.06±1.08 3.03±1.14 1% 51%
7 2.06±0.17 2.05±0.16 1% 52%
8 0.27±0.14 0.27±0.14 1% 50%
9 4.98±0.23 4.97±0.23 0% 52%
10 0.13±0.02 0.14±0.02 -4% 43%
11 0.10±0.01 0.10±0.01 -3% 38%
12 0.12±0.01 0.12±0.01 -1% 47%
13 0.08±0.01 0.08±0.01 -1% 47%
14 0.72±0.23 0.73±0.24 -1% 49%
15 1.46±0.25 1.47±0.25 -0% 49%
16 2.44±1.29 2.45±1.29 -0% 50%
17 1.51±0.27 1.52±0.25 -1% 49%
18 4.69±0.58 4.68±0.57 0% 50%

The above data was generated by running scenes defined in the motion-testing repository
The SHA1 for viamrobotics:main is: 900738751a11aa6be03dd90a3932f38e1c35f187
The SHA1 for raybjork:RSDK-4120 is: 900738751a11aa6be03dd90a3932f38e1c35f187

  • 10 samples were taken for each scene
  • A timeout of 5.0 seconds was imposed for each trial

@raybjork raybjork merged commit 0ba8458 into viamrobotics:main Oct 30, 2024
19 checks passed
@raybjork raybjork deleted the RSDK-4120 branch October 30, 2024 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to test This pull request is marked safe to test from a trusted zone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants