This PR adds the class 'DQ_SerialVrepRobot' #108

ffasilva · 2024-01-26T16:58:46Z

Main instructions

By submitting this pull request, you automatically agree that you have read and accepted the following conditions:

Anyone wanting to propose a new modification or introduce new functionality should reach out to the team first, as proposed modifications that do not comply with the library's development philosophy and style, do not follow the library's architecture, do not introduce a clear and general benefit to the library other than to the person who proposed the modification will likely be rejected with no further discussion.
Support for DQ Robotics is given voluntarily and it is not the developers' role to educate and/or convince anyone of their vision.
Any possible response and its timeliness will be based on the relevance, accuracy, and politeness of a request and the following discussion.
You are familiar with the development workflow.
Each pull request should contain only individual changes (several changes of the same type are allowed on the same pull request).
Refactoring or modifying internal implementation that is working is not allowed unless comprehensively discussed with and approved by @bvadorno and @mmmarinho.

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, and YouBotVrepRobot for compatibility with the new signatures imposed by DQ_SerialVrepRobot.

An example of use is given in 9.

Rationale

The class DQ_SerialVrepRobot already exists in the C++ library but was missing on MATLAB.

…pRobot' as required by 'DQ_SerialVrepRobot'.

…ity with the class 'DQ_SerialVrepRobot'.

…d 'DQ_SerialVrepRobot'.

…ethods on the class header.

