-
Notifications
You must be signed in to change notification settings - Fork 617
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
Add LED pattern API for easily animating addressable LEDs #6344
Conversation
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.
This API looks amazing!
What is planned for C++ parity?
wpilibj/src/main/java/edu/wpi/first/wpilibj/AddressableLEDBufferView.java
Outdated
Show resolved
Hide resolved
wpilibj/src/main/java/edu/wpi/first/wpilibj/AddressableLEDBuffer.java
Outdated
Show resolved
Hide resolved
No plans for C++ at the moment. The Java API is something I wrote for my FRC team's use so they could make LED patterns without needing to figure out animations; I figured I would contribute it upstream so it could help other teams. But I'm not a C++ developer, and I won't have time to learn the cpp-isms to write a quality port between mentoring my team and resuming work on robotbuilder2. |
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.
Think you might wanna base this off main 😉
Ah right, I forgot this PR was targeting the development 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.
Overall, looks pretty good! A few doc comments are missing / incomplete, though: ApplyTo(std::span<>, LEDWriterFn)
is missing @param
, ApplyTo(std::span<>)
is missing the entire doc comment, and ScrollAtRelativeSpeed()
is missing @param
and @return
.
Unfortunately, I haven't been able to spend enough time to completely review the logic, but nothing jumps out as wrong. I will say that it would be nice to be able to configure the starting time so that you could guarantee that patterns like the scroll will start in a predictable state at the start of a command regardless of when the command is started, but that could be added later.
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.
The C++ implementations need lifetime fixes. Since the LEDPattern is a value type, it's unsafe to capture references to it. Similarly std::span is a reference type that needs to be copied into a std::vector when captured.
It appears the floormod/floordiv change broke tests? |
Since FloorDiv() and FloorMod() have no unit tests, we should probably add some. As for the bug, my guess is the xor sign check gives the wrong answer if the integers are mismatched sizes. We can either constrain the parameters and return type to be the same integral type, whichever it is, or we could make the sign check more robust. |
Add LEDReader and LEDWriter helper interfaces to facilitate composing simple patterns into more complex ones, e.g. `LEDPattern.solid(Color.kBlue).breathe(Seconds.of(0.75))` Add a view class for splitting a single large buffer into smaller distinct sections, which is useful for dealing with long chained LED strips mounted on different parts of a robot Remove addressableled example rainbow test; it's redundant with the LEDPattern tests
Default behavior in LEDReader does the same thing, no need to duplicate it in the buffer class
Permits views of views, for example Requires views to use separate reader/writer fields because we can't represent intersection types directly in fields. It would require a generic type bound on the view class, which would pollute field declarations (eg `View<View<Buffer>>` for a view-of-a-view)
Converting from initializer_list requires span contents to be const
MathExtras is LLVM - don't mix it in there
Co-authored-by: Tyler Veness <[email protected]>
FloorDiv() and FloorMod() only work for signed inputs.
Add LEDReader and LEDWriter helper interfaces to facilitate composing simple patterns into more complex ones, e.g.
LEDPattern.solid(Color.kBlue).breathe(Seconds.of(0.75))
. Pattern composition relies on changing out the write behavior; for example,offsetBy
increments the indexes to write to; whileblink
will switch between playing a base pattern and turning off all the LEDs.Add a view class for splitting a single large buffer into smaller distinct sections, which is useful for dealing with long chained LED strips mounted on different parts of a robot. Views cannot be written directly to an LED strip (in fact, trying to do so won't even compile)
Adds some utility methods to the
Color
class for interpolating between two colors, and support color representations with 32-bit integers to avoid object allocationsExamples
Command based subsystem:
Animated rainbow:
Flag:
Playing animations for an alliance color based on driverstation data: