Skip to content

Conversation

@DeltaDizzy
Copy link
Contributor

Because SysIdRoutine is contained within the Commands v2 library, an equivalent is needed for Commands v3 to enable SysId usage.

Feedback sought on child command names.

@DeltaDizzy DeltaDizzy requested a review from a team as a code owner October 26, 2025 01:18
@github-actions github-actions bot added the 2027 2027 target label Oct 26, 2025
@DeltaDizzy
Copy link
Contributor Author

Additional questions:

  1. Should SysIdRoutine's Mechanism inner class be distinguished from Commands v3's Mechanism by a rename or namespace qualification? Currently I have it renamed to SysIdMechanism but am not sure.
  2. Should the removal of null parameters in favor of defaults and use of static Config factories be ported to the Commands v2 version of SysIdRoutine?

Copy link
Contributor

@Gold856 Gold856 left a comment

Choose a reason for hiding this comment

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

We don't do m_ for public fields. At least, that's the general convention across WPILib (it's not super formalized). Separately, we should clean it up in the v2 SysIdRoutine.

@DeltaDizzy
Copy link
Contributor Author

We don't do m_ for public fields. At least, that's the general convention across WPILib (it's not super formalized). Separately, we should clean it up in the v2 SysIdRoutine.

Cleanup of v2 is planned for a followup PR because I wanted to keep this PR focused, but if it's not too much of a scope creep I can do it here I suppose.

* multiple times in a single logfile, the user will need to select which of the tests to use for
* the fit in the analysis tool.
*/
public class SysIdRoutine extends SysIdRoutineLog {
Copy link
Member

Choose a reason for hiding this comment

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

This class should have tests. The v2 SysIdRoutine tests are a good starting point to copy over.

@SamCarlberg SamCarlberg added the component: commands v3 WPILib commands library version 3 label Oct 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2027 2027 target component: commands v3 WPILib commands library version 3

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants