CyberCoyotes / 2024-Crescendo

2024 FRC competition robot code, Java command-based framework.
Other
1 stars 1 forks source link

Shooter Subsystem - questions #140

Closed JediScoy closed 7 months ago

JediScoy commented 8 months ago

Why aren't we using a closed loop approach with the shooter flywheels? In 2022 Rapid React we faked it with percent output and various check & guess wait commands. Mid season I realized velocity control mode would be better. This was confirmed by talking with Connor W and looking at other teams' shooter codes. Closed-Loop Control

The updated versions:

From CTRE website "A Velocity closed loop can be used to maintain a target velocity (in rotations per second). This can be useful for controlling flywheels, where a velocity needs to be maintained for accurate shooting."

Seems like the current code is trying to use DutyCycleOut and "fake" velocity checks

JediScoy commented 8 months ago

There is a run lambda in a periodic method for the shooter subsystem. I won't pretend to fully understand lambdas. I know they are an inline, shorthand for getting values/output and in this case return for that method. Is this sketchy?

I get that writing full commands for something simple can be overkill, but for modularity of code (i.e. using in autonomous or other macros) regular commands are going to be needed, per early season Issues posts.

Shorthand for testing is fine, but its time to go with full commands IMO.

JediScoy commented 8 months ago

See also #110

Fruggg commented 8 months ago

Closed loop is overkill. If you want, knock yourself out, but I see no point when we are using a method effectively saying "go as fast as possible." I also have no idea what "fake" velocity check means in this context. I see no lambda in the periodic. Are you referring to Run()?

JediScoy commented 8 months ago

this.run(() -> SetOutput(input))

Apologies, there is a lambda but its in the Command RunShooter() immediately underneath the periodic(). I realize that there is increase in the use of lambda for FRC, the new-command-base was officially updated in 2020 to included inline commands, and WPILib documentation on Java programming has been using it to an increasing level. I'm not saying its wrong, but it's a fairly recent phenomenon.

Being that we are utilizing a Command based framework, I feel that we need the modularity of subclassing even at the expense of verbosity. If not for my benefit, then for the benefit of incoming programmers. Most programmers I've talked to, even the ones that use them, can tell you they work but not how. Shooting is obviously something that will be used in multiple cases. I was told previously to avoid using commands within subsystem. It could cause confusion and trouble. I realize that is not at all an academic argument.

PathPlanner auton builder requires NamedCommands and most of the examples I've seen thus far have separate commands. Looking at the official documentation it appears at least one of the NamedCommands is a reference to a subsystem command . So again, its not wrong, its just new-ish.

I'm trying to give you some context for my questions and concerns. If you to educate us, then educate us on the use of inline lambdas.

JediScoy commented 8 months ago

As for "If you want, knock yourself out, but I see no point when we are using a method effectively saying 'go as fast as possible.'"

Careful.

Its my understanding that the shooting method we are using is NOT effective. I've heard that from all of the mentors. To the point where it is part of the reason for redesigning the shooter. It may be overkill in your estimation, but experience with these wheels as flywheels - yes, they are small mass - has shown velocity to be the better choice. A velocity closed loop for using with flywheels is literally in the documentation. Make it happen. If its still inconsistent, then by all means point out that I was wrong. I'm ok with being wrong.

The fake velocity reference is because you are using GetVelocity() with a .getVelocity and running checks against it with IsVelocityReqMet(). Perhaps roundabout is better than the use a "fake" check. Just use the actual velocity control mode.

Fruggg commented 8 months ago

As for "Careful."

I'll watch my mouth for now and do the thing. But whoever loses the ego contest owes the other coffee and a donut.

JediScoy commented 8 months ago

I'm slightly overweight, 46 so random things hurt now, losing my hearing, make enough money to pay my bills with an occasional splurge, and get humbled in what used to be fun recreational activities; my ego (mostly) died a while back. I'll even spot you the coffee and donuts.