Skip to content

Flywheels: Check to see if motor configs are applied properly#204

Merged
Fatblabs merged 9 commits intomainfrom
Config-check
Apr 9, 2026
Merged

Flywheels: Check to see if motor configs are applied properly#204
Fatblabs merged 9 commits intomainfrom
Config-check

Conversation

@Fatblabs
Copy link
Copy Markdown
Contributor

@Fatblabs Fatblabs commented Apr 9, 2026

Description: When applying motor configuations, we now check to see if this has failed. If it has, try to apply it at least 5 times with a default of 10 miliseconds before giving up and continuing on with its' business.

Maybe another pr for all other subsystems later.

…f this has failed. If it has, try to apply it at least 5 times with a slightly extend timeout of 30 miliseconds before giving up and continuing on with its' business.
@Fatblabs Fatblabs requested a review from jamesdooley4 April 9, 2026 06:08
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the TalonFX motor configuration in the Flywheels subsystem by introducing a new applyConfig helper method. This method implements a retry mechanism with a timeout for applying configurations, logging successful applications and reporting critical errors if all attempts fail. Feedback suggests that the MAX_APPLY_CONFIG_ATTEMPTS and MAX_APPLY_CONFIG_TIMEOUT constants should be static final, with MAX_APPLY_CONFIG_ATTEMPTS being an int. Additionally, the applyConfig method should be made private for better encapsulation, and its logging messages should be more specific, including the motor ID. The error message for failed configurations should also utilize the MAX_APPLY_CONFIG_ATTEMPTS constant instead of a hardcoded value.

@Fatblabs Fatblabs requested a review from jamesdooley4 April 9, 2026 06:31
@Jetblackdragon Jetblackdragon linked an issue Apr 9, 2026 that may be closed by this pull request
@Fatblabs
Copy link
Copy Markdown
Contributor Author

Fatblabs commented Apr 9, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a more robust motor configuration process by implementing a retry mechanism with logging and error reporting in the Flywheels subsystem. The feedback focuses on aligning the configuration timeout with the pull request description, ensuring constants and variables follow standard Java naming conventions, and correcting minor typos and string formatting in the log messages.

@Fatblabs
Copy link
Copy Markdown
Contributor Author

Fatblabs commented Apr 9, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request enhances the Flywheels subsystem by implementing a more robust configuration application process for TalonFX motor controllers. It introduces a retry mechanism for applying configurations, complete with logging for successful applications and critical error reporting for failures. The reviewer suggests making the newly added configuration constants static to optimize memory usage.

jamesdooley4
jamesdooley4 previously approved these changes Apr 9, 2026
Copy link
Copy Markdown
Member

@jamesdooley4 jamesdooley4 left a comment

Choose a reason for hiding this comment

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

Just one minor comment

@Fatblabs Fatblabs merged commit 05933cb into main Apr 9, 2026
2 checks passed
@Fatblabs Fatblabs deleted the Config-check branch April 9, 2026 16:10
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.

Flywheel leader motor heats up more than follower

2 participants