Merge AddressableLED subsystem #31
Conversation
src/main/java/frc/robot/subsystems/addressableled/AddressableLEDConstants.java
Show resolved
Hide resolved
src/main/java/frc/robot/subsystems/addressableled/AddressableLEDSubsystem.java
Show resolved
Hide resolved
src/main/java/frc/robot/subsystems/addressableled/AddressableLEDSubsystem.java
Show resolved
Hide resolved
…ffold for testing command
| import edu.wpi.first.units.measure.Distance; | ||
|
|
||
| public class AddressableLEDConstants { | ||
| public record Range(int low, int high) {} |
There was a problem hiding this comment.
What does range do? It's not immediately obvious, so there should be a javadoc here
There was a problem hiding this comment.
Like Aceius said, it's still not clear what the purpose of Range is. The parameters are obvious, so you can remove the javadoc comments for those, but you need to make it clear when and where Range is used. Someone should not have to read through to code and use context to understand the purpose of this
I didn't forget to run Spotless this time!
…/reefscape-2025 into addressable-led
src/main/java/frc/robot/commands/test/AddressableLEDTestCommand.java
Outdated
Show resolved
Hide resolved
| import edu.wpi.first.units.measure.Distance; | ||
|
|
||
| public class AddressableLEDConstants { | ||
| public record Range(int low, int high) {} |
There was a problem hiding this comment.
Like Aceius said, it's still not clear what the purpose of Range is. The parameters are obvious, so you can remove the javadoc comments for those, but you need to make it clear when and where Range is used. Someone should not have to read through to code and use context to understand the purpose of this
src/main/java/frc/robot/subsystems/superstructure/wrist/WristConstants.java
Outdated
Show resolved
Hide resolved
The original LED subsystem was based on
AddressableLED, however it was migrated to a direct PWM controller after it was found that the original LED strips did not support the protocol used by WPILib andAddressableLED. A strip that supportsAddressableLED's protocol is now being added, hence this PR consisting of a slightly-updated version of the original LED subsystem implementation based onAddressableLED. The PWM-based LED controller should remain in-tree for now.