-
Notifications
You must be signed in to change notification settings - Fork 27
RSDK-9291 Add MoveThroughJointPositions to Arm component #324
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
Merged
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
fd1a03b
first pass at adding new arm endpoint
raybjork 97cfba9
remove line
raybjork ae21c2c
update arm components
lia-viam 8be6465
add tests
lia-viam fb87828
Merge branch 'main' of github.com:viamrobotics/viam-cpp-sdk into move…
lia-viam 9309563
lint format
lia-viam d80a02b
remove original implementation
lia-viam ac9a0b6
change construction
lia-viam b5cf377
use add
lia-viam 16d59c6
remove unused var decl
lia-viam 30b77de
use const auto&
lia-viam 0ecd214
use emplace_back
lia-viam df26f4e
add todo comment
lia-viam 978ad1d
Merge branch 'main' into movethroughjointpos
lia-viam File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Elsewhere in the SDK, we use xtensor to represent multi-dimensional arrays. It might be worth considering doing that here, as vector<vector isn't the easiest type to work with (for instance, there is no assurance that the inner vectors are all the same length, if that constraint applies here).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this because I'm new to C++ and thought this would be an easy solution. Tensors sound much better I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have a look at what the mlmodel stuff does with xtensor. Don't be alarmed it if looks a little intense, beausec it is doing something much more complex than what you need since the dimensionality and datatypes can all vary there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made the other changes but I'm going to save this one for later as a TODO in the interest of unblocking development on this module for now so it can land in this week's release
@raybjork for posterity, is it the case that that all the inner vectors are the same length? and is that something we potentially know in advance for either this UR arm or for a particular arm device?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think technically a user would be able to send an array of arrays of different lengths, but doing so would be incorrect and should cause an error to be thrown imo. So I think using tensors here does make sense.
You could maybe derive the size of the inner vector by looking at the kinematics file but doing this is outside the scope of the SDK functions and enforcing the correct length should be left to the driver instead.