Upgrade driver to support controller mode other than position control #16
Conversation
…cally_set_pid Support dynamically setting PID with SETPID
* Add velocity safety limit checker * add logic for driver communicating to client why it is cutting off
…nfig_yaml Single source of truth for yaml config
…t_zero_current_limit Remove current limit by default
| inline void WritePosition(int32_t int_pos, | ||
| int32_t moving_time_ms, | ||
| int32_t acc_time_ms) { | ||
| buffer_ptr_[2] = acc_time_ms; | ||
| buffer_ptr_[3] = moving_time_ms; | ||
| buffer_ptr_[4] = int_pos; | ||
| buffer_ptr_ += NUM_DWORDS; | ||
| } | ||
|
|
||
| inline void WriteCurrent(float tau) { | ||
| int16_t int_current = | ||
| static_cast<int16_t>(std::round(NEWTON_METER_TO_INT_CURRENT * tau)); | ||
| buffer_ptr_[0] = (int_current << 16) | PWM_LIMIT; | ||
| buffer_ptr_ += NUM_DWORDS; | ||
| } | ||
|
|
||
| inline void WritePWM(float tau) { | ||
| int16_t int_pwm = | ||
| static_cast<int16_t>(std::round(NEWTON_METER_TO_INT_PWM * tau)); | ||
| // We need to put the 2 bytes (signed) integer PWM in the position of the | ||
| // lower 2 bytes at buffer's [0]. This is how we do it. | ||
| uint16_t unsigned_pwm = static_cast<uint16_t>(int_pwm); | ||
| buffer_ptr_[0] = 0 | unsigned_pwm; | ||
| buffer_ptr_ += NUM_DWORDS; | ||
| } | ||
|
|
||
| inline void WriteVelocity(int32_t int_velocity) { | ||
| buffer_ptr_[2] = int_velocity; | ||
| buffer_ptr_ += NUM_DWORDS; | ||
| } |
There was a problem hiding this comment.
Nit: Could be consistent and perform the conversions from real units to Dynamixel units within the functions for WritePosition and WriteVelocity similar to how WriteCurrent and WritePWM are written.
There was a problem hiding this comment.
I do not like this either. I actually deliberately implemented it this way so that the command buffer does not need to know about the workbench.
For some reason dynamixel has this annoying design that converting actually need dxl_wb as input.
I still think it is slightly better to keep dxl_wb out of this class.
Agree. We will nee to test this with Jerry's checkpoint. |
breakds
dismissed stale reviews from le-horizon and QuantuMope
The merge-base changed after approval.
Recently we are planning to use torque control for the gripper motor, while keeping all the rest of the motors with position control.
Previously, only position control is supported in the "wx armor" driver. This PR extends it so that current control, velocity control and PWM control are now supported. Specifically, a flag (environment variable) is introduced to turn on "PWM control" for the gripper, which approximately resembles the torque control.
The following changes are made to implement the above idea:
joint_motorsare added so that we can directly iterate over the (main) motor of each joint.TORQUEcontrol mode is removed fromOpModebecause it is not supported by the hardware.WX_ARMOR_GRIPPER_PWM_CTRLis introduced as the flag to turn PWM control on gripper on and off.syncWriteis now writing 5 bytes on each motor instead of 3 bytes. The two extra bytes are for the goal PWM, goal current and goal velocity.SetPositionis renamed toSendCommandas it is no longer position control only.SendCommandandInitWriteHandleris clearner. Specifically, which goal to set is now purely based on theOpModeof the motor.How is it tested?
This is tested with the canonical policy on the deployment tool, with both the gripper pwm control turned on and off.
Note
We probably need to monitor the joint position of the gripper while in PWM mode so that it does not "over-open" the fingers.