Add MoveThroughJointPositions to Arm component #324
Conversation
| @@ -103,6 +109,20 @@ class Arm : public Component, public Stoppable { | |||
| virtual void move_to_joint_positions(const std::vector<double>& positions, | |||
| const ProtoStruct& extra) = 0; | |||
|
|
|||
| /// @brief Move each joint on the arm through the positions specified in @param positions | |||
| /// @param options optional specifications to be obeyed during the motion. | |||
| inline void move_through_joint_positions(const std::vector<std::vector<double>>& positions, | |||
There was a problem hiding this comment.
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.
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.
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.
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.
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.
| *options.max_acc_degs_per_sec2); | ||
| } | ||
|
|
||
| for (const std::vector<double>& pos : positions) { |
There was a problem hiding this comment.
You can say const auto& pos here, if you like.
|
|
||
| positions.reserve(request->positions_size()); | ||
| for (const auto& values : request->positions()) { | ||
| positions.push_back({values.values().begin(), values.values().end()}); |
There was a problem hiding this comment.
This can possibly be written positions.emplace_back(values.values().begin(), values.values().end) which will avoid a move and construct the vector in place.
|
going to try to close and reopen because the license/cla task is pending |
I took a first pass at adding this new function to the sdk but don't completely understand everything that is required in doing this, so I'm hoping this can be a launching point to learn more about coding practices in this environment