ffasilva · 2024-06-21T16:00:17Z

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 get_q_from_vrep and send_q_to_vrep (which I'll have to update in the third lesson of the tutorial), they only work for scenes with dynamics disabled because they use the vrep_interface.set_joint_positions method for actuating on the robots.

The fourth lesson of the tutorial is about dynamic simulations and methods such as set_joint_target_positions, get_joint_velocities, etc. I could do some weird workaround of submitting a new pull request just updating LBR4pVrepRobot and YouBotVrepRobot to continue with the tutorials, but this fix is already covered by ace84af in this pull request.

Kind regards,
Frederico

bvadorno · 2024-06-23T16:54:35Z

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,
Bruno

mmmarinho · 2024-06-24T06:38:38Z

@bvadorno sure!

@juanjqo are you taking a look at this one first or should I?

ffasilva · 2024-06-24T14:46:23Z

Hi @bvadorno,

@ffasilva, could you please clarify if this modification will break the second lesson of the CoppeliaSim-with-DQ-Robotics tutorial?

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,
Frederico

juanjqo · 2024-06-24T16:46:48Z

@bvadorno sure!

@juanjqo are you taking a look at this one first or should I?

@mmmarinho could you review it first please? Currently, I don't have access to a Matlab license.

mmmarinho

@ffasilva

Thanks for this. I've written a few comments about the suggested changes.

interfaces/vrep/DQ_SerialVrepRobot.m

interfaces/vrep/DQ_VrepRobot.m

mmmarinho · 2024-07-02T11:39:41Z

interfaces/vrep/robots/YouBotVrepRobot.m

@@ -17,8 +17,8 @@
 %       will become "youBot#0", a third robot, "youBot#1", and so on.
 %
 %   YouBotVrepRobot Methods:
-%       send_q_to_vrep - Sends the joint configurations to VREP


Careful removing methods, we need at least one version of backwards compatibility

Backwards compatibility added in a02ad1b.

In the review view, this is still deleted and not added anywhere else. Maybe I'm doing something wrong?

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(); end

As 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_vrep

yields 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.0001

That's the desired result, right? The deprecated method send_q_to_vrep works similarly.

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

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.

Thanks, @ffasilva. Your reasoning is sensible as you have minimized the number of modifications, and the example still runs. Therefore, your solution is acceptable.

…ep()' method.

…he methods's comments.

…e comments.

…_vrep()' and 'get_q_from_vrep()'.

…opyright's year.

ffasilva · 2024-07-23T14:46:37Z

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,
Frederico

mmmarinho

Hi @ffasilva. Thank you, I've added some comments.

mmmarinho · 2024-07-26T13:29:00Z

interfaces/vrep/robots/YouBotVrepRobot.m


+            %% 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 are some ways to fix this but how about

youBot` does not follow the standard naming convention of/in CoppeliaSim.

Fixed in fa35f1b.

mmmarinho · 2024-07-26T13:30:08Z

interfaces/vrep/robots/YouBotVrepRobot.m

@@ -17,8 +17,8 @@
 %       will become "youBot#0", a third robot, "youBot#1", and so on.
 %
 %   YouBotVrepRobot Methods:
-%       send_q_to_vrep - Sends the joint configurations to VREP


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

Please implement the requested changes.

bvadorno · 2024-08-15T13:04:06Z

interfaces/vrep/DQ_VrepRobot.m


-% (C) Copyright 2020 DQ Robotics Developers
+% (C) Copyright 2018-2024 DQ Robotics Developers


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.)

Done in 107be2c.

bvadorno · 2024-08-15T13:16:07Z

interfaces/vrep/DQ_SerialVrepRobot.m

+%           >> 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();


Let's change it to get_configuration() and deprecate get_configuration_space_positions().

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.

@ffasilva nothing only the master branch needs to be deprecated. Only what ended up in a release version.

bvadorno · 2024-08-15T13:16:51Z

interfaces/vrep/DQ_SerialVrepRobot.m

+%
+%   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.


Let's change it to get_configuration() and deprecate get_configuration_space_positions().

Comment updated in bee94d7.

bvadorno · 2024-08-15T13:17:09Z

interfaces/vrep/DQ_SerialVrepRobot.m

+%
+%   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.


Let's change it to set_configuration() and deprecate set_configuration_space_positions().

Updated comment in c9ef19e.

bvadorno · 2024-08-15T13:17:44Z

interfaces/vrep/DQ_SerialVrepRobot.m

+%       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.


set_target_configuration()

Updated in 5099cb7.

bvadorno · 2024-08-15T13:24:23Z

interfaces/vrep/robots/LBR4pVrepRobot.m

@@ -8,7 +8,7 @@
 %           >> vi.connect('127.0.0.1',19997);
 %           >> vrep_robot = LBR4pVrepRobot("LBR4p", vi);
 %           >> vi.start_simulation();
-%           >> robot.get_q_from_vrep();
+%           >> robot.get_configuration_space_positions();


Please change the methods' names according to my comments on the other files. The legacy names should be deprecated.

Updated in ecab5b5.

bvadorno · 2024-08-15T13:25:52Z

interfaces/vrep/robots/YouBotVrepRobot.m

@@ -8,7 +8,7 @@
 %           >> vi.connect('127.0.0.1',19997);
 %           >> vrep_robot = YouBotVrepRobot("youBot", vi);
 %           >> vi.start_simulation();
-%           >> robot.get_q_from_vrep();
+%           >> robot.get_configuration_space_positions();


Same as in the other files.

Updated in 05873d5.

bvadorno · 2024-08-15T13:29:34Z

interfaces/vrep/robots/YouBotVrepRobot.m

@@ -17,8 +17,8 @@
 %       will become "youBot#0", a third robot, "youBot#1", and so on.
 %
 %   YouBotVrepRobot Methods:
-%       send_q_to_vrep - Sends the joint configurations to VREP


Thanks, @ffasilva. Your reasoning is sensible as you have minimized the number of modifications, and the example still runs. Therefore, your solution is acceptable.

bvadorno · 2024-08-15T13:30:13Z

interfaces/vrep/robots/YouBotVrepRobot.m

@@ -17,11 +17,11 @@
 %       will become "youBot#0", a third robot, "youBot#1", and so on.
 %
 %   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


Same as before: please change the name and deprecate legacy names.

Updated in 8b53f6d.

bvadorno · 2024-08-15T13:30:23Z

interfaces/vrep/robots/YouBotVrepRobot.m

 %       kinematics - Obtains the DQ_Kinematics implementation of this robot

-% (C) Copyright 2020 DQ Robotics Developers
+% (C) Copyright 2018-2024 DQ Robotics Developers


Updated in badb11a.

… method.

…ration' method.

…velocities' method.

…ration_velocities' method.

…plemented in this pull request.

…et_configuration'.

bvadorno

Hi @ffasilva,

Please create a pull request for the C++ version to ensure compatibility with the new names.

Thanks!
Bruno

ffasilva added 3 commits January 26, 2024 15:29

Added the class 'DQ_SerialVrepRobot'. Also modified the class 'DQ_Vre…

b3a97f6

…pRobot' as required by 'DQ_SerialVrepRobot'.

Updated classes 'LBR4pVrepRobot' and 'YouBotVrepRobot' for compatibil…

ace84af

…ity with the class 'DQ_SerialVrepRobot'.

Fixed a problem with joint naming on the classes 'YouBotVrepRobot' an…

7afe99f

…d 'DQ_SerialVrepRobot'.

ffasilva mentioned this pull request Jan 26, 2024

This PR adds an example of use of the class 'DQ_SerialVrepRobot' with a KUKA LBR4 robot dqrobotics/matlab-examples#9

Open

[DQ_SerialVrepRobot] Improved documentation of DQ_SerialVrepRobot's m…

0588c2a

…ethods on the class header.

bvadorno requested a review from juanjqo April 3, 2024 13:56

ffasilva mentioned this pull request Jun 21, 2024

From zero to hero: Static World (part 2) dqrobotics/learning-dqrobotics-with-coppeliasim#4

Draft

bvadorno assigned bvadorno and mmmarinho Jun 23, 2024

mmmarinho self-requested a review June 24, 2024 06:37

ffasilva mentioned this pull request Jun 24, 2024

CoppeliaSim interface with DQ Robotics: From zero to hero dqrobotics/learning-dqrobotics-with-coppeliasim#2

Open

mmmarinho requested changes Jul 2, 2024

View reviewed changes

ffasilva added 11 commits July 23, 2024 13:37

[DQ_SerialVrepRobot.m] Removed old comment refering to 'get_q_from_vr…

0fd50ac

…ep()' method.

[DQ_SerialVrepRobot.m] Updated copyright's year.

ef38693

[DQ_SerialVrepRobot.m] Added reference to C++ version.

217f7cd

[DQ_SerialVrepRobot.m] Text fixes in the 'set' methods comments.

aa73594

[DQ_SerialVrepRobot.m] Updated specifications of the joint modes in t…

3c4d66e

…he methods's comments.

[DQ_SerialVrepRobot.m] Fixed copy and paste gore in the methods' usag…

e0f1a98

…e comments.

[DQ_VrepRobot.m] Added backwards compatibility for methods 'send_q_to…

a02ad1b

…_vrep()' and 'get_q_from_vrep()'.

[LBR4pVrepRobot.m] Updated usage and methods sections. Also updated c…

33147de

…opyright's year.

[LBR4pVrepRobot.m] Updated copyright's year.

ec0901f

[YouBotVrepRobot.m] Updated usage section and copyright's year.

de2dc64

[DQ_VrepRobot.m] Updated methods section and copyright's year.

1a2b767

ffasilva requested a review from mmmarinho July 23, 2024 14:50

mmmarinho requested changes Jul 26, 2024

View reviewed changes

[YouBotVrepRobot.m] Small text fix in the constructor's comments.

fa35f1b

ffasilva requested a review from mmmarinho July 29, 2024 10:34

[YouBotVrepRobot.m] Fixed small wrong number of joints and missing cu…

7e30416

…stom 'base_frame_name' in the constructor.

ffasilva mentioned this pull request Aug 6, 2024

[REQUEST] Update example "vrep/constrained_kinematic_control" to use the class LBR4pVrepRobot dqrobotics/matlab-examples#11

Open

bvadorno self-requested a review August 15, 2024 12:54

bvadorno requested changes Aug 15, 2024

View reviewed changes

ffasilva added 16 commits August 15, 2024 18:05

[DQ_SerialVrepRobot.m] Updated copyright date.

107be2c

[DQ_SerialVrepRobot.m] Updated usage comments.

c934c9c

[DQ_SerialVrepRobot.m] Updated comment describing 'get_configuration'…

bee94d7

… method.

[DQ_SerialVrepRobot.m] Updated comment describing 'set_configuration'…

c9ef19e

… method.

[DQ_SerialVrepRobot.m] Updated comment describing 'set_target_configu…

5099cb7

…ration' method.

[DQ_SerialVrepRobot.m] Updated comment describing 'get_configuration_…

c3356ae

…velocities' method.

[DQ_SerialVrepRobot.m] Updated comment describing 'set_target_configu…

6ac8182

…ration_velocities' method.

[DQ_SerialVrepRobot.m] Removed the comment describing a method not im…

be6a446

…plemented in this pull request.

[DQ_SerialVrepRobot.m] Renamed methods' names as requested by Bruno.

8a60f03

[DQ_VrepRobot.m] Renamed methods' as requested by Bruno.

0cf51a6

[LBR4pVrepRobot.m] Updated copyright.

17f4506

[LBR4pVrepRobot.m] Updated usage comments.

ecab5b5

[YouBotVrepRobot.m] Renamed 'get_configuration_space_positions' to 'g…

05873d5

…et_configuration'.

[YouBotVrepRobot.m] Renamed 'set_configuration_space_positions' to 's…

8b53f6d

…et_configuration'.

[YouBotVrepRobot.m] Updated copyright.

badb11a

Merge branch 'dqrobotics:master' into dq-serial-vrep-robot

4778488

ffasilva requested a review from bvadorno August 15, 2024 21:54

bvadorno approved these changes Aug 16, 2024

View reviewed changes

bvadorno merged commit 1eb3459 into dqrobotics:master Aug 16, 2024
1 of 2 checks passed

ffasilva deleted the dq-serial-vrep-robot branch August 16, 2024 09:46

bvadorno approved these changes Aug 16, 2024

View reviewed changes


		%% 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


		% (C) Copyright 2020 DQ Robotics Developers
		% (C) Copyright 2018-2024 DQ Robotics Developers

This PR adds the class 'DQ_SerialVrepRobot' #108

This PR adds the class 'DQ_SerialVrepRobot' #108

Conversation

ffasilva commented Jan 26, 2024 • edited Loading

Main instructions

Description of changes

Rationale

ffasilva commented Jun 21, 2024

bvadorno commented Jun 23, 2024

mmmarinho commented Jun 24, 2024

ffasilva commented Jun 24, 2024

juanjqo commented Jun 24, 2024

mmmarinho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ffasilva Aug 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ffasilva commented Jul 23, 2024

mmmarinho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bvadorno left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bvadorno left a comment

Choose a reason for hiding this comment

ffasilva commented Jan 26, 2024 •

edited

Loading

ffasilva Aug 6, 2024 •

edited

Loading