Updates Sources to Resolve PR Comments

Author: JLLeitschuh

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

@@ -47,31 +47,39 @@ public final OutputSocket[] getOutputSockets() {
      */
     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;
-    }
-
     /**
      * Starts this source.
      * A source whose {@link #canStopAndStart()} returns true can also be stopped and started and stopped multiple times.
      *
      * @return The source object that created the camera
      * @throws IOException If the source fails to be started
      */
-    protected abstract Source start() throws IOException;
+    public <T extends Source> T start(EventBus eventBus) throws IOException {
+        start();
+        eventBus.register(this);
+        return (T) this;
+    }
+
+    /**
+     * Any method that overrides this method should post a {@link edu.wpi.grip.core.events.SourceStartedEvent}
+     * to the {@link EventBus} if is successfully starts.
+     * @throws IOException If the source fails to be started
+     */
+    protected abstract void start() throws IOException;
 
     /**
      * Stops this source.
      * This will stop the source publishing new socket values after this method returns.
      * A source whose {@link #canStopAndStart()} returns true can also be stopped and started and stopped multiple times.
      *
+     * Any method that overrides this method should post a {@link edu.wpi.grip.core.events.SourceStoppedEvent}
+     * to the {@link EventBus} if is successfully stops.
+     *
      * @return The source that was stopped
-     * @throws TimeoutException if the thread running the source fails to stop
+     * @throws TimeoutException if the thread running the source fails to stop.
+     * @throws IOException If there is a problem stopping the Source
      */
-    public abstract Source stop() throws TimeoutException;
+    public void stop() throws TimeoutException, IOException { /* no op */ }
 
     /**
      * Used to indicate if the source is running or stopped

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

@@ -128,9 +128,9 @@ public void createFromProperties(EventBus eventBus, Properties properties) throw
     /**
      * Starts the video capture from the source device
      */
-    public CameraSource start() throws IOException, IllegalStateException {
+    protected void start() throws IOException, IllegalStateException {
         final OpenCVFrameConverter.ToMat convertToMat = new OpenCVFrameConverter.ToMat();
-        synchronized (this.frameThread) {
+        synchronized (this) {
             if (this.frameThread.isPresent()) {
                 throw new IllegalStateException("The video retrieval thread has already been started.");
             }
@@ -179,7 +179,6 @@ public CameraSource start() throws IOException, IllegalStateException {
             this.frameThread = Optional.of(frameExecutor);
         }
         eventBus.post(new SourceStartedEvent(this));
-        return this;
     }
 
     /**
@@ -188,8 +187,8 @@ public CameraSource start() throws IOException, IllegalStateException {
      * @throws TimeoutException      If the thread running the Webcam fails to join this one after a timeout.
      * @throws IllegalStateException If the camera was already stopped
      */
-    public CameraSource stop() throws TimeoutException, IllegalStateException {
-        synchronized (this.frameThread) {
+    public void stop() throws TimeoutException, IllegalStateException {
+        synchronized (this) {
             if (frameThread.isPresent()) {
                 final Thread ex = frameThread.get();
                 ex.interrupt();
@@ -219,12 +218,11 @@ public CameraSource stop() throws TimeoutException, IllegalStateException {
         }
         eventBus.post(new SourceStoppedEvent(this));
         frameRateOutputSocket.setValue(0);
-        return this;
     }
 
     @Override
     public boolean isRunning() {
-        synchronized (this.frameThread) {
+        synchronized (this) {
             return this.frameThread.isPresent() && this.frameThread.get().isAlive();
         }
     }

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

@@ -84,15 +84,9 @@ public void createFromProperties(EventBus eventBus, Properties properties) throw
         this.initialize(eventBus, path);
     }
 
-    public ImageFileSource start() throws IOException {
+    protected void start() throws IOException {
         this.started = true;
         loadImage(this.path);
-        return this;
-    }
-
-    @Override
-    public ImageFileSource stop() {
-        return this;
     }
 
     @Override

ui/src/main/java/edu/wpi/grip/ui/pipeline/SourceView.java

@@ -25,6 +25,7 @@
 import javafx.scene.layout.VBox;
 
 import java.io.IOException;
+import java.util.concurrent.TimeoutException;
 
 /**
  * A JavaFX control that represents a {@link Source}.  <code>SourceView</code>s are somewhat analogous to
@@ -76,12 +77,14 @@ public SourceView(EventBus eventBus, Source source) {
                 event.consume();
                 if (!startStopButton.isSelected()) try {
                     source.start(eventBus);
+                    // If this fails then an SourceStartedEvent will not be posted
                 } catch (IOException e) {
                     eventBus.post(new FatalErrorEvent(e));
                 }
                 else try {
                     source.stop();
-                } catch (Exception e) {
+                    // If this fails then an SourceStoppedEvent will not be posted
+                } catch (TimeoutException | IOException e) {
                     eventBus.post(new FatalErrorEvent(e));
                 }
             });