Skip to content

11 program hopper#31

Merged
Samson560915 merged 19 commits intomainfrom
11-program-hopper
Jan 31, 2026
Merged

11 program hopper#31
Samson560915 merged 19 commits intomainfrom
11-program-hopper

Conversation

@Samson560915
Copy link
Copy Markdown
Contributor

Follows turret subsystem. SIM runs, however it is not responsive to PID tuning. Constants need to be set and states need to be finished.

@Samson560915 Samson560915 linked an issue Jan 26, 2026 that may be closed by this pull request
@Samson560915 Samson560915 self-assigned this Jan 26, 2026
@Samson560915
Copy link
Copy Markdown
Contributor Author

@aidnem Could you look at this branch/run SIM and see if/why PID tuning is completely unresponsive?

@godmar
Copy link
Copy Markdown
Contributor

godmar commented Jan 26, 2026

I'm merging main into this branch to get the TestMode update.

@godmar
Copy link
Copy Markdown
Contributor

godmar commented Jan 26, 2026

  • DiskSim.java and DiskSimAdapter.java aren't needed anymore, are they? If so, please remove them.
  • you have a homingAngle variable? I thought the hopper was just spinning

@aidnem Could you look at this branch/run SIM and see if/why PID tuning is completely unresponsive?

What do you mean, specifically?
In shop, Aiden used the Phoenix Tuner to connect to the motors in sim. This "poking" appeared to have an effect.

Check the documentation for the TalonFXConfig, which are used directly in coppercore.

Copy link
Copy Markdown
Contributor

@aidnem aidnem left a comment

Choose a reason for hiding this comment

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

I had a couple comments, hopefully these should help your sim become responsive once again.

Comment thread src/main/java/frc/robot/subsystems/hopper/HopperSubsystem.java Outdated
Comment thread src/main/java/frc/robot/subsystems/hopper/HopperSubsystem.java
Comment thread src/main/java/frc/robot/constants/HopperConstants.java Outdated
Comment thread src/main/java/frc/robot/subsystems/hopper/HopperSubsystem.java
@godmar
Copy link
Copy Markdown
Contributor

godmar commented Jan 28, 2026

@Samson560915 one thing you need to do here is Resolve the conversations. When you got comments, react to them, and if that concludes the conversation hit "Resolve conversation." Do not leave conversations open longer than required.

…ocity instead of setting voltage, now the velocity doesn't change in SIM at all.
@aidnem
Copy link
Copy Markdown
Contributor

aidnem commented Jan 29, 2026

@Samson560915 do you want me to take care of merge conflicts for you?

@Samson560915
Copy link
Copy Markdown
Contributor Author

@aidnem Sure! That would be amazing! Thank you so much!

@aidnem
Copy link
Copy Markdown
Contributor

aidnem commented Jan 29, 2026

@Samson560915 Done 👍 . Do you think we can get this merged tonight? I'd like to be "done with subsystems" as soon as possible.

@Samson560915
Copy link
Copy Markdown
Contributor Author

@aidnem I don't know, I did something and now it doesn't spin (velocity does not change). Do you know if I can simulate the velocity changing using the profiled control to spin the motor?

@aidnem
Copy link
Copy Markdown
Contributor

aidnem commented Jan 29, 2026

@Samson560915

Do you know if I can simulate the velocity changing using the profiled control to spin the motor?

When commanding a velocity, the sim should handle all of that

I've done some digging, by adding a print statement here:

  public void setToTargetVelocity() {
    System.out.println("Set to target velocity " + targetVelocity.in(RPM) + " RPM");
    motor.controlToVelocityProfiled(targetVelocity);
  }

This revealed the following situation: target velocity is always 0.

image

The only place you're setting the value is here:

image

This value is false in your JSON:

image

Also, none of your PID gains are present in the JSON.

If you add this line in JSON constants before the line where the constants are loaded, and run sim once, it'll write them out to JSON from the java file:

    jsonHandler.saveObject(new HopperConstants(), "HopperConstants.json");

If you fix all of these issues, you should be set.

@Samson560915
Copy link
Copy Markdown
Contributor Author

@aidnem
Screenshot 2026-01-29 151744
It changes target Velocity, but the SIM seems very off.
Screenshot 2026-01-29 152034

@aidnem
Copy link
Copy Markdown
Contributor

aidnem commented Jan 29, 2026

@Samson560915 all of your gains are 0 except for kA. this means that the only time it's applying any power is when the profile commands acceleration. Try tuning the PIDs in test mode. The general process is:

  1. Start all gains at 0
  2. Increase kP to something small like 0.1 or 1
  3. Double kP until it overshoots and oscillates around the target
  4. Increase kD until it doesn't oscillate

In between all gains changes, make sure you change your target RPM around to see how it responds to commands.

@Samson560915
Copy link
Copy Markdown
Contributor Author

@aidnem Is it supposed to be in test mode when I click
Screenshot 2026-01-29 155134

@aidnem
Copy link
Copy Markdown
Contributor

aidnem commented Jan 30, 2026

Yes. you also have to select the right hopper test mode elastic.

