[wpilib] Clean up drive class implementations #1691
Conversation
calcmogul
force-pushed
the
new-drive
branch
2 times, most recently
from
b28e760
to
cc7919c
Compare
|
What is the merge plan for this? |
|
Good question. The Drive, PIDController, LinearFilter, and command-based rewrites are all in the same boat, so whatever we decide can be applied to all of them, essentially. The main issue is namespace conflicts. We can put them in an "experimental" namespace, but having effectively half the API like that feels strange. We could also try making a subpackage based on the year. Either way, there would be breakage (albeit minor) when it gets moved out of the subpackage after the old package's deprecation period. |
| public void normalize(double maxSpeed) { | ||
| double maxMagnitude = Math.max(Math.abs(left), Math.abs(right)); | ||
| if (maxMagnitude > maxSpeed) { | ||
| left /= maxMagnitude * maxSpeed; |
There was a problem hiding this comment.
Why are other classes
out *= speed/magnitude
but this is
out /= magnitude*speed
There was a problem hiding this comment.
Dangit. I missed that one.
There was a problem hiding this comment.
Is one better than the other?
There was a problem hiding this comment.
Yeah. One is actually correct. :)
calcmogul
force-pushed
the
new-drive
branch
2 times, most recently
from
15fc72c
to
ef98998
Compare
calcmogul
force-pushed
the
new-drive
branch
2 times, most recently
from
008d99f
to
4863903
Compare
calcmogul
force-pushed
the
new-drive
branch
3 times, most recently
from
2d8fb54
to
b65817b
Compare
calcmogul
force-pushed
the
new-drive
branch
4 times, most recently
from
a7765d5
to
108eb04
Compare
calcmogul
changed the title
calcmogul
force-pushed
the
new-drive
branch
6 times, most recently
from
ca71ded
to
b78fa05
Compare
calcmogul
force-pushed
the
new-drive
branch
3 times, most recently
from
b2ea89b
to
875b617
Compare
calcmogul
force-pushed
the
new-drive
branch
from
2afafc3
to
d7ebe6f
Compare
| @@ -31,7 +33,7 @@ bool GenericHID::GetRawButtonReleased(int button) { | |||
| } | |||
|
|
|||
| double GenericHID::GetRawAxis(int axis) const { | |||
| return m_ds->GetStickAxis(m_port, axis); | |||
| return ApplyDeadband(m_ds->GetStickAxis(m_port, axis), m_deadband); | |||
There was a problem hiding this comment.
Commonly teams would want to apply different deadbands to different axes, for example the twist on a joystick, or the triggers on a gamepad, will likely have different deadzones to the X/Y axes.
It seems that this would not be useful to teams that want to utilise these other axes.
There was a problem hiding this comment.
That use case isn't supported by the current drive class implementation either; the same deadband in the drive class applies to all axes.
There was a problem hiding this comment.
Sure, but it looks like this would apply regardless of whether you're using the axis for driving.
calcmogul
force-pushed
the
new-drive
branch
5 times, most recently
from
b76a2ef
to
15da046
Compare
Left and right sides of the differential drive and mecanum drive classes are now both forward and the "inversion multiplier" was removed. The deadband functionality was moved to GenericHID::GetRawAxis() since the deadband is generally applied to joystick axes. MotorSafety was removed since that's handled by the drivetrain's motor controllers already.
|
Superseded by #3340. |
These return the inverse kinematic values, so they can be composed with
feedback controllers. The internal implementation for DifferentialDrive
was massively simplified as well.
Input limits were removed since the normalization takes care of that.
The deadband functionality was moved to GenericHID::GetRawAxis class
since the deadband is generally applied to joystick axes.
MotorSafety was removed since that's handled by the drivetrain's motor
controllers already.