Skip to content

Conversation

koolpoolo
Copy link
Contributor

@koolpoolo koolpoolo commented Sep 29, 2025

helps address #177

// various former controls that were previously used and could be referenced in
// the future

// operatorController
Copy link

@markpete markpete Sep 29, 2025

Choose a reason for hiding this comment

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

How likely is this to be used in the future? If it hasn't been used in 6 months, just delete it unless you have a specific clear plan for re-use.

Future coders can always pull it back from git history.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no its for archives

@markpete
Copy link

Passing driveBaseWrapper around to all of the routines is fine and aligns with the programming standards the team uses, so fine leave as is. However, assuming there is ever only a single driveBaseWrapper instance at a time, you might consider implementing a Singleton pattern inside of the DriveBaseWrapper class, that way you can get the instance inside of the routines where you need it instead of having to pass it down through the call stacks all over the place.

Note that this is a bit more of an advanced coding construct (so totally fine not to do it this way).
https://www.geeksforgeeks.org/system-design/singleton-design-pattern/

Copy link

@markpete markpete left a comment

Choose a reason for hiding this comment

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

Approved pending comments are addressed.

Copy link

@markpete markpete left a comment

Choose a reason for hiding this comment

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

Please address comments.

jamesdooley4
jamesdooley4 previously approved these changes Sep 30, 2025
@Jetblackdragon Jetblackdragon marked this pull request as draft October 1, 2025 20:18
@iamawesomecat
Copy link
Contributor

Test @ field calibration at bordie

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.

8 participants