1198159 / Bonk2021

0 stars 0 forks source link

Some feedback, questions and notes captured in an issue. #1

Open bobtseattle opened 2 years ago

bobtseattle commented 2 years ago

Some feedback now that I have had a chance to look deeper.

You had asked about removing the main class. I think we should stick with keeping Main.java and the standard. I see no technical advantages but certainly could add to possible confusion. I would recommend heeding the warnings in the wpilib docs to not touch main.

What is the advantage to initializing RobotContainer and OI in the Robot constructor instead of robotInit?

You may consider reverting to the WPILIB recommendations on structure. We don't want to hit any conflicts with the FMS during the competition or cause confusion if new members of the team copy this project for future projects.

https://github.com/1198159/Bonk2021/blob/88af49f5914aae80504b55152780b514588fd6f9/src/main/java/frc/team2412/robot/RobotMap.java#L21 Are these configs correct? They seem reversed.

You were asking about where to put default commands.

Have you confirmed all the hardcoded values (like track width and wheel size) in drive base subsystem? https://github.com/1198159/Bonk2021/blob/88af49f5914aae80504b55152780b514588fd6f9/src/main/java/frc/team2412/robot/subsystems/DrivebaseSubsystem.java

Should there be any limits on the VictorySpinCommand?

Did you try your code on the simulator? Seems like the ideal use of the simulator since it is just the drive base.

What is the plan for autonomous mode? Does the drivebasesubsystem have all the methods it needs to setup and follow trajectories?

1198159 commented 2 years ago

moving main class i am fairly certain will not conflict with fms, but i could double check with some of the head developers of wpilib the configs have been relocated to DrivebaseSubsystem due to an issue personally, i feel like the main class just causes more confusion because it just exists, and most programmers wont even touch the Robot class anyway so they dont have to worry about it. However, if we are going to do a multi-Robot project, the Main class makes sense to hotswap between Robot instances so i could probably care less about the main class than other code choices.

robotContainer and OI (OperatorInterface) being in the constructor is highly beneficial because it means more code gets ran before things start so errors occur sooner, also its unnecessary processing time wasted when initializing auto. To me it also makes sense to put it in the constructor because it makes more sense to create everything at once.

VictorySpinCommand is sadly not going to be used, and auto plan is to drive for 3 seconds (which we have tested)

An interesting thing is we arent even following current WPILib reccomendations with our old standard, the current WPILib rec is to not have RobotMap and create hardware in the constructor, the only reason we are on the standard we are on is because its what we have done in the past it seems to me.

No i have not checked the values they are james-s the wheel dia is def wrong, and i believe drivebase is ready to do ramsete (ask james though)

Additionally, oblog works great

So thats my reasons behind my design choices, tl;dr i could revert to main class if its a serious concern, but im really for the changes i made to everything else, I will also clean up the Configurable interface and add it to the library, its very useful because you can create static instances of a configuration and apply it to motors which is awesome.

Thanks, Alex

On Wed, Oct 6, 2021 at 8:35 PM Bob T @.***> wrote:

Some feedback now that I have had a chance to look deeper.

You had asked about removing the main class. I think we should stick with keeping Main.java and the standard. I see no technical advantages but certainly could add to possible confusion. I would recommend heeding the warnings in the wpilib docs to not touch main.

What is the advantage to initializing RobotContainer and OI in the Robot constructor instead of robotInit?

You may consider reverting to the WPILIB recommendations on structure. We don't want to hit any conflicts with the FMS during the competition or cause confusion if new members of the team copy this project for future projects.

https://github.com/1198159/Bonk2021/blob/88af49f5914aae80504b55152780b514588fd6f9/src/main/java/frc/team2412/robot/RobotMap.java#L21 Are these configs correct? They seem reversed.

  • rightConfig = ()->driveLeftMotor2;
  • leftConfig = ()->driveRightMotor1

You were asking about where to put default commands.

Have you confirmed all the hardcoded values (like track width and wheel size) in drive base subsystem?

https://github.com/1198159/Bonk2021/blob/88af49f5914aae80504b55152780b514588fd6f9/src/main/java/frc/team2412/robot/subsystems/DrivebaseSubsystem.java

Should there be any limits on the VictorySpinCommand?

Did you try your code on the simulator? Seems like the ideal use of the simulator since it is just the drive base.

What is the plan for autonomous mode? Does the drivebasesubsystem have all the methods it needs to setup and follow trajectories?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/1198159/Bonk2021/issues/1, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKYXXRE2KEHFKDDP7YGYEELUFUIQPANCNFSM5FQI5C6Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.