dfki-ric-underactuated-lab / torque_limited_simple_pendulum

Torque Limited Simple Pendulum Underactuated System
BSD 3-Clause "New" or "Revised" License
54 stars 19 forks source link

Fixed Control law error in LQRController #24

Closed Haricharan1212 closed 1 year ago

Haricharan1212 commented 1 year ago

Bugs:

Changes Made:

Testing:

shivesh1210 commented 1 year ago

Thanks for the PR. Delta y is not needed because we perform a change of coordinates around that configuration. If you choose to do so, you have to also correct your L133 where the expression on LHS should be Delta y S Delta y.

Haricharan1212 commented 1 year ago

It is updated now

Haricharan1212 commented 1 year ago

Sir,

I have updated the function now. The class is now more modular and has more features. Please look into merging the changes.

Regards, Haricharan B, Second Year Undergraduate, Indian Institute of Technology, Madras

On Mon, Dec 26, 2022 at 12:05 AM Shivesh Kumar @.***> wrote:

Thanks for the PR. Delta y is not needed because we perform a change of coordinates around that configuration. If you choose to do so, you have to also correct your L133 where the expression on LHS should be Delta y S Delta y.

— Reply to this email directly, view it on GitHub https://github.com/dfki-ric-underactuated-lab/torque_limited_simple_pendulum/pull/24#issuecomment-1364722711, or unsubscribe https://github.com/notifications/unsubscribe-auth/AWI4KLTT5KE3DT2ROV7KC53WPCHY5ANCNFSM6AAAAAATI24PHE . You are receiving this because you authored the thread.Message ID: <dfki-ric-underactuated-lab/torque_limited_simple_pendulum/pull/24/c1364722711 @github.com>

fwiebe commented 1 year ago

Hi @Haricharan1212 Thanks for your PR. I have the following comments to your changes:

  1. I think there is a mistake in the A matrix. If you introduce inertia and evaluate the matrix at an arbitrary goal, I think the lower left entry should be

    -self.mass*self.g*self.len / self.moment_of_inertia*np.cos(self.goal[0])

    instead of:

    -self.g * np.cos(self.goal[0])/(2 * self.len)

  2. The angle wrapping is missing. The position error should be wrapped so that there is no discontinuity around the goal position. The angle should be wrapped like

    delta_pos = pos - self.goal[0] delta_pos_wrapped = (delta_pos + np.pi) % (2*np.pi) - np.pi

  3. I would propose to rename the variable moment_of_inertia to simply inertia. This is then consistent with the other controllers and the pendulum plant.

If you can address these comments, I will gladly merge your PR.

Best Regards

Haricharan1212 commented 1 year ago

Hi,

I think I've addressed the above comments.

Best Regards