-
Notifications
You must be signed in to change notification settings - Fork 0
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
add support for multiple PID slots in MotorController #49
base: main
Are you sure you want to change the base?
Conversation
if PID values are set via config, support live refreshes of these values when set() is called.
98510ec
to
27717a5
Compare
@@ -23,16 +30,17 @@ public double getSensorVelocity() { | |||
} | |||
|
|||
@Override | |||
public void set(final ControlMode mode, final double value) { | |||
public void set( | |||
final ControlMode mode, final double value, int slot, double arbitraryFeedForward) { | |||
switch (mode) { | |||
case PercentOutput: | |||
delegate.set(mode, value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should these other control modes also pass along the slot and arbFF?
|
||
@Override | ||
public void setOutputRange(final double minOutput, final double maxOutput, int slot) { | ||
if (slot != 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the behavior might be confusing to the user - if an output range is set on Slot 0 but not on Slot 1, then the range bound will apply to both slots without warning the user
.logRaw( | ||
Severity.WARNING, | ||
"Ignoring slot for setOutputRange - unsupported on Talon"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
potentially confusing behavior, as above
} | ||
|
||
@Override | ||
public void set(final ControlMode mode, double value) { | ||
public void set(final ControlMode mode, double value, int slot, double arbitraryFeedForward) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this have m_device.selectProfileSlot
like in CANVictorMotorController
?
.logRaw( | ||
Severity.WARNING, | ||
"Ignoring slot for setOutputRange - unsupported on Talon"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
potentially confusing behavior, as above
Description
MotorController
.LocalMotorController
set
is called.follow
wrt inverting the follower motor.How Has This Been Tested?
Needs testing. Sending for early feedback.