search

sangteak601 / mujoco_ros2_control

MIT License
11 stars 3 forks source link

Effort based control #5

Closed sangteak601 closed 2 months ago

sangteak601 commented 3 months ago

Addresses #4 Implementing effort based position/velocity control. PID gains are set inside ros2_control tag in URDF.

Alse addresses #6 Implementing F/T sensor

sangteak601 commented 3 months ago

@dyackzan @pac48 This is an implementation of effort based control. If you have a chance, please take a look.

sangteak601 commented 3 months ago

Apologies guys. I have accidentally pushed FT sensor feature. I will make feature branch next time.

dyackzan commented 2 months ago

Can also update this line of the README in this PR since FTS support is being implemented https://github.com/sangteak601/mujoco_ros2_control/blob/da724407216656e707a7017fa86cb5a696c4ef0d/README.md?plain=1#L35

sangteak601 commented 2 months ago

As we discussed in the meeting yesterday, it might also be worth removing the details that check for limits in this package. The MuJoCo MJCF will enforce position and effort limits for the actuators which I believe is similar to hardware behavior.

Regarding this, I have a question. In hypothetical scenario where position limit is [-1,1] and position command is -2. Would it be desirable to keep applying force on the joint? If I remove limit check, robot will keep applying force on the joint to move the joint to -2. Additionally, if I use non-zero value for I gain, it will eventually reach the max force limit.

Edit: As discussed in the meeting, applying force at the limit position will be valid action. Thus, I will remove limit check for position and velocity. I will still need effort limit check since I'm not using MuJoCo's actuators to apply force.

sangteak601 commented 2 months ago

I tested and reviewed the FTS additions and it's looking good to me! I think these changes are ready to go in.

It's probably a good idea to prioritize removing the direct joint position control next and replace it with your PID position control you implemented here as I mentioned in this issue. But you can do that in a separate PR if you want.

Thanks. I will create another PR to remove it.