Skip to content
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

This PR adds methods to get an object's inertial parameters in CoppeliaSim to the class 'DQ_VrepInterface' #109

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

ffasilva
Copy link
Member

@ffasilva ffasilva commented Feb 16, 2024

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 to the class DQ_VrepInterface:

  • The properties:
    • DF_LUA_SCRIPT_API
    • ST_CHILD
  • The methods:
    • call_script_function()
    • get_center_of_mass()
    • get_mass()
    • get_inertia_matrix()

It diverges from the implementation of its C++ counterpart in the following regards:

  • DF_LUA_SCRIPT_API stores the default name (/DQRoboticsApiCommandServer) for the child script in CoppeliaSim with the LUA methods necessary to read an object's inertia parameters;
  • Method call_script_function() follows the signature of the underlying CoppeliaSim API method simxCallScriptFunction;
  • Methods get_center_of_mass() and get_inertia_matrix() pass the relative reference frame parameter as an int to method call_script_function().

An example of use is given in 10.

Rationale

The class DQ_VrepInterface already has the methods call_script_function(), get_center_of_mass(), get_mass(), and get_inertia_matrix() in the C++ library but they were missing on MATLAB.

The reasons for the divergence with the C++ implementation are:

  • It's cleaner and more transparent to have de default LUA child script name defined as a constant property of the class rather than a constant input for the methods;
  • The C++ implementation of call_script_function() scrambles the input arguments of the underlying CoppeliaSim API method simxCallScriptFunction (e.g., function_name goes from being the third argument to being the first). This is unnecessarily confusing and adds no real benefit to its usage.
  • As the methods get_center_of_mass() and get_inertia_matrix() are in their C++ implementation, they do not allow for arbitrary reference frames. More details on that are given in 10, where I discuss the modifications I've made to the default LUA script used for getting the inertial parameters of an object.

Kind regards,
Frederico

…' and methods 'call_script_function()', 'get_center_of_mass()', 'get_mass()', and 'get_inertia_matrix()'.
… DQ. Also simplified how methods 'get_center_of_mass()', 'get_mass()', and 'get_inertia_matrix()' handle their returns from 'call_script_function()'.
@juanjqo
Copy link
Member

juanjqo commented Aug 15, 2024

@ffasilva Thank you for your PR. Right now, we are trying to avoid new proposals that contribute to the divergence between the languages since those usually require a lot of time, and we are working on the new release. As discussed internally, we decided to implement the methods get_center_of_mass(), get_mass(), and get_inertia_matrix() following the current implementation of C++/Python.

@ffasilva ffasilva marked this pull request as draft August 16, 2024 09:38
@ffasilva
Copy link
Member Author

Hi @juanjqo,

I refactored get_center_of_mass(), get_mass(), and get_inertia_matrix() to have the same behavior of the C++ implementation. Namely,

vi.get_inertia_matrix(objectname);
vi.get_center_of_mass(objectname);

return the inertia matrix and the center of mass with respect to the shape frame, whereas

vi.get_inertia_matrix(objectname, vi.ABSOLUTE_FRAME);
vi.get_center_of_mass(objectname, vi.ABSOLUTE_FRAME);

return those parameters with respect to the inertial frame. The behavior is the same when using the new or old Lua API.

Let me know what you think.

Kind regards,
Frederico

@ffasilva ffasilva marked this pull request as ready for review August 29, 2024 15:07
@juanjqo
Copy link
Member

juanjqo commented Sep 13, 2024

@ffasilva thank you. Please check the comments I made in your proposed example dqrobotics/matlab-examples#10 (comment).

Cheers,

Juancho

…t_inertia_matrix()' and 'get_center_of_mass()' methods.
@ffasilva
Copy link
Member Author

Hi @juanjqo,

I fixed the bug you mentioned in 10 with d29125d.

Please let me know if you have other suggestions.

Kind regards,
Frederico

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants