What I am trying to do is get the robot to spin it’s arm mechanism if the beam break is broken. the code builds but I cannot deploy it because I think, since I am calling the armsubsystem from the BeamBreak subsystem the RIO log is telling me I am trying to create a new SPARKmax which has already been created. My most recent attempt was to use a getter method to get an instance of the subsystem from the Robot Container file. I’m not sure what the correct way to do this is or if I should go about it differently.
I then tried to create an LED subsystem and was using the WPIlib to help me use AddressableLEDs, but I didn’t get far as I am getting a build error here with my {} brackets and I’m not sure what I’m doing wrong?
This is a test board I have the beam break connected to right now. Once we get it attached to our competition robot, we’d like it to turn the LED’s to say…green, so that the driver knows we have obtained a game piece with the intake. Possible also stow the intake. When I started with the test board, it only had an arm. I can read the voltage from the beam break fine, but any command I try to run from it hasn’t worked. At one point, I did move the beam break do the arm subsystem, but that didn’t work for me either. What do you mean by “You shouldn’t be instantiating commands in your subsystems except as a factory method to return the command” - where should I put the command?
It’s your choice how you want to organize your project. However, a subsystem is a logical grouping of related hardware (e.g. sensors and actuators), and commands are generally discrete things you can do with that hardware. You’ll have to decide what’s related enough to be grouped together. We don’t know what your robot design is, so it’s hard to make specific recommendations.
Let’s assume for a moment that your LEDs are used purely by your arm to signal that a note has been picked up, and the beambreak is the sensor used to detect a note. This likely isn’t far from the truth.
I would have a single “ArmSubsystem” that contains the LED stuff, the beambreak stuff, and any motors or whatever else is necessary for the arm to work as designed. Then I would define commands to set the LED output (whether on/off, or color-changing, or a pattern if addressable), and to control the motors.
The older paradigm is to have discrete command classes, so that’s one way to do it. A more modern approach is to define commands functionally via lambdas. The most recent approach is to define methods on your subsystem that return command objects. This is a form of factory pattern that allows your commands to natively access subsystem functionality without exposing it through a bunch of subsystem methods. The wpilib docs and example projects show all of these options.
But yes, as @Hollisms suggested, you should give the docs a read-through.
I have been through the Command Based Programming section of the WPIlib a few times, and between that and the Zero to Autonomous Youtube series - I have found them both really helpful. Problem with sensors and and LEDs is that the help in WPIib is written for Timed Robot, so I"m not sure how to structure it. What if I write a command for the if-then statement part of what I’m trying to do with the beam break? Then I think I could require more than one subsystem? I’m not sure how to structure that though
I don’t know if this is exactly your problem, but if you just can’t make your arm rotate, it could be because you don’t call them in robotContainer.java
You can do this by following this example:
Putting Intake intake = new Intake(); (this is an example, in case you put it like yours)
in line 15
And then putting: intake.setDefaultCommand(new intakeCommand(intake));
(according to what you posted previously)
in line 39
In my case these are my intake classes if it helps
package frc.robot.subsystens;
import edu.wpi.first.wpilibj.motorcontrol.PWMSparkMax;
import edu.wpi.first.wpilibj2.command.SubsystemBase;
import frc.robot.Contants.MotorPorst;
public class Intake extends SubsystemBase {
private final PWMSparkMax intake =
new PWMSparkMax(MotorPorst.motorIntake);
public void setSpeed(double speed) {
intake.set(speed);
}
public void stopintake() {
intake.set(0);
}
}
package frc.robot.commands.teleop;
import edu.wpi.first.wpilibj.Joystick;
import edu.wpi.first.wpilibj2.command.CommandBase;
import frc.robot.Contants.Axis;
import frc.robot.Contants.Joysticks;
import frc.robot.Contants.MotorSpeeds;
import frc.robot.subsystens.Intake;
public class intakeCommand extends CommandBase {
private final Joystick shooterStick = new Joystick(Joysticks.shooterStick);
Intake m_IntakeSubsystem;
public intakeCommand(Intake driveSubsystem) {
m_IntakeSubsystem = driveSubsystem;
addRequirements(m_IntakeSubsystem);
}
@Override
public void initialize() {}
@Override
public void execute() {
if (shooterStick.getRawAxis(Axis.leftTrigger) != 0) {
m_IntakeSubsystem.setSpeed(shooterStick.getRawAxis(Axis.leftTrigger) * MotorSpeeds.intakeSpeed);
} else if (shooterStick.getRawAxis(Axis.rightTrigger) != 0) {
m_IntakeSubsystem.setSpeed(-shooterStick.getRawAxis(Axis.rightTrigger) * MotorSpeeds.intakeSpeed);
} else {
m_IntakeSubsystem.setSpeed(0);
}
}
@Override
public boolean isFinished() {
return false;
}
}
I’d also probably personally combine the beam break sensor into whatever subsystem it is more adjacent to. Assuming the beam break sensor is on your Intake and detecting a Note incoming I’d include it in your Intake subsystem, add the isBeamBroken() method or similar, and then use it to trigger the command on the Arm Subsystem. So this would look like:
Trigger m_beamBreak = new Trigger(()->m_intake.isBeamBroken());
m_beamBreak.onTrue(m_arm.startMotorCommand());
You could also possibly setup the Trigger object also in your subsystem, then access it through your RobotContainer, leading to
Which I think is pretty cool as it minimizes the logic at the top level, and very clearly shows that an action in one subsystem is triggering the other.
Nice progression of thinking. Nobody ever said the simplest solution is the easiest to find. But… I’m optimistic that if I can just get the students to write out what they want to accomplish they may find that the succinct code you wrote at the end can be gotten to directly. That’s the beauty of the Command-based shortcuts is they closely match what is needed in English. [Again they sat down in front of VSC and started typing their extends CommandBase [with deprecated error] then asked what do we want? Sigh.]
This is a great pattern. I’d have the trigger be public final rather than having a getter method, probably, but i very much think this is the “correct” architecture to express this sort of thing.
I like this solution, so I am trying to implement it. The code deploys now, but doesn’t work. The beam break doesn’t start the motor when broken. I think my mistake is in this part of the command: the intellisense underlines “isBeamBroken” in the
private boolean isBeamBroken; line and says The value of the field BeamCommand.isBeamBroken is not usedJava(570425421)
public class BeamCommand extends Command {
private final ArmSubsystem m_armSubsystem;
private final BeamBreak m_beamBreak;
private boolean isBeamBroken;
Assuming OP is actually headed toward an Intake subsystem that should own the beam break sensor because it uses it to turn off (otherwise @jdaming is right and all this should be in Arm), I think this is your and lint programs preference:
In Intake: public final Trigger m_beamBreak = new Trigger(this::isBeamBroken);
In RobotContainer: m_intake.beamBreak.onTrue(m_arm::startMotor);
I also hound the students to wordsmith the phrase beamBreak to more like objectInPosition
Thats because it’s not used. You’re calling the subsystem method, but not using the boolean member variable anywhere (its likely not needed, so that’s okay.)
I agree with @Fletch1373 on the error message being both correct and ignorable. Once you have it working you should consider cleaning it up.
And @SLAB-Mr.Thomas is spot on. For testing purposes, map the BeamCommand to a button on the joystick.
Once you have that working try making it the default command on your arm/beam break subsystem or probably better follow the trigger pattern mentioned by @ExploitSage.
Absolutely. Keeping names as close to the problem-space as possible (instead of the solution-space) is one of the most important principles in writing good code.
P.S. I personally detest hungarian notation (i.e. m_), especially for members that are part of an object’s public interface, but the WPILib standard is what it is.
Edit: WPILib standard allows omitting m_ on public fields, per @Peter_Johnson.