First draft outreach bot 16.0, drive only #6

madmosserc · 2025-08-19T01:38:51Z

It built successfully on my machine.
It did not build successfully when I pushed to GitHub.

srimanachanta · 2025-08-19T05:56:16Z

build.gradle

+// AdvantageKit log replay from the command line. Set the
+// value to "true" to enable the sim GUI by default (this
+// is the standard WPILib behavior).
+wpi.sim.addGui().defaultEnabled = false


should be true

srimanachanta · 2025-08-19T13:34:30Z

build.gradle

            // or from command line. If not found an exception will be thrown.
            // You can use getTeamOrDefault(team) instead of getTeamNumber if you
            // want to store a team number in this file.
-            team = project.frc.getTeamOrDefault(540)


leave this as it was, not technically necessary but can help in a pinch during comp if you need to swap to a new computer without being able to transfer everything.

build.gradle

srimanachanta · 2025-08-19T13:38:53Z

src/main/java/frc/robot/Constants.java

+  // Disable LEDs, will reduce software and electrical overhead but disable hardware alerts
+  public static final boolean ENABLE_LEDs = false;


nit: remove

srimanachanta · 2025-08-19T13:40:47Z

src/main/java/frc/robot/Constants.java

+  // Disable the AdvantageKit logger from running
+  public static final boolean ENABLE_LOGGING = true;


feature not implemented, either implement or remove

srimanachanta · 2025-08-20T13:50:33Z

src/main/java/frc/robot/subsystems/drive/DriveConstants.java

+  static final double mk4iDriveGearing = (50.0 / 14.0) * (19.0 / 25.0) * (45.0 / 15.0);
+  static final double mk4iTurnGearing = 12.8;


Should be renamed to MK4, im assuming you have confirmed we are using L1 ratio

srimanachanta · 2025-08-20T13:55:21Z

src/main/java/frc/robot/subsystems/drive/DriveConstants.java

