Merge pull request #85 from rhauch/issue-84

Issue 84 - Simplify engine and executor

Author: rhauch

build.properties

@@ -1,4 +1,4 @@
-strongback.version=1.1.7
+strongback.version=1.2.0-Beta1
 #
 # The build will download a specific version of the WPILib given by the following URL
 # and install it into the 'libs/wpilib' folder. To use a different version of WPILib,

build.xml

@@ -88,7 +88,6 @@
 
             <doctitle><![CDATA[<h1>Strongback Java Library</h1>]]></doctitle>
             <bottom><![CDATA[<i>Copyright &#169; ${current.year} Strongback and individual contributors. All Rights Reserved.</i>]]></bottom>
-            <tag name="todo" scope="all" description="To do:"/>
             <group title="Strongback" packages="org.strongback"/>
             <group title="Components" packages="org.strongback.components*:org.strongback.hardware*"/>
             <group title="Drives" packages="org.strongback.drive"/>

strongback-tests/src/org/strongback/ExecutableTimerTest.java

@@ -20,19 +20,15 @@
 
 import java.util.concurrent.TimeUnit;
 
-import junit.framework.AssertionFailedError;
-
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.strongback.Logger.Level;
-import org.strongback.Strongback.Configurator.TimerMode;
+
+import junit.framework.AssertionFailedError;
 
 /**
  * This test attempts to measure the timing of the {@link Strongback#executor() Strongback Executor} instance at various periods
- * using the system clock and various {@link TimerMode}s. On OS X 10.10.3, the {@link TimerMode#BUSY} is clearly the most
- * accurate down to 1 millisecond, whereas {@link TimerMode#PARK} and {@link TimerMode#SLEEP} are relatively inaccurate and
- * imprecise for intervals even as high as 20ms.
  * <p>
  * The code committed into Git are small execution rates and sample sizes to minimize the time required to run the tests. As
  * such, the results are probably not terribly meaningful. To obtain more meaningful results on your own platform, simply adjust
@@ -44,28 +40,22 @@ public class ExecutableTimerTest {
 
     @BeforeClass
     public static void beforeAll() {
-        Strongback.configure().useSystemLogger(Level.ERROR).recordNoData().recordNoEvents().initialize();
-        Strongback.start();
+        Strongback.configure().setLogLevel(Level.ERROR).recordNoData().recordNoEvents().initialize();
     }
 
     @AfterClass
     public static void afterAll() {
-        Strongback.shutdown();
-    }
-
-    @Test
-    public void shouldMeasureAndPrintTimingHistogramUsingBusyMode() throws InterruptedException {
-        runTimer(TimerMode.BUSY, 4, 2000);
+        Strongback.stop();
     }
 
     @Test
-    public void shouldMeasureAndPrintTimingHistogramUsingParkMode() throws InterruptedException {
-        runTimer(TimerMode.PARK, 5, 2000);
+    public void shouldMeasureAndPrintTimingHistogramFor4MillisecondPeriod() throws InterruptedException {
+        runTimer(4, 2000);
     }
 
     @Test
-    public void shouldMeasureAndPrintTimingHistogramUsingSleepMode() throws InterruptedException {
-        runTimer(TimerMode.SLEEP, 6, 2000);
+    public void shouldMeasureAndPrintTimingHistogramFor10MillisecondPeriod() throws InterruptedException {
+        runTimer(10, 2000);
     }
 
     /**
@@ -75,14 +65,14 @@ public void shouldMeasureAndPrintTimingHistogramUsingSleepMode() throws Interrup
      * @param millisecondExecutionPeriod the execution period in milliseconds
      * @param sampleTimeInMilliseconds the sample time in milliseconds
      */
-    protected void runTimer(TimerMode mode, int millisecondExecutionPeriod, int sampleTimeInMilliseconds) {
+    protected void runTimer(int millisecondExecutionPeriod, int sampleTimeInMilliseconds) {
         try {
+            Strongback.stop();
             Strongback.configure()
-                      .useExecutionTimerMode(mode)
-                      .useExecutionPeriod(millisecondExecutionPeriod, TimeUnit.MILLISECONDS)
-                      .initialize();
+                      .useExecutionPeriod(millisecondExecutionPeriod, TimeUnit.MILLISECONDS);
+            Strongback.start();
             assertThat(ExecutableTimer.measureTimingAndPrint(Strongback.executor(),
-                                                             mode.toString() + " for " + millisecondExecutionPeriod + " ms",
+                                                             " for " + millisecondExecutionPeriod + " ms",
                                                              sampleTimeInMilliseconds / millisecondExecutionPeriod)
                                       .await(10, TimeUnit.SECONDS)
                                       .isComplete());

strongback-tests/src/org/strongback/StrongbackTest.java

@@ -0,0 +1,89 @@
+/*
+ * Strongback
+ * Copyright 2015, Strongback and individual contributors by the @authors tag.
+ * See the COPYRIGHT.txt in the distribution for a full listing of individual
+ * contributors.
+ *
+ * Licensed under the MIT License; you may not use this file except in
+ * compliance with the License. You may obtain a copy of the License at
+ * http://opensource.org/licenses/MIT
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.strongback;
+
+import static org.fest.assertions.Assertions.assertThat;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.strongback.Logger.Level;
+import org.strongback.components.Clock;
+
+/**
+ * Tests that check the functionality of the {@link Strongback.Engine}.
+ */
+public class StrongbackTest {
+
+    private static final SystemLogger LOGGER = new SystemLogger();
+
+    private Clock clock;
+    private Strongback.Engine engine;
+
+    @Before
+    public void beforeEach() {
+        LOGGER.enable(Level.INFO);
+        clock = Clock.system();
+        engine = new Strongback.Engine(clock, LOGGER);
+    }
+
+    @After
+    public void afterEach() {
+        try {
+            if (engine != null && engine.isRunning()) {
+                engine.stop();
+            }
+        } finally {
+            engine = null;
+        }
+    }
+
+    @Test
+    public void shouldNotBeRunningWhenCreated() {
+        assertThat(engine.isRunning()).isFalse();
+    }
+
+    @Test
+    public void shouldStartWithDefaultConfiguration() {
+        engine.logConfiguration();
+        assertThat(engine.isRunning()).isFalse();
+        assertThat(engine.start()).isTrue();
+        assertThat(engine.isRunning()).isTrue();
+    }
+
+    @Test
+    public void shouldAllowChangingExecutionPeriodWhenNotRunning() {
+        assertThat(engine.isRunning()).isFalse();
+        assertThat(engine.getExecutionPeriod()).isEqualTo(20);
+        engine.setExecutionPeriod(5);
+        assertThat(engine.getExecutionPeriod()).isEqualTo(5);
+        assertThat(engine.start()).isTrue();
+        engine.logConfiguration();
+    }
+
+    @Test
+    public void shouldNotAllowChangingExecutionPeriodWhenRunning() {
+        assertThat(engine.isRunning()).isFalse();
+        assertThat(engine.start()).isTrue();
+        assertThat(engine.getExecutionPeriod()).isEqualTo(20);
+        LOGGER.enable(Level.OFF);
+        assertThat(engine.setExecutionPeriod(5)).isFalse();
+        LOGGER.enable(Level.INFO);
+        assertThat(engine.getExecutionPeriod()).isEqualTo(20);
+    }
+
+}

strongback/src/org/strongback/Executables.java

@@ -16,41 +16,89 @@
 
 package org.strongback;
 
-import java.util.Iterator;
+import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
 
 import org.strongback.annotation.ThreadSafe;
 
 /**
  * A simple threadsafe list of {@link Executable} instances.
+ *
  * @author Randall Hauch
  */
 @ThreadSafe
-final class Executables implements Executor, Iterable<Executable> {
+final class Executables implements Executor {
 
-    private final CopyOnWriteArrayList<Executable> executables = new CopyOnWriteArrayList<>();
+    private final CopyOnWriteArrayList<Executable> highPriority = new CopyOnWriteArrayList<>();
+    private final CopyOnWriteArrayList<Executable> mediumPriority = new CopyOnWriteArrayList<>();
+    private final CopyOnWriteArrayList<Executable> lowPriority = new CopyOnWriteArrayList<>();
 
     Executables() {
     }
 
     @Override
-    public boolean register(Executable r) {
-        return executables.addIfAbsent(r);
+    public boolean register(Executable r, Priority priority) {
+        if (priority != null) {
+            unregister(r);
+            switch (priority) {
+                case HIGH:
+                    mediumPriority.remove(r);
+                    lowPriority.remove(r);
+                    return highPriority.addIfAbsent(r);
+                case MEDIUM:
+                    highPriority.remove(r);
+                    lowPriority.remove(r);
+                    return mediumPriority.addIfAbsent(r);
+                case LOW:
+                    highPriority.remove(r);
+                    mediumPriority.remove(r);
+                    return lowPriority.addIfAbsent(r);
+            }
+        }
+        return false;
     }
 
     @Override
     public boolean unregister(Executable r) {
-        return executables.remove(r);
+        if (r != null) {
+            // Remove from all
+            boolean removed = highPriority.remove(r);
+            removed = mediumPriority.remove(r) || removed;
+            removed = lowPriority.remove(r) || removed;
+            return removed;
+        }
+        return false;
     }
 
     @Override
     public void unregisterAll() {
-        executables.clear();
+        highPriority.clear();
+        mediumPriority.clear();
+        lowPriority.clear();
     }
 
-    @Override
-    public Iterator<Executable> iterator() {
-        return executables.iterator();
+    public List<Executable> lowPriorityExecutables() {
+        return lowPriority;
+    }
+
+    public List<Executable> mediumPriorityExecutables() {
+        return mediumPriority;
+    }
+
+    public List<Executable> highPriorityExecutables() {
+        return highPriority;
+    }
+
+    protected Executable[] lowPriorityExecutablesAsArrays() {
+        return lowPriority.toArray(new Executable[0]); // will be reallocated with correct size
+    }
+
+    protected Executable[] mediumPriorityExecutablesAsArrays() {
+        return mediumPriority.toArray(new Executable[0]); // will be reallocated with correct size
+    }
+
+    protected Executable[] highPriorityExecutablesAsArrays() {
+        return highPriority.toArray(new Executable[0]); // will be reallocated with correct size
     }
 
 }

strongback/src/org/strongback/Executor.java

@@ -24,21 +24,45 @@
 @ThreadSafe
 public interface Executor {
 
+    public static enum Priority {
+        HIGH, MEDIUM, LOW;
+    }
+
+    /**
+     * Register a high priority {@link Executable} task so that it is called repeatedly on Strongback's executor thread.
+     *
+     * @param task the executable task
+     * @return {@code true} if the executable task was registered for the first time as a high priority, or {@code false} if
+     *         {@code task} was null or was already registered with this executor at high priority
+     * @deprecated Use {@link #register(Executable, Priority)} instead
+     */
+    @Deprecated
+    default boolean register(Executable task) {
+        return register(task, Priority.HIGH);
+    }
+
     /**
-     * Register an {@link Executable} task to be called repeatedly.
+     * Register an {@link Executable} task with the given priority so that it is called repeatedly on Strongback's executor
+     * thread. If the given task is already registered with a different priority, this method reassigns it to the desired
+     * priority; if the given task is already registered with the desired priority, this method does nothing.
+     * <p>
+     * This executor runs high priority tasks every cycle, medium priority tasks slightly every other cycles, and low priority
+     * tasks every 4 cycles. All {@link Executable} tasks are called on the first cycle.
      *
-     * @param r the executable task
-     * @return true if the executable task was registered, or false if it was null or was already registered with this executor
+     * @param task the executable task
+     * @param priority the priority of the executable; may not be null
+     * @return {@code true} if the executable task was registered for the first time at the given priority, or {@code false} if
+     *         {@code task} was null or was already registered with this executor at the given priority
      */
-    public boolean register(Executable r);
+    public boolean register(Executable task, Priority priority);
 
     /**
      * Unregister an {@link Executable} task to no longer be called.
      *
-     * @param r the executable task
+     * @param task the executable task
      * @return true if the executable task was unregistered, or false if it was null or not registered with this executor
      */
-    public boolean unregister(Executable r);
+    public boolean unregister(Executable task);
 
     /**
      * Unregister all {@link Executable} tasks.