democat3457/swerve-lib

Mk-generic builder doesn't work for NEOs and .withLayout

Closed this issue · 6 comments

markzakharyan commented

Hi, thanks for maintaining the swervelib library. Team 6560 really appreciates it :)

I recently migrated over to the new Mk-generic format to instantiate the drivetrain, and all NEOs on the robot no longer work (as in, they don't move at all).

For reference, here's our old code:

m_frontLeftModule = Mk4iSwerveModuleHelper.createFalcon500Neo(
    tab.getLayout("Front Left Module", BuiltInLayouts.kList)
        .withSize(2, 4)
        .withPosition(0, 0),
    Mk4iSwerveModuleHelper.GearRatio.L2,
    FRONT_LEFT_MODULE_DRIVE_MOTOR,
    FRONT_LEFT_MODULE_STEER_MOTOR,
    FRONT_LEFT_MODULE_STEER_ENCODER,
    FRONT_LEFT_MODULE_STEER_OFFSET);

m_frontRightModule = Mk4iSwerveModuleHelper.createFalcon500Neo(
    tab.getLayout("Front Right Module", BuiltInLayouts.kList)
        .withSize(2, 4)
        .withPosition(2, 0),
    Mk4iSwerveModuleHelper.GearRatio.L2,
    FRONT_RIGHT_MODULE_DRIVE_MOTOR,
    FRONT_RIGHT_MODULE_STEER_MOTOR,
    FRONT_RIGHT_MODULE_STEER_ENCODER,
    FRONT_RIGHT_MODULE_STEER_OFFSET);

m_backLeftModule = Mk4iSwerveModuleHelper.createFalcon500Neo(
    tab.getLayout("Back Left Module", BuiltInLayouts.kList)
        .withSize(2, 4)
        .withPosition(4, 0),
    Mk4iSwerveModuleHelper.GearRatio.L2,
    BACK_LEFT_MODULE_DRIVE_MOTOR,
    BACK_LEFT_MODULE_STEER_MOTOR,
    BACK_LEFT_MODULE_STEER_ENCODER,
    BACK_LEFT_MODULE_STEER_OFFSET);

m_backRightModule = Mk4iSwerveModuleHelper.createFalcon500Neo(
    tab.getLayout("Back Right Module", BuiltInLayouts.kList)
        .withSize(2, 4)
        .withPosition(6, 0),
    Mk4iSwerveModuleHelper.GearRatio.L2,
    BACK_RIGHT_MODULE_DRIVE_MOTOR,
    BACK_RIGHT_MODULE_STEER_MOTOR,
    BACK_RIGHT_MODULE_STEER_ENCODER,
    BACK_RIGHT_MODULE_STEER_OFFSET);

And here's the code after changing to the Mk-generic builder (note that the .withLayout is commented because it results in a runtime exception):

m_frontLeftModule = new MkSwerveModuleBuilder()
                                // .withLayout(tab.getLayout("Front Left Module", BuiltInLayouts.kList)
                                //         .withSize(2, 4)
                                //         .withPosition(0, 0))
                                .withGearRatio(SdsModuleConfigurations.MK4I_L2)
                                .withDriveMotor(MotorType.FALCON, FRONT_LEFT_MODULE_DRIVE_MOTOR)
                                .withSteerMotor(MotorType.NEO, FRONT_LEFT_MODULE_STEER_MOTOR)
                                .withSteerEncoderPort(FRONT_LEFT_MODULE_STEER_ENCODER)
                                .withSteerOffset(FRONT_LEFT_MODULE_STEER_OFFSET)
                                .build();

                m_frontRightModule = new MkSwerveModuleBuilder()
                                // .withLayout(tab.getLayout("Front Left Module", BuiltInLayouts.kList)
                                //         .withSize(2, 4)
                                //         .withPosition(0, 0))
                                .withGearRatio(SdsModuleConfigurations.MK4I_L2)
                                .withDriveMotor(MotorType.FALCON, FRONT_RIGHT_MODULE_DRIVE_MOTOR)
                                .withSteerMotor(MotorType.NEO, FRONT_RIGHT_MODULE_STEER_MOTOR)
                                .withSteerEncoderPort(FRONT_RIGHT_MODULE_STEER_ENCODER)
                                .withSteerOffset(FRONT_RIGHT_MODULE_STEER_OFFSET)
                                .build();

                m_backLeftModule = new MkSwerveModuleBuilder()
                                // .withLayout(tab.getLayout("Front Left Module", BuiltInLayouts.kList)
                                //         .withSize(2, 4)
                                //         .withPosition(0, 0))
                                .withGearRatio(SdsModuleConfigurations.MK4I_L2)
                                .withDriveMotor(MotorType.FALCON, BACK_LEFT_MODULE_DRIVE_MOTOR)
                                .withSteerMotor(MotorType.NEO, BACK_LEFT_MODULE_STEER_MOTOR)
                                .withSteerEncoderPort(BACK_LEFT_MODULE_STEER_ENCODER)
                                .withSteerOffset(BACK_LEFT_MODULE_STEER_OFFSET)
                                .build();

                m_backRightModule = new MkSwerveModuleBuilder()
                                // .withLayout(tab.getLayout("Front Left Module", BuiltInLayouts.kList)
                                //         .withSize(2, 4)
                                //         .withPosition(0, 0))
                                .withGearRatio(SdsModuleConfigurations.MK4I_L2)
                                .withDriveMotor(MotorType.FALCON, BACK_RIGHT_MODULE_DRIVE_MOTOR)
                                .withSteerMotor(MotorType.NEO, BACK_RIGHT_MODULE_STEER_MOTOR)
                                .withSteerEncoderPort(BACK_RIGHT_MODULE_STEER_ENCODER)
                                .withSteerOffset(BACK_RIGHT_MODULE_STEER_OFFSET)
                                .build();

Essentially, the NEOs don't spin at all, and the .withLayout feature results in a runtime exception on robotInit. Any help would be greatly appreciated.

Javaru commented

I just migrated our code to the new builder today. We are using NEOs for both the Drive and the Steer motors. And its working properly. I do see a couple of issues that may resolve your problems.

In regards to the Layout, it looks like you have a copy & paste error. In your new commented out code, you are setting the same position (0,0) for each panel. In your old code, you were modifying the position for each module (0,0), (2,0), (4,0), (6,0). So in the new code, you are trying to put 4 widgets on top of each other. And thus the Runtime Exception. Also, you have the same name, "Front Left Module", for all for layouts.

As for the steer motor, @democat3457 can confirm if this is the issue, but you may need to set a MkModuleConfiguration, especially given the mixing of motor types. The MkModuleConfiguration contains the PID settings used for the steer motors, (and possibly some other configurations). So its absence could easily result in the steer motors having issue. In the old create{motorType}() functions, setting a Mk4ModuleConfiguration was optional. And I think its optional in the builder as well, but with the mix of motors, you may need to set it. Pass it in as a constructor parameter to the builder:

new MkSwerveModuleBuilder(MkModuleConfiguration.getDefaultSteerNEO())
        .withLayout(...)
        ...

I set one just because we initially thought we needed to tweak our PID settings, But ended up using default values. But I kept it as a reminder of its "availability". And so when I converted to the builder, I switched the Mk4ModuleConfiguration to a MkModuleConfiguration.

If it helps any, here is our code. We're using Kotlin, but it reads pretty similar to Java. (just note Kotlin does not use the keyword new when calling constructors.) We use a (custom) moduleConfig data class to hold various values.

val frontLeftSwerveModule = createSwerveModule(config.moduleGroup.frontLeft)

fun createSwerveModule(moduleConfig): SwerveModule {
    val shuffleboardLayout = shuffleboardTab.getLayout(moduleConfig.name, BuiltInLayouts.kList)
            .withSize(2, 3)
            .withPosition(colIndex++, rowIndex)
    val mkModuleConfiguration = MkModuleConfiguration.getDefaultSteerNEO()
    // note: PID values for steer can be set on the mkModuleConfiguration  
    //       if they ever need to be adjusted
    return MkSwerveModuleBuilder(mkModuleConfiguration)
            .withLayout(shuffleboardLayout)
            .withGearRatio(SdsModuleConfigurations.MK4_L2)
            .withDriveMotor(MotorType.NEO, moduleConfig.driveMotorCanId)
            .withSteerMotor(MotorType.NEO, moduleConfig.steerMotorCanId)
            .withSteerEncoderPort(moduleConfig.steerAbsEncoderCanId)
            .withSteerOffset(moduleConfig.offsetAngle.toRadians())
            .build()
}

Hope that helps. And if not, hopefully democat can jump in and assist.

@democat3457 - I, and my Team 3838, want to echo @markzakharyan thanks. We sincerely appreciate you maintaining this library. Simply put, we would not have a nice new functional swerve drive this year without your library. We would be stuck with a large and not so useful paper-weight :) So THANK YOU!!

  • 👍1
