This PR adds the class 'DQ_SerialVrepRobot'#108
Conversation
…pRobot' as required by 'DQ_SerialVrepRobot'.
…ity with the class 'DQ_SerialVrepRobot'.
…d 'DQ_SerialVrepRobot'.
…ethods on the class header.
|
Hi @dqrobotics/developers, Could we please prioritize this pull request? I cannot do the fourth, and last, lesson of the learning-dqrobotics-with-coppeliasim tutorials without it being merged into the master. The only robots available in the public DQ Robotics interface with CoppeliaSim are LBR4pVrepRobot and YouBotVrepRobot. Besides having the obsolete methods of The fourth lesson of the tutorial is about dynamic simulations and methods such as Kind regards, |
|
Hi @mmmarinho, Can you please take this one? As you designed the original classes, you are better positioned to review the new classes. @ffasilva, could you please clarify if this modification will break the second lesson of the CoppeliaSim-with-DQ-Robotics tutorial? Kind regards, |
|
Hi @bvadorno,
I don't see how it would. Lesson 2 doesn't use any robotic manipulator. However, this pull request will break the ending of lesson 3, hence why I left it as a draft. Kind regards, |
@mmmarinho could you review it first please? Currently, I don't have access to a Matlab license. |
| % will become "youBot#0", a third robot, "youBot#1", and so on. | ||
| % | ||
| % YouBotVrepRobot Methods: | ||
| % send_q_to_vrep - Sends the joint configurations to VREP |
There was a problem hiding this comment.
Careful removing methods, we need at least one version of backwards compatibility
There was a problem hiding this comment.
In the review view, this is still deleted and not added anywhere else. Maybe I'm doing something wrong?
There was a problem hiding this comment.
In a02ad1b I added backwards compatibility to both send_q_to_vrep and get_q_from_vrep to the class DQ_VrepRobot, where they used to be abstract methods.
function send_q_to_vrep(obj, q)
% For backwards compatibility only. Do not use this method.
warning('Deprecated. Use set_configuration_space_positions() instead.')
obj.set_configuration_space_positions(q)
end
function q = get_q_from_vrep(obj)
% For backwards compatibility only. Do not use this method.
warning('Deprecated. Use get_configuration_space_positions() instead.')
q = obj.get_configuration_space_positions();
endAs such, the use of something similar to
vi = DQ_VrepInterface();
vi.connect('127.0.0.1',19997);
vrep_robot = YouBotVrepRobot("youBot", vi);
vrep_robot.get_q_from_vrepyields the result
Warning: Deprecated. Use get_configuration_space_positions() instead.
> In DQ_VrepRobot/get_q_from_vrep (line 65)
ans =
1.2068
-1.8492
3.6463
0.0001
0.5395
0.9149
1.2685
-0.0001That's the desired result, right? The deprecated method send_q_to_vrep works similarly.
There was a problem hiding this comment.
Hi, @ffasilva ,
The desired result is to leave the original code unchanged as much as possible and work the new code around it.
I understand why you chose to do it this way, but I'll condition the acceptance of this to these examples working without modification:
https://github.com/dqrobotics/matlab-examples/tree/master/vrep
There was a problem hiding this comment.
Hi @mmmarinho,
I see what you mean, but I figured that deprecating the methods in all the concrete classes that inherit from DQ_VrepRobot, rather than just doing it directly on the abstract class itself, would have generated even more modifications to the original code.
As for your request, the deprecation of the methods is working as intended. The simulation-ram-paper runs without any problems:
However, the constrained_kinematic_control example don't use the VrepRobot classes at all. Which means it obviously works, but it also doesn't help us validate my modifications. I raised an issue to update this example in the future.
Kind regards,
Frederico
P.S.: I caught two bugs in the constructor of the YouBotVrepRobot class. I fixed them here 7e30416.
There was a problem hiding this comment.
Thanks, @ffasilva. Your reasoning is sensible as you have minimized the number of modifications, and the example still runs. Therefore, your solution is acceptable.
…he methods's comments.
…_vrep()' and 'get_q_from_vrep()'.
|
Hi @mmmarinho, Thank you for the review. I implemented the requested modifications and fixed small details in the documentation. Please let me know if there is still something missing. Kind regards, |
| function obj = YouBotVrepRobot(robot_name, vrep_interface) | ||
| obj@DQ_SerialVrepRobot("youBot", 7, robot_name, vrep_interface); | ||
|
|
||
| %% youBot don't follow the standard name convention on CoppeliaSim. Also, the use of 'set_names()', as is done in the C++ implementation, is not supported on a constructor in MATLAB |
There was a problem hiding this comment.
There are some ways to fix this but how about
youBot` does not follow the standard naming convention of/in CoppeliaSim.
| % will become "youBot#0", a third robot, "youBot#1", and so on. | ||
| % | ||
| % YouBotVrepRobot Methods: | ||
| % send_q_to_vrep - Sends the joint configurations to VREP |
There was a problem hiding this comment.
In the review view, this is still deleted and not added anywhere else. Maybe I'm doing something wrong?
…stom 'base_frame_name' in the constructor.
bvadorno
left a comment
There was a problem hiding this comment.
Please implement the requested changes.
| % get_configuration_space_positions - Obtains the joint configurations from VREP | ||
|
|
||
| % (C) Copyright 2020 DQ Robotics Developers | ||
| % (C) Copyright 2018-2024 DQ Robotics Developers |
There was a problem hiding this comment.
DQ Robotics, as we know it now, started in 2011. Please amend the copyright date (remember that this is legally binding, so we must take it seriously.)
interfaces/vrep/DQ_SerialVrepRobot.m
Outdated
| % >> vi.connect('127.0.0.1',19997); | ||
| % >> vrep_robot = DQ_SerialVrepRobot("my_robot", 7, "my_robot", vi); | ||
| % >> vi.start_simulation(); | ||
| % >> vrep_robot.get_configuration_space_positions(); |
There was a problem hiding this comment.
Let's change it to get_configuration() and deprecate get_configuration_space_positions().
There was a problem hiding this comment.
Updated comment in c934c9c. However, there is no need to deprecate get_configuration_space_positions() as this is the pull requesting proposing the first MATLAB version of DQ_SerialVrepRobot. We willl have to deprecate the methods in the C++ version, though.
There was a problem hiding this comment.
@ffasilva nothing only the master branch needs to be deprecated. Only what ended up in a release version.
interfaces/vrep/DQ_SerialVrepRobot.m
Outdated
| % | ||
| % DQ_SerialVrepRobot Methods: | ||
| % get_joint_names - Gets the joint names of the robot in the CoppeliaSim scene. | ||
| % set_configuration_space_positions - Sets the joint configurations of the robot in the CoppeliaSim scene. |
There was a problem hiding this comment.
Let's change it to get_configuration() and deprecate get_configuration_space_positions().
interfaces/vrep/DQ_SerialVrepRobot.m
Outdated
| % | ||
| % DQ_SerialVrepRobot Methods: | ||
| % get_joint_names - Gets the joint names of the robot in the CoppeliaSim scene. | ||
| % set_configuration_space_positions - Sets the joint configurations of the robot in the CoppeliaSim scene. |
There was a problem hiding this comment.
Let's change it to set_configuration() and deprecate set_configuration_space_positions().
interfaces/vrep/DQ_SerialVrepRobot.m
Outdated
| % get_joint_names - Gets the joint names of the robot in the CoppeliaSim scene. | ||
| % set_configuration_space_positions - Sets the joint configurations of the robot in the CoppeliaSim scene. | ||
| % get_configuration_space_positions - Gets the joint configurations of the robot in the CoppeliaSim scene. | ||
| % set_target_configuration_space_positions - Sets the joint configurations of the robot in the CoppeliaSim scene as a target configuration for the joint controllers. |
| % >> vrep_robot = LBR4pVrepRobot("LBR4p", vi); | ||
| % >> vi.start_simulation(); | ||
| % >> robot.get_q_from_vrep(); | ||
| % >> robot.get_configuration_space_positions(); |
There was a problem hiding this comment.
Please change the methods' names according to my comments on the other files. The legacy names should be deprecated.
| % >> vrep_robot = YouBotVrepRobot("youBot", vi); | ||
| % >> vi.start_simulation(); | ||
| % >> robot.get_q_from_vrep(); | ||
| % >> robot.get_configuration_space_positions(); |
| % will become "youBot#0", a third robot, "youBot#1", and so on. | ||
| % | ||
| % YouBotVrepRobot Methods: | ||
| % send_q_to_vrep - Sends the joint configurations to VREP |
There was a problem hiding this comment.
Thanks, @ffasilva. Your reasoning is sensible as you have minimized the number of modifications, and the example still runs. Therefore, your solution is acceptable.
| % YouBotVrepRobot Methods: | ||
| % send_q_to_vrep - Sends the joint configurations to VREP | ||
| % get_q_from_vrep - Obtains the joint configurations from VREP | ||
| % set_configuration_space_positions - Sends the joint configurations to VREP |
There was a problem hiding this comment.
Same as before: please change the name and deprecate legacy names.
| % kinematics - Obtains the DQ_Kinematics implementation of this robot | ||
|
|
||
| % (C) Copyright 2020 DQ Robotics Developers | ||
| % (C) Copyright 2018-2024 DQ Robotics Developers |
…velocities' method.
…ration_velocities' method.
…plemented in this pull request.
…et_configuration'.
…et_configuration'.

Main instructions
By submitting this pull request, you automatically agree that you have read and accepted the following conditions:
Description of changes
Hi, @dqrobotics/developers,
I've added the the class
DQ_SerialVrepRobot. It follows the implementation of its C++ counterpart.I also had to update the classes
DQ_VrepRobot,LBR4pVrepRobot, andYouBotVrepRobotfor compatibility with the new signatures imposed byDQ_SerialVrepRobot.An example of use is given in 9.
Rationale
The class
DQ_SerialVrepRobotalready exists in the C++ library but was missing on MATLAB.