Skip to content

Season 2022 sussy play ppid omg#76

Open
robot256 wants to merge 4 commits intoseason-2022-PowerPlayfrom
season-2022-SussyPlay-PPID-omg
Open

Season 2022 sussy play ppid omg#76
robot256 wants to merge 4 commits intoseason-2022-PowerPlayfrom
season-2022-SussyPlay-PPID-omg

Conversation

@robot256
Copy link

Not sure this will work, but I will review it here.

@robot256 robot256 marked this pull request as draft April 20, 2023 11:21
@robot256 robot256 marked this pull request as ready for review April 20, 2023 18:22
PIDController(LiftPIDCoefficients.kI, LiftPIDCoefficients.kI, LiftPIDCoefficients.kD, LiftPIDCoefficients.kF),
LIFT_MAX_EXTENSION,
LIFT_MIN_EXTENSION,
LINEAR_SLIDE_TOLERANCE,
Copy link
Author

Choose a reason for hiding this comment

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

Need to make this Lift-specific. It's only used in the lift (fetcher tolerance is a literal constant now)

null,
with (FetcherTelePIDCoefficients){PIDController(kP, kI, kD, kF)},
FETCHER_MAX_EXTENSION,
25,
Copy link
Author

Choose a reason for hiding this comment

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

Make this a constant for FETCHER_TOLERANCE

const val LIFT_MAX_EXTENSION = 900
const val LIFT_MAX_EXTENSION = 3.02
const val LIFT_MIN_EXTENSION = 0.992
const val LIFT_SENSITIVITY = 16.0
Copy link
Author

Choose a reason for hiding this comment

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

With the max range of the lift changed from 900 counts to 2.0 volts, make sure this was scaled down similarly.

const val FETCHER_SENSITIVITY = 44.0
const val FETCHER_MAX_EXTENSION = 900
const val LINEAR_SLIDE_TOLERANCE = 5
const val LINEAR_SLIDE_TOLERANCE = 5.0
Copy link
Author

Choose a reason for hiding this comment

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

With the max range of the lift changed from 900 counts to 2.0 volts, make sure this was scaled down similarly.

const val kF = 0.0

}

Copy link
Author

Choose a reason for hiding this comment

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

With the max range of the lift changed from 900 counts to 2.0 volts, the gains probably need to be scaled up so that more power is generated from a small difference in positions.

init {
assert(maxExtensionPosition > minExtensionPosition) { "maxExtensionPosition must be greater than minExtensionPosition" }
assert(maxCorrectionPower > 0 && maxCorrectionPower < 1.0) { "maxExtensionPower must be between 0 and 1"}
assert(tolerance >= 0) { "tolerance must be greater than or equal to 0" }
Copy link
Author

Choose a reason for hiding this comment

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

Needs to be > 0

liftPotentiometer),
fetcher = LinearSlide(fetcher,
null,
with (FetcherTelePIDCoefficients){PIDController(kP, kI, kD, kF)},
Copy link
Author

Choose a reason for hiding this comment

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

Wait what? Fetcher was not supposed to change in the PR!!!!

Comment on lines +167 to +170
with(LiftPIDCoefficients){
hardware.lift.motor.setVelocityPIDFCoefficients(kP, kI, kD, kF)
hardware.lift.motor.setPositionPIDCoefficients(15.0)
}
Copy link
Author

Choose a reason for hiding this comment

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

This should be deleted. We are not using the internal lift PID anymore

// telemetry.addData("Fetcher power", hardware.fetcher.extensionPower)
telemetry.addData("Lift encoder value: ", hardware.lift.extensionEncoder)
// telemetry.addData("Lift encoder value: ", hardware.lift.extensionEncoder)
telemetry.addData("Lift encoder value: ", hardware.lift.motor.encoderPosition)
Copy link
Author

Choose a reason for hiding this comment

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

Delete this entry, made redundant below.

telemetry.addData("Lift encoder value: ", hardware.lift.extensionEncoder)
// telemetry.addData("Lift encoder value: ", hardware.lift.extensionEncoder)
telemetry.addData("Lift encoder value: ", hardware.lift.motor.encoderPosition)
telemetry.addData("Lift set point: ", hardware.lift.extensionSetPoint)
Copy link
Author

Choose a reason for hiding this comment

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

Delete this entry, made redundant below.

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.

2 participants