Sources should not throw exceptions when they fail to load

Author: JLLeitschuh

core/src/main/java/edu/wpi/grip/core/Source.java

@@ -45,4 +45,17 @@ public final OutputSocket[] getOutputSockets() {
      * @see #getProperties()
      */
     public abstract void createFromProperties(EventBus eventBus, Properties properties) throws IOException;
+
+
+    public <T extends Source> T start(EventBus eventBus) throws IOException {
+        final T source = (T) start();
+        eventBus.register(source);
+        return source;
+    };
+
+    protected abstract Source start() throws IOException;
+
+    public abstract Source stop() throws Exception;
+
+    public abstract boolean isRunning();
 }

core/src/main/java/edu/wpi/grip/core/events/SourceStartedEvent.java

@@ -0,0 +1,19 @@
+package edu.wpi.grip.core.events;
+
+
+import edu.wpi.grip.core.Source;
+
+/**
+ * An event that occurs when a source is started.
+ */
+public class SourceStartedEvent {
+    private final Source source;
+
+    public SourceStartedEvent(Source source){
+        this.source = source;
+    }
+
+    public Source getSource() {
+        return this.source;
+    }
+}

core/src/main/java/edu/wpi/grip/core/events/SourceStoppedEvent.java

@@ -0,0 +1,18 @@
+package edu.wpi.grip.core.events;
+
+import edu.wpi.grip.core.Source;
+
+/**
+ * An event that occurs when a source is stopped.
+ */
+public class SourceStoppedEvent {
+    private final Source source;
+
+    public SourceStoppedEvent(Source source){
+        this.source = source;
+    }
+
+    public Source getSource() {
+        return this.source;
+    }
+}

core/src/main/java/edu/wpi/grip/core/sources/CameraSource.java

@@ -11,6 +11,8 @@
 import edu.wpi.grip.core.Source;
 import edu.wpi.grip.core.events.UnexpectedThrowableEvent;
 import edu.wpi.grip.core.events.SourceRemovedEvent;
+import edu.wpi.grip.core.events.SourceStartedEvent;
+import edu.wpi.grip.core.events.SourceStoppedEvent;
 import org.bytedeco.javacpp.opencv_core.Mat;
 import org.bytedeco.javacv.*;
 
@@ -42,7 +44,7 @@ public class CameraSource extends Source {
     private OutputSocket<Mat> frameOutputSocket;
     private OutputSocket<Number> frameRateOutputSocket;
     private Optional<Thread> frameThread;
-    private Optional<FrameGrabber> grabber;
+    private FrameGrabber grabber;
 
     /**
      * Creates a camera source that can be used as an input to a pipeline
@@ -72,7 +74,6 @@ public CameraSource(EventBus eventBus, String address) throws IOException {
      * Used for serialization
      */
     public CameraSource() {
-        this.grabber = Optional.empty();
         this.frameThread = Optional.empty();
     }
 
@@ -81,9 +82,8 @@ private void initialize(EventBus eventBus, FrameGrabber frameGrabber, String nam
         this.name = name;
         this.frameOutputSocket = new OutputSocket<>(eventBus, imageOutputHint);
         this.frameRateOutputSocket = new OutputSocket<>(eventBus, frameRateOutputHint);
-
-        this.eventBus.register(this);
-        this.startVideo(frameGrabber);
+        this.grabber = frameGrabber;
+        eventBus.register(this);
     }
 
     @Override
@@ -127,109 +127,117 @@ public void createFromProperties(EventBus eventBus, Properties properties) throw
     }
 
     /**
-     * Starts the video capture from the
-     *
-     * @param grabber A JavaCV {@link FrameGrabber} instance to capture from
+     * Starts the video capture from the source device
      */
