From f7a27c5d6ad5f55b12bb0c3e4b3efe3051501453 Mon Sep 17 00:00:00 2001 From: dejabot Date: Sun, 5 Jan 2025 02:19:23 -0800 Subject: [PATCH 1/4] remove Rule.Builder and simplify RuleEngine.addRule(), letting it return a Rule that can be modified further, eg with a finishedTriggeringProcedure. --- .../java/com/team766/framework3/Rule.java | 80 +----- .../com/team766/framework3/RuleEngine.java | 13 +- .../team766/framework3/RuleEngineTest.java | 270 +++++++----------- .../java/com/team766/framework3/RuleTest.java | 38 ++- 4 files changed, 141 insertions(+), 260 deletions(-) diff --git a/src/main/java/com/team766/framework3/Rule.java b/src/main/java/com/team766/framework3/Rule.java index 5999e7f9e..f79efdc4a 100644 --- a/src/main/java/com/team766/framework3/Rule.java +++ b/src/main/java/com/team766/framework3/Rule.java @@ -24,8 +24,8 @@ * public class MyRules extends RuleEngine { * public MyRules() { * // add rule to spin up the shooter when the boxop presses the right trigger on the gamepad - * rules.add(Rule.create("spin up shooter", gamepad.getButton(InputConstants.XBOX_RT)). - * withNewlyTriggeringProcedure(() -> new ShooterSpin(shooter))); + * rules.add("spin up shooter", gamepad.getButton(InputConstants.XBOX_RT), + * () -> new ShooterSpin(shooter))); * ... * } * } @@ -49,55 +49,6 @@ enum TriggerType { FINISHED } - /** - * Simple Builder for {@link Rule}s. Configure Rules via this Builder; these fields will be immutable - * in the rule the Builder constructs. - * - * Instances of this Builder are created via {@link Rule#create} to simplify syntax. - */ - public static class Builder { - private final String name; - private final BooleanSupplier predicate; - private Supplier newlyTriggeringProcedure; - private Supplier finishedTriggeringProcedure; - - private Builder(String name, BooleanSupplier predicate) { - this.name = name; - this.predicate = predicate; - } - - /** Specify a creator for the Procedure that should be run when this rule starts triggering. */ - public Builder withNewlyTriggeringProcedure(Supplier action) { - this.newlyTriggeringProcedure = action; - return this; - } - - public Builder withNewlyTriggeringProcedure( - Set> reservations, Runnable action) { - this.newlyTriggeringProcedure = - () -> new FunctionalInstantProcedure(reservations, action); - return this; - } - - /** Specify a creator for the Procedure that should be run when this rule was triggering before and is no longer triggering. */ - public Builder withFinishedTriggeringProcedure(Supplier action) { - this.finishedTriggeringProcedure = action; - return this; - } - - public Builder withFinishedTriggeringProcedure( - Set> reservations, Runnable action) { - this.finishedTriggeringProcedure = - () -> new FunctionalInstantProcedure(reservations, action); - return this; - } - - // called by {@link RuleEngine#addRule}. - /* package */ Rule build() { - return new Rule(name, predicate, newlyTriggeringProcedure, finishedTriggeringProcedure); - } - } - private final String name; private final BooleanSupplier predicate; private final Map> triggerProcedures = @@ -107,15 +58,8 @@ public Builder withFinishedTriggeringProcedure( private TriggerType currentTriggerType = TriggerType.NONE; - public static Builder create(String name, BooleanSupplier predicate) { - return new Builder(name, predicate); - } - - private Rule( - String name, - BooleanSupplier predicate, - Supplier newlyTriggeringProcedure, - Supplier finishedTriggeringProcedure) { + /* package */ Rule( + String name, BooleanSupplier predicate, Supplier newlyTriggeringProcedure) { if (predicate == null) { throw new IllegalArgumentException("Rule predicate has not been set."); } @@ -131,12 +75,18 @@ private Rule( triggerReservations.put( TriggerType.NEWLY, getReservationsForProcedure(newlyTriggeringProcedure)); } + } - if (finishedTriggeringProcedure != null) { - triggerProcedures.put(TriggerType.FINISHED, finishedTriggeringProcedure); - triggerReservations.put( - TriggerType.FINISHED, getReservationsForProcedure(finishedTriggeringProcedure)); - } + /** Specify a creator for the Procedure that should be run when this rule was triggering before and is no longer triggering. */ + public Rule withFinishedTriggeringProcedure(Supplier action) { + triggerProcedures.put(TriggerType.FINISHED, action); + triggerReservations.put(TriggerType.FINISHED, getReservationsForProcedure(action)); + return this; + } + + public Rule withFinishedTriggeringProcedure(Set> reservations, Runnable action) { + return withFinishedTriggeringProcedure( + () -> new FunctionalInstantProcedure(reservations, action)); } private Set> getReservationsForProcedure(Supplier supplier) { diff --git a/src/main/java/com/team766/framework3/RuleEngine.java b/src/main/java/com/team766/framework3/RuleEngine.java index 78d7ed3d0..4edea0c22 100644 --- a/src/main/java/com/team766/framework3/RuleEngine.java +++ b/src/main/java/com/team766/framework3/RuleEngine.java @@ -14,6 +14,8 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.BooleanSupplier; +import java.util.function.Supplier; /** * {@link RuleEngine}s manage and process a set of {@link Rule}s. Subclasses should add rules via @@ -41,11 +43,18 @@ public Category getLoggerCategory() { return Category.RULES; } - protected void addRule(Rule.Builder builder) { - Rule rule = builder.build(); + protected Rule addRule(String name, BooleanSupplier condition, Supplier action) { + Rule rule = new Rule(name, condition, action); rules.add(rule); int priority = rulePriorities.size(); rulePriorities.put(rule, priority); + return rule; + } + + protected Rule addRule( + String name, BooleanSupplier condition, Mechanism mechanism, Runnable action) { + return addRule( + name, condition, () -> new FunctionalInstantProcedure(Set.of(mechanism), action)); } @VisibleForTesting diff --git a/src/test/java/com/team766/framework3/RuleEngineTest.java b/src/test/java/com/team766/framework3/RuleEngineTest.java index aa496c904..e6a79702a 100644 --- a/src/test/java/com/team766/framework3/RuleEngineTest.java +++ b/src/test/java/com/team766/framework3/RuleEngineTest.java @@ -67,13 +67,13 @@ public void testAddRuleAndGetPriority() { new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> new FakeProcedure(2, Set.of(fm1)))); + "fm1_p0", + new ScheduledPredicate(0), + () -> new FakeProcedure(2, Set.of(fm1))); addRule( - Rule.create("fm1_p1", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> new FakeProcedure(2, Set.of(fm1)))); + "fm1_p1", + new ScheduledPredicate(0), + () -> new FakeProcedure(2, Set.of(fm1))); } }; @@ -96,13 +96,13 @@ public void testRunNonConflictingRules() { new RuleEngine() { { addRule( - Rule.create("fm1", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> new FakeProcedure(2, Set.of(fm1)))); + "fm1", + new ScheduledPredicate(0), + () -> new FakeProcedure(2, Set.of(fm1))); addRule( - Rule.create("fm2", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> new FakeProcedure(2, Set.of(fm2)))); + "fm2", + new ScheduledPredicate(0), + () -> new FakeProcedure(2, Set.of(fm2))); } }; @@ -146,17 +146,13 @@ public void testFinishedProcedureBumpsNewlyProcedureForSameRule() { new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p0", 1, Set.of(fm1))) - .withFinishedTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procfin_p0", - 1, - Set.of(fm1, fm2)))); + "fm1_p0", + new ScheduledPredicate(0), + () -> new FakeProcedure("fm1procnew_p0", 1, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> + new FakeProcedure( + "fm1procfin_p0", 1, Set.of(fm1, fm2))); } }; @@ -188,28 +184,18 @@ public void testRunRulePriorities() { new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1proc_p0", - 0, - Set.of(fm1, fm2)))); + "fm1_p0", + new ScheduledPredicate(0), + () -> new FakeProcedure("fm1proc_p0", 0, Set.of(fm1, fm2))); addRule( - Rule.create("fm1_p1", new PeriodicPredicate(2)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1proc_p1", - 0, - Set.of(fm1, fm3)))); + "fm1_p1", + new PeriodicPredicate(2), + () -> new FakeProcedure("fm1proc_p1", 0, Set.of(fm1, fm3))); addRule( - Rule.create("fm3_p2", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm3proc_p2", 0, Set.of(fm3)))); + "fm3_p2", + new ScheduledPredicate(0), + () -> new FakeProcedure("fm3proc_p2", 0, Set.of(fm3))); } }; @@ -250,30 +236,18 @@ public void testRunHigherPriorityRuleStillBeingRun() { new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1proc_p0", - 2, - Set.of(fm1, fm2)))); + "fm1_p0", + new ScheduledPredicate(0), + () -> new FakeProcedure("fm1proc_p0", 2, Set.of(fm1, fm2))); addRule( - Rule.create("fm1_p1", new ScheduledPredicate(1)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1proc_p1", - 2, - Set.of(fm1, fm2)))); + "fm1_p1", + new ScheduledPredicate(1), + () -> new FakeProcedure("fm1proc_p1", 2, Set.of(fm1, fm2))); addRule( - Rule.create("fm1_p2", new ScheduledPredicate(3)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1proc_p2", - 2, - Set.of(fm1, fm2)))); + "fm1_p2", + new ScheduledPredicate(3), + () -> new FakeProcedure("fm1proc_p2", 2, Set.of(fm1, fm2))); } }; @@ -321,21 +295,13 @@ public void testRunLowerPriorityRuleBumped() { new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(1)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1proc_p0", - 2, - Set.of(fm1, fm2)))); + "fm1_p0", + new ScheduledPredicate(1), + () -> new FakeProcedure("fm1proc_p0", 2, Set.of(fm1, fm2))); addRule( - Rule.create("fm1_p1", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1proc_p1", - 4, - Set.of(fm1, fm2)))); + "fm1_p1", + new ScheduledPredicate(0), + () -> new FakeProcedure("fm1proc_p1", 4, Set.of(fm1, fm2))); } }; @@ -363,21 +329,15 @@ public void testRuleResetIgnoredLowerPriorityRule() { new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p0", 2, Set.of(fm1)))); + "fm1_p0", + new ScheduledPredicate(0), + () -> new FakeProcedure("fm1procnew_p0", 2, Set.of(fm1))); addRule( - Rule.create("fm1_p1", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p1", 1, Set.of(fm1))) - .withFinishedTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procfin_p1", 1, Set.of(fm2)))); + "fm1_p1", + new ScheduledPredicate(0), + () -> new FakeProcedure("fm1procnew_p1", 1, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> new FakeProcedure("fm1procfin_p1", 1, Set.of(fm2))); } }; @@ -404,21 +364,15 @@ public void testRuleResetIgnoredLowerPriorityRuleHigherPriorityRulePreviouslySch new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p0", 2, Set.of(fm1)))); + "fm1_p0", + new ScheduledPredicate(0), + () -> new FakeProcedure("fm1procnew_p0", 2, Set.of(fm1))); addRule( - Rule.create("fm1_p1", new ScheduledPredicate(1)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p1", 1, Set.of(fm1))) - .withFinishedTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procfin_p1", 1, Set.of(fm2)))); + "fm1_p1", + new ScheduledPredicate(1), + () -> new FakeProcedure("fm1procnew_p1", 1, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> new FakeProcedure("fm1procfin_p1", 1, Set.of(fm2))); } }; @@ -453,21 +407,15 @@ public void testRuleResetBumpedLowerPriorityRule() { new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(1)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p0", 2, Set.of(fm1)))); + "fm1_p0", + new ScheduledPredicate(1), + () -> new FakeProcedure("fm1procnew_p0", 2, Set.of(fm1))); addRule( - Rule.create("fm1_p1", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p1", 2, Set.of(fm1))) - .withFinishedTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procfin_p1", 2, Set.of(fm2)))); + "fm1_p1", + new ScheduledPredicate(0), + () -> new FakeProcedure("fm1procnew_p1", 2, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> new FakeProcedure("fm1procfin_p1", 2, Set.of(fm2))); } }; @@ -494,25 +442,17 @@ public void testLowerPriorityRuleRunsWhenProcedureFromHigherPriorityRuleFinishes new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(0, 4)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p0", 0, Set.of(fm1))) - .withFinishedTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procfin_p0", 0, Set.of(fm1)))); + "fm1_p0", + new ScheduledPredicate(0, 4), + () -> new FakeProcedure("fm1procnew_p0", 0, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> new FakeProcedure("fm1procfin_p0", 0, Set.of(fm1))); addRule( - Rule.create("fm1_p1", new ScheduledPredicate(1)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p1", 0, Set.of(fm1))) - .withFinishedTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procfin_p1", 0, Set.of(fm1)))); + "fm1_p1", + new ScheduledPredicate(1), + () -> new FakeProcedure("fm1procnew_p1", 0, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> new FakeProcedure("fm1procfin_p1", 0, Set.of(fm1))); } }; @@ -563,25 +503,17 @@ public void testRuleCalledAgainAfterBeingReset() { new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(1)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p0", 0, Set.of(fm1))) - .withFinishedTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procfin_p0", 0, Set.of(fm1)))); + "fm1_p0", + new ScheduledPredicate(1), + () -> new FakeProcedure("fm1procnew_p0", 0, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> new FakeProcedure("fm1procfin_p0", 0, Set.of(fm1))); addRule( - Rule.create("fm1_p1", new ScheduledPredicate(0, 4)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p1", 1, Set.of(fm1))) - .withFinishedTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procfin_p1", 1, Set.of(fm1)))); + "fm1_p1", + new ScheduledPredicate(0, 4), + () -> new FakeProcedure("fm1procnew_p1", 1, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> new FakeProcedure("fm1procfin_p1", 1, Set.of(fm1))); } }; @@ -633,25 +565,17 @@ public void testRuleResetPreventsFinishedForLongTrigger() { new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(1)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p0", 0, Set.of(fm1))) - .withFinishedTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procfin_p0", 0, Set.of(fm1)))); + "fm1_p0", + new ScheduledPredicate(1), + () -> new FakeProcedure("fm1procnew_p0", 0, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> new FakeProcedure("fm1procfin_p0", 0, Set.of(fm1))); addRule( - Rule.create("fm1_p1", new ScheduledPredicate(0, 3)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p1", 1, Set.of(fm1))) - .withFinishedTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procfin_p1", 1, Set.of(fm1)))); + "fm1_p1", + new ScheduledPredicate(0, 3), + () -> new FakeProcedure("fm1procnew_p1", 1, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> new FakeProcedure("fm1procfin_p1", 1, Set.of(fm1))); } }; diff --git a/src/test/java/com/team766/framework3/RuleTest.java b/src/test/java/com/team766/framework3/RuleTest.java index 3b7730a8b..b37a54257 100644 --- a/src/test/java/com/team766/framework3/RuleTest.java +++ b/src/test/java/com/team766/framework3/RuleTest.java @@ -41,10 +41,7 @@ protected void setUp() {} @Test public void testCreate() { - Rule alwaysTrue = - Rule.create("always true", () -> true) - .withNewlyTriggeringProcedure(() -> Procedure.NO_OP) - .build(); + Rule alwaysTrue = new Rule("always true", () -> true, () -> Procedure.NO_OP); assertNotNull(alwaysTrue); assertEquals("always true", alwaysTrue.getName()); } @@ -52,10 +49,7 @@ public void testCreate() { @Test public void testEvaluate() { // start with simple test of a NONE->NEWLY->CONTINUING->CONTINUING sequence - Rule alwaysTrue = - Rule.create("always true", () -> true) - .withNewlyTriggeringProcedure(() -> Procedure.NO_OP) - .build(); + Rule alwaysTrue = new Rule("always true", () -> true, () -> Procedure.NO_OP); assertEquals(Rule.TriggerType.NONE, alwaysTrue.getCurrentTriggerType()); alwaysTrue.evaluate(); assertEquals(TriggerType.NEWLY, alwaysTrue.getCurrentTriggerType()); @@ -66,9 +60,10 @@ public void testEvaluate() { // test a full cycle: NONE->NEWLY->CONTINUING->FINISHED->NONE->NEWLY->... Rule duckDuckGooseGoose = - Rule.create("duck duck goose goose", new DuckDuckGooseGoosePredicate()) - .withNewlyTriggeringProcedure(() -> Procedure.NO_OP) - .build(); + new Rule( + "duck duck goose goose", + new DuckDuckGooseGoosePredicate(), + () -> Procedure.NO_OP); assertEquals(Rule.TriggerType.NONE, duckDuckGooseGoose.getCurrentTriggerType()); duckDuckGooseGoose.evaluate(); assertEquals(TriggerType.NEWLY, duckDuckGooseGoose.getCurrentTriggerType()); @@ -89,10 +84,11 @@ public void testGetMechanismsToReserve() { final Set> finishedMechanisms = Set.of(new FakeMechanism()); Rule duckDuckGooseGoose = - Rule.create("duck duck goose goose", new DuckDuckGooseGoosePredicate()) - .withNewlyTriggeringProcedure(newlyMechanisms, () -> {}) - .withFinishedTriggeringProcedure(finishedMechanisms, () -> {}) - .build(); + new Rule( + "duck duck goose goose", + new DuckDuckGooseGoosePredicate(), + () -> new FunctionalInstantProcedure(newlyMechanisms, () -> {})) + .withFinishedTriggeringProcedure(finishedMechanisms, () -> {}); // NONE assertEquals(Collections.emptySet(), duckDuckGooseGoose.getMechanismsToReserve()); @@ -101,12 +97,13 @@ public void testGetMechanismsToReserve() { duckDuckGooseGoose.evaluate(); assertEquals(newlyMechanisms, duckDuckGooseGoose.getMechanismsToReserve()); - // nothing between NEWLLY and FINISHED + // nothing between NEWLY and FINISHED duckDuckGooseGoose.evaluate(); assertEquals(Collections.emptySet(), duckDuckGooseGoose.getMechanismsToReserve()); // FINISHED duckDuckGooseGoose.evaluate(); + System.out.println("X: " + duckDuckGooseGoose.toString()); assertEquals(finishedMechanisms, duckDuckGooseGoose.getMechanismsToReserve()); // check NONE again @@ -121,10 +118,11 @@ public void testGetMechanismsToReserve() { @Test public void testGetProcedureToRun() { Rule duckDuckGooseGoose = - Rule.create("duck duck goose goose", new DuckDuckGooseGoosePredicate()) - .withNewlyTriggeringProcedure(() -> new TrivialProcedure("newly")) - .withFinishedTriggeringProcedure(() -> new TrivialProcedure("finished")) - .build(); + new Rule( + "duck duck goose goose", + new DuckDuckGooseGoosePredicate(), + () -> new TrivialProcedure("newly")) + .withFinishedTriggeringProcedure(() -> new TrivialProcedure("finished")); // NONE assertNull(duckDuckGooseGoose.getProcedureToRun()); From 896041ac6b64ca8f5b5a9898952ef28ec5f38160 Mon Sep 17 00:00:00 2001 From: dejabot Date: Sun, 5 Jan 2025 11:38:33 -0800 Subject: [PATCH 2/4] "seal" rules after ruleengine using them is run() the first time. this prevents accidental modification of rules after the containing ruleengine starts getting used. also switch to a LinkedHashMap for storing rules and tweak some of the methods used in unit tests. --- .../java/com/team766/framework3/Rule.java | 10 +++++ .../com/team766/framework3/RuleEngine.java | 34 +++++++++++------ .../team766/framework3/RuleEngineTest.java | 37 ++++++++++++++++--- 3 files changed, 65 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/team766/framework3/Rule.java b/src/main/java/com/team766/framework3/Rule.java index f79efdc4a..c76ae2288 100644 --- a/src/main/java/com/team766/framework3/Rule.java +++ b/src/main/java/com/team766/framework3/Rule.java @@ -57,6 +57,7 @@ enum TriggerType { Maps.newEnumMap(TriggerType.class); private TriggerType currentTriggerType = TriggerType.NONE; + private boolean sealed = false; /* package */ Rule( String name, BooleanSupplier predicate, Supplier newlyTriggeringProcedure) { @@ -79,6 +80,11 @@ enum TriggerType { /** Specify a creator for the Procedure that should be run when this rule was triggering before and is no longer triggering. */ public Rule withFinishedTriggeringProcedure(Supplier action) { + if (sealed) { + throw new IllegalStateException( + "Cannot modify rules once they've been evaluated in the RuleEngine"); + } + triggerProcedures.put(TriggerType.FINISHED, action); triggerReservations.put(TriggerType.FINISHED, getReservationsForProcedure(action)); return this; @@ -107,6 +113,10 @@ public String getName() { return currentTriggerType; } + /* package */ void seal() { + sealed = true; + } + /* package */ void reset() { currentTriggerType = TriggerType.NONE; } diff --git a/src/main/java/com/team766/framework3/RuleEngine.java b/src/main/java/com/team766/framework3/RuleEngine.java index 4edea0c22..4556bdd84 100644 --- a/src/main/java/com/team766/framework3/RuleEngine.java +++ b/src/main/java/com/team766/framework3/RuleEngine.java @@ -10,8 +10,7 @@ import edu.wpi.first.wpilibj2.command.CommandScheduler; import java.util.HashMap; import java.util.HashSet; -import java.util.LinkedList; -import java.util.List; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; import java.util.function.BooleanSupplier; @@ -32,9 +31,10 @@ public class RuleEngine implements LoggingBase { private static record RuleAction(Rule rule, Rule.TriggerType triggerType) {} - private final List rules = new LinkedList<>(); + private final LinkedHashMap rules = new LinkedHashMap<>(); private final Map rulePriorities = new HashMap<>(); private BiMap ruleMap = HashBiMap.create(); + private boolean sealed = false; protected RuleEngine() {} @@ -45,7 +45,7 @@ public Category getLoggerCategory() { protected Rule addRule(String name, BooleanSupplier condition, Supplier action) { Rule rule = new Rule(name, condition, action); - rules.add(rule); + rules.put(name, rule); int priority = rulePriorities.size(); rulePriorities.put(rule, priority); return rule; @@ -58,12 +58,13 @@ protected Rule addRule( } @VisibleForTesting - /* package */ Map getRuleNameMap() { - Map namedRules = new HashMap<>(); - for (Rule rule : rules) { - namedRules.put(rule.getName(), rule); - } - return namedRules; + /* package */ int size() { + return rules.size(); + } + + @VisibleForTesting + /* package */ Rule getRuleByName(String name) { + return rules.get(name); } @VisibleForTesting @@ -82,7 +83,18 @@ protected Rule getRuleForTriggeredProcedure(Command command) { return (ruleAction == null) ? null : ruleAction.rule; } + private void sealRules() { + for (Rule rule : rules.values()) { + rule.seal(); + } + } + public final void run() { + if (!sealed) { + sealRules(); + sealed = true; + } + Set> mechanismsToUse = new HashSet<>(); // TODO(MF3): when creating a Procedure, check that the reservations are the same as @@ -90,7 +102,7 @@ public final void run() { // evaluate each rule ruleLoop: - for (Rule rule : rules) { + for (Rule rule : rules.values()) { try { rule.evaluate(); diff --git a/src/test/java/com/team766/framework3/RuleEngineTest.java b/src/test/java/com/team766/framework3/RuleEngineTest.java index e6a79702a..3c0b9edd4 100644 --- a/src/test/java/com/team766/framework3/RuleEngineTest.java +++ b/src/test/java/com/team766/framework3/RuleEngineTest.java @@ -3,12 +3,12 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import com.team766.TestCase3; import edu.wpi.first.wpilibj2.command.Command; import edu.wpi.first.wpilibj2.command.CommandScheduler; -import java.util.Map; import java.util.Set; import java.util.function.BooleanSupplier; import org.junit.jupiter.api.Test; @@ -58,6 +58,34 @@ public boolean getAsBoolean() { private final FakeMechanism2 fm2 = new FakeMechanism2(); private final FakeMechanism3 fm3 = new FakeMechanism3(); + @Test + public void testSeal() { + // test that we can modify rules before we call run + RuleEngine rulesOne = + new RuleEngine() { + { + addRule("rule1_1", () -> true, () -> Procedure.NO_OP) + .withFinishedTriggeringProcedure(() -> Procedure.NO_OP); + } + }; + rulesOne.run(); + + // test that + RuleEngine rulesTwo = + new RuleEngine() { + { + addRule("rule2_1", () -> true, () -> Procedure.NO_OP); + } + }; + rulesTwo.run(); + + assertThrows( + IllegalStateException.class, + () -> + rulesTwo.getRuleByName("rule2_1") + .withFinishedTriggeringProcedure(() -> Procedure.NO_OP)); + } + @Test public void testAddRuleAndGetPriority() { // simply test that rules we add are added - and at the expected priority @@ -77,12 +105,11 @@ public void testAddRuleAndGetPriority() { } }; - Map namedRules = myRules.getRuleNameMap(); // make sure we have 2 rules - assertEquals(2, namedRules.size()); + assertEquals(2, myRules.size()); // with priorities based on insertion order, starting at 0 - assertEquals(0, myRules.getPriorityForRule(namedRules.get("fm1_p0"))); - assertEquals(1, myRules.getPriorityForRule(namedRules.get("fm1_p1"))); + assertEquals(0, myRules.getPriorityForRule(myRules.getRuleByName("fm1_p0"))); + assertEquals(1, myRules.getPriorityForRule(myRules.getRuleByName("fm1_p1"))); } @Test From 5a4a3aeee5c09fe7bcca6e4296a0b21b22979954 Mon Sep 17 00:00:00 2001 From: Ryan Cahoon Date: Wed, 2 Oct 2024 03:43:46 -0700 Subject: [PATCH 3/4] Rule action persistence policies --- .vscode/settings.json | 14 +- .../FunctionalInstantProcedure.java | 7 +- .../framework3/FunctionalProcedure.java | 7 +- .../java/com/team766/framework3/Rule.java | 73 +++++- .../com/team766/framework3/RuleEngine.java | 96 +++++++- .../team766/framework3/RulePersistence.java | 23 ++ .../team766/framework3/RuleEngineTest.java | 230 +++++++++++++++++- .../java/com/team766/framework3/RuleTest.java | 40 ++- 8 files changed, 470 insertions(+), 20 deletions(-) create mode 100644 src/main/java/com/team766/framework3/RulePersistence.java diff --git a/.vscode/settings.json b/.vscode/settings.json index 714bc4416..f25587d5b 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -33,5 +33,17 @@ "diffEditor.ignoreTrimWhitespace": false, "java.format.settings.url": "https://raw.githubusercontent.com/google/styleguide/gh-pages/eclipse-java-google-style.xml", "java.checkstyle.configuration": "${workspaceFolder}/.github/linters/checkstyle.xml", - "java.checkstyle.version": "9.3" + "java.checkstyle.version": "9.3", + "java.completion.favoriteStaticMembers": [ + "com.team766.framework3.RulePersistence.*", + "org.junit.Assert.*", + "org.junit.Assume.*", + "org.junit.jupiter.api.Assertions.*", + "org.junit.jupiter.api.Assumptions.*", + "org.junit.jupiter.api.DynamicContainer.*", + "org.junit.jupiter.api.DynamicTest.*", + "org.mockito.Mockito.*", + "org.mockito.ArgumentMatchers.*", + "org.mockito.Answers.*" + ] } diff --git a/src/main/java/com/team766/framework3/FunctionalInstantProcedure.java b/src/main/java/com/team766/framework3/FunctionalInstantProcedure.java index 2a78d02ab..131369f2e 100644 --- a/src/main/java/com/team766/framework3/FunctionalInstantProcedure.java +++ b/src/main/java/com/team766/framework3/FunctionalInstantProcedure.java @@ -6,7 +6,12 @@ public final class FunctionalInstantProcedure extends InstantProcedure { private final Runnable runnable; public FunctionalInstantProcedure(Set> reservations, Runnable runnable) { - super(runnable.toString(), reservations); + this(runnable.toString(), reservations, runnable); + } + + public FunctionalInstantProcedure( + String name, Set> reservations, Runnable runnable) { + super(name, reservations); this.runnable = runnable; } diff --git a/src/main/java/com/team766/framework3/FunctionalProcedure.java b/src/main/java/com/team766/framework3/FunctionalProcedure.java index 69d13c1b8..141c416ce 100644 --- a/src/main/java/com/team766/framework3/FunctionalProcedure.java +++ b/src/main/java/com/team766/framework3/FunctionalProcedure.java @@ -7,7 +7,12 @@ public final class FunctionalProcedure extends Procedure { private final Consumer runnable; public FunctionalProcedure(Set> reservations, Consumer runnable) { - super(runnable.toString(), reservations); + this(runnable.toString(), reservations, runnable); + } + + public FunctionalProcedure( + String name, Set> reservations, Consumer runnable) { + super(name, reservations); this.runnable = runnable; } diff --git a/src/main/java/com/team766/framework3/Rule.java b/src/main/java/com/team766/framework3/Rule.java index c76ae2288..2271475de 100644 --- a/src/main/java/com/team766/framework3/Rule.java +++ b/src/main/java/com/team766/framework3/Rule.java @@ -49,26 +49,82 @@ enum TriggerType { FINISHED } + /** Policy for canceling actions when the rule is in a given state. */ + enum Cancellation { + /** Do not cancel any previous actions. */ + DO_NOT_CANCEL, + /** Cancel the action previously scheduled when the rule was in the NEWLY state. */ + CANCEL_NEWLY_ACTION, + } + private final String name; private final BooleanSupplier predicate; private final Map> triggerProcedures = Maps.newEnumMap(TriggerType.class); private final Map>> triggerReservations = Maps.newEnumMap(TriggerType.class); + private final Cancellation cancellationOnFinish; private TriggerType currentTriggerType = TriggerType.NONE; private boolean sealed = false; /* package */ Rule( - String name, BooleanSupplier predicate, Supplier newlyTriggeringProcedure) { + String name, + BooleanSupplier predicate, + RulePersistence rulePersistence, + Supplier onTriggeringProcedure) { if (predicate == null) { throw new IllegalArgumentException("Rule predicate has not been set."); } - if (newlyTriggeringProcedure == null) { - throw new IllegalArgumentException("Newly triggering Procedure is not defined."); + if (onTriggeringProcedure == null) { + throw new IllegalArgumentException("On-triggering Procedure is not defined."); } + final Supplier newlyTriggeringProcedure = + switch (rulePersistence) { + case ONCE -> { + this.cancellationOnFinish = Cancellation.DO_NOT_CANCEL; + yield onTriggeringProcedure; + } + case ONCE_AND_HOLD -> { + this.cancellationOnFinish = Cancellation.CANCEL_NEWLY_ACTION; + yield () -> { + final Procedure procedure = onTriggeringProcedure.get(); + if (procedure == null) { + return null; + } + return new FunctionalProcedure( + procedure.getName(), + procedure.reservations(), + context -> { + procedure.run(context); + context.waitFor(() -> false); + }); + }; + } + case REPEATEDLY -> { + this.cancellationOnFinish = Cancellation.CANCEL_NEWLY_ACTION; + yield () -> { + final Procedure procedure = onTriggeringProcedure.get(); + if (procedure == null) { + return null; + } + return new FunctionalProcedure( + procedure.getName(), + procedure.reservations(), + context -> { + Procedure currentProcedure = procedure; + while (currentProcedure != null) { + context.runSync(currentProcedure); + context.yield(); + currentProcedure = onTriggeringProcedure.get(); + } + }); + }; + } + }; + this.name = name; this.predicate = predicate; if (newlyTriggeringProcedure != null) { @@ -95,7 +151,7 @@ public Rule withFinishedTriggeringProcedure(Set> reservations, Runn () -> new FunctionalInstantProcedure(reservations, action)); } - private Set> getReservationsForProcedure(Supplier supplier) { + private static Set> getReservationsForProcedure(Supplier supplier) { if (supplier != null) { Procedure procedure = supplier.get(); if (procedure != null) { @@ -142,10 +198,11 @@ public String getName() { } /* package */ Set> getMechanismsToReserve() { - if (triggerReservations.containsKey(currentTriggerType)) { - return triggerReservations.get(currentTriggerType); - } - return Collections.emptySet(); + return triggerReservations.getOrDefault(currentTriggerType, Collections.emptySet()); + } + + /* package */ Cancellation getCancellationOnFinish() { + return cancellationOnFinish; } /* package */ Procedure getProcedureToRun() { diff --git a/src/main/java/com/team766/framework3/RuleEngine.java b/src/main/java/com/team766/framework3/RuleEngine.java index 4556bdd84..f545eb063 100644 --- a/src/main/java/com/team766/framework3/RuleEngine.java +++ b/src/main/java/com/team766/framework3/RuleEngine.java @@ -1,5 +1,7 @@ package com.team766.framework3; +import static com.team766.framework3.RulePersistence.ONCE_AND_HOLD; + import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.BiMap; import com.google.common.collect.HashBiMap; @@ -14,6 +16,7 @@ import java.util.Map; import java.util.Set; import java.util.function.BooleanSupplier; +import java.util.function.Consumer; import java.util.function.Supplier; /** @@ -43,18 +46,90 @@ public Category getLoggerCategory() { return Category.RULES; } - protected Rule addRule(String name, BooleanSupplier condition, Supplier action) { - Rule rule = new Rule(name, condition, action); + protected Rule addRule( + String name, + BooleanSupplier condition, + RulePersistence rulePersistence, + Supplier action) { + Rule rule = new Rule(name, condition, rulePersistence, action); rules.put(name, rule); int priority = rulePriorities.size(); rulePriorities.put(rule, priority); return rule; } + protected Rule addRule(String name, BooleanSupplier condition, Supplier action) { + return addRule(name, condition, ONCE_AND_HOLD, action); + } + protected Rule addRule( - String name, BooleanSupplier condition, Mechanism mechanism, Runnable action) { + String name, + BooleanSupplier condition, + RulePersistence rulePersistence, + Set> mechanisms, + Consumer action) { + return addRule( + name, + condition, + rulePersistence, + () -> new FunctionalProcedure(mechanisms, action)); + } + + protected Rule addRule( + String name, + BooleanSupplier condition, + Set> mechanisms, + Consumer action) { + return addRule(name, condition, ONCE_AND_HOLD, mechanisms, action); + } + + protected Rule addRule( + String name, + BooleanSupplier condition, + RulePersistence rulePersistence, + Mechanism mechanism, + Consumer action) { + return addRule(name, condition, rulePersistence, Set.of(mechanism), action); + } + + protected Rule addRule( + String name, + BooleanSupplier condition, + Mechanism mechanism, + Consumer action) { + return addRule(name, condition, ONCE_AND_HOLD, mechanism, action); + } + + protected Rule addRule( + String name, + BooleanSupplier condition, + RulePersistence rulePersistence, + Set> mechanisms, + Runnable action) { return addRule( - name, condition, () -> new FunctionalInstantProcedure(Set.of(mechanism), action)); + name, + condition, + rulePersistence, + () -> new FunctionalInstantProcedure(mechanisms, action)); + } + + protected Rule addRule( + String name, BooleanSupplier condition, Set> mechanisms, Runnable action) { + return addRule(name, condition, ONCE_AND_HOLD, mechanisms, action); + } + + protected Rule addRule( + String name, + BooleanSupplier condition, + RulePersistence rulePersistence, + Mechanism mechanism, + Runnable action) { + return addRule(name, condition, rulePersistence, Set.of(mechanism), action); + } + + protected Rule addRule( + String name, BooleanSupplier condition, Mechanism mechanism, Runnable action) { + return addRule(name, condition, ONCE_AND_HOLD, mechanism, action); } @VisibleForTesting @@ -107,7 +182,7 @@ public final void run() { rule.evaluate(); // see if the rule is triggering - Rule.TriggerType triggerType = rule.getCurrentTriggerType(); + final Rule.TriggerType triggerType = rule.getCurrentTriggerType(); if (triggerType != Rule.TriggerType.NONE) { log(Severity.INFO, "Rule " + rule.getName() + " triggering: " + triggerType); @@ -169,6 +244,17 @@ public final void run() { } // we're good to proceed + + if (triggerType == Rule.TriggerType.FINISHED + && rule.getCancellationOnFinish() + == Rule.Cancellation.CANCEL_NEWLY_ACTION) { + var newlyCommand = + ruleMap.inverse().get(new RuleAction(rule, Rule.TriggerType.NEWLY)); + if (newlyCommand != null) { + newlyCommand.cancel(); + } + } + Procedure procedure = rule.getProcedureToRun(); if (procedure == null) { continue; diff --git a/src/main/java/com/team766/framework3/RulePersistence.java b/src/main/java/com/team766/framework3/RulePersistence.java new file mode 100644 index 000000000..b6e5dafd7 --- /dev/null +++ b/src/main/java/com/team766/framework3/RulePersistence.java @@ -0,0 +1,23 @@ +package com.team766.framework3; + +/** + * Policies for how to handle a Rule's action when the action completes or the Rule stops triggering. + */ +public enum RulePersistence { + /** + * When the action completes, don't do anything. Any Mechanism reservations that the action held + * are released. Also, the action may continue running after the Rule stops triggering. + */ + ONCE, + /** + * When the action completes, don't do anything but retain the Mechanism reservations that the + * action held until the Rule stops triggering. If the Rule stops triggering before the action + * has completed, then the action will be terminated. + */ + ONCE_AND_HOLD, + /** + * When the action completes, start executing the action again, until the Rule stops triggering. + * The action will be terminated when the Rule stops triggering. + */ + REPEATEDLY, +} diff --git a/src/test/java/com/team766/framework3/RuleEngineTest.java b/src/test/java/com/team766/framework3/RuleEngineTest.java index 3c0b9edd4..95db1acba 100644 --- a/src/test/java/com/team766/framework3/RuleEngineTest.java +++ b/src/test/java/com/team766/framework3/RuleEngineTest.java @@ -1,6 +1,10 @@ package com.team766.framework3; +import static com.team766.framework3.RulePersistence.ONCE; +import static com.team766.framework3.RulePersistence.ONCE_AND_HOLD; +import static com.team766.framework3.RulePersistence.REPEATEDLY; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -10,6 +14,7 @@ import edu.wpi.first.wpilibj2.command.Command; import edu.wpi.first.wpilibj2.command.CommandScheduler; import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.BooleanSupplier; import org.junit.jupiter.api.Test; @@ -64,7 +69,7 @@ public void testSeal() { RuleEngine rulesOne = new RuleEngine() { { - addRule("rule1_1", () -> true, () -> Procedure.NO_OP) + addRule("rule1_1", () -> true, ONCE, () -> Procedure.NO_OP) .withFinishedTriggeringProcedure(() -> Procedure.NO_OP); } }; @@ -74,7 +79,7 @@ public void testSeal() { RuleEngine rulesTwo = new RuleEngine() { { - addRule("rule2_1", () -> true, () -> Procedure.NO_OP); + addRule("rule2_1", () -> true, ONCE, () -> Procedure.NO_OP); } }; rulesTwo.run(); @@ -97,10 +102,12 @@ public void testAddRuleAndGetPriority() { addRule( "fm1_p0", new ScheduledPredicate(0), + ONCE, () -> new FakeProcedure(2, Set.of(fm1))); addRule( "fm1_p1", new ScheduledPredicate(0), + ONCE, () -> new FakeProcedure(2, Set.of(fm1))); } }; @@ -125,10 +132,12 @@ public void testRunNonConflictingRules() { addRule( "fm1", new ScheduledPredicate(0), + ONCE, () -> new FakeProcedure(2, Set.of(fm1))); addRule( "fm2", new ScheduledPredicate(0), + ONCE, () -> new FakeProcedure(2, Set.of(fm2))); } }; @@ -175,6 +184,7 @@ public void testFinishedProcedureBumpsNewlyProcedureForSameRule() { addRule( "fm1_p0", new ScheduledPredicate(0), + ONCE, () -> new FakeProcedure("fm1procnew_p0", 1, Set.of(fm1))) .withFinishedTriggeringProcedure( () -> @@ -213,15 +223,18 @@ public void testRunRulePriorities() { addRule( "fm1_p0", new ScheduledPredicate(0), + ONCE, () -> new FakeProcedure("fm1proc_p0", 0, Set.of(fm1, fm2))); addRule( "fm1_p1", new PeriodicPredicate(2), + ONCE, () -> new FakeProcedure("fm1proc_p1", 0, Set.of(fm1, fm3))); addRule( "fm3_p2", new ScheduledPredicate(0), + ONCE, () -> new FakeProcedure("fm3proc_p2", 0, Set.of(fm3))); } }; @@ -265,15 +278,18 @@ public void testRunHigherPriorityRuleStillBeingRun() { addRule( "fm1_p0", new ScheduledPredicate(0), + ONCE, () -> new FakeProcedure("fm1proc_p0", 2, Set.of(fm1, fm2))); addRule( "fm1_p1", new ScheduledPredicate(1), + ONCE, () -> new FakeProcedure("fm1proc_p1", 2, Set.of(fm1, fm2))); addRule( "fm1_p2", new ScheduledPredicate(3), + ONCE, () -> new FakeProcedure("fm1proc_p2", 2, Set.of(fm1, fm2))); } }; @@ -324,10 +340,12 @@ public void testRunLowerPriorityRuleBumped() { addRule( "fm1_p0", new ScheduledPredicate(1), + ONCE, () -> new FakeProcedure("fm1proc_p0", 2, Set.of(fm1, fm2))); addRule( "fm1_p1", new ScheduledPredicate(0), + ONCE, () -> new FakeProcedure("fm1proc_p1", 4, Set.of(fm1, fm2))); } }; @@ -358,10 +376,12 @@ public void testRuleResetIgnoredLowerPriorityRule() { addRule( "fm1_p0", new ScheduledPredicate(0), + ONCE, () -> new FakeProcedure("fm1procnew_p0", 2, Set.of(fm1))); addRule( "fm1_p1", new ScheduledPredicate(0), + ONCE, () -> new FakeProcedure("fm1procnew_p1", 1, Set.of(fm1))) .withFinishedTriggeringProcedure( () -> new FakeProcedure("fm1procfin_p1", 1, Set.of(fm2))); @@ -393,10 +413,12 @@ public void testRuleResetIgnoredLowerPriorityRuleHigherPriorityRulePreviouslySch addRule( "fm1_p0", new ScheduledPredicate(0), + ONCE, () -> new FakeProcedure("fm1procnew_p0", 2, Set.of(fm1))); addRule( "fm1_p1", new ScheduledPredicate(1), + ONCE, () -> new FakeProcedure("fm1procnew_p1", 1, Set.of(fm1))) .withFinishedTriggeringProcedure( () -> new FakeProcedure("fm1procfin_p1", 1, Set.of(fm2))); @@ -436,10 +458,12 @@ public void testRuleResetBumpedLowerPriorityRule() { addRule( "fm1_p0", new ScheduledPredicate(1), + ONCE, () -> new FakeProcedure("fm1procnew_p0", 2, Set.of(fm1))); addRule( "fm1_p1", new ScheduledPredicate(0), + ONCE, () -> new FakeProcedure("fm1procnew_p1", 2, Set.of(fm1))) .withFinishedTriggeringProcedure( () -> new FakeProcedure("fm1procfin_p1", 2, Set.of(fm2))); @@ -471,12 +495,14 @@ public void testLowerPriorityRuleRunsWhenProcedureFromHigherPriorityRuleFinishes addRule( "fm1_p0", new ScheduledPredicate(0, 4), + ONCE, () -> new FakeProcedure("fm1procnew_p0", 0, Set.of(fm1))) .withFinishedTriggeringProcedure( () -> new FakeProcedure("fm1procfin_p0", 0, Set.of(fm1))); addRule( "fm1_p1", new ScheduledPredicate(1), + ONCE, () -> new FakeProcedure("fm1procnew_p1", 0, Set.of(fm1))) .withFinishedTriggeringProcedure( () -> new FakeProcedure("fm1procfin_p1", 0, Set.of(fm1))); @@ -532,12 +558,14 @@ public void testRuleCalledAgainAfterBeingReset() { addRule( "fm1_p0", new ScheduledPredicate(1), + ONCE, () -> new FakeProcedure("fm1procnew_p0", 0, Set.of(fm1))) .withFinishedTriggeringProcedure( () -> new FakeProcedure("fm1procfin_p0", 0, Set.of(fm1))); addRule( "fm1_p1", new ScheduledPredicate(0, 4), + ONCE, () -> new FakeProcedure("fm1procnew_p1", 1, Set.of(fm1))) .withFinishedTriggeringProcedure( () -> new FakeProcedure("fm1procfin_p1", 1, Set.of(fm1))); @@ -594,12 +622,14 @@ public void testRuleResetPreventsFinishedForLongTrigger() { addRule( "fm1_p0", new ScheduledPredicate(1), + ONCE, () -> new FakeProcedure("fm1procnew_p0", 0, Set.of(fm1))) .withFinishedTriggeringProcedure( () -> new FakeProcedure("fm1procfin_p0", 0, Set.of(fm1))); addRule( "fm1_p1", new ScheduledPredicate(0, 3), + ONCE, () -> new FakeProcedure("fm1procnew_p1", 1, Set.of(fm1))) .withFinishedTriggeringProcedure( () -> new FakeProcedure("fm1procfin_p1", 1, Set.of(fm1))); @@ -638,4 +668,200 @@ public void testRuleResetPreventsFinishedForLongTrigger() { step(); // 3 } + + /** Test ONCE RulePersistence policy */ + @Test + public void testOncePersistence() { + AtomicReference predicateEndsFirstProc = new AtomicReference<>(); + AtomicReference actionEndsFirstProc = new AtomicReference<>(); + RuleEngine myRules = + new RuleEngine() { + { + addRule( + "predicate_ends_first", + new ScheduledPredicate(0, 1), + ONCE, + () -> { + var proc = + new FakeProcedure( + "predicate_ends_first_proc", 10, Set.of(fm1)); + predicateEndsFirstProc.set(proc); + return proc; + }); + addRule( + "action_ends_first", + new ScheduledPredicate(0, 10), + ONCE, + () -> { + var proc = + new FakeProcedure( + "action_ends_first_proc", 1, Set.of(fm2)); + actionEndsFirstProc.set(proc); + return proc; + }); + } + }; + + myRules.run(); + + // check that both action Procedures are scheduled + Command cmd1 = CommandScheduler.getInstance().requiring(fm1); + assertNotNull(cmd1); + assertTrue(cmd1.getName().endsWith("predicate_ends_first_proc")); + Command cmd2 = CommandScheduler.getInstance().requiring(fm2); + assertNotNull(cmd2); + assertTrue(cmd2.getName().endsWith("action_ends_first_proc")); + + step(); + myRules.run(); + step(); + myRules.run(); + + // ONCE actions should be allowed to run after the rule has stopped triggering. + assertEquals(2, predicateEndsFirstProc.get().age()); + assertFalse(predicateEndsFirstProc.get().isEnded()); + cmd1 = CommandScheduler.getInstance().requiring(fm1); + assertNotNull(cmd1); + assertTrue(cmd1.getName().endsWith("predicate_ends_first_proc")); + + // If a ONCE action completes, it should end and mechanism reservations released. + assertTrue(actionEndsFirstProc.get().isEnded()); + cmd2 = CommandScheduler.getInstance().requiring(fm2); + assertNull(cmd2); + } + + /** Test ONCE_AND_HOLD RulePersistence policy */ + @Test + public void testOnceAndHoldPersistence() { + AtomicReference predicateEndsFirstProc = new AtomicReference<>(); + AtomicReference actionEndsFirstProc = new AtomicReference<>(); + RuleEngine myRules = + new RuleEngine() { + { + addRule( + "predicate_ends_first", + new ScheduledPredicate(0, 1), + ONCE_AND_HOLD, + () -> { + var proc = + new FakeProcedure( + "predicate_ends_first_proc", 10, Set.of(fm1)); + predicateEndsFirstProc.set(proc); + return proc; + }); + addRule( + "action_ends_first", + new ScheduledPredicate(0, 10), + ONCE_AND_HOLD, + () -> { + var proc = + new FakeProcedure( + "action_ends_first_proc", 1, Set.of(fm2)); + actionEndsFirstProc.set(proc); + return proc; + }); + } + }; + + myRules.run(); + + // check that both action Procedures are scheduled + Command cmd1 = CommandScheduler.getInstance().requiring(fm1); + assertNotNull(cmd1); + assertTrue(cmd1.getName().endsWith("predicate_ends_first_proc")); + Command cmd2 = CommandScheduler.getInstance().requiring(fm2); + assertNotNull(cmd2); + assertTrue(cmd2.getName().endsWith("action_ends_first_proc")); + + step(); + myRules.run(); + step(); + myRules.run(); + + // ONCE_AND_HOLD actions should be cancelled after the rule has stopped triggering. + assertTrue(predicateEndsFirstProc.get().isEnded()); + cmd1 = CommandScheduler.getInstance().requiring(fm1); + assertNull(cmd1); + + // If a ONCE_AND_HOLD action completes, it should end but mechanism reservations are + // retained. + assertTrue(actionEndsFirstProc.get().isEnded()); + cmd2 = CommandScheduler.getInstance().requiring(fm2); + assertNotNull(cmd2); + assertTrue(cmd2.getName().endsWith("action_ends_first_proc")); + } + + /** Test REPEATEDLY RulePersistence policy */ + @Test + public void testRepeatedlyPersistence() { + AtomicReference predicateEndsFirstProc = new AtomicReference<>(); + AtomicReference actionEndsFirstProc = new AtomicReference<>(); + RuleEngine myRules = + new RuleEngine() { + { + addRule( + "predicate_ends_first", + new ScheduledPredicate(0, 1), + REPEATEDLY, + () -> { + var proc = + new FakeProcedure( + "predicate_ends_first_proc", 10, Set.of(fm1)); + predicateEndsFirstProc.set(proc); + return proc; + }); + addRule( + "action_ends_first", + new ScheduledPredicate(0, 10), + REPEATEDLY, + () -> { + var proc = + new FakeProcedure( + "action_ends_first_proc", 1, Set.of(fm2)); + actionEndsFirstProc.set(proc); + return proc; + }); + } + }; + + myRules.run(); + + // check that both action Procedures are scheduled + Command cmd1 = CommandScheduler.getInstance().requiring(fm1); + assertNotNull(cmd1); + assertTrue(cmd1.getName().endsWith("predicate_ends_first_proc")); + Command cmd2 = CommandScheduler.getInstance().requiring(fm2); + assertNotNull(cmd2); + assertTrue(cmd2.getName().endsWith("action_ends_first_proc")); + + step(); + myRules.run(); + step(); + myRules.run(); + step(); + + // REPEATEDLY actions should be cancelled after the rule has stopped triggering. + assertTrue(predicateEndsFirstProc.get().isEnded()); + cmd1 = CommandScheduler.getInstance().requiring(fm1); + assertNull(cmd1); + + // If a REPEATEDLY action completes, another instance should be started. + assertFalse(actionEndsFirstProc.get().isEnded()); + cmd2 = CommandScheduler.getInstance().requiring(fm2); + assertNotNull(cmd2); + assertTrue(cmd2.getName().endsWith("action_ends_first_proc")); + + final FakeProcedure previousActionInstance = actionEndsFirstProc.get(); + + myRules.run(); + step(); + myRules.run(); + step(); + + assertTrue(previousActionInstance.isEnded()); + assertFalse(actionEndsFirstProc.get().isEnded()); + cmd2 = CommandScheduler.getInstance().requiring(fm2); + assertNotNull(cmd2); + assertTrue(cmd2.getName().endsWith("action_ends_first_proc")); + } } diff --git a/src/test/java/com/team766/framework3/RuleTest.java b/src/test/java/com/team766/framework3/RuleTest.java index b37a54257..86108c634 100644 --- a/src/test/java/com/team766/framework3/RuleTest.java +++ b/src/test/java/com/team766/framework3/RuleTest.java @@ -1,9 +1,13 @@ package com.team766.framework3; +import static com.team766.framework3.RulePersistence.ONCE; +import static com.team766.framework3.RulePersistence.ONCE_AND_HOLD; +import static com.team766.framework3.RulePersistence.REPEATEDLY; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import com.team766.framework3.Rule.Cancellation; import com.team766.framework3.Rule.TriggerType; import java.util.Collections; import java.util.Set; @@ -41,7 +45,7 @@ protected void setUp() {} @Test public void testCreate() { - Rule alwaysTrue = new Rule("always true", () -> true, () -> Procedure.NO_OP); + Rule alwaysTrue = new Rule("always true", () -> true, ONCE, () -> Procedure.NO_OP); assertNotNull(alwaysTrue); assertEquals("always true", alwaysTrue.getName()); } @@ -49,7 +53,7 @@ public void testCreate() { @Test public void testEvaluate() { // start with simple test of a NONE->NEWLY->CONTINUING->CONTINUING sequence - Rule alwaysTrue = new Rule("always true", () -> true, () -> Procedure.NO_OP); + Rule alwaysTrue = new Rule("always true", () -> true, ONCE, () -> Procedure.NO_OP); assertEquals(Rule.TriggerType.NONE, alwaysTrue.getCurrentTriggerType()); alwaysTrue.evaluate(); assertEquals(TriggerType.NEWLY, alwaysTrue.getCurrentTriggerType()); @@ -63,6 +67,7 @@ public void testEvaluate() { new Rule( "duck duck goose goose", new DuckDuckGooseGoosePredicate(), + ONCE, () -> Procedure.NO_OP); assertEquals(Rule.TriggerType.NONE, duckDuckGooseGoose.getCurrentTriggerType()); duckDuckGooseGoose.evaluate(); @@ -77,6 +82,35 @@ public void testEvaluate() { assertEquals(TriggerType.NEWLY, duckDuckGooseGoose.getCurrentTriggerType()); } + @Test + public void testGetCancellation() { + Rule ruleWithOnce = + new Rule( + "always true", + new DuckDuckGooseGoosePredicate(), + ONCE, + () -> Procedure.NO_OP); + assertEquals(Cancellation.DO_NOT_CANCEL, ruleWithOnce.getCancellationOnFinish()); + + Rule ruleWithOnceAndHold = + new Rule( + "always true", + new DuckDuckGooseGoosePredicate(), + ONCE_AND_HOLD, + () -> Procedure.NO_OP); + assertEquals( + Cancellation.CANCEL_NEWLY_ACTION, ruleWithOnceAndHold.getCancellationOnFinish()); + + Rule ruleWithRepeatedly = + new Rule( + "always true", + new DuckDuckGooseGoosePredicate(), + REPEATEDLY, + () -> Procedure.NO_OP); + assertEquals( + Cancellation.CANCEL_NEWLY_ACTION, ruleWithRepeatedly.getCancellationOnFinish()); + } + @Test public void testGetMechanismsToReserve() { final Set> newlyMechanisms = @@ -87,6 +121,7 @@ public void testGetMechanismsToReserve() { new Rule( "duck duck goose goose", new DuckDuckGooseGoosePredicate(), + ONCE, () -> new FunctionalInstantProcedure(newlyMechanisms, () -> {})) .withFinishedTriggeringProcedure(finishedMechanisms, () -> {}); @@ -121,6 +156,7 @@ public void testGetProcedureToRun() { new Rule( "duck duck goose goose", new DuckDuckGooseGoosePredicate(), + ONCE, () -> new TrivialProcedure("newly")) .withFinishedTriggeringProcedure(() -> new TrivialProcedure("finished")); From 4d02198ecfa89e6660df17081a0cba0fb3bec165 Mon Sep 17 00:00:00 2001 From: Ryan Cahoon Date: Sun, 12 Jan 2025 22:55:40 -0800 Subject: [PATCH 4/4] Change "rules.add" to "addRule" in example code --- src/main/java/com/team766/framework3/Rule.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/team766/framework3/Rule.java b/src/main/java/com/team766/framework3/Rule.java index c76ae2288..15a423d4a 100644 --- a/src/main/java/com/team766/framework3/Rule.java +++ b/src/main/java/com/team766/framework3/Rule.java @@ -24,8 +24,8 @@ * public class MyRules extends RuleEngine { * public MyRules() { * // add rule to spin up the shooter when the boxop presses the right trigger on the gamepad - * rules.add("spin up shooter", gamepad.getButton(InputConstants.XBOX_RT), - * () -> new ShooterSpin(shooter))); + * addRule("spin up shooter", gamepad.getButton(InputConstants.XBOX_RT), + * () -> new ShooterSpin(shooter))); * ... * } * }