Skip to content

Finding Fixes in the Rewrite Dry Run#581

Draft
28PaulHauge wants to merge 3 commits into
mainfrom
finding-fixes-rewriteDryRun
Draft

Finding Fixes in the Rewrite Dry Run#581
28PaulHauge wants to merge 3 commits into
mainfrom
finding-fixes-rewriteDryRun

Conversation

@28PaulHauge

Copy link
Copy Markdown
Contributor

No description provided.

@28PaulHauge 28PaulHauge self-assigned this Apr 19, 2026

@claude claude Bot left a comment

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.

This PR modernizes switch statements throughout the codebase and adds missing @OverRide annotations — good cleanup overall. However, there is one critical breaking change in WoodBotDrivetrain.TunerSwerveDrivetrain that would cause CompBot and PracticeBot to silently use WoodBot's drivetrain constants at runtime.

TalonFX::new,
TalonFX::new,
CANcoder::new,
WoodBotDrivetrain.drivetrainConstants,

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.

Breaking change affecting all robot configs. This constructor now ignores its drivetrainConstants parameter and hardcodes WoodBotDrivetrain.drivetrainConstants. Since CommandSwerveDrivetrain extends WoodBotDrivetrain.TunerSwerveDrivetrain (see CommandSwerveDrivetrain.java line 40), every config (CB, PB, WB, SIM) that creates a CommandSwerveDrivetrain passes its own constants through super() — but those constants are now silently discarded here. CompBot and PracticeBot would use WoodBot CAN bus name, Pigeon2 ID, etc. Change this back to drivetrainConstants (the parameter).

TalonFX::new,
CANcoder::new,
drivetrainConstants,
WoodBotDrivetrain.drivetrainConstants,

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.

Same issue: the drivetrainConstants parameter is ignored here too. Note the 4-arg constructor at line 325 was left unchanged and correctly passes the parameter through — the inconsistency suggests lines 282 and 305 were unintentionally changed. Change back to drivetrainConstants (the parameter).

@JRTaylord
JRTaylord marked this pull request as draft April 21, 2026 00:29
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