Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,4 @@ fabric.properties

# Exclude local preference files generated by VSCode editors.
.vscode/
.idea/

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we actually want to remove the entire .idea directory.
If we want it to be auto imported, we can uncomment these lines.
Even if we do want to do this, it would probably be better to have a separate discussion on this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spderman3333 either revert, or address in a separate PR. Generally, you want to avoid bundling unrelated changes in the same PR (this here being some random config change that has nothing to do with the PR Title, which is all about fixing some logical error in the motor drive impl).

3 changes: 0 additions & 3 deletions .idea/.gitignore

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, keep config changes out of this PR.

This file was deleted.

6 changes: 0 additions & 6 deletions .idea/compiler.xml

This file was deleted.

6 changes: 0 additions & 6 deletions .idea/google-java-format.xml

This file was deleted.

5 changes: 0 additions & 5 deletions .idea/misc.xml

This file was deleted.

6 changes: 0 additions & 6 deletions .idea/vcs.xml

This file was deleted.

8 changes: 8 additions & 0 deletions lib/src/main/java/com/team2813/lib2813/control/Motor.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,12 @@ public interface Motor {
* @return The current applied current
*/
Current getAppliedCurrent();

/** Stops the motor. */
void disable();
Comment thread
spderman3333 marked this conversation as resolved.

/** Stops the motor. */
default void stopMotor() {
disable();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
import java.util.ArrayList;
import java.util.List;

/**
* Wrapper class for SparkMax brushed and brushless motor controllers. Deprecated as we will likely
* not use SparkMaxes again.
*/
@Deprecated(forRemoval = true)
public class SparkMaxWrapper implements PIDMotor {
private final List<SparkMax> followers = new ArrayList<>();
private final SparkBase motor;
Expand Down Expand Up @@ -81,6 +86,15 @@ public Current getAppliedCurrent() {
return Units.Amps.of(motor.getOutputCurrent());
}

/**
* WARNING: due to the end of support of SparkMaxWrapper, there is no evidence that this method
* will work. Proceed with caution!
*/
@Override
public void disable() {
motor.stopMotor();
}

@Override
public void setPosition(double position) {
encoder.setPosition(position);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,31 @@ public TalonFX motor() {
return motor;
}

/**
* Sets the behavior the motor should exhibit upon receiving a request to stop:
* "<i>disable()</i>".
*
* <ul>
* <li>Coast: The motor stops applying an input, but continues to move with its inertia.
* <li>Brake: The motor stops applying an input, and actively opposes its inertia.
* </ul>
Comment on lines +169 to +172

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enumerating the neutral modes is probably a bad idea because they are already present in the Phoenix API docs, and may change in the future

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, its easier for anyone reading, that does not want to open a whole new tab.

*
* @param mode
*/
public void setNeutralMode(NeutralModeValue mode) {
motor.setNeutralMode(mode);
}

/**
* Sends a disable command to the motor, placing it in its neutral value.
*
* @see TalonFXWrapper#setNeutralMode(NeutralModeValue)
*/
@Override
public void disable() {
motor.disable();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are probably looking for motor.stopMotor(), or even better, motor.set(new NeutralOut()) to be more explicit with the intention. The documentation from motor.disable() implies that it stops the motor controller instead of the output of the motor.

@kcooney kcooney Oct 11, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spderman3333 given all the confusion as to the behavior of disable() on the various vendor motor classes, perhaps the new method you are adding to Motor should be named stopMotor(), not disable().

}

@Override
public void configPIDF(int slot, double p, double i, double d, double f) {
SlotConfigs conf = new SlotConfigs();
Expand Down
Loading
Loading