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

[BUG] MATLAB dq_kinematics_plot modifying the object #107

Open
ffasilva opened this issue Nov 3, 2023 · 1 comment
Open

[BUG] MATLAB dq_kinematics_plot modifying the object #107

ffasilva opened this issue Nov 3, 2023 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@ffasilva
Copy link
Member

ffasilva commented Nov 3, 2023


Code of Conduct

By submitting this report you automatically agree that you've read and accepted the following conditions.

  • Support for DQ Robotics is given voluntarily and it's 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.
  • If a DQ Robotics member replies, the user must let them know if their response solves their issue or not.
  • Any suggestion/advice/request made by anyone, as well intentioned as they might be, might not be incorporated into DQ Robotics.

Discussed in #103

Bug description

In brief, dq_kinematics_plot(robot,q) function on MATLAB is modifying the received object robot. Namely, it sets robot.q and forces it to be a row vector.

To Reproduce

Run the following:

Code

ax18 = Ax18ManipulatorRobot.kinematics();
ax18.q
q = [0 0 0 0 0]';
plot(ax18, q);
ax18.q

Output

ans =
[]

ans =
0 0 0 0 0 

Expected behavior

  • The plot function should not have altered robot.q.

Expected output

ans =
[]

ans =
[]

Environment:

  • OS: Ubuntu 20.04
  • dqrobotics version 20.04.0.1
  • MATLAB version 2023b
@ffasilva ffasilva added the bug Something isn't working label Nov 3, 2023
@bvadorno bvadorno self-assigned this Nov 3, 2023
@bvadorno
Copy link
Member

Hi, @ffasilva.

This might be a good time to pick this one, but you'll need to improve the description of this issue. I understand what you meant generally because we talked asynchronously via other communication channels, but it's not good enough as the description of this issue must be precise and self-contained.

The method dq_kinematics_plot does not exist. Please include the standard description ClassName.method(). The method, in this case, is plot(). You must specify the class; otherwise, you'll force the person handling the bug to dig into the library because of this poor description because there are many plot methods across many classes.

Secondly, although I agree that in this case, the plot function shouldn't change the value of q, this is not necessarily a conceptual problem in OOP, and you must be careful not to conflate concepts, as I'm aware that you are primarily concerned with encapsulation in this case.

The first parameter in MATLAB methods is the object owning that method. For example, my_method(obj, q) is equivalent to obj.my_method(q). Therefore, if q is an attribute of obj and private, my_method can still change the value of q. This is conceptually similar to a setter method. As a matter of fact, if you let my_method = set, this is precisely the behavior you would expect. On the other hand, this wouldn't be true if q was an attribute of a superclass and my_method belonged to the class unless q was protected or public.

This is why specifying the class in your issue is essential. Also, please add brief information explaining why q shouldn't be modified by the plot method (unsafe, side effects, etc). Friendly warning: there is no point in ranting or claiming that q should be private or protected because we decided elsewhere that we wouldn't change it now due to side effects.

After you improve the description of this issue, feel free to start a pull request to fix it.

Kind regards,
Bruno

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants