Skip to content
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

Added implementation for scaling joint speeds based on gear ratios #41

Open
wants to merge 35 commits into
base: Arm_Control_System
Choose a base branch
from

Conversation

JackZautner
Copy link
Contributor

Pending hardware testing. Do this one before #34

Tzanccc and others added 25 commits October 25, 2022 08:52
…pplication

Joint state publisher application
* Starting to implement new design

* Continuing implementation, doing simple parts first to remove big issues

* Further implementation

* Only Differential stuff and mains left

* Up to mains complete

* Removing old arm motor files, still in VC for reference

* Fixed duplicating copies, need to resolve further

* Fixed copying by allowing safer copying.  Need to double check path planning

* All fixed up.  Added test script

* Additional changes, some motion planning is possible, need to refine motor/encoder directions

* Adding direction multipliers to formalize motor reversal in direct speed converters.  Need equivalent for differential speed converters.

* Updating constants

* First autonomous actions on the arm

* Adding legend

* Adjusting parameters

* Tuning joint limits

* Feature/over current fault implementation (#31)

* Added functionality to retrieve over current status, updated build settings

* Added functionality to stop all joints when a motor is over current

* Added implementation for action server to stop when an over current fault is detected

* set mock arm launch to launch the mock roboclaw

* Added functionality to stop the action server when a motor is over current for too long

* Fixed code in compliance with PR

* Address fix in Motor.cpp

* Corresponding const-change in Motor.hpp

* Const-correctness in ArmControlActionServer.cpp

---------

Co-authored-by: Ben Nowotny <44074469+bennowotny@users.noreply.github.com>

* Removed I terms (need synchronization before we can do that)

---------

Co-authored-by: Ben Nowotny <bennowotny65535@gmail.com>
Co-authored-by: WR-BaseStation <wisconsinrobotics@cae.wisc.edu>
Co-authored-by: Ben Nowotny <44074469+bennowotny@users.noreply.github.com>
Copy link
Contributor

@bennowotny bennowotny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, this did not pass hardware testing; the arm was stuttering rather than taking a smooth motion to the target. The cause for this is not apparent, so we may need more testing to find the root cause.

src/wr_control_drive_arm/src/ArmControlActionServer.cpp Outdated Show resolved Hide resolved
Base automatically changed from arm-test-jsp-and-controller to test/rviz March 14, 2023 01:03
Base automatically changed from test/rviz to Arm_Control_System March 15, 2023 21:52
@JackZautner
Copy link
Contributor Author

I have realized that 1b774df does not explicitly state this but it only contains changes for the direct joint to motor speed converters.

Copy link
Contributor

@bennowotny bennowotny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work; small edits.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In simulation, the gear ratio is always 1.

auto currJoint = goal->trajectory.joint_names.at(i);

// The position monitor whose velocity is currently being scaled
auto currPosMtr = namedPositionMonitors.at(currJoint);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid copying the std::shared_ptr:

Suggested change
auto currPosMtr = namedPositionMonitors.at(currJoint);
auto& currPosMtr = *namedPositionMonitors.at(currJoint);

@@ -2,6 +2,7 @@
#define DIRECT_JOINT_TO_MOTOR_SPEED_CONVERTER_H

#include "Motor.hpp"
#include "SingleEncoderJointPositionMonitor.hpp"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Include should be unused:

Suggested change
#include "SingleEncoderJointPositionMonitor.hpp"

Comment on lines 60 to 67
auto getEncoderConfigFromParams(const XmlRpcValue &params, const std::string &jointName) -> EncoderConfiguration {
return {.countsPerRotation = static_cast<int32_t>(params[jointName]["counts_per_rotation"]),
.offset = static_cast<int32_t>(params[jointName]["offset"])};
return {.countsPerRotation = static_cast<int32_t>(params[jointName]["encoder_parameters"]["counts_per_rotation"]),
.offset = static_cast<int32_t>(params[jointName]["encoder_parameters"]["offset"])};
}

auto getMotorConfigFromParams(const XmlRpcValue &params, const std::string &jointName) -> MotorConfiguration {
return {.gearRatio = static_cast<double>(params[jointName]["motor_configurations"]["gear_ratio"])};
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUGGESTION: Since this is defined in two places the same way, maybe it makes sense to abstract these functions to their own header files and link the implementation in CMake.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't do (for now lol)

Comment on lines 81 to 128
XmlRpcValue encParams;
pn.getParam("encoder_parameters", encParams);
XmlRpcValue armParams;
pn.getParam("arm_parameters", armParams);

namedJointPositionMonitors.try_emplace("elbowPitch_joint",

"aux1",
RoboclawChannel::A,
getEncoderConfigFromParams(encParams, "elbow"),
getEncoderConfigFromParams(armParams, "elbow"),
getMotorConfigFromParams(armParams, "elbow"),
n);
namedJointPositionMonitors.try_emplace("elbowRoll_joint",

"aux1",
RoboclawChannel::B,
getEncoderConfigFromParams(encParams, "forearmRoll"),
getEncoderConfigFromParams(armParams, "forearmRoll"),
getMotorConfigFromParams(armParams, "forearmRoll"),
n);
namedJointPositionMonitors.try_emplace("shoulder_joint",

"aux0",
RoboclawChannel::B,
getEncoderConfigFromParams(encParams, "shoulder"),
getEncoderConfigFromParams(armParams, "shoulder"),
getMotorConfigFromParams(armParams, "shoulder"),
n);
namedJointPositionMonitors.try_emplace("turntable_joint",

"aux0",
RoboclawChannel::A,
getEncoderConfigFromParams(encParams, "turntable"),
getEncoderConfigFromParams(armParams, "turntable"),
getMotorConfigFromParams(armParams, "turntable"),
n);
namedJointPositionMonitors.try_emplace("wristPitch_joint",

"aux2",
RoboclawChannel::A,
getEncoderConfigFromParams(encParams, "wristPitch"),
getEncoderConfigFromParams(armParams, "wristPitch"),
getMotorConfigFromParams(armParams, "wristPitch"),
n);
namedJointPositionMonitors.try_emplace("wristRoll_link",
"aux2",
RoboclawChannel::B,
getEncoderConfigFromParams(encParams, "wristRoll"),
getEncoderConfigFromParams(armParams, "wristRoll"),
getMotorConfigFromParams(armParams, "wristRoll"),
n);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUGGESTION: This may be abstract-able as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't do (for now lol)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants