Refactor Operation Interface (#586)

* Refactor Operation Interface

- Operations now use local state variables for input and output sockets rather than regenerating them on the fly
- Moved name, description, category, icon, and aliases to a separate description object
- Socket classes are now interfaces
- Refactors Generated Operations to be in source code
- Changed socket get methods to return List<FooSocket> instead of FooSocket<?>[]

Author: JLLeitschuh

buildSrc/src/main/java/edu/wpi/gripgenerator/FileParser.java

@@ -10,11 +10,7 @@
 import com.github.javaparser.ast.type.PrimitiveType;
 import edu.wpi.gripgenerator.defaults.DefaultValueCollector;
 import edu.wpi.gripgenerator.defaults.EnumDefaultValue;
-import edu.wpi.gripgenerator.defaults.ObjectDefaultValue;
 import edu.wpi.gripgenerator.defaults.PrimitiveDefaultValue;
-import edu.wpi.gripgenerator.settings.DefinedMethod;
-import edu.wpi.gripgenerator.settings.DefinedMethodCollection;
-import edu.wpi.gripgenerator.settings.DefinedParamType;
 import edu.wpi.gripgenerator.templates.OperationList;
 
 import java.io.ByteArrayInputStream;
@@ -113,9 +109,6 @@ public Expression getDefaultValue(String defaultValue) {
             returnMap.putAll(parseOpenImgprc(compilationUnit, collector, operationList));
 
         }
-
-        // Generate the Operation List class last
-        returnMap.put(operationList.getClassName(), operationList.getDeclaration());
         return returnMap;
     }
 
@@ -126,70 +119,6 @@ public static Map<String, CompilationUnit> parseOpenImgprc(CompilationUnit imgpr
         OpenCVEnumVisitor enumVisitor = new OpenCVEnumVisitor(baseClassName, collector);
         enumVisitor.visit(imgprocDeclaration, compilationUnits);
         compilationUnits.putAll(enumVisitor.generateCompilationUnits());
-
-        DefinedMethodCollection collection = new DefinedMethodCollection(baseClassName,
-                new DefinedMethod("Sobel", false, "Mat", "Mat"
-                ).addDescription("Find edges by calculating the requested derivative order for the given image."),
-                new DefinedMethod("medianBlur", false, "Mat", "Mat"
-                ).addDescription("Apply a Median blur to an image."),
-                new DefinedMethod("GaussianBlur", false,
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("Size").setDefaultValue(new ObjectDefaultValue("Size", "1", "1"))
-                ).addDescription("Apply a Gaussian blur to an image."),
-                new DefinedMethod("Laplacian", "Mat", "Mat"
-                ).addDescription("Find edges by calculating the Laplacian for the given image."),
-                new DefinedMethod("dilate", false,
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("Mat").setDefaultValue(new ObjectDefaultValue("Mat"))
-                ).addDescription("Expands areas of higher values in an image."),
-                new DefinedMethod("Canny", false,
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("Mat", DefinedParamType.DefinedParamDirection.OUTPUT)
-                ).addDescription("Apply a \\\"canny edge detection\\\" algorithm to an image."),
-                new DefinedMethod("threshold", false,
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("double"),
-                        new DefinedParamType("double"),
-                        new DefinedParamType("int").setLiteralDefaultValue("THRESH_BINARY")
-                ).addDescription("Apply a fixed-level threshold to each array element in an image."),
-                new DefinedMethod("adaptiveThreshold", false,
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("double"),
-                        new DefinedParamType("int").setLiteralDefaultValue("ADAPTIVE_THRESH_MEAN_C"),
-                        new DefinedParamType("int").setLiteralDefaultValue("THRESH_BINARY")
-                ).addDescription("Transforms a grayscale image to a binary image)."),
-                new DefinedMethod("erode", false,
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("Mat").setDefaultValue(new ObjectDefaultValue("Mat"))
-                ).addDescription("Expands areas of lower values in an image."),
-                new DefinedMethod("cvtColor", false,
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("int").setLiteralDefaultValue("COLOR_BGR2BGRA")
-                ).addDescription("Convert an image from one color space to another."),
-                new DefinedMethod("applyColorMap", true,
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("int").setLiteralDefaultValue("COLORMAP_AUTUMN")
-                ).addDescription("Apply a MATLAB equivalent colormap to an image."),
-                new DefinedMethod("resize", false,
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("Size").setDefaultValue(new ObjectDefaultValue("Size"))
-                ).addDescription("Resize the image to the specified size."),
-                new DefinedMethod("rectangle", false,
-                        new DefinedParamType("Mat", DefinedParamType.DefinedParamDirection.INPUT_AND_OUTPUT),
-                        new DefinedParamType("Point")
-                ).addDescription("Draw a rectangle (outline or filled) on an image.")
-        ).setDirectionDefaults(DefinedParamType.DefinedParamDirection.OUTPUT, "dst")
-                .setIgnoreDefaults("dtype", "ddepth");
-        new OpenCVMethodVisitor(collection).visit(imgprocDeclaration, compilationUnits);
-        collection.generateCompilationUnits(collector, compilationUnits, operations);
         return compilationUnits;
     }
 
