-
Notifications
You must be signed in to change notification settings - Fork 3
adding leds to the lib, UNTESTED #1
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
base: 2024
Are you sure you want to change the base?
Conversation
PerfectlyInternal
left a comment
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.
a few minor changes to keep stuff in order, overall looks like it should work
| public class Leds extends SubsystemBase { | ||
| private final LedConfig config; // Configuration object for LEDs | ||
|
|
||
| private Spark LEDController; // Spark object for controlling LEDs |
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.
move this to the config so the user can pass their own spark if theyd like
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.
confused about this, how would i do that
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.
take a look at other subsystems that use non-motor hardware & their configs
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.
all sybsystems haev motors 💀
| * - config: Configuration object for LEDs | ||
| */ | ||
| public Leds(LedConfig config) { | ||
| this.LEDController = new Spark(config.LEDPort); // Initialize LED controller |
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.
remember to change references to the controller when its in the config, and also check to see if it should be inited from port or just using the object passed in the config
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.
imo it should be inited from the port, makes it less confusing in config
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.
If the user wants to provide their own spark object, we should support that
No description provided.