+        .turnMotorId(2)
+        .driveMotorId(3)
+        .encoderChannel(0)
+        .encoderOffset(


Encoder offset should be done on module for CTRE CANCoders.
https://v6.docs.ctr-electronics.com/en/stable/docs/hardware-reference/cancoder/index.html#zeroing-the-cancoder

srimanachanta · 2025-08-20T13:55:48Z

src/main/java/frc/robot/subsystems/drive/DriveConstants.java

+        .encoderOffset(Rotation2d.fromRadians(-1.7285578862473199)) // .rotateBy(Rotation2d.kPi))
+        .driveGearing(mk4iDriveGearing)
+        .turnGearing(mk4iTurnGearing)
+        .turnInverted(true)


Not sure if turn should be inverted on MK4, look into that in previous code.

srimanachanta · 2025-08-20T13:56:53Z

src/main/java/frc/robot/subsystems/drive/Module.java

-        turnKd.initDefault(0.0);
+    switch (Constants.getRobot()) {
+      case OUTREACHBOT -> {
+        drivekP.initDefault(0.1);


ALWAYS ALWAYS reset to zero before you have tuned the values for safety purposes. If you arent sure the code works, these values could casue the robot to hurt others, or more importantly, itself.

srimanachanta · 2025-08-20T13:59:26Z

src/main/java/frc/robot/subsystems/drive/ModuleIOSpark.java

+  private final SparkMax turnSpark;
+  private final RelativeEncoder driveEncoder;
+  private final RelativeEncoder turnRelativeEncoder;
+  private final AnalogEncoder turnAbsoluteEncoder;


16.0 uses a CTRE CANCoder, not an analog enocder. This code needs to be updated and rewritten to support the CANCoder per the current CTRE docs. It may be beneficial to reference the AdvantageKit TalonFX Swerve Template which uses CANCoders

srimanachanta

Good work on making the changes you made, you addressed most of the issues that were present. The main issue here is understanding working with the CANcoder and the Phoenix 6 API.

Some additional recommendations:

Get in the habit of making multiple smaller commits instead of one big commit, this allows you to split up changes and your work into more digestible segments. It also allows others to understand your process of working through stuff.
Git already allows you to go back and get older versions of your code. Get in the habit of deleting unneeded code instead of just commenting it out. Commenting out a lot of code like how you have done here leads to the code base just being unreadable overall. As a general rule of thumb, if you're commenting out more than a block of code, you probably should be making a commit instead.
You have a lot of really good questions throughout this code and I can somewhat see your thought process based on the decision you made in designing your code. I suggest you sit with Aayan and see if you can research the answers to some of these questions if he doesn't already know the answer.

srimanachanta · 2025-09-23T16:27:08Z

src/main/java/frc/robot/subsystems/drive/DriveConstants.java

+  }
+
+  @Builder
+  record ModuleConfig(


There seems to be some confusion about the actual module configuration based on what I've seen in your code so far. There are two NEOs used for turn and steer and a single CANCoder used as the absolute encoder for the module. Based on this, you should refactor this record to reflect the following:

@Builder record ModuleConfig( int turnMotorId, int driveMotorId, int absoluteEncoderId, double driveGearing, double turnGearing, ) {}

srimanachanta · 2025-09-23T16:45:03Z

src/main/java/frc/robot/subsystems/drive/ModuleIOSpark.java

+  private final SparkMax turnSpark;
+  private final RelativeEncoder driveEncoder;
+  private final RelativeEncoder turnRelativeEncoder;
+  private final AbsoluteEncoder turnAbsoluteEncoder;


Ditto on my previous comment on the module configuration. I'm going to assume you got this idea from a REV-only template/example code. Remember that the module you are defining uses REV's APIs for the SparkMax and CTRE's API for the CANCoder. Because the hardware belongs to both vendors, there is no natural/built in way to integrate the two like how you have done here. Instead, you need to define an instance of CANCoder and manually call the refresh and update methods to update a StatusSignal<Double> which you then use to seed the PID algorithm on the SparkMax by setting the initial position of the RelativeEncoder as you previously had.

Here is the rough idea:
With your hardware objects:
https://v6.docs.ctr-electronics.com/en/stable/docs/api-reference/api-usage/status-signals.html
https://v6.docs.ctr-electronics.com/en/stable/docs/api-reference/api-usage/configuration.html

private final CANcoder turnAbsoluteEncoder; // Define needed StatusSignals // Later down in the constructor turnAbsoluteEncoder = new CANcoder(config.encoderId()); // Configure other non-persistant settings on the CANcoder like unit conversion and polling rates. You can find this is the AdvantageKit templates for some sensible values.

In the IO Update Call:
https://v6.docs.ctr-electronics.com/en/stable/docs/api-reference/api-usage/status-signals.html#refreshing-the-signal-value

// Refresh the status signal for needed signals

Finally, the helper method to get the absolute encoder position

// Update to get value from the CANcoder instead of from a REV API Encoder.

I don't understand how the StatusSignal object fits into this structure. I tried adding a StatusSignal object, but I'm not sure how it gets used later in the code or how to reconcile the fact that getAbsolutePosition returns a StatusSignal < Angle > object, but we initialize a StatusSignal < Double > object.

Everything else makes sense and is complete.

I googled and AI searched for solutions, but didn't find anything that worked.

Sorry StatusSignal<Double> was me speculating. The correct initialization is to use StatusSignal<Angle>. See here for example.

As for why we need the status signal:
Unlike the REV API which uses getters to get the last value updated by the encoder, Phoenix uses an object to represent the specific signal/sensor value you want to poll and basically "caches" this value in the form of a status signal. Basically, just a wrapper around the signal you want to use. The thing that makes this tricky is that this status signals always points towards the signal value at the time it was created. In order to update it to the last available sensor value, and update or "refresh" method must be called.

If you're curious why have this system as apposed to REV's getter setter system, by using a status signal, you can associate the sensor value with a bunch of other important metadata markers like timestamp, latency, units, or error condition of the data. Unlike Phoenix, REV's API doesn't guarantee that these values would be synchronized with the sensor value reported by the getter, making the metadata markers pretty useless.

…onfigure other non-persistant settings on the CANcoder like unit conversion and polling rates. You can find this is the AdvantageKit templates for some sensible values.

First draft outreach bot 16.0, drive only

8294b17

madmosserc requested review from Bluewaves54 and srimanachanta August 19, 2025 01:38

srimanachanta requested changes Aug 20, 2025

View reviewed changes

Fixes and updates from comments

6b81db1

srimanachanta self-requested a review September 23, 2025 16:11

srimanachanta requested changes Sep 23, 2025

View reviewed changes

CANCoder Updates - Did everything, but not sure what this means: // C…

78c2dc6

…onfigure other non-persistant settings on the CANcoder like unit conversion and polling rates. You can find this is the AdvantageKit templates for some sensible values.

madmosserc requested a review from srimanachanta October 13, 2025 04:20

		// Disable LEDs, will reduce software and electrical overhead but disable hardware alerts
		public static final boolean ENABLE_LEDs = false;

		// Disable the AdvantageKit logger from running
		public static final boolean ENABLE_LOGGING = true;

		static final double mk4iDriveGearing = (50.0 / 14.0) * (19.0 / 25.0) * (45.0 / 15.0);
		static final double mk4iTurnGearing = 12.8;

First draft outreach bot 16.0, drive only #6

Are you sure you want to change the base?

First draft outreach bot 16.0, drive only #6

Uh oh!

Conversation

madmosserc commented Aug 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

srimanachanta left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

madmosserc Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

madmosserc Oct 10, 2025 •

edited

Loading