@@ -201,62 +130,6 @@ public static Map<String, CompilationUnit> parseOpenCVCore(CompilationUnit coreD
         enumVisitor.visit(coreDeclaration, compilationUnits);
         compilationUnits.putAll(enumVisitor.generateCompilationUnits());
 
-        DefinedMethodCollection collection = new DefinedMethodCollection(baseClassName,
-                new DefinedMethod("add", true, "Mat", "Mat", "Mat")
-                        .addDescription("Calculate the per-pixel sum of two images."),
-                new DefinedMethod("subtract", true, "Mat", "Mat", "Mat")
-                        .addDescription("Calculate the per-pixel difference between two images."),
-                new DefinedMethod("multiply", false, "Mat", "Mat", "Mat")
-                        .addDescription("Calculate the per-pixel scaled product of two images."),
-                new DefinedMethod("divide", false, "Mat", "Mat", "Mat")
-                        .addDescription("Perform per-pixel division of two images."),
-                new DefinedMethod("scaleAdd", false, "Mat", "double", "Mat", "Mat")
-                        .addDescription("Calculate the sum of two images where one image is multiplied by a scalar."),
-//                new DefinedMethod("normalize", false, "Mat", "Mat"),
-                new DefinedMethod("addWeighted", false, "Mat")
-                        .addDescription("Calculate the weighted sum of two images."),
-                new DefinedMethod("flip", false,
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("int").setLiteralDefaultValue("Y_AXIS"))
-                        .addDescription("Flip image around vertical, horizontal, or both axes."),
-                new DefinedMethod("bitwise_and", true, "Mat", "Mat", "Mat")
-                        .addDescription("Calculate the per-element bitwise conjunction of two images."),
-                new DefinedMethod("bitwise_or", true, "Mat", "Mat", "Mat")
-                        .addDescription("Calculate the per-element bit-wise disjunction of two images."),
-                new DefinedMethod("bitwise_xor", true, "Mat", "Mat", "Mat")
-                        .addDescription("Calculate the per-element bit-wise \\\"exclusive or\\\" on two images."),
-                new DefinedMethod("bitwise_not", true, "Mat", "Mat")
-                        .addDescription("Calculate per-element bit-wise inversion of an image."),
-                new DefinedMethod("absdiff", false, "Mat", "Mat")
-                        .addDescription("Calculate the per-element absolute difference of two images."),
-                new DefinedMethod("compare", true,
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("Mat"),
-                        new DefinedParamType("int").setLiteralDefaultValue("CMP_EQ")
-                ).addDescription("Compare each pixel in two images using a given rule."),
-                new DefinedMethod("max", false, "Mat", "Mat")
-                        .addDescription("Calculate per-element maximum of two images."),
-                new DefinedMethod("min", false, "Mat", "Mat")
-                        .addDescription("Calculate the per-element minimum of two images."),
-                new DefinedMethod("extractChannel", false, "Mat", "Mat")
-                        .addDescription("Extract a single channel from a image."),
-                new DefinedMethod("transpose", false, "Mat", "Mat")
-                        .addDescription("Calculate the transpose of an image.")
-//                new DefinedMethod("sqrt", false, "Mat", "Mat"),
-//                new DefinedMethod("pow", false,
-//                        new DefinedParamType("Mat"),
-//                        new DefinedParamType("double")
-//                                .setDefaultValue(new PrimitiveDefaultValue(new PrimitiveType(PrimitiveType.Primitive.Double), "1"))
-//                )
-        ).setDirectionDefaults(DefinedParamType.DefinedParamDirection.OUTPUT, "dst")
-                .setIgnoreDefaults("dtype", "ddepth");
-        new OpenCVMethodVisitor(collection).visit(coreDeclaration, compilationUnits);
-
-        collection.generateCompilationUnits(collector, compilationUnits, operations);
-
-
         return compilationUnits;
     }
 

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

