From 256e7904fd022a2be79cdc345fdae5e6c68b3a71 Mon Sep 17 00:00:00 2001
From: CoolSpy3 <55305038+CoolSpy3@users.noreply.github.com>
Date: Fri, 20 Nov 2020 21:02:23 -0800
Subject: [PATCH] [wpilibj] SimDeviceSim: Fix sim value changed callback
 (#2880)

---
 .../wpilibj/simulation/SimDeviceSim.java      |  4 +-
 .../wpilibj/simulation/SimDeviceSimTest.java  | 67 +++++++++++++++++--
 2 files changed, 64 insertions(+), 7 deletions(-)

diff --git a/wpilibj/src/main/java/edu/wpi/first/wpilibj/simulation/SimDeviceSim.java b/wpilibj/src/main/java/edu/wpi/first/wpilibj/simulation/SimDeviceSim.java
index 54af515096..bc79f63b8f 100644
--- a/wpilibj/src/main/java/edu/wpi/first/wpilibj/simulation/SimDeviceSim.java
+++ b/wpilibj/src/main/java/edu/wpi/first/wpilibj/simulation/SimDeviceSim.java
@@ -79,8 +79,8 @@ public class SimDeviceSim {
     return new CallbackStore(uid, SimDeviceDataJNI::cancelSimValueCreatedCallback);
   }
 
-  public CallbackStore registerValueChangedCallback(SimValueCallback callback, boolean initialNotify) {
-    int uid = SimDeviceDataJNI.registerSimValueChangedCallback(m_handle, callback, initialNotify);
+  public CallbackStore registerValueChangedCallback(SimValue value, SimValueCallback callback, boolean initialNotify) {
+    int uid = SimDeviceDataJNI.registerSimValueChangedCallback(value.getNativeHandle(), callback, initialNotify);
     return new CallbackStore(uid, SimDeviceDataJNI::cancelSimValueChangedCallback);
   }
 
diff --git a/wpilibj/src/test/java/edu/wpi/first/wpilibj/simulation/SimDeviceSimTest.java b/wpilibj/src/test/java/edu/wpi/first/wpilibj/simulation/SimDeviceSimTest.java
index 2c0c3ee266..608a21c63f 100644
--- a/wpilibj/src/test/java/edu/wpi/first/wpilibj/simulation/SimDeviceSimTest.java
+++ b/wpilibj/src/test/java/edu/wpi/first/wpilibj/simulation/SimDeviceSimTest.java
@@ -40,12 +40,11 @@ class SimDeviceSimTest {
 
     try (SimDevice otherdev = SimDevice.create("testnotDC");
         SimDevice dev1 = SimDevice.create("testDC1")) {
-      SimDeviceSim sim = new SimDeviceSim("testDC1");
       try (
-          CallbackStore callback1 = sim.registerDeviceCreatedCallback("testDC", (name, handle) -> {
+          CallbackStore callback1 = SimDeviceSim.registerDeviceCreatedCallback("testDC", (name, handle) -> {
             callback1Counter.addAndGet(1);
           }, false);
-          CallbackStore callback2 = sim.registerDeviceCreatedCallback("testDC", (name, handle) -> {
+          CallbackStore callback2 = SimDeviceSim.registerDeviceCreatedCallback("testDC", (name, handle) -> {
             callback2Counter.addAndGet(1);
           }, true)) {
         assertEquals(0, callback1Counter.get(), "Callback 1 called early");
@@ -71,8 +70,7 @@ class SimDeviceSimTest {
     AtomicInteger counter = new AtomicInteger(0);
 
     SimDevice dev1 = SimDevice.create("testDF1");
-    SimDeviceSim sim = new SimDeviceSim("testDF1");
-    try (CallbackStore callback = sim.registerDeviceFreedCallback("testDF", (name, handle) -> {
+    try (CallbackStore callback = SimDeviceSim.registerDeviceFreedCallback("testDF", (name, handle) -> {
       counter.addAndGet(1);
     }, false)) {
       assertEquals(0, counter.get(), "Callback called early");
@@ -85,4 +83,63 @@ class SimDeviceSimTest {
 
     assertEquals(1, counter.get(), "Callback called after closure");
   }
+
+  @Test
+  void testValueCreatedCallback() {
+    AtomicInteger callback1Counter = new AtomicInteger(0);
+    AtomicInteger callback2Counter = new AtomicInteger(0);
+
+    try (SimDevice dev1 = SimDevice.create("testVM1")) {
+      dev1.createBoolean("v1", false, false);
+      SimDeviceSim sim = new SimDeviceSim("testVM1");
+      try (
+          CallbackStore callback1 = sim.registerValueCreatedCallback((name, handle, readonly, value) -> {
+            callback1Counter.addAndGet(1);
+          }, false);
+          CallbackStore callback2 = sim.registerValueCreatedCallback((name, handle, readonly, value) -> {
+            callback2Counter.addAndGet(1);
+          }, true)) {
+        assertEquals(0, callback1Counter.get(), "Callback 1 called early");
+        assertEquals(1, callback2Counter.get(), "Callback 2 called early or not initalized with existing devices");
+
+        dev1.createDouble("v2", false, 0);
+
+        assertEquals(1, callback1Counter.get(), "Callback 1 called either more than once or not at all");
+        assertEquals(2, callback2Counter.get(), "Callback 2 called either more or less than twice");
+      }
+      dev1.createBoolean("v3", false, false);
+
+      assertEquals(1, callback1Counter.get(), "Callback 1 called after closure");
+      assertEquals(2, callback2Counter.get(), "Callback 2 called after closure");
+    }
+  }
+
+  @Test
+  void testValueChangedCallback() {
+    AtomicInteger callback1Counter = new AtomicInteger(0);
+    AtomicInteger callback2Counter = new AtomicInteger(0);
+
+    try (SimDevice dev1 = SimDevice.create("testVC1")) {
+      SimBoolean val = dev1.createBoolean("v1", false, false);
+      SimDeviceSim sim = new SimDeviceSim("testVM1");
+      try (
+          CallbackStore callback1 = sim.registerValueChangedCallback(val, (name, handle, readonly, value) -> {
+            callback1Counter.addAndGet(1);
+          }, false);
+          CallbackStore callback2 = sim.registerValueChangedCallback(val, (name, handle, readonly, value) -> {
+            callback2Counter.addAndGet(1);
+          }, true)) {
+        assertEquals(0, callback1Counter.get(), "Callback 1 called early");
+        assertEquals(1, callback2Counter.get(), "Callback 2 called early or not initalized with existing devices");
+
+        val.set(true);
+
+        assertEquals(1, callback1Counter.get(), "Callback 1 called either more than once or not at all");
+        assertEquals(2, callback2Counter.get(), "Callback 2 called either more or less than twice");
+      }
+      val.set(false);
+      assertEquals(1, callback1Counter.get(), "Callback 1 called after closure");
+      assertEquals(2, callback2Counter.get(), "Callback 2 called after closure");
+    }
+  }
 }