1757WestwoodRobotics / RobotBase

a base swerve drive robot for 1757
MIT License
0 stars 5 forks source link

Landon's Review Comments #1

Open LandonBayer opened 9 months ago

LandonBayer commented 9 months ago

explain or fix right now.

LandonBayer commented 9 months ago

Line ~102 of README.md: Add a step to download pipenv Line 117 of README.md: remove Lines 118-139 of README.md: move to another section to avoid confusion Line 181 of README.md: is that supposed to look like a comment or be ran as it's own command?

Line 1 of defensestate.py: what is the difference here, just an updated library thing?

Line 32 of absoluterelativedrive.py: why no more brackets? line 50 of absoluterelativedrive.py: why no more alliance side switch?

New Auto.auto: is this just a new thing for pathplanner now? (no more manual path stuff yay!)

Line 38 of requirements.txt: it has navx there, do we need pigeon too?

line 33 of robot.py: what is SignalLogger?

lines 60-70 of robotcontainer.py: what is dat named commands thing (explain)

lmaxwell24 commented 9 months ago

New Auto.auto is a pathplanner 2024 path requirements, navx is put there as a backup, actually wouldn't be needed, comes with the pipenv file as robotpy-all, pigeon is part of phoenix6 library

absoluterelativedrive in order to have pathplanner-ness, not doing consistently a forwards-is-blue-forwards, so now odometry is your own field origin no more brackets because now the function takes (*args) which will take an infinite number and convert it into a set, instead of using a list, 2024 migration

defensestate: yeah commands fully in python

robot.py signallogger: a way to log the CTRE signals on a bot (position, velocity, control) basically control loop logging so we can view how the motors behave over time later in phoenix tuner

named commands: see here named commands are commands that can run during an auto, so we can trigger events within the autons

LandonBayer commented 9 months ago

Line 36 of physics.py: what's typing.callable again? line 89 of physics.py: damn we be simulating voltage inputs okay okay sheeesh line 159 of physics.py: this order of operations is lowk confusing line 282 of physics.py: wat is feed_enable

will do more sometime tmrw :)

lmaxwell24 commented 9 months ago

L36: callable is a function, in that it takes in nothing and returns a TalonFXSimState, since the sim states will update as calls are made L89: yeah thats how DCMotorSim operates, based on V=IR and motor values, given that you'll know the resistance created based on rotation and inertia and the likes L159: take the swerve position, which is in rotations, and is the motor, divide it by the gear ratio (so it'll be the actual angle of the swerve) and then multiply to get radians, which can go into Rotation2d L282: read this

LandonBayer commented 9 months ago

line 322 of constants.py: why did the drive gain go from .15 to .001 lol line 325: what is a V gain line 328: why did it go from .6 to 4 line 330: why D gain from 12 to 0 lines 540/541: are these just like random or some number u found or smthing? line 544: why is default location now just 0,0,0 instead of middle of the field?

line 21 of visionsubsystem.py: why is it loading from 2023 (for examples ig, but this should be 2024 now) line like 35: is our pose estimation automatic using PhotonVision now? general thing for vision: i like it now it's much more readable :)

line 12 of motorsimulator.py: why is it called SimTalon if it can take a diff motor as an input? also, why does this even exist if DCMotorSim already exists? line 40: Whats the diff between Falcon and FalconFOC? line 55: isn't this already in like robotcontainer or whatever? why do we have to do it here? (there was smthing with input voltage, update, etc)

line 14 of simcoder: yay no more ctre check! line 24: why does it need to multiply by 360? is it like 1 per revolution but as a float?

simtalon.py: why is there motorsimulator, simcoder, AND simtalon dawg line 61: why is the if isReversed in front of it what da lines 64-81: do we really need to smartdash publish every gain lol lines 85-87: what do these values mean (0,0,false,0,0,false etc) line 93: why is the update at 100 huh shouldn't it be 50 bc 50 hz?

drive system looks fine but i speed read it

also we really need to test the pigeon, I can try to help u find it in the lab if I have time

LandonBayer commented 9 months ago

On newer commits!!

line 26 of simcoder.py: Why is it simply return Rotation2d instead of Rotation2d.fromRadians or smthing?

commit dacfc0c: is names just to make stuff easier to identify, or is there any real purpose besides that (im fine with it to be clear) also on that commit: where are the names input?

line 108 of operatorinterface.py: We should test that POV actually works, I trust that it should but yknow

I just suddenly realized I can actually comment on lines now ig?? So keep a lookout for those instead