@@ -58,7 +58,7 @@ public void onConnectionAdded(ConnectionAddedEvent event) {
 
     @Subscribe
     public void onOutputChanged(SocketChangedEvent e) {
-        if (e.getSocket() == outputSocket) {
+        if (e.isRegarding(outputSocket)) {
             inputSocket.setValueOptional(outputSocket.getValue());
         }
     }
@@ -76,14 +76,14 @@ public void removeConnection(StepRemovedEvent e) {
         // Remove this connection if one of the steps it was connected to was removed
         for (OutputSocket socket : e.getStep().getOutputSockets()) {
             if (socket == this.outputSocket) {
-                this.eventBus.post(new ConnectionRemovedEvent(this));
+                remove();
                 return;
             }
         }
 
         for (InputSocket socket : e.getStep().getInputSockets()) {
             if (socket == this.inputSocket) {
-                this.eventBus.post(new ConnectionRemovedEvent(this));
+                remove();
                 return;
             }
         }
@@ -94,9 +94,16 @@ public void removeConnection(SourceRemovedEvent e) {
         // Remove this connection if it's from a source that was removed
         for (OutputSocket socket : e.getSource().getOutputSockets()) {
             if (socket == this.outputSocket) {
-                this.eventBus.post(new ConnectionRemovedEvent(this));
+                remove();
                 return;
             }
         }
     }
+
+    /**
+     * Removes this connection from the pipeline.
+     */
+    public void remove() {
+        eventBus.post(new ConnectionRemovedEvent(this));
+    }
 }

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

@@ -12,6 +12,10 @@
 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.sockets.InputSocket;
+import edu.wpi.grip.core.sockets.InputSocketImpl;
+import edu.wpi.grip.core.sockets.OutputSocket;
+import edu.wpi.grip.core.sockets.OutputSocketImpl;
 import edu.wpi.grip.core.sources.CameraSource;
 import edu.wpi.grip.core.sources.ImageFileSource;
 import edu.wpi.grip.core.sources.MultiImageFileSource;
@@ -109,6 +113,9 @@ public <I> void hear(TypeLiteral<I> type, TypeEncounter<I> encounter) {
 
         bind(ConnectionValidator.class).to(Pipeline.class);
         bind(Source.SourceFactory.class).to(Source.SourceFactoryImpl.class);
+
+        bind(InputSocket.Factory.class).to(InputSocketImpl.FactoryImpl.class);
+        bind(OutputSocket.Factory.class).to(OutputSocketImpl.FactoryImpl.class);
         install(new FactoryModuleBuilder()
                 .implement(CameraSource.class, CameraSource.class)
                 .build(CameraSource.Factory.class));

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

@@ -6,11 +6,11 @@
 import com.google.inject.Injector;
 import edu.wpi.grip.core.events.ExceptionClearedEvent;
 import edu.wpi.grip.core.events.ExceptionEvent;
+import edu.wpi.grip.core.operations.CVOperations;
 import edu.wpi.grip.core.operations.Operations;
 import edu.wpi.grip.core.operations.network.GRIPNetworkModule;
 import edu.wpi.grip.core.serialization.Project;
 import edu.wpi.grip.core.sources.GRIPSourcesHardwareModule;
-import edu.wpi.grip.generated.CVOperations;
 
 import javax.inject.Inject;
 import java.io.File;
@@ -27,6 +27,7 @@ public class Main {
     @Inject private PipelineRunner pipelineRunner;
     @Inject private EventBus eventBus;
     @Inject private Operations operations;
+    @Inject private CVOperations cvOperations;
     @Inject private Logger logger;
 
     @SuppressWarnings("PMD.SystemPrintln")
@@ -45,7 +46,7 @@ public void start(String[] args) throws IOException, InterruptedException {
         }
 
         operations.addOperations();
-        CVOperations.addOperations(eventBus);
+        cvOperations.addOperations();
 
         final String projectPath = args[0];
 

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

@@ -1,112 +1,40 @@
 package edu.wpi.grip.core;
 
-import com.google.common.collect.ImmutableSet;
-import com.google.common.eventbus.EventBus;
 import edu.wpi.grip.core.sockets.InputSocket;
 import edu.wpi.grip.core.sockets.OutputSocket;
-import edu.wpi.grip.core.sockets.SocketsProvider;
 
-import java.io.InputStream;
-import java.util.Optional;
+import java.util.List;
 
 /**
- * The common interface used by <code>Step</code>s in a pipeline to call various operations.  There is usually only one
- * instance of any class that implements <code>Operation</code>, which is called whenever that operation is used.
+ * The common interface used by <code>Step</code>s in a pipeline to call various operations. Each instance of an
+ * operation in the pipeline is handled by a unique instance of that {@code Operation} class.
  */
 public interface Operation {
 
-    enum Category {
-        IMAGE_PROCESSING,
-        FEATURE_DETECTION,
-        NETWORK,
-        LOGICAL,
-        OPENCV,
-        MISCELLANEOUS,
-    }
-
-    /**
-     * @return The unique user-facing name of the operation, such as "Gaussian Blur"
-     */
-    String getName();
-
-    /**
-     * @return Any old unique user-facing names of the operation. This is used to preserve compatibility with
-     * old versions of GRIP if the operation name changes.
-     */
-    default ImmutableSet<String> getAliases() {
-        return ImmutableSet.of();
-    }
-
-
-    /**
-     * @return A description of the operation.
-     */
-    String getDescription();
-
-    /**
-     * @return What category the operation falls under.  This is used to organize them in the GUI
-     */
-    default Category getCategory() {
-        return Category.MISCELLANEOUS;
-    }
-
     /**
-     * @return An {@link InputStream} of a 128x128 image to show the user as a representation of the operation.
-     */
-    default Optional<InputStream> getIcon() {
-        return Optional.empty();
-    }
-
-    default SocketsProvider createSockets(EventBus eventBus) {
-        return new SocketsProvider(createInputSockets(eventBus), createOutputSockets(eventBus));
-    }
-    /**
-     * @param eventBus The Guava {@link EventBus} used by the application.
-     * @return An array of sockets for the inputs that the operation expects.
-     */
-    InputSocket<?>[] createInputSockets(EventBus eventBus);
-
-    /**
-     * @param eventBus The Guava {@link EventBus} used by the application.
-     * @return An array of sockets for the outputs that the operation produces.
+     * @return A list of sockets for the inputs that the operation expects.
+     *
+     * @implNote The returned list should be immutable (i.e. read-only)
      */
-    OutputSocket<?>[] createOutputSockets(EventBus eventBus);
+    List<InputSocket> getInputSockets();
 
     /**
-     * Override this to provide persistent per-step data
+     * @return A list of sockets for the outputs that the operation produces.
+     *
+     * @implNote The returned list should be immutable (i.e. read-only)
      */
-    default Optional<?> createData() {
-        return Optional.empty();
-    }
+    List<OutputSocket> getOutputSockets();
 
     /**
-     * Perform the operation on the specified inputs, storing the results in the specified outputs.
-     *
-     * @param inputs  An array obtained from {@link #createInputSockets(EventBus)}. The caller can set the value of
-     *                each socket to an actual parameter for the operation.
-     * @param outputs An array obtained from {@link #createOutputSockets(EventBus)}. The outputs of the operation will
-     *                be stored in these sockets.
-     * @param data    Optional data to be passed to the operation
+     * Performs this {@code Operation}.
      */
-    default void perform(InputSocket<?>[] inputs, OutputSocket<?>[] outputs, Optional<?> data) {
-        perform(inputs, outputs);
-    }
-
-    default void perform(InputSocket<?>[] inputs, OutputSocket<?>[] outputs) {
-        throw new UnsupportedOperationException("Perform was not overridden");
-    }
+    void perform();
 
     /**
      * Allows the step to clean itself up when removed from the pipeline.
      * This should only be called by {@link Step#setRemoved()} to ensure correct synchronization.
-     *
-     * @param inputs  An array obtained from {@link #createInputSockets(EventBus)}. The caller can set the value of
-     *                each socket to an actual parameter for the operation.
-     * @param outputs An array obtained from {@link #createOutputSockets(EventBus)}. The outputs of the operation will
-     *                be stored in these sockets.
-     * @param data    Optional data to be passed to the operation
      */
-    default void cleanUp(InputSocket<?>[] inputs, OutputSocket<?>[] outputs, Optional<?> data) {
+    default void cleanUp() {
         /* no-op */
     }
 }