-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Units Library #10
base: master
Are you sure you want to change the base?
Conversation
build.gradle
Outdated
// or from command line. If not found an exception will be thrown. | ||
// You can use getTeamOrDefault(team) instead of getTeamNumber if you | ||
// want to store a team number in this file. | ||
team = frc.getTeamNumber() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name not indicative of type
team_number
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't change this because it's setting a property that's used by the Gradle plugin we use
|
||
// ramsete constants (tested for most robots) | ||
public static final double kB = 2.0; | ||
public static final double kZeta = 0.7; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good naming here
public static final Trajectory TEST_TRAJECTORY = | ||
TrajectoryGenerator.generateTrajectory( | ||
new Pose2d(0, 0, new Rotation2d(0)), | ||
List.of( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is there a newline here?
src/main/java/org/team4159/frc/robot/subsystems/Drivetrain.java
Outdated
Show resolved
Hide resolved
src/main/java/org/team4159/frc/robot/subsystems/Drivetrain.java
Outdated
Show resolved
Hide resolved
}); | ||
StringBuilder name = new StringBuilder(); | ||
boolean slashed = false; | ||
for (int i = sorted_units.size() - 1; i >= 0; i--) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need some newlines in the mehtod to organize the code better
@Override | ||
public String symbol() { | ||
ArrayList<Map.Entry<Unit, Integer>> sorted_units = new ArrayList<>(getUnits().entrySet()); | ||
sorted_units.sort((a, b) -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seperate this into another function and pass that function into the sort
|
||
@Override | ||
public String toString() { | ||
return amount + " " + unit.symbol(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does Java automatically convert doubles to string when concatenating with string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
do we still want to merge this, or should i close the pr? |
Adds a lot of utilities to the library
DONT MERGE