Jackson is a very heavy library; it supports loads of features that we
don't need, and historically has caused issues due to long class loading
times (a little over 2 seconds to load AprilTagFieldLayout). This often
manifests as a help request in the form of "my robot disables when I do
X, but doesn't disable when doing X in subsequent attempts until code
restart." While SC has brought down Jackson loading times significantly,
with AprilTagFieldLayout loads taking only 330 milliseconds, that's
still a rather long delay, and while libraries should handle any JSON
loading ahead of time to prevent delays in auto/teleop, it would still
be good to make the worst case better to reduce user frustration.
Benchmarks indicate using [Avaje
Jsonb](https://github.com/avaje/avaje-jsonb) to load AprilTagFieldLayout
only takes ~70 ms, a fair chunk of which isn't actually in Avaje Jsonb
(~4 ms is spent on using getResourceAsStream to retrieve the JSON file,
~8 ms is spent on just loading the AprilTag class and its dependencies).
Note that all times listed are end-to-end, meaning nothing else was done
except for the operation being benchmarked, and doing arithmetic on them
can be flawed due to some classes being loaded twice, i.e.,
getResourceAsStream and `new AprilTag()` likely load some of the same
JDK classes and so subtracting both from the Avaje Jsonb load time is
likely slightly incorrect because class loading is being double counted.
For our purposes, it's likely accurate enough and is mostly just for
contextualization.
Benchmarks were run on a Raspberry Pi CM5 with 2 GB of RAM. Source code
for the
[results](https://github.com/user-attachments/files/26471452/benchmark.txt)
can be found in the "Fastjson2" commit
(2456d15ca8ebd17635e607cd40bf8816e77869a1).
Avaje Jsonb uses code generation via annotation processors to generate
the classes needed to do JSON serde and uses service providers to find
them, which will require downstream changes in robot projects, as the
different service providers in each library must be merged together for
Avaje Jsonb to function. We will use the Gradle shadow plugin, as its
already used by the installer and therefore adds zero additional
dependencies.
1. Make the OpMode interface itself periodic; this means the only
differences between `OpMode` and `PeriodicOpMode` are the latter's
methods to add sideloaded periodic callbacks
2. Make OpModeRobot process callbacks in a similar fashion to TimedRobot
and
3. Add some lifecycle functions (discussed below)
4. Pull the callback priority queue from TimedRobot to a new class
called `PeriodicPriorityQueue` so that `TimedRobot` and `OpModeRobot`
have less duplication
5. Fix a typo in the DriverStationJNI class that causes a memory leak
when certain driver station sim calls
6. Port the C++ OpModeRobot tests to Java
`OpModeRobot` now possesses some `IterativeRobotBase`-stye lifecycle
functions; these functions
1. `robotPeriodic`
2. `simulationInit` and `simulationPeriodic`
3. `disabledInit`, `disabledPeriodic`, and `disabledExit`
(note that `simulationInit` and `disabledInit` may be renamed to match
wpilibsuite#8719)
`OpModeRobot` also now processes `OpMode` changes (by the Driver
Station) in its `loopFunc` method, similar to
`IterativeRobotBase.loopFunc` processing game mode changes; `loopFunc`
is, similarly to `TimedRobot`, provided as a default `Callback`
---------
Signed-off-by: Zach Harel <zach@zharel.me>
Co-authored-by: Joseph Eng <91924258+KangarooKoala@users.noreply.github.com>
When downloading a patch to fix linting errors, it's annoying to have to
unzip it, particularly when it's a single file. This PR updates the
`upload-artifact` action to v7, which allows for uploading an artifact
without zipping it. It also sets archive to false for all patches
generated by linting.
Commands are no longer able to outlive their schedule-site's scope,
regardless of how they were scheduled (set as a default command, bound
to a trigger, or manually scheduled)
As a consequence, default commands need better tracking so the default
command setting can be released when their scope exits and the next-most
appropriate default command can be rescheduled (eg, an opmode sets a
default command, then the globally-scoped default is restored when the
opmode exits). Some complexity is required here to make it work well for
edge cases.
Like `schedule()`, `setDefaultCommand()` will immediately start the new
default command if called inside of another command to avoid 1-loop
delays. However, this does not apply when called by the _current_
default command, as it would result in attempting to cancel the default
command while it's mounted (which is impossible and would throw an
exception)
```java
class Robot extends OpModeRobot {
final Drive drive = new Drive();
final CommandXboxController controller = new CommandXboxController(1);
public Robot() {
// global default command, active unless overridden in an opmode or command
drive.setDefaultCommand(drive.stop());
// global trigger binding, always active
controller.rightBumper().onTrue(drive.setX());
}
}
@Teleop
class ExampleOpMode extends PeriodicOpMode {
public ExampleOpMode(Robot robot) {
// opmode-specific default command
robot.drive.setDefaultCommand(robot.drive.operatorControl(robot.controller));
// opmode-specific binding
robot.controller.leftBumper().whileTrue(robot.drive.stop());
// opmode-specific binding that takes precedence over the global binding
// because it happens last; it "wins out" over the `setX()` binding
robot.controller.rightBumper().onTrue(robot.drive.selfTest());
}
@Override
public void periodic() {
Scheduler.getDefault().run();
}
}
```
`Color::FromHSV` didn't match the Java `Color.fromHSV` in some saturated
edge cases, introducing an off-by-one error when the HSV color should
correspond complete saturation of one or two of the primary colors.
Example:
- Java: `Color.fromHSV(0, 255, 255) -> (255, 0, 0)`
- C++: `Color::FromHSV(0, 255, 255) -> (255, 1, 1)`
This also means the following methods are also transitively affected:
- `AddressableLED::LEDData::SetHSV`
- `LEDPattern::Rainbow`
This off-by-one error is introduced by a rounding error from the chroma
calculation, which was dividing by 256 rather than the appropriate
maximum value of 255 like in Java:
7ca35e5678/wpilibj/src/main/java/edu/wpi/first/wpilibj/util/Color.java (L176-L177)
Also port appropriate tests from Java to C++ to catch this bug.
I found this bug when I tried to port `AddressableLEDBuffer` to RobotPy.
Codex found the root cause :)
There's changes to the diagnostic output and a performance improvement
for autodiff setup. I also updated Java's Options docs to more closely
match upstream.
```
> Task :javacPlugin:javadoc
/home/tav/frc/wpilib/allwpilib/javacPlugin/src/main/java/org/wpilib/javacplugin/OpModeAnnotationValidator.java:31: warning: invalid input: '<'
* <li>Name must be <= 32 characters
^
/home/tav/frc/wpilib/allwpilib/javacPlugin/src/main/java/org/wpilib/javacplugin/OpModeAnnotationValidator.java:32: warning: invalid input: '<'
* <li>Group must be <= 12 characters
^
/home/tav/frc/wpilib/allwpilib/javacPlugin/src/main/java/org/wpilib/javacplugin/OpModeAnnotationValidator.java:33: warning: invalid input: '<'
* <li>Description must be <= 64 characters
^
3 warnings
```
These weren't caught by the `docs:generateJavaDocs` task because the
javacPlugin docs aren't included there.
Since sched_setscheduler() requires non-RT priorities to be 0, we can
use that as a sentinel value for disabling RT and condense the Java API
to just two functions with fewer parameters. The thread priority setter
is deprecated since only experts should use it.
The HAL Notifier thread priority setter was replaced with setting the
priority in the thread itself.
The C++ Notifier non-RT and RT constructors were deduplicated.
The real-time scheduler was changed from SCHED_FIFO to SCHED_RR, which
is SCHED_FIFO with threads allowed to run for a maximum time quantum
before yielding (100 ms by default).
Each Bazel Windows CI job is currently redundantly evaluating the build
actions for both the `opt` and `dbg` compilation modes.
If we group the debug artifact builds together (instead of grouping by
the shared/static library artifacts), the `dbg` compilation mode actions
no longer need to be executed across multiple CI jobs in the matrix.
(Yes, the CI jobs are sharing the Bazel cache, but there's still
overhead in checking the action cache for each action. It's also
possible that two jobs will race to execute the same action.)
| Total actions per job | [Before] | After |
|----------------------------|---------:|------:|
| Windows x86-64 | 12277 | 10179 |
| Windows x86-64 Static | 11947 | n/a |
| Windows x86-64 Debug | n/a | 9895 |
| Windows ARM64 | 5518 | 3420 |
| Windows ARM64 Static | 5304 | n/a |
| Windows ARM64 Debug | n/a | 3272 |
| Windows System Core | 4836 | 2916 |
| Windows System Core Static | 4576 | n/a |
| Windows System Core Debug | n/a | 2916 |
[Before]:
https://github.com/wpilibsuite/allwpilib/actions/runs/23781230818
This should hopefully translate to shorter wall-clock time Windows CI
jobs.
The wrapper includes reverse mode autodiff, the Problem DSL, and the
optimal control problem API. I wrote it by directly translating the
upstream
[API](https://github.com/SleipnirGroup/Sleipnir/tree/main/include/sleipnir)
and [tests](https://github.com/SleipnirGroup/Sleipnir/tree/main/test) to
Java (i.e., copy-paste-modify).
I replaced the ArmFeedforward and Ellipse2d JNIs with implementations
using the Sleipnir Java bindings. Switching dev binary JNIs to release
by default sped up wpimath test runs from several minutes to 7 seconds.
In https://github.com/wpilibsuite/allwpilib/issues/8681 we discovered
that multicast service types need to be valid (end with _tcp or _udp),
or else errors are silently swallowed. Let's make our C++ unit test use
a valid name and also check that it works. I think if we
should/shouldn't do this is up for debate still.
I also discovered two bugs in the JNI code that lead to incorrect
results being returned
- Return array index was always 0
- Use of JLocal for the return value seems to mean that the array will
always be NULL in java