-    private synchronized void startVideo(FrameGrabber grabber) throws IOException {
+    public CameraSource start() throws IOException, IllegalStateException {
         final OpenCVFrameConverter.ToMat convertToMat = new OpenCVFrameConverter.ToMat();
-        if (this.frameThread.isPresent()) {
-            throw new IllegalStateException("The video retrieval thread has already been started.");
-        }
-        if (this.grabber.isPresent()) {
-            throw new IllegalStateException("The Frame Grabber has already been started.");
-        }
-        try {
-            grabber.start();
-        } catch (FrameGrabber.Exception e) {
-            throw new IOException("A problem occurred trying to start the frame grabber for " + this.name, e);
-        }
+        synchronized (this.frameThread) {
+            if (this.frameThread.isPresent()) {
+                throw new IllegalStateException("The video retrieval thread has already been started.");
+            }
+            try {
+                grabber.start();
+            } catch (FrameGrabber.Exception e) {
+                throw new IOException("A problem occurred trying to start the frame grabber for " + this.name, e);
+            }
 
-        // Store the grabber only once it has been started in the case that there is an exception.
-        this.grabber = Optional.of(grabber);
+            final Thread frameExecutor = new Thread(() -> {
+                long lastFrame = System.currentTimeMillis();
+                while (!Thread.interrupted()) {
+                    final Frame videoFrame;
+                    try {
+                        videoFrame = grabber.grab();
+                    } catch (FrameGrabber.Exception e) {
+                        throw new IllegalStateException("Failed to grab image", e);
+                    }
 
-        final Thread frameExecutor = new Thread(() -> {
-            long lastFrame = System.currentTimeMillis();
-            while (!Thread.interrupted()) {
-                final Frame videoFrame;
-                try {
-                    videoFrame = grabber.grab();
-                } catch (FrameGrabber.Exception e) {
-                    throw new IllegalStateException("Failed to grab image", e);
-                }
-                final Mat frameMat = convertToMat.convert(videoFrame);
+                    final Mat frameMat = convertToMat.convert(videoFrame);
 
-                if (frameMat == null || frameMat.isNull()) {
-                    throw new IllegalStateException("The camera returned a null frame Mat");
+                    if (frameMat == null || frameMat.isNull()) {
+                        throw new IllegalStateException("The camera returned a null frame Mat");
+                    }
+
+                    frameMat.copyTo(frameOutputSocket.getValue().get());
+                    frameOutputSocket.setValue(frameOutputSocket.getValue().get());
+                    long thisMoment = System.currentTimeMillis();
+                    frameRateOutputSocket.setValue(1000 / (thisMoment - lastFrame));
+                    lastFrame = thisMoment;
                 }
+            }, "Camera");
 
-                frameMat.copyTo(frameOutputSocket.getValue().get());
-                frameOutputSocket.setValue(frameOutputSocket.getValue().get());
-                long thisMoment = System.currentTimeMillis();
-                frameRateOutputSocket.setValue(1000 / (thisMoment - lastFrame));
-                lastFrame = thisMoment;
-            }
-        }, "Camera");
-        frameExecutor.setUncaughtExceptionHandler(
-                (thread, exception) -> {
-                    eventBus.post(new UnexpectedThrowableEvent(exception, "Webcam Frame Grabber Thread crashed with uncaught exception"));
-                    try {
-                        stopVideo();
-                    } catch (TimeoutException e) {
-                        eventBus.post(new UnexpectedThrowableEvent(e, "Webcam Frame Grabber could not be stopped!"));
+            frameExecutor.setUncaughtExceptionHandler(
+                    (thread, exception) -> {
+                        eventBus.post(new UnexpectedThrowableEvent(exception, "Camera Frame Grabber Thread crashed with uncaught exception"));
+                        try {
+                            stop();
+                        } catch (TimeoutException e) {
+                            eventBus.post(new UnexpectedThrowableEvent(e, "Camera Frame Grabber could not be stopped!"));
+                        }
                     }
-                }
-        );
-        frameExecutor.setDaemon(true);
-        frameExecutor.start();
-        frameThread = Optional.of(frameExecutor);
+            );
+            frameExecutor.setDaemon(true);
+            frameExecutor.start();
+            this.frameThread = Optional.of(frameExecutor);
+        }
+        eventBus.post(new SourceStartedEvent(this));
+        return this;
     }
 
     /**
      * Stops the video feed from updating the output socket.
      *
-     * @throws TimeoutException If the thread running the Webcam fails to join this one after a timeout.
+     * @throws TimeoutException      If the thread running the Webcam fails to join this one after a timeout.
+     * @throws IllegalStateException If the camera was already stopped
      */
-    private void stopVideo() throws TimeoutException {
-        if (frameThread.isPresent()) {
-            final Thread ex = frameThread.get();
-            ex.interrupt();
-            try {
-                ex.join(TimeUnit.SECONDS.toMillis(2));
-                if (ex.isAlive()) {
-                    throw new TimeoutException("Unable to terminate video feed from Web Camera");
-                }
-            } catch (InterruptedException e) {
-                Thread.currentThread().interrupt();
-                //TODO: Move this into a logging framework
-                System.out.println("Caught Exception:");
-                e.printStackTrace();
-            } finally {
-                frameThread = Optional.empty();
-                // This will always run even if a timeout exception occurs
+    public CameraSource stop() throws TimeoutException, IllegalStateException {
+        synchronized (this.frameThread) {
+            if (frameThread.isPresent()) {
+                final Thread ex = frameThread.get();
+                ex.interrupt();
                 try {
-                    grabber.ifPresent(grabber -> {
-                        try {
-                            grabber.stop();
-                        } catch (FrameGrabber.Exception e) {
-                            throw new IllegalStateException("A problem occurred trying to stop the frame grabber", e);
-                        }
-                    });
+                    ex.join(TimeUnit.SECONDS.toMillis(500));
+                    if (ex.isAlive()) {
+                        throw new TimeoutException("Unable to terminate video feed from Web Camera");
+                    }
+                } catch (InterruptedException e) {
+                    Thread.currentThread().interrupt();
+                    //TODO: Move this into a logging framework
+                    System.out.println("Caught Exception:");
+                    e.printStackTrace();
                 } finally {
-                    // This will always run even if we fail to stop the grabber
-                    grabber = Optional.empty();
+                    // Clean up this resource as you can't restart a stopped thread
+                    this.frameThread = Optional.empty();
+                    // This will always run even if a timeout exception occurs
+                    try {
+                        grabber.stop();
+                    } catch (FrameGrabber.Exception e) {
+                        throw new IllegalStateException("A problem occurred trying to stop the frame grabber", e);
+                    }
                 }
+            } else {
+                throw new IllegalStateException("Tried to stop a Webcam that is already stopped.");
             }
-        } else {
-            throw new IllegalStateException("Tried to stop a Webcam that is already stopped.");
+        }
+        eventBus.post(new SourceStoppedEvent(this));
+        frameRateOutputSocket.setValue(0);
+        return this;
+    }
+
+    @Override
+    public boolean isRunning() {
+        synchronized (this.frameThread) {
+            return this.frameThread.isPresent() && this.frameThread.get().isAlive();
         }
     }
 
     @Subscribe
     public void onSourceRemovedEvent(SourceRemovedEvent event) throws TimeoutException {
         if (event.getSource() == this) {
-            this.stopVideo();
-            this.eventBus.unregister(this);
+            try {
+                this.stop();
+            } finally {
+                this.eventBus.unregister(this);
+            }
         }
     }
 

core/src/main/java/edu/wpi/grip/core/sources/ImageFileSource.java

@@ -7,6 +7,7 @@
 import edu.wpi.grip.core.SocketHint;
 import edu.wpi.grip.core.SocketHints;
 import edu.wpi.grip.core.Source;
+import edu.wpi.grip.core.events.SourceStartedEvent;
 import org.bytedeco.javacpp.opencv_core.Mat;
 import org.bytedeco.javacpp.opencv_imgcodecs;
 
@@ -31,6 +32,8 @@ public class ImageFileSource extends Source {
     private String path;
     private final SocketHint<Mat> imageOutputHint = SocketHints.Inputs.createMatSocketHint("Image", true);
     private OutputSocket<Mat> outputSocket;
+    private EventBus eventBus;
+    private boolean started = false;
 
     /**
      * @param eventBus The event bus for the pipeline.
@@ -80,10 +83,25 @@ public void createFromProperties(EventBus eventBus, Properties properties) throw
         if (path == null) {
             throw new IllegalArgumentException("Cannot create ImageFileSource without a path.");
         }
-
         this.initialize(eventBus, path);
     }
 
+    public ImageFileSource start() throws IOException {
+        this.started = true;
+        loadImage(this.path);
+        return this;
+    }
+
+    @Override
+    public ImageFileSource stop() {
+        return this;
+    }
+
+    @Override
+    public boolean isRunning() {
+        return this.started;
+    }
+
     /**
      * Loads the image and posts an update to the {@link EventBus}
      *
@@ -108,5 +126,6 @@ private void loadImage(String path, final int flags) throws IOException {
             // TODO Output Error to GUI about invalid url
             throw new IOException("Error loading image " + path);
         }
+        this.eventBus.post(new SourceStartedEvent(this));
     }
 }

core/src/test/java/edu/wpi/grip/core/sources/ImageFileSourceTest.java

@@ -32,7 +32,7 @@ public void setUp() throws URISyntaxException {
     public void testLoadImageToMat() throws IOException {
         // Given above setup
         // When
-        final ImageFileSource fileSource = new ImageFileSource(eventBus, this.imageFile);
+        final ImageFileSource fileSource = new ImageFileSource(eventBus, this.imageFile).start(eventBus);
         OutputSocket<Mat> outputSocket = fileSource.getOutputSockets()[0];
 
         // Then
@@ -45,7 +45,7 @@ public void testLoadImageToMat() throws IOException {
 
     @Test(expected = IOException.class)
     public void testReadInTextFile() throws IOException {
-        final ImageFileSource fileSource = new ImageFileSource(eventBus, this.textFile);
+        final ImageFileSource fileSource = new ImageFileSource(eventBus, this.textFile).start(eventBus);
         OutputSocket<Mat> outputSocket = fileSource.getOutputSockets()[0];
         assertTrue("No matrix should have been returned.", outputSocket.getValue().get().empty());
     }
@@ -54,7 +54,7 @@ public void testReadInTextFile() throws IOException {
     public void testReadInFileWithoutExtension() throws MalformedURLException, IOException {
         final File testFile = new File("temp" + File.separator +"fdkajdl3eaf");
 
-        final ImageFileSource fileSource = new ImageFileSource(eventBus, testFile);
+        final ImageFileSource fileSource = new ImageFileSource(eventBus, testFile).start(eventBus);
         OutputSocket<Mat> outputSocket = fileSource.getOutputSockets()[0];
         assertTrue("No matrix should have been returned.", outputSocket.getValue().get().empty());
     }