search

Team2168 / 2016_Main_Robot

Code for the 2016 season
0 stars 0 forks source link

Move Shooter Constants to RobotMap #36

Closed NotInControl closed 8 years ago

NotInControl commented 8 years ago

Move these constants to Robot Map. Unless there is a good reason to keep them in the subsystem.

https://github.com/Team2168/2016_Main_Robot/blob/master/src/org/team2168/subsystems/Shooter.java#L29-L30

jcorcoran commented 8 years ago

The reason to keep them in the subsystem is that they shouldn't be needed by anyone else in the. ode. Moving them out into RobotMap make the code note difficult to follow.

Plus the more things you dump in RobotMap, the harder that file gets to read.

PWM channel numbers belong in robot map because there's potential for two subsystems to try to take the same port number. Having subsystem config values accessible globally doesn't really make sense to me as you're not going to understand which parameter you need to tweak without looking at the subsystem code. And only that one subsystem needs access to those variables. Splitting it up into multiple files just makes things harder to use IMO.

Sent from mobile.

NotInControl commented 8 years ago

Ok you can keep as is and close both issues

Sent on a Sprint Samsung Galaxy S® 5

-------- Original message -------- From: James Date:02/15/2016 11:24 AM (GMT-05:00) To: Team2168/2016_Main_Robot 2016_Main_Robot@noreply.github.com Cc: Kevin Subject: Re: [2016_Main_Robot] Move Shooter Constants to RobotMap (#36)

The reason to keep them in the subsystem is that they shouldn't be needed by anyone else in the. ode. Moving them out into RobotMap make the code note difficult to follow.

Plus the more things you dump in RobotMap, the harder that file gets to read.

PWM channel numbers belong in robot map because there's potential for two subsystems to try to take the same port number. Having subsystem config values accessible globally doesn't really make sense to me as you're not going to understand which parameter you need to tweak without looking at the subsystem code. And only that one subsystem needs access to those variables. Splitting it up into multiple files just makes things harder to use IMO.

Sent from mobile.

Reply to this email directly or view it on GitHubhttps://github.com/Team2168/2016_Main_Robot/issues/36#issuecomment-184280107.