Skip to content

Conversation

tk2217
Copy link
Contributor

@tk2217 tk2217 commented Mar 15, 2025

No description provided.

@tk2217 tk2217 requested a review from a team as a code owner March 15, 2025 19:49
@samfreund
Copy link
Member

not the linting 💀

@samfreund samfreund changed the title dynamic controller switching fix: dynamic controller switching Mar 15, 2025
@samfreund samfreund changed the title fix: dynamic controller switching feat: dynamic controller switching Mar 15, 2025
@samfreund
Copy link
Member

I gotta admit, this feels a little excessive. I think all that we really need is changing the controller checking to default to an xbox controller in sim. In a case where we need to use test mode, I expect that we'll likely be changing code as well, so it's not a big deal to need to redeploy.

Copy link
Member

@samfreund samfreund left a comment

Choose a reason for hiding this comment

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

This is looking pretty good. I'd like to see some testing where we determine that there aren't going to be any edge cases, and I want to see that if we have a random disconnect it'll automatically pick the controllers back up. Also, merge main into this. After I see that testing, this should be good to go.

if (Constants.IOConstants.kTestMode) {
System.out.println("Test Mode Enabled\nNot for competition use");
}
this.m_robotContainer.updateControllerConnections();
Copy link
Member

Choose a reason for hiding this comment

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

idk how I feel about this calling every single control loop. That feels maybe excessive.

Copy link
Member

Choose a reason for hiding this comment

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

If you can demonstrate it doesn't make an impact tho I'm fine with it.

if (Constants.IOConstants.kTestMode) {
System.out.println("Test Mode Enabled\nNot for competition use");
}
this.m_robotContainer.updateControllerConnections();
Copy link
Member

Choose a reason for hiding this comment

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

If you can demonstrate it doesn't make an impact tho I'm fine with it.

@samfreund samfreund mentioned this pull request Mar 18, 2025
@samfreund samfreund marked this pull request as draft March 25, 2025 03:35
HENRYMARTIN5
HENRYMARTIN5 previously approved these changes Mar 31, 2025
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.

4 participants