Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wilsonville tweaks #62

Merged
merged 10 commits into from
Mar 27, 2025
Merged

Wilsonville tweaks #62

merged 10 commits into from
Mar 27, 2025

Conversation

TheFlameFish
Copy link
Contributor

Tweaks made during Wilsonville that probably don't fit into specific PRs.

Note with the wrist that it seems significant changes need to be made in software and maybe hardware; some of the changes made to the wrist are kind of band-aid, but I think work for the purpose of this PR.

Copy link
Contributor

@nolanhergert nolanhergert left a comment

Choose a reason for hiding this comment

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

One more thing. Was the change to the wrist code in the last match reverted? I can't tell from the review and I wasn't close enough to the code to see what changed.

@@ -39,7 +39,7 @@ public static final class Module {
public static final int TALON_TURN_MOTOR_3 = 2;
public static final int TALON_CANCODER_3 = 23;

public static final double WHEEL_RADIUS = Units.inchesToMeters(1.906);
public static final double WHEEL_RADIUS = Units.inchesToMeters(1.9375);
Copy link
Contributor

Choose a reason for hiding this comment

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

We were seeing alignment issues all comp. I specifically remember an offset in Y during auto.

While I don't think this is the ultimate root cause, it could be a factor. It's not required to revert for this review, I would just keep an eye on it.

@@ -100,7 +100,7 @@ public void run() {
signalsLock.lock();
try {
if (isCANFD) {
BaseStatusSignal.waitForAll(2.0 / DriveConstants.Module.ODOMETRY_FREQUENCY, signals);
BaseStatusSignal.waitForAll(4.0 / DriveConstants.Module.ODOMETRY_FREQUENCY, signals);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is potentially a really good fix. Can we try setting it to 1.5 or 1.0 in the lab and see if the issue (last CAN ID of wire is not getting enough time during init sequence, causing timeout) manifests itself more reliably to make sure we found the root cause? Then we can tell drive team they don't need to keep twisting the wheels on the field.

@TheFlameFish
Copy link
Contributor Author

TheFlameFish commented Mar 26, 2025

One more thing. Was the change to the wrist code in the last match reverted? I can't tell from the review and I wasn't close enough to the code to see what changed.

I'm not sure what change you're referring to, but I don't think we made any code changes after the last match so I don't think so.

If you're referring to the wrist getting stuck, I think that was more of a hardware issue (or maybe a software issue uncovered by a hardware issue), and wasn't caused by any code changes we made. I'm not sure though. Belle and Christian understand the issue much better than I do

@TheFlameFish TheFlameFish merged commit 828ab39 into main Mar 27, 2025
4 checks passed
@TheFlameFish TheFlameFish deleted the wilsonville-tweaks branch March 27, 2025 17:28
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.

3 participants