From 1ab5b66829534c31e9cdc0782ae94aab68183d88 Mon Sep 17 00:00:00 2001
From: Mohammad Durrani <46766905+mdurrani808@users.noreply.github.com>
Date: Sat, 31 Dec 2022 18:29:36 -0500
Subject: [PATCH] Clean up front end, remove decision margin and error bits,
 remove target family selector  (#652)

* clean up front end ui

* address changes

* Further tweaks to camera default gains to help make sure users get a good first impression

* even more saner defaults

* Even even more camera sane defaults

* lint

* lint pt 2

* unit test fixup

Co-authored-by: Chris <chrisgerth010592@gmail.com>
---
 .../src/views/PipelineViews/AprilTagTab.vue   | 316 ++++++++----------
 .../vision/pipeline/AprilTagPipeline.java     |  23 +-
 .../pipeline/AprilTagPipelineSettings.java    |  14 +-
 .../vision/pipeline/CVPipelineSettings.java   |   8 +-
 .../Calibration3dPipelineSettings.java        |   2 +-
 .../ColoredShapePipelineSettings.java         |   1 +
 .../pipeline/DriverModePipelineSettings.java  |   1 +
 .../pipeline/ReflectivePipelineSettings.java  |   2 +
 .../vision/processes/VisionModule.java        |  12 +-
 .../vision/pipeline/AprilTagTest.java         |   2 +
 10 files changed, 168 insertions(+), 213 deletions(-)

diff --git a/photon-client/src/views/PipelineViews/AprilTagTab.vue b/photon-client/src/views/PipelineViews/AprilTagTab.vue
index aa842868b..2fd47b736 100644
--- a/photon-client/src/views/PipelineViews/AprilTagTab.vue
+++ b/photon-client/src/views/PipelineViews/AprilTagTab.vue
@@ -1,196 +1,166 @@
 <template>
   <div>
     <v-select
-      v-model="selectedFamily"
-      dark
-      color="accent"
-      item-color="secondary"
-      label="Select target family"
-      :items="familyList"
-      @input="handlePipelineUpdate('tagFamily', familyList.indexOf(selectedFamily))"
-    />
-    <v-select
-      v-model="selectedModel"
-      dark
-      color="accent"
-      item-color="secondary"
-      label="Select a target model"
-      :items="targetList"
-      item-text="name"
-      item-value="data"
-      @input="handlePipelineUpdate('targetModel', targetList.indexOf(selectedModel) + 6)"
+        v-model="selectedFamily"
+        dark
+        color="accent"
+        item-color="secondary"
+        label="Select target family"
+        :items="familyList"
+        @input="handlePipelineUpdate('tagFamily', familyList.indexOf(selectedFamily))"
     />
     <CVslider
-      v-model="decimate"
-      class="pt-2"
-      slider-cols="8"
-      name="Decimate"
-      min="1"
-      max="8"
-      step="1.0"
-      tooltip="Increases FPS at the expense of range by reducing image resolution initially"
-      @input="handlePipelineData('decimate')"
+        v-model="decimate"
+        class="pt-2"
+        slider-cols="8"
+        name="Decimate"
+        min="1"
+        max="8"
+        step="1.0"
+        tooltip="Increases FPS at the expense of range by reducing image resolution initially"
+        @input="handlePipelineData('decimate')"
     />
     <CVslider
-      v-model="blur"
-      class="pt-2"
-      slider-cols="8"
-      name="Blur"
-      min="0"
-      max="5"
-      step=".01"
-      tooltip="Gaussian blur added to the image, high FPS cost for slightly decreased noise"
-      @input="handlePipelineData('blur')"
+        v-model="blur"
+        class="pt-2"
+        slider-cols="8"
+        name="Blur"
+        min="0"
+        max="5"
+        step=".01"
+        tooltip="Gaussian blur added to the image, high FPS cost for slightly decreased noise"
+        @input="handlePipelineData('blur')"
     />
     <CVslider
-      v-model="threads"
-      class="pt-2"
-      slider-cols="8"
-      name="Threads"
-      min="1"
-      max="8"
-      step="1"
-      tooltip="Number of threads spawned by the AprilTag detector"
-      @input="handlePipelineData('threads')"
+        v-model="threads"
+        class="pt-2"
+        slider-cols="8"
+        name="Threads"
+        min="1"
+        max="8"
+        step="1"
+        tooltip="Number of threads spawned by the AprilTag detector"
+        @input="handlePipelineData('threads')"
     />
     <CVswitch
-      v-model="refineEdges"
-      class="pt-2"
-      slider-cols="8"
-      name="Refine Edges"
-      tooltip="Further refines the apriltag corner position initial estimate, suggested left on"
-      @input="handlePipelineData('refineEdges')"
+        v-model="refineEdges"
+        class="pt-2"
+        slider-cols="8"
+        name="Refine Edges"
+        tooltip="Further refines the apriltag corner position initial estimate, suggested left on"
+        @input="handlePipelineData('refineEdges')"
     />
     <CVslider
-      v-model="hammingDist"
-      class="pt-2 pb-4"
-      slider-cols="8"
-      name="Max error bits"
-      min="0"
-      max="10"
-      step="1"
-      tooltip="Maximum number of error bits to correct; potential tags with more will be thrown out. For smaller tags (like 16h5), set this as low as possible."
-      @input="handlePipelineData('hammingDist')"
+        v-model="decisionMargin"
+        class="pt-2 pb-4"
+        slider-cols="8"
+        name="Decision Margin Cutoff"
+        min="0"
+        max="250"
+        step="1"
+        tooltip="Tags with a 'margin' (decoding quality score) less than this wil be rejected. Increase this to reduce the number of false positive detections"
+        @input="handlePipelineData('decisionMargin')"
     />
     <CVslider
-      v-model="decisionMargin"
-      class="pt-2 pb-4"
-      slider-cols="8"
-      name="Decision Margin Cutoff"
-      min="0"
-      max="250"
-      step="1"
-      tooltip="Tags with a 'margin' (decoding quality score) less than this wil be rejected. Increase this to reduce the number of false positive detections"
-      @input="handlePipelineData('decisionMargin')"
-    />
-    <CVslider
-      v-model="numIterations"
-      class="pt-2 pb-4"
-      slider-cols="8"
-      name="Pose Estimation Iterations"
-      min="0"
-      max="500"
-      step="1"
-      tooltip="Number of iterations the pose estimation algorithm will run, 50-100 is a good starting point"
-      @input="handlePipelineData('numIterations')"
+        v-model="numIterations"
+        class="pt-2 pb-4"
+        slider-cols="8"
+        name="Pose Estimation Iterations"
+        min="0"
+        max="500"
+        step="1"
+        tooltip="Number of iterations the pose estimation algorithm will run, 50-100 is a good starting point"
+        @input="handlePipelineData('numIterations')"
     />
   </div>
 </template>
 
 <script>
-    import CVslider from '../../components/common/cv-slider'
-    import CVswitch from '../../components/common/cv-switch'
+import CVslider from '../../components/common/cv-slider'
+import CVswitch from '../../components/common/cv-switch'
 
-    export default {
-        name: "AprilTag",
-        components: {
-          CVslider,
-          CVswitch,
-        },
-        data() {
-            return {
-              familyList: ["tag36h11", "tag25h9", "tag16h5"],
-              // Selected model is offset (ew) by 6 from the photon ordinal, as we only wanna show the 36h11 and 16h5 options
-              targetList: ['6.5in (36h11) AprilTag', '6in (16h5) AprilTag'], //Keep in sync with TargetModel.java
-            }
-        },
-        computed: {
-          selectedModel: {
-              get() {
-                  let ret = this.$store.getters.currentPipelineSettings.targetModel - 6
-                  return this.targetList[ret];
-              },
-              set(val) {
-                  this.$store.commit("mutatePipeline", {"targetModel": this.targetList.indexOf(val) + 6})
-              }
-          },
-          selectedFamily: {
-            get() {
-                let ret = this.$store.getters.currentPipelineSettings.tagFamily
-                return this.familyList[ret];
-            },
-            set(val) {
-                this.$store.commit("mutatePipeline", {"tagFamily": this.familyList.indexOf(val)})
-            }
-          },
-          decimate: {
-            get() {
-                return this.$store.getters.currentPipelineSettings.decimate
-            },
-            set(val) {
-                this.$store.commit("mutatePipeline", {"decimate": val});
-            }
-          },
-          hammingDist: {
-            get() {
-                return this.$store.getters.currentPipelineSettings.hammingDist
-            },
-            set(val) {
-                this.$store.commit("mutatePipeline", {"hammingDist": val});
-            }
-          },
-          decisionMargin: {
-            get() {
-                return this.$store.getters.currentPipelineSettings.decisionMargin
-            },
-            set(val) {
-                this.$store.commit("mutatePipeline", {"decisionMargin": val});
-            }
-          },
-          numIterations: {
-            get() {
-                return this.$store.getters.currentPipelineSettings.numIterations
-            },
-            set(val) {
-                this.$store.commit("mutatePipeline", {"numIterations": val});
-            }
-          },
-          blur: {
-            get() {
-                return this.$store.getters.currentPipelineSettings.blur
-            },
-            set(val) {
-                this.$store.commit("mutatePipeline", {"blur": val});
-            }
-          },
-          threads: {
-            get() {
-                return this.$store.getters.currentPipelineSettings.threads
-            },
-            set(val) {
-                this.$store.commit("mutatePipeline", {"threads": val});
-            }
-          },
-          refineEdges: {
-            get() {
-                return this.$store.getters.currentPipelineSettings.refineEdges
-            },
-            set(val) {
-                this.$store.commit("mutatePipeline", {"refineEdges": val});
-            }
-          },
-        },
-        methods: {
-        }
+export default {
+  name: "AprilTag",
+  components: {
+    CVslider,
+    CVswitch,
+  },
+  data() {
+    return {
+      familyList: ["tag36h11", "tag25h9", "tag16h5"],
+      // Selected model is offset (ew) by 6 from the photon ordinal, as we only wanna show the 36h11 and 16h5 options
+      targetList: ['6in (16h5) AprilTag'], //Keep in sync with TargetModel.java
     }
+  },
+  computed: {
+    selectedModel: {
+      get() {
+        let ret = this.$store.getters.currentPipelineSettings.targetModel - 6
+        return this.targetList[ret];
+      },
+      set(val) {
+        this.$store.commit("mutatePipeline", {"targetModel": this.targetList.indexOf(val) + 6})
+      }
+    },
+    selectedFamily: {
+      get() {
+        let ret = this.$store.getters.currentPipelineSettings.tagFamily
+        return this.familyList[ret];
+      },
+      set(val) {
+        this.$store.commit("mutatePipeline", {"tagFamily": this.familyList.indexOf(val)})
+      }
+    },
+    decimate: {
+      get() {
+        return this.$store.getters.currentPipelineSettings.decimate
+      },
+      set(val) {
+        this.$store.commit("mutatePipeline", {"decimate": val});
+      }
+    },
+    decisionMargin: {
+      get() {
+        return this.$store.getters.currentPipelineSettings.decisionMargin
+      },
+      set(val) {
+        this.$store.commit("mutatePipeline", {"decisionMargin": val});
+      }
+    },
+    numIterations: {
+      get() {
+        return this.$store.getters.currentPipelineSettings.numIterations
+      },
+      set(val) {
+        this.$store.commit("mutatePipeline", {"numIterations": val});
+      }
+    },
+    blur: {
+      get() {
+        return this.$store.getters.currentPipelineSettings.blur
+      },
+      set(val) {
+        this.$store.commit("mutatePipeline", {"blur": val});
+      }
+    },
+    threads: {
+      get() {
+        return this.$store.getters.currentPipelineSettings.threads
+      },
+      set(val) {
+        this.$store.commit("mutatePipeline", {"threads": val});
+      }
+    },
+    refineEdges: {
+      get() {
+        return this.$store.getters.currentPipelineSettings.refineEdges
+      },
+      set(val) {
+        this.$store.commit("mutatePipeline", {"refineEdges": val});
+      }
+    },
+  },
+  methods: {
+  }
+}
 </script>
diff --git a/photon-core/src/main/java/org/photonvision/vision/pipeline/AprilTagPipeline.java b/photon-core/src/main/java/org/photonvision/vision/pipeline/AprilTagPipeline.java
index 165bd304f..0ed76354d 100644
--- a/photon-core/src/main/java/org/photonvision/vision/pipeline/AprilTagPipeline.java
+++ b/photon-core/src/main/java/org/photonvision/vision/pipeline/AprilTagPipeline.java
@@ -69,28 +69,7 @@ public class AprilTagPipeline extends CVPipeline<CVPipelineResult, AprilTagPipel
         // target model for the draw 3d targets pipeline to work...
 
         // for now, hard code tag width based on enum value
-        double tagWidth;
-
-        // This needs
-        switch (settings.targetModel) {
-            case k200mmAprilTag:
-                {
-                    tagWidth = Units.inchesToMeters(3.25 * 2);
-                    break;
-                }
-            case k6in_16h5:
-                {
-                    tagWidth = Units.inchesToMeters(3 * 2);
-                    break;
-                }
-            default:
-                {
-                    // guess at 200mm?? If it's zero everything breaks, but it should _never_ be zero. Unless
-                    // users select the wrong model...
-                    tagWidth = 0.16;
-                    break;
-                }
-        }
+        double tagWidth = Units.inchesToMeters(3 * 2); // for 6in 16h5 tag.
 
         // AprilTagDetectorParams aprilTagDetectionParams =
         //         new AprilTagDetectorParams(
diff --git a/photon-core/src/main/java/org/photonvision/vision/pipeline/AprilTagPipelineSettings.java b/photon-core/src/main/java/org/photonvision/vision/pipeline/AprilTagPipelineSettings.java
index 072faf074..f476b640f 100644
--- a/photon-core/src/main/java/org/photonvision/vision/pipeline/AprilTagPipelineSettings.java
+++ b/photon-core/src/main/java/org/photonvision/vision/pipeline/AprilTagPipelineSettings.java
@@ -24,17 +24,15 @@ import org.photonvision.vision.target.TargetModel;
 
 @JsonTypeName("AprilTagPipelineSettings")
 public class AprilTagPipelineSettings extends AdvancedPipelineSettings {
-    public AprilTagFamily tagFamily = AprilTagFamily.kTag36h11;
+    public AprilTagFamily tagFamily = AprilTagFamily.kTag16h5;
     public int decimate = 1;
     public double blur = 0;
-    public int threads = 1;
+    public int threads = 4; // Multiple threads seems to be better performance on most platforms
     public boolean debug = false;
     public boolean refineEdges = true;
     public int numIterations = 200;
-
-    // TODO is this a legit, reasonable default?
-    public int hammingDist = 1;
-    public int decisionMargin = 30;
+    public int hammingDist = 0;
+    public int decisionMargin = 35;
 
     // 3d settings
 
@@ -42,7 +40,9 @@ public class AprilTagPipelineSettings extends AdvancedPipelineSettings {
         super();
         pipelineType = PipelineType.AprilTag;
         outputShowMultipleTargets = true;
-        targetModel = TargetModel.k200mmAprilTag;
+        targetModel = TargetModel.k6in_16h5;
+        cameraExposure = 20;
+        cameraAutoExposure = false;
         ledMode = false;
     }
 
diff --git a/photon-core/src/main/java/org/photonvision/vision/pipeline/CVPipelineSettings.java b/photon-core/src/main/java/org/photonvision/vision/pipeline/CVPipelineSettings.java
index ac6838d5f..f8653a03a 100644
--- a/photon-core/src/main/java/org/photonvision/vision/pipeline/CVPipelineSettings.java
+++ b/photon-core/src/main/java/org/photonvision/vision/pipeline/CVPipelineSettings.java
@@ -40,14 +40,14 @@ public class CVPipelineSettings implements Cloneable {
     public String pipelineNickname = "New Pipeline";
     public boolean cameraAutoExposure = false;
     // manual exposure only used if cameraAutoExposure if false
-    public double cameraExposure = 100;
+    public double cameraExposure = 20;
     public int cameraBrightness = 50;
     // Currently only used by a few cameras (notably the zero-copy Pi Camera driver) with the Gain
     // quirk
-    public int cameraGain = 50;
+    public int cameraGain = 75;
     // Currently only used by the zero-copy Pi Camera driver
-    public int cameraRedGain = 18;
-    public int cameraBlueGain = 24;
+    public int cameraRedGain = 11;
+    public int cameraBlueGain = 20;
     public int cameraVideoModeIndex = 0;
     public FrameDivisor streamingFrameDivisor = FrameDivisor.NONE;
     public boolean ledMode = false;
diff --git a/photon-core/src/main/java/org/photonvision/vision/pipeline/Calibration3dPipelineSettings.java b/photon-core/src/main/java/org/photonvision/vision/pipeline/Calibration3dPipelineSettings.java
index 529aa0c6d..196c9e7d6 100644
--- a/photon-core/src/main/java/org/photonvision/vision/pipeline/Calibration3dPipelineSettings.java
+++ b/photon-core/src/main/java/org/photonvision/vision/pipeline/Calibration3dPipelineSettings.java
@@ -30,7 +30,7 @@ public class Calibration3dPipelineSettings extends AdvancedPipelineSettings {
 
     public Calibration3dPipelineSettings() {
         super();
-
+        this.cameraAutoExposure = true;
         this.inputShouldShow = true;
         this.outputShouldShow = true;
     }
diff --git a/photon-core/src/main/java/org/photonvision/vision/pipeline/ColoredShapePipelineSettings.java b/photon-core/src/main/java/org/photonvision/vision/pipeline/ColoredShapePipelineSettings.java
index e615b3980..7f00089ed 100644
--- a/photon-core/src/main/java/org/photonvision/vision/pipeline/ColoredShapePipelineSettings.java
+++ b/photon-core/src/main/java/org/photonvision/vision/pipeline/ColoredShapePipelineSettings.java
@@ -45,6 +45,7 @@ public class ColoredShapePipelineSettings extends AdvancedPipelineSettings {
     public ColoredShapePipelineSettings() {
         super();
         pipelineType = PipelineType.ColoredShape;
+        cameraExposure = 20;
     }
 
     @Override
diff --git a/photon-core/src/main/java/org/photonvision/vision/pipeline/DriverModePipelineSettings.java b/photon-core/src/main/java/org/photonvision/vision/pipeline/DriverModePipelineSettings.java
index 25911458e..70beebf7a 100644
--- a/photon-core/src/main/java/org/photonvision/vision/pipeline/DriverModePipelineSettings.java
+++ b/photon-core/src/main/java/org/photonvision/vision/pipeline/DriverModePipelineSettings.java
@@ -31,5 +31,6 @@ public class DriverModePipelineSettings extends CVPipelineSettings {
         pipelineIndex = PipelineManager.DRIVERMODE_INDEX;
         pipelineType = PipelineType.DriverMode;
         inputShouldShow = true;
+        cameraAutoExposure = true;
     }
 }
diff --git a/photon-core/src/main/java/org/photonvision/vision/pipeline/ReflectivePipelineSettings.java b/photon-core/src/main/java/org/photonvision/vision/pipeline/ReflectivePipelineSettings.java
index 7376f3962..b06dcadde 100644
--- a/photon-core/src/main/java/org/photonvision/vision/pipeline/ReflectivePipelineSettings.java
+++ b/photon-core/src/main/java/org/photonvision/vision/pipeline/ReflectivePipelineSettings.java
@@ -27,5 +27,7 @@ public class ReflectivePipelineSettings extends AdvancedPipelineSettings {
     public ReflectivePipelineSettings() {
         super();
         pipelineType = PipelineType.Reflective;
+        cameraExposure = 6;
+        cameraGain = 20;
     }
 }
diff --git a/photon-core/src/main/java/org/photonvision/vision/processes/VisionModule.java b/photon-core/src/main/java/org/photonvision/vision/processes/VisionModule.java
index 98fa1cba9..3a9f2df47 100644
--- a/photon-core/src/main/java/org/photonvision/vision/processes/VisionModule.java
+++ b/photon-core/src/main/java/org/photonvision/vision/processes/VisionModule.java
@@ -104,14 +104,14 @@ public class VisionModule {
         if (cameraQuirks.hasQuirk(CameraQuirk.Gain)) {
             pipelineManager.userPipelineSettings.forEach(
                     it -> {
-                        if (it.cameraGain == -1) it.cameraGain = 20; // Sane default
+                        if (it.cameraGain == -1) it.cameraGain = 75; // Sane default
                     });
         }
         if (cameraQuirks.hasQuirk(CameraQuirk.AWBGain)) {
             pipelineManager.userPipelineSettings.forEach(
                     it -> {
-                        if (it.cameraRedGain == -1) it.cameraRedGain = 16; // Sane defaults
-                        if (it.cameraBlueGain == -1) it.cameraBlueGain = 16;
+                        if (it.cameraRedGain == -1) it.cameraRedGain = 11; // Sane defaults
+                        if (it.cameraBlueGain == -1) it.cameraBlueGain = 20;
                     });
         }
 
@@ -401,7 +401,7 @@ public class VisionModule {
         }
         if (cameraQuirks.hasQuirk(CameraQuirk.Gain)) {
             // If the gain is disabled for some reason, re-enable it
-            if (pipelineSettings.cameraGain == -1) pipelineSettings.cameraGain = 20;
+            if (pipelineSettings.cameraGain == -1) pipelineSettings.cameraGain = 75;
             visionSource.getSettables().setGain(Math.max(0, pipelineSettings.cameraGain));
         } else {
             pipelineSettings.cameraGain = -1;
@@ -409,8 +409,8 @@ public class VisionModule {
 
         if (cameraQuirks.hasQuirk(CameraQuirk.AWBGain)) {
             // If the AWB gains are disabled for some reason, re-enable it
-            if (pipelineSettings.cameraRedGain == -1) pipelineSettings.cameraRedGain = 16;
-            if (pipelineSettings.cameraBlueGain == -1) pipelineSettings.cameraBlueGain = 16;
+            if (pipelineSettings.cameraRedGain == -1) pipelineSettings.cameraRedGain = 11;
+            if (pipelineSettings.cameraBlueGain == -1) pipelineSettings.cameraBlueGain = 20;
             visionSource.getSettables().setRedGain(Math.max(0, pipelineSettings.cameraRedGain));
             visionSource.getSettables().setBlueGain(Math.max(0, pipelineSettings.cameraBlueGain));
         } else {
diff --git a/photon-core/src/test/java/org/photonvision/vision/pipeline/AprilTagTest.java b/photon-core/src/test/java/org/photonvision/vision/pipeline/AprilTagTest.java
index 9fede51cd..792ca95fb 100644
--- a/photon-core/src/test/java/org/photonvision/vision/pipeline/AprilTagTest.java
+++ b/photon-core/src/test/java/org/photonvision/vision/pipeline/AprilTagTest.java
@@ -24,6 +24,7 @@ import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.photonvision.common.util.TestUtils;
+import org.photonvision.vision.apriltag.AprilTagFamily;
 import org.photonvision.vision.camera.QuirkyCamera;
 import org.photonvision.vision.frame.provider.FileFrameProvider;
 import org.photonvision.vision.pipeline.result.CVPipelineResult;
@@ -46,6 +47,7 @@ public class AprilTagTest {
         pipeline.getSettings().cornerDetectionAccuracyPercentage = 4;
         pipeline.getSettings().cornerDetectionUseConvexHulls = true;
         pipeline.getSettings().targetModel = TargetModel.k200mmAprilTag;
+        pipeline.getSettings().tagFamily = AprilTagFamily.kTag36h11;
 
         var frameProvider =
                 new FileFrameProvider(