-
Notifications
You must be signed in to change notification settings - Fork 0
led code #10
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
led code #10
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remember integers aren't always perfectly divisible by 2
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure how hard it would be to implement, but it would be possible to also animate halves (eg. top half is the rainbow animation, bottom is the fire animation)? Not necessary though. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| // Copyright (c) 2025 Team 2342 | ||
| // https://github.com/FRCTeamPhoenix | ||
| // | ||
| // This source code is licensed under the MIT License. | ||
| // See the LICENSE file in the root directory of this project. | ||
|
|
||
| package org.team2342.lib.leds; | ||
|
|
||
| import org.littletonrobotics.junction.Logger; | ||
|
|
||
| public class LedIOSim implements LedIO { | ||
| private LedColor firstColor = LedColor.off(); | ||
| private LedColor secondColor = LedColor.off(); | ||
| private LedEffect firstEffect = LedEffect.OFF; | ||
| private LedEffect secondEffect = LedEffect.OFF; | ||
|
|
||
| @Override | ||
| public void setColor(Half half, LedColor color) { | ||
| if (color == null) { | ||
| color = LedColor.off(); | ||
| } | ||
|
|
||
| switch (half) { | ||
| case FIRST: | ||
| firstColor = color; | ||
| firstEffect = LedEffect.SOLID; | ||
| break; | ||
| case SECOND: | ||
| secondColor = color; | ||
| secondEffect = LedEffect.SOLID; | ||
| break; | ||
| case ALL: | ||
| firstColor = color; | ||
| secondColor = color; | ||
| firstEffect = LedEffect.SOLID; | ||
| secondEffect = LedEffect.SOLID; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void setEffect(Half half, LedEffect effect, LedColor color){ | ||
| if (color == null) { | ||
| color = LedColor.off(); | ||
| } | ||
|
|
||
| switch (half) { | ||
| case FIRST: | ||
| firstColor = color; | ||
| firstEffect = effect; | ||
| break; | ||
| case SECOND: | ||
| secondColor = color; | ||
| secondEffect = effect; | ||
| break; | ||
| case ALL: | ||
| firstColor = color; | ||
| secondColor = color; | ||
| firstEffect = effect; | ||
| secondEffect = effect; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void updateInputs(LedIOInputs inputs){ | ||
| inputs.firstHalfColor = firstColor; | ||
| inputs.secondHalfColor = secondColor; | ||
| inputs.firstHalfEffect = firstEffect; | ||
| inputs.secondHalfEffect = secondEffect; | ||
|
|
||
| Logger.recordOutput("LED/FirstHalf/Red", firstColor.red); | ||
| Logger.recordOutput("LED/FirstHalf/Green", firstColor.green); | ||
| Logger.recordOutput("LED/FirstHalf/Blue", firstColor.blue); | ||
| Logger.recordOutput("LED/FirstHalf/Effect", firstEffect.toString()); | ||
|
|
||
| Logger.recordOutput("LED/SecondHalf/Red", secondColor.red); | ||
| Logger.recordOutput("LED/SecondHalf/Green", secondColor.green); | ||
| Logger.recordOutput("LED/SecondHalf/Blue", secondColor.blue); | ||
| Logger.recordOutput("LED/SecondHalf/Effect", secondEffect.toString()); | ||
| } | ||
| } |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid using driver/operator naming, stick to the first/second naming.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comments on the class are nice, but don't explain the functions in the class doc. Just a high-level explanation of what the class does.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you are using a periodic() function, would it useful to have the class extend SubsystemBase so the periodic function gets called automatically? It would also let you use commands to set the LEDs, and let the CommandScheduler handle conflicts.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The default state for the LEDs should always be off, and not flashing white.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't there a better way to do flashing? If you already using animations, why not create a flashing animation? |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| // Copyright (c) 2025 Team 2342 | ||
| // https://github.com/FRCTeamPhoenix | ||
| // | ||
| // This source code is licensed under the MIT License. | ||
| // See the LICENSE file in the root directory of this project. | ||
|
|
||
| package org.team2342.lib.leds; | ||
|
|
||
| import edu.wpi.first.wpilibj.Timer; | ||
| import edu.wpi.first.wpilibj2.command.SubsystemBase; | ||
|
|
||
| public class LedStrip extends SubsystemBase { | ||
| private final LedIO io; | ||
| private LedIO.LedColor firstColor = LedIO.LedColor.off(); | ||
| private LedIO.LedEffect firstEffect = LedIO.LedEffect.OFF; | ||
| private LedIO.LedColor secondColor = LedIO.LedColor.off(); | ||
| private LedIO.LedEffect secondEffect = LedIO.LedEffect.OFF; | ||
|
|
||
| private boolean flashing = false; | ||
| private double lastFlashTime = 0.0; | ||
| private static final double FLASH_PERIOD_SEC = 0.5; | ||
|
|
||
| public LedStrip(LedIO io){ | ||
| this.io = io; | ||
| } | ||
|
|
||
| public void setFirst(LedIO.LedEffect effect, LedIO.LedColor color){ | ||
| firstEffect = effect; | ||
| firstColor = color; | ||
| } | ||
|
|
||
| public void setSecond(LedIO.LedEffect effect, LedIO.LedColor color){ | ||
| secondEffect = effect; | ||
| secondColor = color; | ||
| } | ||
|
|
||
| @Override | ||
| public void periodic() { | ||
| updateFlashing(); | ||
| applyEffect(LedIO.Half.FIRST, firstEffect, firstColor); | ||
| applyEffect(LedIO.Half.SECOND, secondEffect, secondColor); | ||
| } | ||
|
|
||
| private void updateFlashing() { | ||
| double now = Timer.getFPGATimestamp(); | ||
| if (now - lastFlashTime >= FLASH_PERIOD_SEC) { | ||
| flashing = !flashing; | ||
| lastFlashTime = now; | ||
| } | ||
| } | ||
|
|
||
| private void applyEffect(LedIO.Half half, LedIO.LedEffect effect, LedIO.LedColor color){ | ||
| switch (effect) { | ||
| case SOLID: | ||
| io.setColor(half, color); | ||
| break; | ||
| case FLASHING: | ||
| io.setColor(half, (flashing ? color : LedIO.LedColor.off())); | ||
| break; | ||
| case RAINBOW: | ||
| io.setEffect(half, LedIO.LedEffect.RAINBOW, color); | ||
| break; | ||
| case OFF: | ||
| io.setColor(half, LedIO.LedColor.off()); | ||
| break; | ||
| } | ||
| } | ||
| } |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Going forward, avoid changing stuff in other files not related to the task at hand. These are just comments, so I'll leave them, but if you want to edit something else, make another branch. |
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.
Add a function to set the entire strip (all it needs to do is call setFirst and setSecond, but a single function to do both would be nice)
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.
Avoid using the Phoenix specific Animation type in the generic IO. You could make a custom class that extends Animation, or a class that can convert itself to a Phoenix Animation (eg. PIDFFConfigs).