-
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
Elevator subsystem skeleton creation #14
base: main
Are you sure you want to change the base?
Conversation
src/main/java/competition/electrical_contract/Contract2025.java
Outdated
Show resolved
Hide resolved
src/main/java/competition/electrical_contract/Contract2025.java
Outdated
Show resolved
Hide resolved
src/main/java/competition/subsystems/elevator/ElevatorSubsystem.java
Outdated
Show resolved
Hide resolved
src/main/java/competition/subsystems/elevator/ElevatorSubsystem.java
Outdated
Show resolved
Hide resolved
src/main/java/competition/subsystems/elevator/ElevatorSubsystem.java
Outdated
Show resolved
Hide resolved
//common TargetHeights | ||
} | ||
double defaultElevatorPower; | ||
final DoubleProperty elevatorPower; |
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.
⭐ ⭐ Making the elevator go up vs down usually needs VERY different powers. And maybe we just have a gamepad control it with the joystick in the short-run?
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.
Agree, but gamepad control should go into the HumanInput
of the maintainer.
src/main/java/competition/subsystems/elevator/ElevatorSubsystem.java
Outdated
Show resolved
Hide resolved
…488/TeamXbot2025 into Elevator-Subsystem-Skeleton
…488/TeamXbot2025 into Elevator-Subsystem-Skeleton
…488/TeamXbot2025 into Elevator-Subsystem-Skeleton
//common TargetHeights | ||
} | ||
double defaultElevatorPower; | ||
final DoubleProperty elevatorPower; |
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.
Agree, but gamepad control should go into the HumanInput
of the maintainer.
this.contract = contract; | ||
|
||
//was going to make this ephemeral but cant find it | ||
this.elevatorTargetHeight = pf.createPersistentProperty("Elevator Target", 0.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.
just turn these into double
and then use aKitLog.record
in periodic()
to save them
} | ||
} | ||
|
||
public void rise(){ |
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.
recommend renaming to "raise".
Raise & Lower are a semantic pair.
Rise & Fall are a semantic pair.
|
||
@Override | ||
public void refreshDataFrame() { | ||
//to be implemented later |
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.
just do motor.refreshDataFrame()
for any motor(s) you have.
public void refreshDataFrame() { | ||
//to be implemented later | ||
} | ||
|
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.
also add a periodic()
where you call motor.periodic()
|
||
import javax.inject.Inject; | ||
|
||
public class ElevatorMaintainerCommand extends BaseCommand { |
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.
this needs to extend BaseMaintainerCommand
|
||
import javax.inject.Inject; | ||
|
||
public class SetElevatorTargetHeightCommand extends BaseCommand { |
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.
this needs to extend BaseSetpointCommand
Creating elevator subsystem skeleton
Asana task URL: https://app.asana.com/0/1208731987242938/1209174882393849/f
Whats changing?
Basic Elevator Commands inlcluding rise, lower, stop, along with basic power setting to elevator motors, also added abstract methods for motor device info in electrical contract, and implemented them in competition contract 2025.
Questions/notes for reviewers
Does the Elevator subsystem have to extend a BaseSetpointSubsystem rather than a BaseSubsystem
Also are the motors supposed to be XCANTalon's or is it the XCANMotorControllers
What the heck is motion magic and do i need to use that for elevator power logic in the future?
No tests, just a basic skeleton
PR feedback legend