-
Notifications
You must be signed in to change notification settings - Fork 7
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
Unified command interface #3
Comments
Currently, we have sendRadCommand / sendDegreeCommand to access the full command that can be sent to the motor. The full command is: sendRadCommand(cmd_pos, cmd_vel, Kp, Kd, feedforward_tau). While working with the sendVelocity/Position command, the respective gains Kp/Kd are also needed. Do you then envision the command's arguments as follows: sendPositionCommand(cmd_pos, Kp)? This doesn't exist for sendTorqueCommand as the motors have direct torque control and all other arguments can be set to zero. |
So I think the commands could be something like this: std::map<int, motorState> SetMotorCommand(std::map<int, motorCommand> motorRadCommands);
std::map<int, motorState> SetPositionCommand(std::map<int, motorCommand> motorRadCommands);
std::map<int, motorState> SetVelocityCommand(std::map<int, motorCommand> motorRadCommands);
std::map<int, motorState> SetTorqueCommand(std::map<int, motorCommand> motorRadCommands);
However inside the function the nonrelevant fields will be set to zero. So in the case of the position, we could send - position, kp, kd, and everything is explicitly set to zero. Also, the SetPositionCommand would internally call the SetMotorCommand |
That is interesting. Although, the
How about a different struct for position/velocity/torque commands? Or is that too confusing/convoluted? |
Sure, a separate struct could work too. If the same is to be kept, the functions should mention in their docstrings that they mask out the unused values internally. |
The command interface should include 4 commands -
And 2-3 should internally mask out options and use 1 to actually set a command.
Original issue:
ntnu-arl#2
The text was updated successfully, but these errors were encountered: