Merge pull request #553 from JLLeitschuh/feat/projectSettingsProvider

Makes SettingsProvider an interface

Author: JLLeitschuh

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

@@ -11,6 +11,7 @@
 import com.google.inject.spi.TypeListener;
 import edu.wpi.grip.core.events.UnexpectedThrowableEvent;
 import edu.wpi.grip.core.serialization.Project;
+import edu.wpi.grip.core.settings.SettingsProvider;
 import edu.wpi.grip.core.sources.CameraSource;
 import edu.wpi.grip.core.sources.ImageFileSource;
 import edu.wpi.grip.core.sources.MultiImageFileSource;
@@ -100,6 +101,9 @@ public <I> void hear(TypeLiteral<I> type, TypeEncounter<I> encounter) {
 
         bind(EventBus.class).toInstance(eventBus);
 
+        // Allow for just injecting the settings provider, instead of the whole pipeline
+        bind(SettingsProvider.class).to(Pipeline.class);
+
         install(new FactoryModuleBuilder().build(new TypeLiteral<Connection.Factory<Object>>() {
         }));
 

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

@@ -3,16 +3,17 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.eventbus.EventBus;
 import com.google.common.eventbus.Subscribe;
+import com.google.inject.Inject;
 import com.google.inject.Singleton;
 import com.thoughtworks.xstream.annotations.XStreamAlias;
 import com.thoughtworks.xstream.annotations.XStreamOmitField;
 import edu.wpi.grip.core.events.*;
 import edu.wpi.grip.core.settings.ProjectSettings;
+import edu.wpi.grip.core.settings.SettingsProvider;
 import edu.wpi.grip.core.sockets.InputSocket;
 import edu.wpi.grip.core.sockets.OutputSocket;
 import edu.wpi.grip.core.sockets.SocketHint;
 
-import javax.inject.Inject;
 import java.util.*;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReadWriteLock;
@@ -33,7 +34,7 @@
  */
 @Singleton
 @XStreamAlias(value = "grip:Pipeline")
-public class Pipeline implements ConnectionValidator {
+public class Pipeline implements ConnectionValidator, SettingsProvider {
 
     @Inject
     @XStreamOmitField
@@ -166,11 +167,8 @@ public Set<Connection> getConnections() {
         return Collections.unmodifiableSet(this.connections);
     }
 
-    /*
-     * @return The current per-project settings.  This object may become out of date if the settings are edited
-     * by the user, so objects requiring a preference value should also subscribe to {@link ProjectSettingsChangedEvent}
-     * to get updates.
-     */
+
+    @Override
     public ProjectSettings getProjectSettings() {
         return settings;
     }

core/src/main/java/edu/wpi/grip/core/operations/network/networktables/NTManager.java

@@ -7,10 +7,8 @@
 import edu.wpi.first.wpilibj.networktables.NetworkTable;
 import edu.wpi.first.wpilibj.networktables.NetworkTablesJNI;
 import edu.wpi.first.wpilibj.tables.ITable;
-import edu.wpi.grip.core.Pipeline;
 import edu.wpi.grip.core.PipelineRunner;
 import edu.wpi.grip.core.events.ProjectSettingsChangedEvent;
-import edu.wpi.grip.core.events.StepRemovedEvent;
 import edu.wpi.grip.core.operations.network.Manager;
 import edu.wpi.grip.core.operations.network.MapNetworkPublisher;
 import edu.wpi.grip.core.operations.network.MapNetworkPublisherFactory;
@@ -23,6 +21,7 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -33,6 +32,10 @@
  */
 @Singleton
 public class NTManager implements Manager, MapNetworkPublisherFactory {
+    /*
+     * Nasty hack that is unavoidable because of how NetworkTables works.
+     */
+    private static final AtomicInteger publisherCount = new AtomicInteger(0);
 
     /**
      * Information from:
@@ -52,7 +55,6 @@ public class NTManager implements Manager, MapNetworkPublisherFactory {
     }};
     private final Logger logger = Logger.getLogger(getClass().getName());
 
-    @Inject private Pipeline pipeline;
     @Inject private PipelineRunner pipelineRunner;
     @Inject private GRIPMode gripMode;
 
@@ -108,18 +110,6 @@ public void updateSettings(ProjectSettingsChangedEvent event) {
         }
     }
 
-    /**
-     * If there are no NTPublishAnnotatedOperation steps, we can shut down NetworkTables
-     */
-    @Subscribe
-    public void disableNetworkTables(StepRemovedEvent event) {
-        if (!pipeline.getSteps().stream().anyMatch(step -> step.getOperation() instanceof NTPublishAnnotatedOperation)) {
-            synchronized (NetworkTable.class) {
-                NetworkTable.shutdown();
-            }
-        }
-    }
-
     private static final class NTPublisher<P> extends MapNetworkPublisher<P> {
         private final ImmutableSet<String> keys;
         private Optional<String> name = Optional.empty();
@@ -170,6 +160,13 @@ public void close() {
             if(name.isPresent()) {
                 deleteOldTable(name.get());
             }
+            synchronized (NetworkTable.class) {
+                // This publisher is no longer used.
+                if (NTManager.publisherCount.addAndGet(-1) == 0) {
+                    // We are the last publisher so shut it down
+                    NetworkTable.shutdown();
+                }
+            }
         }
 
         private ITable getTable() {
@@ -188,6 +185,8 @@ private static ITable getRootTable() {
 
     @Override
     public <P>MapNetworkPublisher<P> create(Set<String> keys) {
+        // Keep track of ever publisher created.
+        publisherCount.getAndAdd(1);
         return new NTPublisher<>(keys);
     }
 }

core/src/main/java/edu/wpi/grip/core/settings/SettingsProvider.java

@@ -0,0 +1,16 @@
+package edu.wpi.grip.core.settings;
+
+import edu.wpi.grip.core.events.ProjectSettingsChangedEvent;
+
+/**
+ * Provides access to the project's settings.
+ * This should be injected to get access to the settings.
+ */
+public interface SettingsProvider {
+    /**
+     * @return The current per-project settings.  This object may become out of date if the settings are edited
+     * by the user, so objects requiring a preference value should also subscribe to {@link ProjectSettingsChangedEvent}
+     * to get updates.
+     */
+    ProjectSettings getProjectSettings();
+}

ui/src/main/java/edu/wpi/grip/ui/DeployController.java

@@ -5,11 +5,11 @@
 import com.google.common.hash.Hashing;
 import com.google.common.io.LineReader;
 import com.google.common.io.Resources;
-import edu.wpi.grip.core.Pipeline;
 import edu.wpi.grip.core.events.ProjectSettingsChangedEvent;
 import edu.wpi.grip.core.events.StopPipelineEvent;
 import edu.wpi.grip.core.serialization.Project;
 import edu.wpi.grip.core.settings.ProjectSettings;
+import edu.wpi.grip.core.settings.SettingsProvider;
 import edu.wpi.grip.ui.util.StringInMemoryFile;
 import javafx.application.Platform;
 import javafx.beans.binding.Bindings;
@@ -81,7 +81,7 @@ public class DeployController {
     @Inject
     private Project project;
     @Inject
-    private Pipeline pipeline;
+    private SettingsProvider settingsProvider;
     @Inject
     private Logger logger;
 
@@ -93,7 +93,7 @@ public void initialize() {
         command.bind(Bindings.concat(javaHome.textProperty(), "/bin/java ", jvmArgs.textProperty(), " -jar '",
                 deployDir.textProperty(), "/", GRIP_JAR, "' '", deployDir.textProperty(), "/", projectFile.textProperty(), "'"));
 
-        loadSettings(pipeline.getProjectSettings());
+        loadSettings(settingsProvider.getProjectSettings());
     }
 
     @Subscribe
@@ -115,7 +115,7 @@ private void loadSettings(ProjectSettings settings) {
     private void saveSettings() {
         // If the settings are updated in the deploy dialog, we still want to save them in the persistent project
         // settings, so they don't get reset the next time the project is opened to the settings are edited.
-        final ProjectSettings settings = pipeline.getProjectSettings();
+        final ProjectSettings settings = settingsProvider.getProjectSettings();
         settings.setDeployAddress(address.getText());
         settings.setDeployUser(user.getText());
         settings.setDeployJavaHome(javaHome.getText());

ui/src/main/java/edu/wpi/grip/ui/MainWindowController.java

@@ -9,6 +9,7 @@
 import edu.wpi.grip.core.events.ProjectSettingsChangedEvent;
 import edu.wpi.grip.core.serialization.Project;
 import edu.wpi.grip.core.settings.ProjectSettings;
+import edu.wpi.grip.core.settings.SettingsProvider;
 import edu.wpi.grip.core.util.SafeShutdown;
 import edu.wpi.grip.core.util.service.SingleActionListener;
 import edu.wpi.grip.ui.components.StartStoppableButton;
@@ -54,6 +55,8 @@ public class MainWindowController {
     @Inject
     private Pipeline pipeline;
     @Inject
+    private SettingsProvider settingsProvider;
+    @Inject
     private PipelineRunner pipelineRunner;
     @Inject
     private StartStoppableButton.Factory startStoppableButtonFactory;
@@ -196,7 +199,7 @@ public boolean saveProjectAs() throws IOException {
 
     @FXML
     public void showProjectSettingsEditor() {
-        final ProjectSettings projectSettings = pipeline.getProjectSettings().clone();
+        final ProjectSettings projectSettings = settingsProvider.getProjectSettings().clone();
 
         ProjectSettingsEditor projectSettingsEditor = new ProjectSettingsEditor(root, projectSettings);
         projectSettingsEditor.showAndWait().ifPresent(buttonType -> {