Proposed modifications to dev

[dogoepp]

Notes:

• maybe we should not throw generic Errors. If there is no matching error type, let us make one.
• set_goal_positions lacks checks on the actuator max and min angles.
• furthermore, if this method is specific to actuators using Protocol 2, it should be in protocol_specific_packets.hpp

In relation to #61. @PedroDesRobots, comments on these three points ? May I merge ?

[PedroDesRobots]

• The error matches correctly error type (concerning vector size )
• In my next commit, I will add max and min angles check. For the method set_goal_positions, take care when you rename it because different tools already use this function... so I don't think that it's a good idea to change the name. We can made this hack :

  template <typename Id, typename Pos> static InstructionPacket set_goal_positions_angle(const std::vector& ids, const std::vector& positions) { return set_goal_positions<Id,Pos>(ids,positions) }

Like this we got our function name set_goal_positions_angle but we dont break others implementation which use the old function (set_goal_positions)

• This method is unfortunatly not specific at protocol 2 because MX28 with protocol 1 are able to use it too.

We could merge a more consistent dev version :+1:

[dogoepp]

• The error matches correctly error type (concerning vector size )

I was thinking about adding an exception type for "vector sizes that do not match". Does it make sense to you ? We seem to be having more and more of these, since bulk and sync commands came around.

• For the method set_goal_positions, take care when you rename it because different tools already use this function... so I don't think that it's a good idea to change the name. We can made this hack :

template <typename Id, typename Pos> static InstructionPacket set_goal_positions_angle(const std::vector& ids, const std::vector& positions) { return set_goal_positions<Id,Pos>(ids,positions) }

Like this we got our function name set_goal_positions_angle but we dont break others implementation which use the old function (set_goal_positions)

The method set_goal_angles was first written in commit cd572d8e11 and the commit 17827a40440eb changed its behavior. This is the commit that broke the API stability and the current branch is only reverting it back to what it was. I believe that it is also more coherent with the rest of the similar methods of servo: they get _angle appended when their input is meant to be in radians instead of the actuator-specific unit (which I call ticks). If you agree with this, I'll change the only call there is to this method, at line 437 of utility.hpp.

• This method is unfortunatly not specific at protocol 2 because MX28 with protocol 1 are able to use it too.

Tricky problem. I'll have to think, as it would be best if the library would not let you compile or at least would complain at run time if the method is used with the wrong kind of actuator.

We could merge a more consistent dev version +1

Do you mean that this branch good to merge for you, or do you mean that it needs changes ?

[PedroDesRobots]

@dogoepp,

• "vector sizes mismatch" takes its sense since bulk and sync. I obviously agree to add it.

• I understand. I let you change the method name in utility.hpp : set_goal_positions(old) -> set_goal_positions_angle(new) @Aneoshun takes care of this change, it will probably affect your code.

• Concerning the branch, it needs changes (don't merge it yet).

[dogoepp]

The two requested changes were made. I'll carry on with my review and further modifications. Please let me know what need to be done before merging in dev.

Also, it would be nice if we could test these changes either with real actuators or with unit a test base.

[dogoepp]

Any news @PedroDesRobots?

[PedroDesRobots]

I will merge dev branch and dev-dogoepp, principally for your VecotrEmptyError and VectorSizesDifferError classes for commit https://github.com/resibots/libdynamixel/pull/62/commits/380319afa4bb3b03db41c06d60141d8805989bf3 and commits https://github.com/resibots/libdynamixel/pull/62/commits/ca8b2a5fc0c0eb3b08a0686b49df7921c1d73391 will be replaced by abstraction method. . In #61, I've done some commit and solved the abstraction model for bulk and sync instructions, then no more tricks!