hollisms commented

Agree Team 3201 also thanks you! Saved us countless hours and is much appreciated!

  • 😄1
democat3457 commented

No problem to everyone! I enjoy maintaining this :)

Anyway, regarding the problem, @Javaru is right in saying that passing a MkModuleConfiguration would fix the issue - just use MkModuleConfiguration.getDefaultSteerNEO() like they also use. The reason behind this is actually unintended behavior that I'll fix in the next update: passing no arguments to the builder constructor results in a MkModuleConfiguration being made with no steer PID, so it defaults to using whatever is on the Spark MAX currently - which could very well be 0. I'll try and get this update out later today!

democat3457 commented

@markzakharyan alright, 2023.1.2.1 should be released now, with fixes to default constructor steer PID values.

markzakharyan commented

In regards to the Layout, it looks like you have a copy & paste error. In your new commented out code, you are setting the same position (0,0) for each panel. In your old code, you were modifying the position for each module (0,0), (2,0), (4,0), (6,0). So in the new code, you are trying to put 4 widgets on top of each other. And thus the Runtime Exception. Also, you have the same name, "Front Left Module", for all for layouts.

@Javaru Well... I feel like an idiot now LOL. You're absolutely right about the shuffleboard layout.

You guys were right about the MkModuleConfiguration.getDefaultSteerNEO(). I just passed it in and swervelib works again. We had to use one of the Falcons from our swerve chassis bot on our main bot, so we have 3 modules with NEO turn motors and Falcon drive motors, and 1 module with two NEO motors. Nevertheless, swervelib still works perfectly.

Thanks for the help, everyone :)

  • 👍1
Javaru commented

Well... I feel like an idiot now LOL. You're absolutely right about the shuffleboard layout.

There's not a developer out there that hasn't made a copy & paste error. And it's always hard to proofread our own code :) Good luck to you and your team.