dqrobotics / matlab

The DQ Robotics library in MATLAB
https://dqrobotics.github.io
GNU Lesser General Public License v3.0
26 stars 12 forks source link

[BUG] MATLAB dq_kinematics_plot modifying the object #107

Open ffasilva opened 10 months ago

ffasilva commented 10 months ago

Code of Conduct

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

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

Expected output

ans =
[]

ans =
[]

Environment:

bvadorno commented 1 month ago

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