From 82aca6139b3bb118837533a959376e462e118c11 Mon Sep 17 00:00:00 2001 From: aidnem <> Date: Tue, 8 Oct 2024 21:50:43 -0400 Subject: [PATCH 01/15] WIP - try to port monitors but the packages can't see each other yet so it's all broken --- monitors/build.gradle | 72 ++++++++++++++++++ .../java/coppercore/monitors/Monitor.java | 73 +++++++++++++++++++ monitors/src/test/java/MonitorTests.java | 73 +++++++++++++++++++ settings.gradle | 2 +- .../wpi_interface/MonitoredSubsystem.java | 40 ++++++++++ 5 files changed, 259 insertions(+), 1 deletion(-) create mode 100644 monitors/build.gradle create mode 100644 monitors/src/main/java/coppercore/monitors/Monitor.java create mode 100644 monitors/src/test/java/MonitorTests.java create mode 100644 wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java diff --git a/monitors/build.gradle b/monitors/build.gradle new file mode 100644 index 0000000..646994e --- /dev/null +++ b/monitors/build.gradle @@ -0,0 +1,72 @@ +/* + * This file was generated by the Gradle 'init' task. + * + * This generated file contains a sample Java library project to get you started. + * For more details on building Java & JVM projects, please refer to https://docs.gradle.org/8.7/userguide/building_java_projects.html in the Gradle documentation. + */ + +plugins { + // Apply the java-library plugin for API and implementation separation. + id 'java-library' + id "com.diffplug.spotless" version "6.24.0" + +} + +repositories { + // Use Maven Central for resolving dependencies. + mavenCentral() +} + +spotless { +// optional: limit format enforcement to just the files changed by this feature branch +ratchetFrom 'origin/main' + +format 'misc', { + // define the files to apply `misc` to + target '*.gradle', '.gitattributes', '.gitignore' + + // define the steps to apply to those files + trimTrailingWhitespace() + indentWithTabs() // or spaces. Takes an integer argument if you don't like 4 + endWithNewline() +} +java { + // don't need to set target, it is inferred from java + // Allow ignoring certain parts in formatting. + toggleOffOn() + // apply a specific flavor of google-java-format + googleJavaFormat('1.19.2').aosp().reflowLongStrings() + // fix formatting of type annotations + formatAnnotations() +} +} + +compileJava.dependsOn 'spotlessApply' + + +dependencies { + // Use JUnit Jupiter for testing. + testImplementation libs.junit.jupiter + + testRuntimeOnly 'org.junit.platform:junit-platform-launcher' + + // This dependency is exported to consumers, that is to say found on their compile classpath. + api libs.commons.math3 + + // This dependency is used internally, and not exposed to consumers on their own compile classpath. + implementation libs.guava +} + +// Apply a specific Java toolchain to ease working on different environments. +java { + toolchain { + languageVersion = JavaLanguageVersion.of(17) + } +} + +tasks.named('test') { + // Use JUnit Platform for unit tests. + useJUnitPlatform() +} + +compileJava.dependsOn 'spotlessApply' diff --git a/monitors/src/main/java/coppercore/monitors/Monitor.java b/monitors/src/main/java/coppercore/monitors/Monitor.java new file mode 100644 index 0000000..0744573 --- /dev/null +++ b/monitors/src/main/java/coppercore/monitors/Monitor.java @@ -0,0 +1,73 @@ +package coppercore.monitors; + +import java.util.function.BooleanSupplier; + +public class Monitor { + String name; // Name to log the status of the monitor under + boolean sticky; // Should the monitor still report a fault after conditions return to normal? + double timeToFault; // How long the value can be unacceptable before a fault occurs + BooleanSupplier isStateValid; // Supplier with which to check whether the value is acceptable + Runnable faultCallback; // Function to call when the fault happens + + double triggeredTime = 0.0; // Timestamp when monitor was first triggered + // If this value is zero, the monitor has been triggered for less than 1 tick + + boolean triggered = false; // Is the value currently unnacceptable? + boolean faulted = false; // Has the monitor detected a fault? + + public Monitor( + String name, + boolean sticky, + BooleanSupplier isStateValid, + double timeToFault, + Runnable faultCallback) { + this.name = name; + this.sticky = sticky; + this.timeToFault = timeToFault; + this.isStateValid = isStateValid; + + this.faultCallback = faultCallback; + } + + public void periodic(double currentTimeSeconds) { + // currentTimeSeconds doesn't need to be from a specific point in history + // As long as the reference point is always the same, it could be from + // the robot being turned on, initialized, etc. + + triggered = !isStateValid.getAsBoolean(); + if (triggered) { + if (triggeredTime == 0.0) { + triggeredTime = currentTimeSeconds; + } + + if (currentTimeSeconds - triggeredTime > timeToFault) { + faulted = true; + } + } else { + if (!sticky) { + faulted = false; + } + + triggeredTime = 0.0; + } + if (faulted) { + faultCallback.run(); + } + + // TODO: Move logging to MonitoredSubsystem under wpi interface + // Logger.recordOutput("monitors/" + name + "/triggered", triggered); + // Logger.recordOutput("monitors/" + name + "/faulted", faulted); + } + + public boolean isFaulted() { + return faulted; + } + + public boolean isTriggered() { + return triggered; + } + + public void resetStickyFault() { + faulted = false; + } +} diff --git a/monitors/src/test/java/MonitorTests.java b/monitors/src/test/java/MonitorTests.java new file mode 100644 index 0000000..4543f17 --- /dev/null +++ b/monitors/src/test/java/MonitorTests.java @@ -0,0 +1,73 @@ +package coppercore.monitors.test; + +public class MonitorTests { + // TODO: FIGURE OUT HOW TO TEST THIS + // Callbacks are hard :( + + // Mimics a changing robot state + // boolean isStateValid = true; + + // @Test + // public void nonStickyTest() { + // Monitor exampleMonitor = + // new Monitor("exampleMonitor", false, () -> isStateValid, 1.0, () -> {}); + + // exampleMonitor.periodic(0.0); + // Assertions.assertFalse(exampleMonitor.isFaulted()); + // Assertions.assertFalse(exampleMonitor.isTriggered()); + + // isStateValid = false; + // exampleMonitor.periodic(1.0); + // Assertions.assertFalse(exampleMonitor.isFaulted()); + // Assertions.assertFalse(exampleMonitor.isTriggered()); + + // exampleMonitor.periodic(1.25); + // Assertions.assertFalse(exampleMonitor.isFaulted()); + // Assertions.assertFalse(exampleMonitor.isTriggered()); + + // exampleMonitor.periodic(2.0); + // Assertions.assertTrue(exampleMonitor.isFaulted()); + // Assertions.assertTrue(exampleMonitor.isTriggered()); + + // isStateValid = true; + // exampleMonitor.periodic(3.0); + // Assertions.assertFalse(exampleMonitor.isFaulted()); + // Assertions.assertFalse(exampleMonitor.isTriggered()); + // } + + // @Test + // public void stickyTest() { + // // Mimics a changing robot state + // boolean isStateValid = true; + + // Monitor exampleMonitor = + // new Monitor( + // "exampleMonitor", + // true, + // () -> isStateValid, + // 1.0, + // () -> {}); // TODO: Figure out how to test callback + + // exampleMonitor.periodic(0.0); + // Assertions.assertFalse(exampleMonitor.isFaulted()); + // Assertions.assertFalse(exampleMonitor.isTriggered()); + + // isStateValid = false; + // exampleMonitor.periodic(1.0); + // Assertions.assertFalse(exampleMonitor.isFaulted()); + // Assertions.assertFalse(exampleMonitor.isTriggered()); + + // exampleMonitor.periodic(1.25); + // Assertions.assertFalse(exampleMonitor.isFaulted()); + // Assertions.assertFalse(exampleMonitor.isTriggered()); + + // exampleMonitor.periodic(2.0); + // Assertions.assertTrue(exampleMonitor.isFaulted()); + // Assertions.assertTrue(exampleMonitor.isTriggered()); + + // isStateValid = true; + // exampleMonitor.periodic(3.0); + // Assertions.assertTrue(exampleMonitor.isFaulted()); + // Assertions.assertFalse(exampleMonitor.isTriggered()); + // } +} diff --git a/settings.gradle b/settings.gradle index cbee54e..754e689 100644 --- a/settings.gradle +++ b/settings.gradle @@ -12,4 +12,4 @@ plugins { rootProject.name = 'coppercore' -include('geometry', 'wpilib_interface', 'math', 'controls', "parameter_tools", 'vision') +include('geometry', 'wpilib_interface', 'math', 'controls', "parameter_tools", 'vision', 'monitors') diff --git a/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java b/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java new file mode 100644 index 0000000..da9e239 --- /dev/null +++ b/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java @@ -0,0 +1,40 @@ +package coppercore.wpi_interface; + +import coppercore.monitors.Monitor; +import edu.wpi.first.wpilibj2.command.SubsystemBase; +import java.util.ArrayList; +import java.util.List; + +public class MonitoredSubsystem extends SubsystemBase { + private List registeredMonitors = new ArrayList(); + + public void addMonitor(Monitor monitor) { + registeredMonitors.add(monitor); + } + + @Override + public void periodic() { + monitoredPeriodic(); + runMonitors(); + } + + /** + * OVERRIDE ME! This function is called every time the subsystem's periodic function is called. + * However, MonitoredSubsytem automatically checks monitors during every periodic run. + * Therefore, this method should be overridden as a replacement for the normal periodic function + * in the implementation of the subsystem. + * + *

This method is called periodically by the {@link CommandScheduler}. Useful for updating + * subsystem-specific state that you don't want to offload to a {@link Command}. Teams should + * try to be consistent within their own codebases about which responsibilities will be handled + * by Commands, and which will be handled here. + */ + public void monitoredPeriodic() {} + + private void runMonitors() { + registeredMonitors.forEach( + monitor -> { + monitor.periodic(); + }); + } +} From 1c31c7b25039e4e852febfeb4f7d2845d4f4e998 Mon Sep 17 00:00:00 2001 From: aidnem <99768676+aidnem@users.noreply.github.com> Date: Wed, 9 Oct 2024 17:24:14 +0000 Subject: [PATCH 02/15] Fix MonitoredSubsystem but its still broken --- .../coppercore/wpi_interface/MonitoredSubsystem.java | 10 +++++++++- wpilib_interface/build.gradle | 1 + 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java b/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java index da9e239..0cab140 100644 --- a/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java +++ b/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java @@ -1,9 +1,11 @@ package coppercore.wpi_interface; import coppercore.monitors.Monitor; +import edu.wpi.first.wpilibj.Timer; import edu.wpi.first.wpilibj2.command.SubsystemBase; import java.util.ArrayList; import java.util.List; +import org.littletonrobotics.junction.Logger; public class MonitoredSubsystem extends SubsystemBase { private List registeredMonitors = new ArrayList(); @@ -34,7 +36,13 @@ public void monitoredPeriodic() {} private void runMonitors() { registeredMonitors.forEach( monitor -> { - monitor.periodic(); + monitor.periodic(Timer.getFPGATimestamp()); + + // TODO: Move logging to MonitoredSubsystem under wpi interface + Logger.recordOutput( + "monitors/" + monitor.getName() + "/triggered", monitor.isTriggered()); + Logger.recordOutput( + "monitors/" + monitor.getName() + "/faulted", monitor.isFaulted()); }); } } diff --git a/wpilib_interface/build.gradle b/wpilib_interface/build.gradle index 946ff8d..ba553fc 100644 --- a/wpilib_interface/build.gradle +++ b/wpilib_interface/build.gradle @@ -19,6 +19,7 @@ def includeDesktopSupport = true // Defining my dependencies. In this case, WPILib (+ friends), and vendor libraries. // Also defines JUnit 5. dependencies { + implementation project(":monitors") implementation 'edu.wpi.first.wpilibj:wpilibj-java:2024.3.2' implementation 'edu.wpi.first.wpilibNewCommands:wpilibNewCommands-java:2024.3.2' implementation 'edu.wpi.first.wpiutil:wpiutil-java:2024.3.2' From 9588742f7d6bac60eb441c14d2de2a9cd52de9e9 Mon Sep 17 00:00:00 2001 From: aidnem <99768676+aidnem@users.noreply.github.com> Date: Wed, 9 Oct 2024 18:39:48 +0000 Subject: [PATCH 03/15] Try to add AdvantageKit, now none of my imports work instead of only some of them --- vision/vendordeps/WPILibNewCommands.json | 72 ++++++++++++------------ wpilib_interface/build.gradle | 20 +++++++ 2 files changed, 56 insertions(+), 36 deletions(-) diff --git a/vision/vendordeps/WPILibNewCommands.json b/vision/vendordeps/WPILibNewCommands.json index 4143e09..4d1a3d4 100644 --- a/vision/vendordeps/WPILibNewCommands.json +++ b/vision/vendordeps/WPILibNewCommands.json @@ -1,38 +1,38 @@ { - "fileName": "WPILibNewCommands.json", - "name": "WPILib-New-Commands", - "version": "1.0.0", - "uuid": "111e20f7-815e-48f8-9dd6-e675ce75b266", - "frcYear": "2024", - "mavenUrls": [], - "jsonUrl": "", - "javaDependencies": [ - { - "groupId": "edu.wpi.first.wpilibNewCommands", - "artifactId": "wpilibNewCommands-java", - "version": "wpilib" - } - ], - "jniDependencies": [], - "cppDependencies": [ - { - "groupId": "edu.wpi.first.wpilibNewCommands", - "artifactId": "wpilibNewCommands-cpp", - "version": "wpilib", - "libName": "wpilibNewCommands", - "headerClassifier": "headers", - "sourcesClassifier": "sources", - "sharedLibrary": true, - "skipInvalidPlatforms": true, - "binaryPlatforms": [ - "linuxathena", - "linuxarm32", - "linuxarm64", - "windowsx86-64", - "windowsx86", - "linuxx86-64", - "osxuniversal" - ] - } - ] + "fileName": "WPILibNewCommands.json", + "name": "WPILib-New-Commands", + "version": "1.0.0", + "uuid": "111e20f7-815e-48f8-9dd6-e675ce75b266", + "frcYear": "2024", + "mavenUrls": [], + "jsonUrl": "", + "javaDependencies": [ + { + "groupId": "edu.wpi.first.wpilibNewCommands", + "artifactId": "wpilibNewCommands-java", + "version": "wpilib" + } + ], + "jniDependencies": [], + "cppDependencies": [ + { + "groupId": "edu.wpi.first.wpilibNewCommands", + "artifactId": "wpilibNewCommands-cpp", + "version": "wpilib", + "libName": "wpilibNewCommands", + "headerClassifier": "headers", + "sourcesClassifier": "sources", + "sharedLibrary": true, + "skipInvalidPlatforms": true, + "binaryPlatforms": [ + "linuxathena", + "linuxarm32", + "linuxarm64", + "windowsx86-64", + "windowsx86", + "linuxx86-64", + "osxuniversal" + ] + } + ] } \ No newline at end of file diff --git a/wpilib_interface/build.gradle b/wpilib_interface/build.gradle index ba553fc..7bd7d76 100644 --- a/wpilib_interface/build.gradle +++ b/wpilib_interface/build.gradle @@ -19,6 +19,9 @@ def includeDesktopSupport = true // Defining my dependencies. In this case, WPILib (+ friends), and vendor libraries. // Also defines JUnit 5. dependencies { + def akitJson = new groovy.json.JsonSlurper().parseText(new File(projectDir.getAbsolutePath() + "/vendordeps/AdvantageKit.json").text) + annotationProcessor "org.littletonrobotics.akit.junction:junction-autolog:$akitJson.version" + implementation project(":monitors") implementation 'edu.wpi.first.wpilibj:wpilibj-java:2024.3.2' implementation 'edu.wpi.first.wpilibNewCommands:wpilibNewCommands-java:2024.3.2' @@ -62,10 +65,27 @@ repositories { mavenLocal() maven { url 'https://frcmaven.wpi.edu/artifactory/release/' + } + maven { + url = uri("https://maven.pkg.github.com/Mechanical-Advantage/AdvantageKit") + credentials { + username = "Mechanical-Advantage-Bot" + password = "\u0067\u0068\u0070\u005f\u006e\u0056\u0051\u006a\u0055\u004f\u004c\u0061\u0079\u0066\u006e\u0078\u006e\u0037\u0051\u0049\u0054\u0042\u0032\u004c\u004a\u006d\u0055\u0070\u0073\u0031\u006d\u0037\u004c\u005a\u0030\u0076\u0062\u0070\u0063\u0051" + } } gradlePluginPortal() } +configurations.all { + exclude group: "edu.wpi.first.wpilibj" +} + +task(checkAkitInstall, dependsOn: "classes", type: JavaExec) { + mainClass = "org.littletonrobotics.junction.CheckInstall" + classpath = sourceSets.main.runtimeClasspath +} +compileJava.finalizedBy checkAkitInstall + spotless { // optional: limit format enforcement to just the files changed by this feature branch ratchetFrom 'origin/main' From 252b4a8531f0e3596018e92b9e0228cdc9ec3e3b Mon Sep 17 00:00:00 2001 From: aidnem <> Date: Wed, 9 Oct 2024 16:58:09 -0400 Subject: [PATCH 04/15] Turns out excluding wpilibj made us unable to import wpilibj, now just need to fix other import issues --- wpilib_interface/build.gradle | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/wpilib_interface/build.gradle b/wpilib_interface/build.gradle index 7bd7d76..040fbb8 100644 --- a/wpilib_interface/build.gradle +++ b/wpilib_interface/build.gradle @@ -76,9 +76,9 @@ repositories { gradlePluginPortal() } -configurations.all { - exclude group: "edu.wpi.first.wpilibj" -} +// configurations.all { +// exclude group: "edu.wpi.first.wpilibj" +// } task(checkAkitInstall, dependsOn: "classes", type: JavaExec) { mainClass = "org.littletonrobotics.junction.CheckInstall" From 9bc953bbf88e29c61ac2b0c6f4b29c718acb6afe Mon Sep 17 00:00:00 2001 From: aidnem <99768676+aidnem@users.noreply.github.com> Date: Wed, 6 Nov 2024 17:55:35 +0000 Subject: [PATCH 05/15] Add builder pattern and fix tests for Monitor --- .../java/coppercore/monitors/Monitor.java | 144 ++++++++++++++++- monitors/src/test/java/MonitorTests.java | 148 ++++++++++-------- 2 files changed, 221 insertions(+), 71 deletions(-) diff --git a/monitors/src/main/java/coppercore/monitors/Monitor.java b/monitors/src/main/java/coppercore/monitors/Monitor.java index 0744573..23ba7c7 100644 --- a/monitors/src/main/java/coppercore/monitors/Monitor.java +++ b/monitors/src/main/java/coppercore/monitors/Monitor.java @@ -3,7 +3,7 @@ import java.util.function.BooleanSupplier; public class Monitor { - String name; // Name to log the status of the monitor under + String name; // Name to log the status of the monitor under, used by MonitoredSubsystem boolean sticky; // Should the monitor still report a fault after conditions return to normal? double timeToFault; // How long the value can be unacceptable before a fault occurs BooleanSupplier isStateValid; // Supplier with which to check whether the value is acceptable @@ -15,6 +15,22 @@ public class Monitor { boolean triggered = false; // Is the value currently unnacceptable? boolean faulted = false; // Has the monitor detected a fault? + /** + * Creates a fault Monitor. This constructor takes all parameters at once. There is also a + * builder pattern supplied under MonitorBuilder. Using the builder is recommended because it + * makes code much more readable, but is not required. + * + * @param name the name of the monitor, which will be used by MonitoredSubsystem for logging. + * @param sticky whether the fault should remain faulted after conditions return to an + * acceptable state. + * @param isStateValid supplier for whether the state is CURRENTLY valid. This doesn't need to + * handle persistence, the monitor class will handle this automatically. + * @param timeToFault the time, in seconds, that isStateValid must return false before the a + * fault is triggered. + * @param faultCallback a function called on every periodic loop while the monitor is in a + * faulted state. + * @see MonitorBuilder + */ public Monitor( String name, boolean sticky, @@ -29,6 +45,13 @@ public Monitor( this.faultCallback = faultCallback; } + /** + * This is the "main loop" of the Monitor. This should be called each in each periodic loop. + * + * @param currentTimeSeconds the current timestamp in seconds. This doesn't need to have a + * specific frame of reference, it is used to detect when conditions have been unnacceptable + * for enough time to fault. + */ public void periodic(double currentTimeSeconds) { // currentTimeSeconds doesn't need to be from a specific point in history // As long as the reference point is always the same, it could be from @@ -40,7 +63,7 @@ public void periodic(double currentTimeSeconds) { triggeredTime = currentTimeSeconds; } - if (currentTimeSeconds - triggeredTime > timeToFault) { + if (currentTimeSeconds - triggeredTime >= timeToFault) { faulted = true; } } else { @@ -53,21 +76,130 @@ public void periodic(double currentTimeSeconds) { if (faulted) { faultCallback.run(); } - - // TODO: Move logging to MonitoredSubsystem under wpi interface - // Logger.recordOutput("monitors/" + name + "/triggered", triggered); - // Logger.recordOutput("monitors/" + name + "/faulted", faulted); } + /** + * Returns a boolean describing whether or not the monitor is faulted. A monitor is considered + * faulted when it has been triggered for a time greater than or equal to its time to fault. A + * monitor is also considered faulted if it is sticky and has ever been faulted. + * + * @return whether or not the monitor is currently faulted. + */ public boolean isFaulted() { return faulted; } + /** + * Returns a boolean describing whether or not the monitor is currently triggered. A monitor is + * triggered when isStateValid returns false. + * + * @return whether or not the monitor is currently triggered + */ public boolean isTriggered() { return triggered; } + /** + * Reset a sticky fault. This means that, if a Monitor is sticky, and is currently faulted, + * calling this function will return it to a non-faulted state. + */ public void resetStickyFault() { faulted = false; } + + /** + * Get the name of the Monitor. + * + * @return a string, the name of the monitor that it uses for logging. + */ + public String getName() { + return name; + } + + /** + * This class is meant to build a fault monitor. Create a builder, then call withName, + * withStickyness, withTimeToFault, and withIsStateValid, and withFaultCallback to configure its + * fields. Once every field is configured, call build() to return a shiny new fault monitor. + */ + public static class MonitorBuilder { + String name; // Name to log the status of the monitor under + boolean sticky; // Should the monitor still report a fault after conditions return to + // normal? + double timeToFault; // How long the value can be unacceptable before a fault occurs + BooleanSupplier + isStateValid; // Supplier with which to check whether the value is acceptable + Runnable faultCallback; // Function to call when the fault happens + + /** + * Sets the name of the monitor. This name will be used when the monitor is logged by + * MonitoredSubsystem. + * + * @param name the name of the monitor, which is used for logging by MonitoredSubsystem or + * can be used for manual logging outside of a MonitoredSubsystem. + * @return the monitor builder, so that successive builder calls can be chained + */ + public MonitorBuilder withName(String name) { + this.name = name; + return this; + } + + /** + * Sets whether or not the monitor is sticky or not, and returns itself. + * + * @param sticky a boolean, whether or not the monitor should remain faulted after + * conditions return to normal. + * @return the monitor builder, so that successive builder calls can be chained. + */ + public MonitorBuilder withStickyness(boolean sticky) { + this.sticky = sticky; + return this; + } + + /** + * Sets how long the monitor may be triggered before it faults, and returns itself. + * + * @param timeToFault a double, how long the monitor can be triggered (in an unnacceptable + * state) before it becomes faulted in seconds. + * @return the monitor builder, so that successive builder calls can be chained. + */ + public MonitorBuilder withTimeToFault(double timeToFault) { + this.timeToFault = timeToFault; + return this; + } + + /** + * Sets the supplier for whether or not the state is currently valid. + * + * @param isStateValid a boolean supplier, which should return true when the state is valid + * and false when the state is invalid. This supplier doesn't need to account for + * timeToFault, this is automatically handled by the monitor. + * @return the monitor, so that successive builder calls can be chained. + */ + public MonitorBuilder withIsStateValidSupplier(BooleanSupplier isStateValid) { + this.isStateValid = isStateValid; + return this; + } + + /** + * Sets how long the monitor may be triggered before it faults, and returns itself. + * + * @param faultCallback a runnable, which will be called periodic unnacceptable state) + * before it becomes faulted in seconds. + * @return the monitor builder, so that successive builder calls can be chained. + */ + public MonitorBuilder withFaultCallback(Runnable faultCallback) { + this.faultCallback = faultCallback; + return this; + } + + /** + * Instantiates a monitor and returns it. This method should be called after all of the + * fields of the monitor are configured using with[Field] methods. + * + * @return a monitor with the fields set by the builder. + */ + public Monitor build() { + return new Monitor(name, sticky, isStateValid, timeToFault, faultCallback); + } + } } diff --git a/monitors/src/test/java/MonitorTests.java b/monitors/src/test/java/MonitorTests.java index 4543f17..42dd1a3 100644 --- a/monitors/src/test/java/MonitorTests.java +++ b/monitors/src/test/java/MonitorTests.java @@ -1,73 +1,91 @@ package coppercore.monitors.test; +import coppercore.monitors.Monitor; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + public class MonitorTests { // TODO: FIGURE OUT HOW TO TEST THIS // Callbacks are hard :( // Mimics a changing robot state - // boolean isStateValid = true; - - // @Test - // public void nonStickyTest() { - // Monitor exampleMonitor = - // new Monitor("exampleMonitor", false, () -> isStateValid, 1.0, () -> {}); - - // exampleMonitor.periodic(0.0); - // Assertions.assertFalse(exampleMonitor.isFaulted()); - // Assertions.assertFalse(exampleMonitor.isTriggered()); - - // isStateValid = false; - // exampleMonitor.periodic(1.0); - // Assertions.assertFalse(exampleMonitor.isFaulted()); - // Assertions.assertFalse(exampleMonitor.isTriggered()); - - // exampleMonitor.periodic(1.25); - // Assertions.assertFalse(exampleMonitor.isFaulted()); - // Assertions.assertFalse(exampleMonitor.isTriggered()); - - // exampleMonitor.periodic(2.0); - // Assertions.assertTrue(exampleMonitor.isFaulted()); - // Assertions.assertTrue(exampleMonitor.isTriggered()); - - // isStateValid = true; - // exampleMonitor.periodic(3.0); - // Assertions.assertFalse(exampleMonitor.isFaulted()); - // Assertions.assertFalse(exampleMonitor.isTriggered()); - // } - - // @Test - // public void stickyTest() { - // // Mimics a changing robot state - // boolean isStateValid = true; - - // Monitor exampleMonitor = - // new Monitor( - // "exampleMonitor", - // true, - // () -> isStateValid, - // 1.0, - // () -> {}); // TODO: Figure out how to test callback - - // exampleMonitor.periodic(0.0); - // Assertions.assertFalse(exampleMonitor.isFaulted()); - // Assertions.assertFalse(exampleMonitor.isTriggered()); - - // isStateValid = false; - // exampleMonitor.periodic(1.0); - // Assertions.assertFalse(exampleMonitor.isFaulted()); - // Assertions.assertFalse(exampleMonitor.isTriggered()); - - // exampleMonitor.periodic(1.25); - // Assertions.assertFalse(exampleMonitor.isFaulted()); - // Assertions.assertFalse(exampleMonitor.isTriggered()); - - // exampleMonitor.periodic(2.0); - // Assertions.assertTrue(exampleMonitor.isFaulted()); - // Assertions.assertTrue(exampleMonitor.isTriggered()); - - // isStateValid = true; - // exampleMonitor.periodic(3.0); - // Assertions.assertTrue(exampleMonitor.isFaulted()); - // Assertions.assertFalse(exampleMonitor.isTriggered()); - // } + private boolean isStateValid = true; + + private boolean getIsStateValid() { + return isStateValid; + } + + @Test + public void nonStickyTest() { + isStateValid = true; + + Monitor exampleMonitor = + new Monitor.MonitorBuilder() + .withName("exampleMonitor") + .withStickyness(false) + .withIsStateValidSupplier(() -> getIsStateValid()) + .withTimeToFault(1.0) + .withFaultCallback(() -> {}) + .build(); + + exampleMonitor.periodic(0.0); + Assertions.assertFalse(exampleMonitor.isFaulted()); + Assertions.assertFalse(exampleMonitor.isTriggered()); + + isStateValid = false; + exampleMonitor.periodic(1.0); + Assertions.assertFalse(exampleMonitor.isFaulted()); + Assertions.assertTrue(exampleMonitor.isTriggered()); + + exampleMonitor.periodic(1.25); + Assertions.assertFalse(exampleMonitor.isFaulted()); + Assertions.assertTrue(exampleMonitor.isTriggered()); + + exampleMonitor.periodic(2.0); + Assertions.assertTrue(exampleMonitor.isFaulted()); + Assertions.assertTrue(exampleMonitor.isTriggered()); + + isStateValid = true; + exampleMonitor.periodic(3.0); + Assertions.assertFalse(exampleMonitor.isFaulted()); + Assertions.assertFalse(exampleMonitor.isTriggered()); + } + + @Test + public void stickyTest() { + // Mimics a changing robot state + isStateValid = true; + + Monitor exampleMonitor = + new Monitor.MonitorBuilder() + .withName("exampleMonitor") + .withStickyness(true) + .withIsStateValidSupplier(() -> getIsStateValid()) + .withTimeToFault(1.0) + .withFaultCallback(() -> {}) + .build(); + + exampleMonitor.periodic(0.0); + Assertions.assertFalse(exampleMonitor.isFaulted()); + Assertions.assertFalse(exampleMonitor.isTriggered()); + + isStateValid = false; + exampleMonitor.periodic(1.0); + Assertions.assertFalse(exampleMonitor.isFaulted()); + Assertions.assertFalse(getIsStateValid()); + Assertions.assertTrue(exampleMonitor.isTriggered()); + + exampleMonitor.periodic(1.25); + Assertions.assertFalse(exampleMonitor.isFaulted()); + Assertions.assertTrue(exampleMonitor.isTriggered()); + + exampleMonitor.periodic(2.0); + Assertions.assertTrue(exampleMonitor.isFaulted()); + Assertions.assertTrue(exampleMonitor.isTriggered()); + + isStateValid = true; + exampleMonitor.periodic(3.0); + Assertions.assertTrue(exampleMonitor.isFaulted()); + Assertions.assertFalse(exampleMonitor.isTriggered()); + } } From 89ded93bb5e7d98f8e6edf6c24f1e99e63fdc59c Mon Sep 17 00:00:00 2001 From: aidnem <99768676+aidnem@users.noreply.github.com> Date: Thu, 7 Nov 2024 15:39:58 +0000 Subject: [PATCH 06/15] Fix WPI imports and MAYBE fix advantagekit, must test out of codespace to confirm --- wpilib_interface/build.gradle | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/wpilib_interface/build.gradle b/wpilib_interface/build.gradle index 040fbb8..9d3185f 100644 --- a/wpilib_interface/build.gradle +++ b/wpilib_interface/build.gradle @@ -16,14 +16,27 @@ java { // Set this to true to enable desktop support. def includeDesktopSupport = true +configurations.all { + // exclude group: "edu.wpi.first.wpilibj" +} + +task(checkAkitInstall, dependsOn: "classes", type: JavaExec) { + mainClass = "org.littletonrobotics.junction.CheckInstall" + classpath = sourceSets.main.runtimeClasspath +} +compileJava.finalizedBy checkAkitInstall + // Defining my dependencies. In this case, WPILib (+ friends), and vendor libraries. // Also defines JUnit 5. dependencies { def akitJson = new groovy.json.JsonSlurper().parseText(new File(projectDir.getAbsolutePath() + "/vendordeps/AdvantageKit.json").text) annotationProcessor "org.littletonrobotics.akit.junction:junction-autolog:$akitJson.version" + annotationProcessor "org.littletonrobotics.akit.junction:junction-autolog:3.2.1" implementation project(":monitors") + implementation 'org.littletonrobotics.akit.junction:junction-core:3.2.1' implementation 'edu.wpi.first.wpilibj:wpilibj-java:2024.3.2' + implementation 'edu.wpi.first.wpiutil:wpiutil-java:2024.3.2' implementation 'edu.wpi.first.wpilibNewCommands:wpilibNewCommands-java:2024.3.2' implementation 'edu.wpi.first.wpiutil:wpiutil-java:2024.3.2' implementation 'edu.wpi.first.wpiunits:wpiunits-java:2024.3.2' @@ -76,15 +89,6 @@ repositories { gradlePluginPortal() } -// configurations.all { -// exclude group: "edu.wpi.first.wpilibj" -// } - -task(checkAkitInstall, dependsOn: "classes", type: JavaExec) { - mainClass = "org.littletonrobotics.junction.CheckInstall" - classpath = sourceSets.main.runtimeClasspath -} -compileJava.finalizedBy checkAkitInstall spotless { // optional: limit format enforcement to just the files changed by this feature branch From b6d5b1485a96fa9174ee4c79919ee65c9ca25166 Mon Sep 17 00:00:00 2001 From: aidnem <99768676+aidnem@users.noreply.github.com> Date: Thu, 7 Nov 2024 11:32:22 -0500 Subject: [PATCH 07/15] GREAT SCOTT GLORY HALLELUJAH IT BUILDS --- wpilib_interface/build.gradle | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/wpilib_interface/build.gradle b/wpilib_interface/build.gradle index 9d3185f..bfc3cfa 100644 --- a/wpilib_interface/build.gradle +++ b/wpilib_interface/build.gradle @@ -17,7 +17,7 @@ java { def includeDesktopSupport = true configurations.all { - // exclude group: "edu.wpi.first.wpilibj" + exclude group: "edu.wpi.first.wpilibj" } task(checkAkitInstall, dependsOn: "classes", type: JavaExec) { @@ -33,6 +33,9 @@ dependencies { annotationProcessor "org.littletonrobotics.akit.junction:junction-autolog:$akitJson.version" annotationProcessor "org.littletonrobotics.akit.junction:junction-autolog:3.2.1" + implementation wpi.java.deps.wpilib() + implementation wpi.java.vendor.java() + implementation project(":monitors") implementation 'org.littletonrobotics.akit.junction:junction-core:3.2.1' implementation 'edu.wpi.first.wpilibj:wpilibj-java:2024.3.2' From 501019d0cec8090d98f7e497bc827fa7d14bce39 Mon Sep 17 00:00:00 2001 From: aidnem <99768676+aidnem@users.noreply.github.com> Date: Thu, 7 Nov 2024 12:17:32 -0500 Subject: [PATCH 08/15] Remove to-do comment that had already been done --- .../main/java/coppercore/wpi_interface/MonitoredSubsystem.java | 1 - 1 file changed, 1 deletion(-) diff --git a/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java b/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java index 0cab140..69549ad 100644 --- a/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java +++ b/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java @@ -38,7 +38,6 @@ private void runMonitors() { monitor -> { monitor.periodic(Timer.getFPGATimestamp()); - // TODO: Move logging to MonitoredSubsystem under wpi interface Logger.recordOutput( "monitors/" + monitor.getName() + "/triggered", monitor.isTriggered()); Logger.recordOutput( From aad9f849474cf01e31640e4ae2b201c4b5343c51 Mon Sep 17 00:00:00 2001 From: aidnem <99768676+aidnem@users.noreply.github.com> Date: Mon, 25 Nov 2024 10:13:57 -0500 Subject: [PATCH 09/15] Simplify boolean logic in Monitor and improve code comments --- .../java/coppercore/monitors/Monitor.java | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/monitors/src/main/java/coppercore/monitors/Monitor.java b/monitors/src/main/java/coppercore/monitors/Monitor.java index 23ba7c7..ed2ddfa 100644 --- a/monitors/src/main/java/coppercore/monitors/Monitor.java +++ b/monitors/src/main/java/coppercore/monitors/Monitor.java @@ -9,8 +9,9 @@ public class Monitor { BooleanSupplier isStateValid; // Supplier with which to check whether the value is acceptable Runnable faultCallback; // Function to call when the fault happens - double triggeredTime = 0.0; // Timestamp when monitor was first triggered - // If this value is zero, the monitor has been triggered for less than 1 tick + double triggeredTime = -1.0; // Timestamp when monitor was first triggered + // If this value is less than or equal to zero, the monitor has been triggered for less than 1 + // tick boolean triggered = false; // Is the value currently unnacceptable? boolean faulted = false; // Has the monitor detected a fault? @@ -59,19 +60,30 @@ public void periodic(double currentTimeSeconds) { triggered = !isStateValid.getAsBoolean(); if (triggered) { - if (triggeredTime == 0.0) { + // If triggered time is less than zero, this means it hasn't been set yet. + // Therefore, this is the first loop that the monitor is triggered and we should + // store the current timestamp to reference how long it's been triggered for later. + if (triggeredTime <= 0.0) { triggeredTime = currentTimeSeconds; } - if (currentTimeSeconds - triggeredTime >= timeToFault) { - faulted = true; - } + // When triggered, the monitor will fault if either: + // - It is already faulted (it can't transition from faulted to non-faulted while + // triggered) + // or + // - It has been triggered for the timeToFault. + faulted = faulted || ((currentTimeSeconds - triggeredTime) >= timeToFault); } else { if (!sticky) { faulted = false; } + // If the monitor isn't triggered, it will only be faulted if it is sticky and already + // faulted. + faulted = faulted && sticky; - triggeredTime = 0.0; + // Use -1 as a sentinel value to indicate that the triggered time hasn't been stored + // yet. + triggeredTime = -1.0; } if (faulted) { faultCallback.run(); From 9a526047e3e392fc4f348a4187b825410fc3cb1f Mon Sep 17 00:00:00 2001 From: aidnem <99768676+aidnem@users.noreply.github.com> Date: Mon, 25 Nov 2024 10:19:58 -0500 Subject: [PATCH 10/15] Update normal comments to be doc comments for fields of Monitor --- .../java/coppercore/monitors/Monitor.java | 39 +++++++++++++++---- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/monitors/src/main/java/coppercore/monitors/Monitor.java b/monitors/src/main/java/coppercore/monitors/Monitor.java index ed2ddfa..38967f3 100644 --- a/monitors/src/main/java/coppercore/monitors/Monitor.java +++ b/monitors/src/main/java/coppercore/monitors/Monitor.java @@ -3,17 +3,40 @@ import java.util.function.BooleanSupplier; public class Monitor { - String name; // Name to log the status of the monitor under, used by MonitoredSubsystem - boolean sticky; // Should the monitor still report a fault after conditions return to normal? - double timeToFault; // How long the value can be unacceptable before a fault occurs - BooleanSupplier isStateValid; // Supplier with which to check whether the value is acceptable - Runnable faultCallback; // Function to call when the fault happens - - double triggeredTime = -1.0; // Timestamp when monitor was first triggered - // If this value is less than or equal to zero, the monitor has been triggered for less than 1 + /** Name to log the status of the monitor under, used by MonitoredSubsystem */ + String name; + + /** Should the monitor still report a fault after conditions return to normal? */ + boolean sticky; + + /** How long the value can be unacceptable before a fault occurs */ + double timeToFault; + + /** Supplier with which to check whether the value is acceptable */ + BooleanSupplier isStateValid; + + /** Function to call when the fault happens */ + Runnable faultCallback; + + /** + * Timestamp when the monitor was first triggered, or -1.0 if the monitor has been triggered for + * less than 1 loop. + */ + double triggeredTime = -1.0; + + // If triggeredTime's value is less than or equal to zero, the monitor has been triggered for + // less than 1 // tick + /** Whether the value is currently unnacceptable */ boolean triggered = false; // Is the value currently unnacceptable? + + /** + * Whether the monitor has detected a fault + * + *

This means either the monitor has been triggered for the 'time to fault', or the monitor + * is sticky and has faulted without being reset. + */ boolean faulted = false; // Has the monitor detected a fault? /** From 19ea93dbb84491894919ffe88df65a6bfb3a9d04 Mon Sep 17 00:00:00 2001 From: aidnem <99768676+aidnem@users.noreply.github.com> Date: Mon, 25 Nov 2024 10:24:24 -0500 Subject: [PATCH 11/15] Remove gradleRIO and manually add wpilib dependencies instead --- wpilib_interface/build.gradle | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/wpilib_interface/build.gradle b/wpilib_interface/build.gradle index bfc3cfa..4badbde 100644 --- a/wpilib_interface/build.gradle +++ b/wpilib_interface/build.gradle @@ -3,7 +3,6 @@ import java.text.SimpleDateFormat plugins { id "java" id "maven-publish" - id "edu.wpi.first.GradleRIO" version "2024.3.2" id "com.peterabeles.gversion" version "1.10" id "com.diffplug.spotless" version "6.24.0" } @@ -16,15 +15,21 @@ java { // Set this to true to enable desktop support. def includeDesktopSupport = true -configurations.all { - exclude group: "edu.wpi.first.wpilibj" -} +// configurations.all { +// exclude group: "edu.wpi.first.wpilibj" +// } task(checkAkitInstall, dependsOn: "classes", type: JavaExec) { mainClass = "org.littletonrobotics.junction.CheckInstall" classpath = sourceSets.main.runtimeClasspath } -compileJava.finalizedBy checkAkitInstall + +// In order to allow installation of wpilib packages, the exclusion on line 19 must be disabled +// However, disabling that exclusion causes advantagekit's install check to fail. +// Therefore, we must also disable this check. +// TODO: Figure out how to manually depend on wpilib while still using checkAkitInstall +// or figure out if it's necessary. +//compileJava.finalizedBy checkAkitInstall // Defining my dependencies. In this case, WPILib (+ friends), and vendor libraries. // Also defines JUnit 5. @@ -33,9 +38,6 @@ dependencies { annotationProcessor "org.littletonrobotics.akit.junction:junction-autolog:$akitJson.version" annotationProcessor "org.littletonrobotics.akit.junction:junction-autolog:3.2.1" - implementation wpi.java.deps.wpilib() - implementation wpi.java.vendor.java() - implementation project(":monitors") implementation 'org.littletonrobotics.akit.junction:junction-core:3.2.1' implementation 'edu.wpi.first.wpilibj:wpilibj-java:2024.3.2' From 6b310db60942875d62fcb13db897a804a0dcbd86 Mon Sep 17 00:00:00 2001 From: aidnem <99768676+aidnem@users.noreply.github.com> Date: Mon, 25 Nov 2024 10:36:14 -0500 Subject: [PATCH 12/15] Add ability to enable and disable logging for MonitoredSubsystem and for each individual Monitor --- .../java/coppercore/monitors/Monitor.java | 54 ++++++++++++++++++- .../wpi_interface/MonitoredSubsystem.java | 24 +++++++-- 2 files changed, 72 insertions(+), 6 deletions(-) diff --git a/monitors/src/main/java/coppercore/monitors/Monitor.java b/monitors/src/main/java/coppercore/monitors/Monitor.java index 38967f3..471d338 100644 --- a/monitors/src/main/java/coppercore/monitors/Monitor.java +++ b/monitors/src/main/java/coppercore/monitors/Monitor.java @@ -18,6 +18,14 @@ public class Monitor { /** Function to call when the fault happens */ Runnable faultCallback; + /** + * Should the monitor be logged by the MonitoredSubsystem? + * + *

Changing this value doesn't change the behavior of the monitor, it exists only to tell + * MonitoredSubsystem whether or not to log the monitor. + */ + boolean loggingEnabled; + /** * Timestamp when the monitor was first triggered, or -1.0 if the monitor has been triggered for * less than 1 loop. @@ -53,6 +61,8 @@ public class Monitor { * fault is triggered. * @param faultCallback a function called on every periodic loop while the monitor is in a * faulted state. + * @param loggingEnabled whether or not the monitor should be logged. This value is only used by + * MonitoredSubsystem to enable or disable logging for each monitor. * @see MonitorBuilder */ public Monitor( @@ -60,13 +70,16 @@ public Monitor( boolean sticky, BooleanSupplier isStateValid, double timeToFault, - Runnable faultCallback) { + Runnable faultCallback, + boolean loggingEnabled) { this.name = name; this.sticky = sticky; this.timeToFault = timeToFault; this.isStateValid = isStateValid; this.faultCallback = faultCallback; + + this.loggingEnabled = loggingEnabled; } /** @@ -151,6 +164,26 @@ public String getName() { return name; } + /** + * Set whether or not the monitor should be logged. + * + *

Changing this value doesn't change the behavior of the monitor, it exists only to tell + * MonitoredSubsystem whether or not to log the monitor. + */ + public void setLoggingEnabled(boolean loggingEnabled) { + this.loggingEnabled = loggingEnabled; + } + + /** + * Get whether or not the monitor should be logged. + * + *

Changing this value doesn't change the behavior of the monitor, it exists only to tell + * MonitoredSubsystem whether or not to log the monitor. + */ + public boolean getLoggingEnabled() { + return loggingEnabled; + } + /** * This class is meant to build a fault monitor. Create a builder, then call withName, * withStickyness, withTimeToFault, and withIsStateValid, and withFaultCallback to configure its @@ -164,6 +197,7 @@ public static class MonitorBuilder { BooleanSupplier isStateValid; // Supplier with which to check whether the value is acceptable Runnable faultCallback; // Function to call when the fault happens + boolean loggingEnabled = true; // Whether or not to log the monitor. Defaults to true. /** * Sets the name of the monitor. This name will be used when the monitor is logged by @@ -227,6 +261,21 @@ public MonitorBuilder withFaultCallback(Runnable faultCallback) { return this; } + /** + * Sets whether or not the monitor should be logged. + * + *

This value is only used to tell the MonitoredSubsystem whether or not to log each + * monitor. This value defaults to true unless withLoggingEnabled(false) is called on the + * builder! + * + * @param loggingEnabled whether or not the monitor should be logged + * @return the monitor builder, so that successive builder calls can be chained. + */ + public MonitorBuilder withLoggingEnabled(boolean loggingEnabled) { + this.loggingEnabled = loggingEnabled; + return this; + } + /** * Instantiates a monitor and returns it. This method should be called after all of the * fields of the monitor are configured using with[Field] methods. @@ -234,7 +283,8 @@ public MonitorBuilder withFaultCallback(Runnable faultCallback) { * @return a monitor with the fields set by the builder. */ public Monitor build() { - return new Monitor(name, sticky, isStateValid, timeToFault, faultCallback); + return new Monitor( + name, sticky, isStateValid, timeToFault, faultCallback, loggingEnabled); } } } diff --git a/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java b/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java index 69549ad..57d4526 100644 --- a/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java +++ b/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java @@ -10,6 +10,8 @@ public class MonitoredSubsystem extends SubsystemBase { private List registeredMonitors = new ArrayList(); + private boolean loggingEnabled = true; + public void addMonitor(Monitor monitor) { registeredMonitors.add(monitor); } @@ -38,10 +40,24 @@ private void runMonitors() { monitor -> { monitor.periodic(Timer.getFPGATimestamp()); - Logger.recordOutput( - "monitors/" + monitor.getName() + "/triggered", monitor.isTriggered()); - Logger.recordOutput( - "monitors/" + monitor.getName() + "/faulted", monitor.isFaulted()); + if (loggingEnabled && monitor.getLoggingEnabled()) { + Logger.recordOutput( + "monitors/" + monitor.getName() + "/triggered", + monitor.isTriggered()); + Logger.recordOutput( + "monitors/" + monitor.getName() + "/faulted", monitor.isFaulted()); + } }); } + + /** + * Set whether or not the monitored subsystem should log its monitors. This is enabled by + * default, but can be disabled if there are RAM issues stemming from too many strings in + * logging. + * + * @param loggingEnabled + */ + public void setLoggingEnabled(boolean loggingEnabled) { + this.loggingEnabled = loggingEnabled; + } } From 70be76e36f3104363986e8c4ab00b81c40aa68d1 Mon Sep 17 00:00:00 2001 From: aidnem <99768676+aidnem@users.noreply.github.com> Date: Mon, 25 Nov 2024 10:38:53 -0500 Subject: [PATCH 13/15] Make MonitoredSubsystem an abstract class to force users to override monitoredPeriodic --- .../java/coppercore/wpi_interface/MonitoredSubsystem.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java b/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java index 57d4526..3dfa77f 100644 --- a/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java +++ b/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java @@ -7,7 +7,7 @@ import java.util.List; import org.littletonrobotics.junction.Logger; -public class MonitoredSubsystem extends SubsystemBase { +public abstract class MonitoredSubsystem extends SubsystemBase { private List registeredMonitors = new ArrayList(); private boolean loggingEnabled = true; @@ -33,7 +33,7 @@ public void periodic() { * try to be consistent within their own codebases about which responsibilities will be handled * by Commands, and which will be handled here. */ - public void monitoredPeriodic() {} + public abstract void monitoredPeriodic(); private void runMonitors() { registeredMonitors.forEach( From 79b80ff5dfe3c9f06c692596f0f1d5259b048940 Mon Sep 17 00:00:00 2001 From: aidnem <99768676+aidnem@users.noreply.github.com> Date: Mon, 25 Nov 2024 10:43:40 -0500 Subject: [PATCH 14/15] Add tests for monitor fault callbacks --- monitors/src/test/java/MonitorTests.java | 32 ++++++++++++++++++++---- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/monitors/src/test/java/MonitorTests.java b/monitors/src/test/java/MonitorTests.java index 42dd1a3..430840e 100644 --- a/monitors/src/test/java/MonitorTests.java +++ b/monitors/src/test/java/MonitorTests.java @@ -5,9 +5,6 @@ import org.junit.jupiter.api.Test; public class MonitorTests { - // TODO: FIGURE OUT HOW TO TEST THIS - // Callbacks are hard :( - // Mimics a changing robot state private boolean isStateValid = true; @@ -15,6 +12,14 @@ private boolean getIsStateValid() { return isStateValid; } + // Number to be incremented by fault callback + private int faultCallbackCalls = 0; + + // Example fault callback to verify that the callback was called + private void callFaultCallback() { + faultCallbackCalls++; + } + @Test public void nonStickyTest() { isStateValid = true; @@ -25,36 +30,45 @@ public void nonStickyTest() { .withStickyness(false) .withIsStateValidSupplier(() -> getIsStateValid()) .withTimeToFault(1.0) - .withFaultCallback(() -> {}) + .withFaultCallback( + () -> { + callFaultCallback(); + }) .build(); exampleMonitor.periodic(0.0); Assertions.assertFalse(exampleMonitor.isFaulted()); Assertions.assertFalse(exampleMonitor.isTriggered()); + Assertions.assertTrue(faultCallbackCalls == 0); isStateValid = false; exampleMonitor.periodic(1.0); Assertions.assertFalse(exampleMonitor.isFaulted()); Assertions.assertTrue(exampleMonitor.isTriggered()); + Assertions.assertTrue(faultCallbackCalls == 0); exampleMonitor.periodic(1.25); Assertions.assertFalse(exampleMonitor.isFaulted()); Assertions.assertTrue(exampleMonitor.isTriggered()); + Assertions.assertTrue(faultCallbackCalls == 0); exampleMonitor.periodic(2.0); Assertions.assertTrue(exampleMonitor.isFaulted()); Assertions.assertTrue(exampleMonitor.isTriggered()); + Assertions.assertTrue(faultCallbackCalls == 1); isStateValid = true; exampleMonitor.periodic(3.0); Assertions.assertFalse(exampleMonitor.isFaulted()); Assertions.assertFalse(exampleMonitor.isTriggered()); + Assertions.assertTrue(faultCallbackCalls == 1); } @Test public void stickyTest() { // Mimics a changing robot state isStateValid = true; + faultCallbackCalls = 0; Monitor exampleMonitor = new Monitor.MonitorBuilder() @@ -62,30 +76,38 @@ public void stickyTest() { .withStickyness(true) .withIsStateValidSupplier(() -> getIsStateValid()) .withTimeToFault(1.0) - .withFaultCallback(() -> {}) + .withFaultCallback( + () -> { + callFaultCallback(); + }) .build(); exampleMonitor.periodic(0.0); Assertions.assertFalse(exampleMonitor.isFaulted()); Assertions.assertFalse(exampleMonitor.isTriggered()); + Assertions.assertTrue(faultCallbackCalls == 0); isStateValid = false; exampleMonitor.periodic(1.0); Assertions.assertFalse(exampleMonitor.isFaulted()); Assertions.assertFalse(getIsStateValid()); Assertions.assertTrue(exampleMonitor.isTriggered()); + Assertions.assertTrue(faultCallbackCalls == 0); exampleMonitor.periodic(1.25); Assertions.assertFalse(exampleMonitor.isFaulted()); Assertions.assertTrue(exampleMonitor.isTriggered()); + Assertions.assertTrue(faultCallbackCalls == 0); exampleMonitor.periodic(2.0); Assertions.assertTrue(exampleMonitor.isFaulted()); Assertions.assertTrue(exampleMonitor.isTriggered()); + Assertions.assertTrue(faultCallbackCalls == 1); isStateValid = true; exampleMonitor.periodic(3.0); Assertions.assertTrue(exampleMonitor.isFaulted()); Assertions.assertFalse(exampleMonitor.isTriggered()); + Assertions.assertTrue(faultCallbackCalls == 2); } } From 67082a1d2bd775a4543d656c26efe092ddd2559d Mon Sep 17 00:00:00 2001 From: aidnem <99768676+aidnem@users.noreply.github.com> Date: Mon, 25 Nov 2024 19:00:58 -0500 Subject: [PATCH 15/15] Fix wpilib_interface's build.gradle so that imports work again --- wpilib_interface/build.gradle | 8 ++------ .../coppercore/wpilib_interface}/MonitoredSubsystem.java | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) rename {wpi_interface/src/main/java/coppercore/wpi_interface => wpilib_interface/src/main/java/coppercore/wpilib_interface}/MonitoredSubsystem.java (98%) diff --git a/wpilib_interface/build.gradle b/wpilib_interface/build.gradle index 4badbde..ff0bd78 100644 --- a/wpilib_interface/build.gradle +++ b/wpilib_interface/build.gradle @@ -34,8 +34,8 @@ task(checkAkitInstall, dependsOn: "classes", type: JavaExec) { // Defining my dependencies. In this case, WPILib (+ friends), and vendor libraries. // Also defines JUnit 5. dependencies { - def akitJson = new groovy.json.JsonSlurper().parseText(new File(projectDir.getAbsolutePath() + "/vendordeps/AdvantageKit.json").text) - annotationProcessor "org.littletonrobotics.akit.junction:junction-autolog:$akitJson.version" + //def akitJson = new groovy.json.JsonSlurper().parseText(new File(projectDir.getAbsolutePath() + "/vendordeps/AdvantageKit.json").text) + //annotationProcessor "org.littletonrobotics.akit.junction:junction-autolog:$akitJson.version" annotationProcessor "org.littletonrobotics.akit.junction:junction-autolog:3.2.1" implementation project(":monitors") @@ -43,15 +43,11 @@ dependencies { implementation 'edu.wpi.first.wpilibj:wpilibj-java:2024.3.2' implementation 'edu.wpi.first.wpiutil:wpiutil-java:2024.3.2' implementation 'edu.wpi.first.wpilibNewCommands:wpilibNewCommands-java:2024.3.2' - implementation 'edu.wpi.first.wpiutil:wpiutil-java:2024.3.2' implementation 'edu.wpi.first.wpiunits:wpiunits-java:2024.3.2' implementation 'edu.wpi.first.wpimath:wpimath-java:2024.3.2' implementation project(":math") - implementation wpi.java.deps.wpilib() - implementation wpi.java.vendor.java() - testImplementation 'org.junit.jupiter:junit-jupiter:5.10.1' testRuntimeOnly 'org.junit.platform:junit-platform-launcher' diff --git a/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java b/wpilib_interface/src/main/java/coppercore/wpilib_interface/MonitoredSubsystem.java similarity index 98% rename from wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java rename to wpilib_interface/src/main/java/coppercore/wpilib_interface/MonitoredSubsystem.java index 3dfa77f..8bdaa61 100644 --- a/wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java +++ b/wpilib_interface/src/main/java/coppercore/wpilib_interface/MonitoredSubsystem.java @@ -1,4 +1,4 @@ -package coppercore.wpi_interface; +package coppercore.wpilib_interface; import coppercore.monitors.Monitor; import edu.wpi.first.wpilibj.Timer;