- also removed expoKV and expoKA and updated motion profile during tuning
@godmar
Copy link
Copy Markdown
Contributor

godmar commented Jan 30, 2026

I merged the latest version of main in 4bd0e30 and changed the LoggedTunableNumber to update acc/jerk in 7730aa1. I removed expoKV and expoKA since you're not using exponential profiles (your code should follow the indexer, not the turret for this.)

@aidnem
Copy link
Copy Markdown
Contributor

aidnem commented Jan 30, 2026

@Samson560915 any progress on tuning PIDs for this? As a very optimistic and aggressive goal, I'd like to get this, the hood, the shooter, and the intake merged by the end of the day tomorrow, so we'd like to get this done as soon as physically possible.
Please let me know if you need any help tuning PIDs. During our team meeting tomorrow, I can go over the process with you as well.

@aidnem
Copy link
Copy Markdown
Contributor

aidnem commented Jan 30, 2026

Here's verification that the code does work once adding a transition that actually lets it transition to test mode from spinning state:

image

I used all gains=0 except I put kP at 16 and max acceleration seemed to be fine at 8000. It could maybe go higher than that.

@aidnem
Copy link
Copy Markdown
Contributor

aidnem commented Jan 31, 2026

@godmar I hate stealing people's tasks, but do you think that, in the interest of getting this merged to un-block the merge of the hood and the shooter, I should just push my changes that I used to fix the test mode & tune PIDs?

Edit: never mind, he pushed his changes so there's no longer any need.

@aidnem
Copy link
Copy Markdown
Contributor

aidnem commented Jan 31, 2026

Excellent:

image

Copy link
Copy Markdown
Contributor

@aidnem aidnem left a comment

Choose a reason for hiding this comment

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

This looks very good. I have a couple very quick changes and then I think it should be ready for merge.

Comment thread src/main/deploy/constants/comp/HopperConstants.json Outdated
Comment thread src/main/java/frc/robot/subsystems/hopper/HopperSubsystem.java Outdated
}

private boolean shouldIdle() {
return false; // TODO: ask if the hopper should be idling at all
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It will eventually have to idle but that will come later when we write more of the action coordination stuff.

Comment thread src/main/java/frc/robot/InitSubsystems.java Outdated
@aidnem
Copy link
Copy Markdown
Contributor

aidnem commented Jan 31, 2026

@Samson560915 as soon as you take care of these 4 things it should be good to go.

…sary casts. Used newLeader method for SIM in InitSubsystems.
Copy link
Copy Markdown
Contributor

@aidnem aidnem left a comment

Choose a reason for hiding this comment

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

Just 1 little thing left !

"hopperKI": 0.0,
"hopperKD": 0.0,
"hopperMotionMagicCruiseVelocityRotationsPerSecond": 3.0,
"maxHopperSetpointVelocityRotationsPerSec": 5.0,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What's the difference between motion magic cruise velocity and max setpoint velocity? Couldn't you just remove the setpoint velocity?

@Samson560915
Copy link
Copy Markdown
Contributor Author

@aidnem By the way, all of the tuning setpoint velocity stuff in in radians per sec, should I change that to rotations per second or does it not matter?

@aidnem
Copy link
Copy Markdown
Contributor

aidnem commented Jan 31, 2026

@Samson560915 yeah I think RPM would be better, if you don't mind making that change really quick.

Copy link
Copy Markdown
Contributor

@aidnem aidnem left a comment

Choose a reason for hiding this comment

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

If you want to change to RPM, go for it, as that would be better. All my other comments are resolved.

@Samson560915
Copy link
Copy Markdown
Contributor Author

@aidnem So, the MotorInputsAutoLogged class only logs in Radians per Second, so if you change everything else to rotations it looks really weird on AdvantageScope, do you want to be to convert the inputs also into rotations per second or do you think we just keep the radians per second?

@aidnem
Copy link
Copy Markdown
Contributor

aidnem commented Jan 31, 2026

@Samson560915 if all values that you log end in their unit (e.g. "RPM" for RPM, like the field names in the AutoLogged inputs, "RadiansPerSecond"), AdvantageScope will automatically convert them and render them correctly when you put them on the same axis.

Then, by clicking the 3 dots above that axis's set of logged fields, you can select the field to render (i believe it defaults to the unit of the first field that you added to the axis).

@aidnem
Copy link
Copy Markdown
Contributor

aidnem commented Jan 31, 2026

@Samson560915 since we aren't really going to be tuning this mechanism precisely for speed (since load will be constantly changing with fuel entering and exiting the hopper) I think we might as well just merge this with radians/sec and then update to RPM later if we think it's really necessary. We can always just convert on AdvantageScope anyway.

@Samson560915
Copy link
Copy Markdown
Contributor Author

@aidnem Sure, so do you want to just merge now?

@aidnem
Copy link
Copy Markdown
Contributor

aidnem commented Jan 31, 2026

Go for it.

@Samson560915 Samson560915 merged commit 4e46422 into main Jan 31, 2026
3 checks passed
@aidnem aidnem deleted the 11-program-hopper branch January 31, 2026 17:03
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.

Program Hopper

5 participants