-
Notifications
You must be signed in to change notification settings - Fork 0
Nh command loop #178
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
base: main
Are you sure you want to change the base?
Nh command loop #178
Changes from all commits
d1453f7
326bc8a
43cd542
edd4828
3e4b61a
2d7a250
e421a7b
ccc1dd7
c1ea130
1b25122
b830a9c
4754c03
8b93d87
7509596
95f708d
1210a5d
296fd66
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,33 +1,51 @@ | ||
| package frc.robot.commands.ramp; | ||
|
|
||
| import edu.wpi.first.wpilibj.Timer; | ||
| import edu.wpi.first.wpilibj2.command.Command; | ||
| import frc.robot.constants.Constants; | ||
| import frc.robot.subsystems.LightStrip; | ||
| import frc.robot.subsystems.Ramp; | ||
| import frc.robot.utils.TimeoutCounter; | ||
|
|
||
| import java.util.function.DoubleSupplier; | ||
|
|
||
| public class RampMove extends Command{ | ||
| public class RampMove extends Command { | ||
|
|
||
| private final Ramp ramp; | ||
| private final DoubleSupplier encoderValue; | ||
| private final TimeoutCounter timeoutCounter; | ||
| private final Timer timer = new Timer(); | ||
|
|
||
| public RampMove(Ramp ramp, DoubleSupplier encoderValue) { | ||
| public RampMove(Ramp ramp, LightStrip lightStrip, DoubleSupplier encoderValue) { | ||
| this.ramp = ramp; | ||
| this.encoderValue = encoderValue; | ||
| this.timeoutCounter = new TimeoutCounter("RampMove", lightStrip); | ||
| addRequirements(this.ramp); | ||
| } | ||
|
|
||
| @Override | ||
| public void initialize() { | ||
| timer.restart(); | ||
| ramp.setRampPos(encoderValue.getAsDouble()); | ||
| } | ||
|
|
||
|
|
||
| @Override | ||
| public void execute() { | ||
| ramp.updateFF(); | ||
| } | ||
|
|
||
| @Override | ||
| public void end(boolean interrupted) { | ||
| timer.stop(); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isFinished() { | ||
| if (timer.hasElapsed(Constants.RAMP_MOVE_TIMEOUT)){ | ||
| timeoutCounter.increaseTimeoutCount(); | ||
| return true; | ||
| } | ||
| return Math.abs(ramp.getRampPos() - ramp.getDesiredPosition()) < Constants.RAMP_MOVE_THRESHOLD; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,7 @@ public void initialize() { | |
|
|
||
| @Override | ||
| public void execute() { | ||
| ramp.updateFF(); | ||
| } | ||
|
|
||
| @Override | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,52 +1,58 @@ | ||
| package frc.robot.subsystems; | ||
|
|
||
| import edu.wpi.first.wpilibj.motorcontrol.Spark; | ||
| import edu.wpi.first.wpilibj.smartdashboard.SmartDashboard; | ||
| import edu.wpi.first.wpilibj2.command.SubsystemBase; | ||
| import frc.robot.constants.Constants; | ||
| import frc.robot.utils.BlinkinPattern; | ||
| import frc.robot.utils.logging.Logger; | ||
| import frc.robot.utils.smartshuffleboard.SmartShuffleboard; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import java.util.concurrent.ConcurrentHashMap; | ||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
| import java.util.concurrent.atomic.AtomicReference; | ||
| import java.util.function.BooleanSupplier; | ||
|
|
||
| public class LightStrip extends SubsystemBase { | ||
| private static final String baseLogName = "/robot/lightstrip/"; | ||
| private final Spark colorSensorPort; | ||
| private BlinkinPattern pattern = BlinkinPattern.WHITE; | ||
| private final Map<BooleanSupplier, BlinkinPattern> predicateLightEvents = new HashMap<>(); | ||
|
|
||
| private final AtomicReference<BlinkinPattern> pattern = new AtomicReference<>(BlinkinPattern.BLACK); | ||
| private final ConcurrentHashMap<BooleanSupplier, BlinkinPattern> predicateLightEvents = new ConcurrentHashMap<>(); | ||
| private final AtomicBoolean running = new AtomicBoolean(true); | ||
| public LightStrip(int port) { | ||
| this.colorSensorPort = new Spark(port); | ||
| } | ||
|
|
||
| @Override | ||
| public void periodic() { | ||
| predicateLightEvents.keySet() | ||
| .stream() | ||
| .filter(BooleanSupplier::getAsBoolean) | ||
| .findFirst() | ||
| .ifPresent(c -> { | ||
| setPattern(predicateLightEvents.get(c)); | ||
| predicateLightEvents.remove(c); | ||
| Logger.logInteger(baseLogName + "predicateSize", predicateLightEvents.size(), Constants.ENABLE_LOGGING); | ||
| new Thread(() -> { | ||
| while (running.get()){ | ||
| for (BooleanSupplier supplier : predicateLightEvents.keySet()){ | ||
| if (supplier.getAsBoolean()){ | ||
| setPattern(predicateLightEvents.get(supplier)); | ||
| predicateLightEvents.remove(supplier); | ||
| break; | ||
| } | ||
| } | ||
| ); | ||
| try { | ||
| Thread.sleep(40); | ||
| } catch (InterruptedException e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| } | ||
| }).start(); | ||
| } | ||
|
|
||
| public void setPattern(BlinkinPattern pattern) { | ||
| this.pattern = pattern; | ||
| this.pattern.set(pattern); | ||
| colorSensorPort.set(pattern.getPwm()); | ||
| SmartDashboard.putString("PATTERN",pattern.toString()); | ||
armadilloz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Logger.logDouble(baseLogName + "pwmSignal", pattern.getPwm(), Constants.ENABLE_LOGGING); | ||
| Logger.logString(baseLogName + "pwmName", pattern.toString(), Constants.ENABLE_LOGGING); | ||
| if (Constants.LED_DEBUG){ | ||
| SmartShuffleboard.put("Lightstrip", "pwm", pattern.getPwm()); | ||
| SmartShuffleboard.put("Lightstrip", "pattern", pattern.toString()); | ||
| } | ||
| } | ||
|
|
||
| public BlinkinPattern getPattern() { | ||
| return pattern; | ||
| return pattern.get(); | ||
| } | ||
| public void scheduleOnTrue(BooleanSupplier callable, BlinkinPattern pattern) { | ||
| predicateLightEvents.put(callable, pattern); | ||
| } | ||
|
|
||
| public void setRunning(boolean running) { | ||
| this.running.set(running); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,8 @@ public class Ramp extends SubsystemBase { | |
| private final NeoPidMotor neoPidMotor; | ||
| private double rampPos = Constants.RAMP_POS; | ||
| private final InterpolatingDoubleTreeMap rampAngleMap = new InterpolatingDoubleTreeMap(); | ||
| private double neoModerFF = Constants.RAMP_PID_FAR_FF; | ||
|
|
||
| public Ramp() { | ||
| neoPidMotor = new NeoPidMotor(Constants.RAMP_ID); | ||
| configureMotor(); | ||
|
|
@@ -32,48 +34,33 @@ private void configureMotor() { | |
| neoPidMotor.setSmartMotionAllowedClosedLoopError(Constants.RAMP_ERROR_RANGE); | ||
| neoPidMotor.setMaxAccel(Constants.RAMP_MAX_RPM_ACCELERATION); | ||
| neoPidMotor.getPidController().setP(Constants.RAMP_PID_P); | ||
| neoPidMotor.getPidController().setFF(Constants.RAMP_PID_FAR_FF); | ||
| neoPidMotor.getPidController().setFF(neoModerFF); | ||
| } | ||
|
|
||
| public void periodic() { | ||
| if (Math.abs(getRampPos() - getDesiredPosition()) <= Constants.RAMP_ELIM_FF_THRESHOLD){ | ||
| neoPidMotor.getPidController().setFF(NeoPidMotor.DEFAULT_FF); | ||
| }else{ | ||
| neoPidMotor.getPidController().setFF(Constants.RAMP_PID_FAR_FF); | ||
| } | ||
| if (Constants.RAMP_DEBUG){ | ||
| SmartShuffleboard.put("Ramp", "Encoder Value", getRampPos()); | ||
| SmartShuffleboard.put("Ramp", "Desired pos", rampPos); | ||
| SmartShuffleboard.put("Ramp", "Reverse Switch Tripped", getReversedSwitchState()); | ||
| SmartShuffleboard.put("Ramp", "Forward Switch Tripped", getForwardSwitchState()); | ||
| // double pidP = SmartShuffleboard.getDouble("Ramp", "PID P", neoPidMotor.getPidController().getP()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code above (setting thePID_FF) can probably be be part of a command rather then run every time in periodic
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since the ramp is a PID and the MoveRamp command finishes instantly, what do you recommend? Reading the document it does say to use the method "Infrequently" because it takes time. I could store the last PID Value and compare against it before setting it?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a functional reason that the command be done immediately? I would think that it would make sense for a command to terminate once the ramp it at the position.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I checked and I don't think it will break anything if I make the ramp move not end immediately. I added the logic to the ramp commands |
||
| // double pidI = SmartShuffleboard.getDouble("Ramp", "PID I", neoPidMotor.getPidController().getI()); | ||
| // double pidD = SmartShuffleboard.getDouble("Ramp", "PID D", neoPidMotor.getPidController().getD()); | ||
| // double pidFF = SmartShuffleboard.getDouble("Ramp", "PID FF", neoPidMotor.getPidController().getFF()); | ||
| // if (pidP != neoPidMotor.getPidController().getP()) neoPidMotor.getPidController().setP(pidP); | ||
| // if (pidI != neoPidMotor.getPidController().getI()) neoPidMotor.getPidController().setI(pidI); | ||
| // if (pidD != neoPidMotor.getPidController().getD()) neoPidMotor.getPidController().setD(pidD); | ||
| // if (pidFF != neoPidMotor.getPidController().getFF()) neoPidMotor.getPidController().setFF(pidFF); | ||
| SmartShuffleboard.put("Driver", "Speaker Close", isShootCloseAngle()) | ||
| .withPosition(9, 0) | ||
| .withSize(1, 1); | ||
| SmartShuffleboard.put("Driver", "Speaker Away", isShootAwayAngle()) | ||
| .withPosition(9, 1) | ||
| .withSize(1, 1); | ||
| SmartShuffleboard.put("Driver", "Amp", isShootAmpAngle()) | ||
| .withPosition(8, 1) | ||
| .withSize(1, 1); | ||
| SmartShuffleboard.put("Ramp", "Forward Switch Tripped", getForwardSwitchState()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why deleting the debug shuffleboard values?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. delete selection was to big when removing pid stuff
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
| SmartShuffleboard.put("Ramp", "Forward Switch Tripped", getForwardSwitchState()); | ||
| Logger.logDouble(baseLogName + "EncoderValue", getRampPos(), Constants.ENABLE_LOGGING); | ||
| Logger.logDouble(baseLogName + "DesiredPos", rampPos, Constants.ENABLE_LOGGING); | ||
| } | ||
| if (Constants.LOG_LIMIT_SWITCHES){ | ||
| Logger.logBoolean(baseLogName + "FWD LMT", getForwardSwitchState(), Constants.ENABLE_LOGGING); | ||
| Logger.logBoolean(baseLogName + "REV LMT", getReversedSwitchState(), Constants.ENABLE_LOGGING); | ||
| } | ||
|
|
||
| Logger.logDouble(baseLogName + "EncoderValue", getRampPos(), Constants.ENABLE_LOGGING); | ||
| Logger.logDouble(baseLogName + "DesiredPos", rampPos, Constants.ENABLE_LOGGING); | ||
| Logger.logBoolean(baseLogName + "FWD LMT", getForwardSwitchState(), Constants.ENABLE_LOGGING); | ||
| Logger.logBoolean(baseLogName + "REV LMT", getReversedSwitchState(), Constants.ENABLE_LOGGING); | ||
|
|
||
| /* | ||
| SmartShuffleboard.put("Driver", "Speaker Close", isShootCloseAngle()) | ||
| .withPosition(9, 0) | ||
| .withSize(1, 1); | ||
| SmartShuffleboard.put("Driver", "Speaker Away", isShootAwayAngle()) | ||
| .withPosition(9, 1) | ||
| .withSize(1, 1); | ||
| SmartShuffleboard.put("Driver", "Amp", isShootAmpAngle()) | ||
| .withPosition(8, 1) | ||
| .withSize(1, 1); | ||
| */ | ||
| } | ||
|
|
||
| public void setRampPos(double targetPosition) { | ||
|
|
@@ -150,4 +137,22 @@ public void setAngle(Rotation2d angleFromGround) { | |
| public double calcPose(Pose2d pose2d, Alignable alignable) { | ||
| return rampAngleMap.get(pose2d.getTranslation().getDistance(new Translation2d(alignable.getX(), alignable.getY()))); | ||
| } | ||
|
|
||
| public void setFF(double feedForward) { | ||
| neoPidMotor.getPidController().setFF(feedForward); | ||
| neoModerFF = feedForward; | ||
| } | ||
| public double getFF(){ | ||
| return neoModerFF; | ||
| } | ||
|
|
||
| public void updateFF() { | ||
| if (Math.abs(getRampPos() - getDesiredPosition()) <= Constants.RAMP_ELIM_FF_THRESHOLD) { | ||
| if (getFF() != NeoPidMotor.DEFAULT_FF) { | ||
| setFF(NeoPidMotor.DEFAULT_FF); | ||
| } | ||
| } else if (getFF() != Constants.RAMP_PID_FAR_FF) { | ||
| setFF(Constants.RAMP_PID_FAR_FF); | ||
| } | ||
| } | ||
| } | ||
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.
Seems like there is no real need to protect the
patternand therunningsince they are primitives (boolean and a reference), though using an atomic reference here does not hurt. The one thing that does require protecting as it is modified from both threads is the map, which is currently vulnerable.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.
changed to concurrent hashmap