From 47c59859ee45db026992b983fb29e166bc73ef2c Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Sat, 14 Nov 2020 21:04:51 -0800 Subject: [PATCH] [sim] Make SimDevice callbacks synchronous (#2861) Asynchronous callbacks are more efficient but pose synchronization challenges; other sim callbacks are synchronous but SimDevice ones were not. --- .../hal/simulation/SimDeviceDataJNI.java | 2 +- .../native/athena/mockdata/SimDeviceData.cpp | 5 +- .../cpp/jni/simulation/SimDeviceDataJNI.cpp | 399 +++++++++--------- .../include/hal/simulation/SimDeviceData.h | 5 +- .../native/sim/mockdata/SimDeviceData.cpp | 11 +- .../main/native/cpp/WSProvider_SimDevice.cpp | 2 +- .../wpilibj/simulation/SimDeviceSim.java | 4 +- .../wpilibj/simulation/SimDeviceSimTest.java | 71 +++- 8 files changed, 286 insertions(+), 213 deletions(-) diff --git a/hal/src/main/java/edu/wpi/first/hal/simulation/SimDeviceDataJNI.java b/hal/src/main/java/edu/wpi/first/hal/simulation/SimDeviceDataJNI.java index ea2a9f6066..f75090e135 100644 --- a/hal/src/main/java/edu/wpi/first/hal/simulation/SimDeviceDataJNI.java +++ b/hal/src/main/java/edu/wpi/first/hal/simulation/SimDeviceDataJNI.java @@ -17,7 +17,7 @@ public class SimDeviceDataJNI extends JNIWrapper { public static native int registerSimDeviceCreatedCallback(String prefix, SimDeviceCallback callback, boolean initialNotify); public static native void cancelSimDeviceCreatedCallback(int uid); - public static native int registerSimDeviceFreedCallback(String prefix, SimDeviceCallback callback); + public static native int registerSimDeviceFreedCallback(String prefix, SimDeviceCallback callback, boolean initialNotify); public static native void cancelSimDeviceFreedCallback(int uid); public static native int getSimDeviceHandle(String name); diff --git a/hal/src/main/native/athena/mockdata/SimDeviceData.cpp b/hal/src/main/native/athena/mockdata/SimDeviceData.cpp index a31160c85d..e2019996f5 100644 --- a/hal/src/main/native/athena/mockdata/SimDeviceData.cpp +++ b/hal/src/main/native/athena/mockdata/SimDeviceData.cpp @@ -23,8 +23,9 @@ int32_t HALSIM_RegisterSimDeviceCreatedCallback( void HALSIM_CancelSimDeviceCreatedCallback(int32_t uid) {} -int32_t HALSIM_RegisterSimDeviceFreedCallback( - const char* prefix, void* param, HALSIM_SimDeviceCallback callback) { +int32_t HALSIM_RegisterSimDeviceFreedCallback(const char* prefix, void* param, + HALSIM_SimDeviceCallback callback, + HAL_Bool initialNotify) { return 0; } diff --git a/hal/src/main/native/cpp/jni/simulation/SimDeviceDataJNI.cpp b/hal/src/main/native/cpp/jni/simulation/SimDeviceDataJNI.cpp index 0b43d24ef6..7f58c166be 100644 --- a/hal/src/main/native/cpp/jni/simulation/SimDeviceDataJNI.cpp +++ b/hal/src/main/native/cpp/jni/simulation/SimDeviceDataJNI.cpp @@ -9,15 +9,13 @@ #include -#include -#include #include -#include #include #include "SimulatorJNI.h" #include "edu_wpi_first_hal_simulation_SimDeviceDataJNI.h" +#include "hal/handles/UnlimitedHandleResource.h" #include "hal/simulation/SimDeviceData.h" using namespace hal; @@ -39,7 +37,6 @@ struct DeviceInfo { HAL_SimValueHandle handle; jobject MakeJava(JNIEnv* env) const; - void CallJava(JNIEnv* env, jobject callobj) const; }; struct ValueInfo { @@ -52,10 +49,6 @@ struct ValueInfo { HAL_Value value; jobject MakeJava(JNIEnv* env) const; - void CallJava(JNIEnv* env, jobject callobj) const; - - private: - std::pair ToValue12() const; }; } // namespace @@ -67,12 +60,7 @@ jobject DeviceInfo::MakeJava(JNIEnv* env) const { (jint)handle); } -void DeviceInfo::CallJava(JNIEnv* env, jobject callobj) const { - env->CallVoidMethod(callobj, simDeviceCallbackCallback, - MakeJString(env, name), (jint)handle); -} - -std::pair ValueInfo::ToValue12() const { +static std::pair ToValue12(const HAL_Value& value) { jlong value1 = 0; jdouble value2 = 0.0; switch (value.type) { @@ -100,164 +88,213 @@ std::pair ValueInfo::ToValue12() const { jobject ValueInfo::MakeJava(JNIEnv* env) const { static jmethodID func = env->GetMethodID(simValueInfoCls, "", "(Ljava/lang/String;IZIJD)V"); - auto [value1, value2] = ToValue12(); + auto [value1, value2] = ToValue12(value); return env->NewObject(simValueInfoCls, func, MakeJString(env, name), (jint)handle, (jboolean)readonly, (jint)value.type, value1, value2); } -void ValueInfo::CallJava(JNIEnv* env, jobject callobj) const { - auto [value1, value2] = ToValue12(); - env->CallVoidMethod(callobj, simValueCallbackCallback, MakeJString(env, name), - (jint)handle, (jboolean)readonly, (jint)value.type, - value1, value2); -} - namespace { -class CallbackStore { +class DeviceCallbackStore { public: - explicit CallbackStore(JNIEnv* env, jobject obj) : m_call{env, obj} {} - ~CallbackStore() { - if (m_cancelCallback) m_cancelCallback(); - } - - void SetCancel(std::function cancelCallback) { - m_cancelCallback = std::move(cancelCallback); - } - void Free(JNIEnv* env) { m_call.free(env); } - jobject Get() const { return m_call; } + void create(JNIEnv* env, jobject obj) { m_call = JGlobal(env, obj); } + void performCallback(const char* name, HAL_SimDeviceHandle handle); + void free(JNIEnv* env) { m_call.free(env); } + void setCallbackId(int32_t id) { callbackId = id; } + int32_t getCallbackId() { return callbackId; } private: wpi::java::JGlobal m_call; - std::function m_cancelCallback; + int32_t callbackId; }; -class CallbackThreadJNI : public wpi::SafeThread { +class ValueCallbackStore { public: - void Main(); - - using DeviceCalls = - std::vector, DeviceInfo>>; - DeviceCalls m_deviceCalls; - using ValueCalls = - std::vector, ValueInfo>>; - ValueCalls m_valueCalls; - - wpi::UidVector, 4> m_callbacks; -}; - -class CallbackJNI { - public: - static CallbackJNI& GetInstance() { - static CallbackJNI inst; - return inst; - } - void SendDevice(int32_t callback, DeviceInfo info); - void SendValue(int32_t callback, ValueInfo info); - - std::pair> AllocateCallback( - JNIEnv* env, jobject obj); - - void FreeCallback(JNIEnv* env, int32_t uid); + void create(JNIEnv* env, jobject obj) { m_call = JGlobal(env, obj); } + void performCallback(const char* name, HAL_SimValueHandle handle, + bool readonly, const HAL_Value& value); + void free(JNIEnv* env) { m_call.free(env); } + void setCallbackId(int32_t id) { callbackId = id; } + int32_t getCallbackId() { return callbackId; } private: - CallbackJNI() { m_owner.Start(); } - - wpi::SafeThreadOwner m_owner; + wpi::java::JGlobal m_call; + int32_t callbackId; }; } // namespace -void CallbackThreadJNI::Main() { +void DeviceCallbackStore::performCallback(const char* name, + HAL_SimDeviceHandle handle) { JNIEnv* env; - JavaVMAttachArgs args; - args.version = JNI_VERSION_1_2; - args.name = const_cast("SimDeviceCallback"); - args.group = nullptr; - jint rs = sim::GetJVM()->AttachCurrentThreadAsDaemon( - reinterpret_cast(&env), &args); - if (rs != JNI_OK) return; - - DeviceCalls deviceCalls; - ValueCalls valueCalls; - - std::unique_lock lock(m_mutex); - while (m_active) { - m_cond.wait(lock, [&] { return !m_active; }); - if (!m_active) break; - - deviceCalls.swap(m_deviceCalls); - valueCalls.swap(m_valueCalls); - - lock.unlock(); // don't hold mutex during callback execution - - for (auto&& call : deviceCalls) { - if (auto store = call.first.lock()) { - if (jobject callobj = store->Get()) { - call.second.CallJava(env, callobj); - if (env->ExceptionCheck()) { - env->ExceptionDescribe(); - env->ExceptionClear(); - } - } - } + JavaVM* vm = sim::GetJVM(); + bool didAttachThread = false; + int tryGetEnv = vm->GetEnv(reinterpret_cast(&env), JNI_VERSION_1_6); + if (tryGetEnv == JNI_EDETACHED) { + // Thread not attached + didAttachThread = true; + if (vm->AttachCurrentThread(reinterpret_cast(&env), nullptr) != 0) { + // Failed to attach, log and return + wpi::outs() << "Failed to attach\n"; + wpi::outs().flush(); + return; } - - for (auto&& call : valueCalls) { - if (auto store = call.first.lock()) { - if (jobject callobj = store->Get()) { - call.second.CallJava(env, callobj); - if (env->ExceptionCheck()) { - env->ExceptionDescribe(); - env->ExceptionClear(); - } - } - } - } - - deviceCalls.clear(); - valueCalls.clear(); - - lock.lock(); + } else if (tryGetEnv == JNI_EVERSION) { + wpi::outs() << "Invalid JVM Version requested\n"; + wpi::outs().flush(); } - // free global references - for (auto&& callback : m_callbacks) callback->Free(env); + env->CallVoidMethod(m_call, simDeviceCallbackCallback, MakeJString(env, name), + (jint)handle); - sim::GetJVM()->DetachCurrentThread(); + if (env->ExceptionCheck()) { + env->ExceptionDescribe(); + } + + if (didAttachThread) { + vm->DetachCurrentThread(); + } } -void CallbackJNI::SendDevice(int32_t callback, DeviceInfo info) { - auto thr = m_owner.GetThread(); - if (!thr) return; - thr->m_deviceCalls.emplace_back(thr->m_callbacks[callback], std::move(info)); - thr->m_cond.notify_one(); +void ValueCallbackStore::performCallback(const char* name, + HAL_SimValueHandle handle, + bool readonly, + const HAL_Value& value) { + JNIEnv* env; + JavaVM* vm = sim::GetJVM(); + bool didAttachThread = false; + int tryGetEnv = vm->GetEnv(reinterpret_cast(&env), JNI_VERSION_1_6); + if (tryGetEnv == JNI_EDETACHED) { + // Thread not attached + didAttachThread = true; + if (vm->AttachCurrentThread(reinterpret_cast(&env), nullptr) != 0) { + // Failed to attach, log and return + wpi::outs() << "Failed to attach\n"; + wpi::outs().flush(); + return; + } + } else if (tryGetEnv == JNI_EVERSION) { + wpi::outs() << "Invalid JVM Version requested\n"; + wpi::outs().flush(); + } + + auto [value1, value2] = ToValue12(value); + env->CallVoidMethod(m_call, simValueCallbackCallback, MakeJString(env, name), + (jint)handle, (jboolean)readonly, (jint)value.type, + value1, value2); + + if (env->ExceptionCheck()) { + env->ExceptionDescribe(); + } + + if (didAttachThread) { + vm->DetachCurrentThread(); + } } -void CallbackJNI::SendValue(int32_t callback, ValueInfo info) { - auto thr = m_owner.GetThread(); - if (!thr) return; - thr->m_valueCalls.emplace_back(thr->m_callbacks[callback], std::move(info)); - thr->m_cond.notify_one(); +static hal::UnlimitedHandleResource* + deviceCallbackHandles; + +namespace { +typedef int32_t (*RegisterDeviceCallbackFunc)(const char* prefix, void* param, + HALSIM_SimDeviceCallback callback, + HAL_Bool initialNotify); +typedef void (*FreeDeviceCallbackFunc)(int32_t uid); +} // namespace + +static SIM_JniHandle AllocateDeviceCallback( + JNIEnv* env, const char* prefix, jobject callback, jboolean initialNotify, + RegisterDeviceCallbackFunc createCallback) { + auto callbackStore = std::make_shared(); + + auto handle = deviceCallbackHandles->Allocate(callbackStore); + + if (handle == HAL_kInvalidHandle) { + return -1; + } + + uintptr_t handleAsPtr = static_cast(handle); + void* handleAsVoidPtr = reinterpret_cast(handleAsPtr); + + callbackStore->create(env, callback); + + auto callbackFunc = [](const char* name, void* param, + HAL_SimDeviceHandle handle) { + uintptr_t handleTmp = reinterpret_cast(param); + SIM_JniHandle jnihandle = static_cast(handleTmp); + auto data = deviceCallbackHandles->Get(jnihandle); + if (!data) return; + + data->performCallback(name, handle); + }; + + auto id = + createCallback(prefix, handleAsVoidPtr, callbackFunc, initialNotify); + + callbackStore->setCallbackId(id); + + return handle; } -std::pair> -CallbackJNI::AllocateCallback(JNIEnv* env, jobject obj) { - auto thr = m_owner.GetThread(); - if (!thr) return std::pair(0, nullptr); - auto store = std::make_shared(env, obj); - return std::pair(thr->m_callbacks.emplace_back(store) + 1, store); +static void FreeDeviceCallback(JNIEnv* env, SIM_JniHandle handle, + FreeDeviceCallbackFunc freeCallback) { + auto callback = deviceCallbackHandles->Free(handle); + freeCallback(callback->getCallbackId()); + callback->free(env); } -void CallbackJNI::FreeCallback(JNIEnv* env, int32_t uid) { - auto thr = m_owner.GetThread(); - if (!thr) return; - if (uid <= 0 || static_cast(uid) >= thr->m_callbacks.size()) return; - --uid; - auto store = std::move(thr->m_callbacks[uid]); - thr->m_callbacks.erase(uid); - store->Free(env); +static hal::UnlimitedHandleResource* + valueCallbackHandles; + +namespace { +typedef void (*FreeValueCallbackFunc)(int32_t uid); +} // namespace + +template +static SIM_JniHandle AllocateValueCallback( + JNIEnv* env, THandle h, jobject callback, jboolean initialNotify, + int32_t (*createCallback)(THandle handle, void* param, + HALSIM_SimValueCallback callback, + HAL_Bool initialNotify)) { + auto callbackStore = std::make_shared(); + + auto handle = valueCallbackHandles->Allocate(callbackStore); + + if (handle == HAL_kInvalidHandle) { + return -1; + } + + uintptr_t handleAsPtr = static_cast(handle); + void* handleAsVoidPtr = reinterpret_cast(handleAsPtr); + + callbackStore->create(env, callback); + + auto callbackFunc = [](const char* name, void* param, + HAL_SimValueHandle handle, HAL_Bool readonly, + const HAL_Value* value) { + uintptr_t handleTmp = reinterpret_cast(param); + SIM_JniHandle jnihandle = static_cast(handleTmp); + auto data = valueCallbackHandles->Get(jnihandle); + if (!data) return; + + data->performCallback(name, handle, readonly, *value); + }; + + auto id = createCallback(h, handleAsVoidPtr, callbackFunc, initialNotify); + + callbackStore->setCallbackId(id); + + return handle; +} + +static void FreeValueCallback(JNIEnv* env, SIM_JniHandle handle, + FreeValueCallbackFunc freeCallback) { + auto callback = valueCallbackHandles->Free(handle); + freeCallback(callback->getCallbackId()); + callback->free(env); } namespace hal { @@ -288,6 +325,16 @@ bool InitializeSimDeviceDataJNI(JNIEnv* env) { simValueCallbackCls, "callbackNative", "(Ljava/lang/String;IZIJD)V"); if (!simValueCallbackCallback) return false; + static hal::UnlimitedHandleResource + cbDevice; + deviceCallbackHandles = &cbDevice; + + static hal::UnlimitedHandleResource + cbValue; + valueCallbackHandles = &cbValue; + return true; } @@ -337,18 +384,9 @@ Java_edu_wpi_first_hal_simulation_SimDeviceDataJNI_registerSimDeviceCreatedCallb (JNIEnv* env, jclass, jstring prefix, jobject callback, jboolean initialNotify) { - auto [uid, store] = - CallbackJNI::GetInstance().AllocateCallback(env, callback); - int32_t cuid = HALSIM_RegisterSimDeviceCreatedCallback( - JStringRef{env, prefix}.c_str(), - reinterpret_cast(static_cast(uid)), - [](const char* name, void* param, HAL_SimDeviceHandle handle) { - int32_t uid = reinterpret_cast(param); - CallbackJNI::GetInstance().SendDevice(uid, DeviceInfo{name, handle}); - }, - initialNotify); - store->SetCancel([cuid] { HALSIM_CancelSimDeviceCreatedCallback(cuid); }); - return uid; + return AllocateDeviceCallback(env, JStringRef{env, prefix}.c_str(), callback, + initialNotify, + &HALSIM_RegisterSimDeviceCreatedCallback); } /* @@ -360,29 +398,22 @@ JNIEXPORT void JNICALL Java_edu_wpi_first_hal_simulation_SimDeviceDataJNI_cancelSimDeviceCreatedCallback (JNIEnv* env, jclass, jint uid) { - CallbackJNI::GetInstance().FreeCallback(env, uid); + FreeDeviceCallback(env, uid, &HALSIM_CancelSimDeviceCreatedCallback); } /* * Class: edu_wpi_first_hal_simulation_SimDeviceDataJNI * Method: registerSimDeviceFreedCallback - * Signature: (Ljava/lang/String;Ljava/lang/Object;)I + * Signature: (Ljava/lang/String;Ljava/lang/Object;Z)I */ JNIEXPORT jint JNICALL Java_edu_wpi_first_hal_simulation_SimDeviceDataJNI_registerSimDeviceFreedCallback - (JNIEnv* env, jclass, jstring prefix, jobject callback) + (JNIEnv* env, jclass, jstring prefix, jobject callback, + jboolean initialNotify) { - auto [uid, store] = - CallbackJNI::GetInstance().AllocateCallback(env, callback); - int32_t cuid = HALSIM_RegisterSimDeviceFreedCallback( - JStringRef{env, prefix}.c_str(), - reinterpret_cast(static_cast(uid)), - [](const char* name, void* param, HAL_SimDeviceHandle handle) { - int32_t uid = reinterpret_cast(param); - CallbackJNI::GetInstance().SendDevice(uid, DeviceInfo{name, handle}); - }); - store->SetCancel([cuid] { HALSIM_CancelSimDeviceFreedCallback(cuid); }); - return uid; + return AllocateDeviceCallback(env, JStringRef{env, prefix}.c_str(), callback, + initialNotify, + &HALSIM_RegisterSimDeviceFreedCallback); } /* @@ -394,7 +425,7 @@ JNIEXPORT void JNICALL Java_edu_wpi_first_hal_simulation_SimDeviceDataJNI_cancelSimDeviceFreedCallback (JNIEnv* env, jclass, jint uid) { - CallbackJNI::GetInstance().FreeCallback(env, uid); + FreeDeviceCallback(env, uid, &HALSIM_CancelSimDeviceFreedCallback); } /* @@ -460,19 +491,9 @@ JNIEXPORT jint JNICALL Java_edu_wpi_first_hal_simulation_SimDeviceDataJNI_registerSimValueCreatedCallback (JNIEnv* env, jclass, jint device, jobject callback, jboolean initialNotify) { - auto [uid, store] = - CallbackJNI::GetInstance().AllocateCallback(env, callback); - int32_t cuid = HALSIM_RegisterSimValueCreatedCallback( - device, reinterpret_cast(static_cast(uid)), - [](const char* name, void* param, HAL_SimValueHandle handle, - HAL_Bool readonly, const HAL_Value* value) { - int32_t uid = reinterpret_cast(param); - CallbackJNI::GetInstance().SendValue( - uid, ValueInfo{name, handle, static_cast(readonly), *value}); - }, - initialNotify); - store->SetCancel([cuid] { HALSIM_CancelSimValueCreatedCallback(cuid); }); - return uid; + return AllocateValueCallback(env, static_cast(device), + callback, initialNotify, + &HALSIM_RegisterSimValueCreatedCallback); } /* @@ -484,7 +505,7 @@ JNIEXPORT void JNICALL Java_edu_wpi_first_hal_simulation_SimDeviceDataJNI_cancelSimValueCreatedCallback (JNIEnv* env, jclass, jint uid) { - CallbackJNI::GetInstance().FreeCallback(env, uid); + FreeValueCallback(env, uid, &HALSIM_CancelSimValueCreatedCallback); } /* @@ -496,19 +517,9 @@ JNIEXPORT jint JNICALL Java_edu_wpi_first_hal_simulation_SimDeviceDataJNI_registerSimValueChangedCallback (JNIEnv* env, jclass, jint handle, jobject callback, jboolean initialNotify) { - auto [uid, store] = - CallbackJNI::GetInstance().AllocateCallback(env, callback); - int32_t cuid = HALSIM_RegisterSimValueChangedCallback( - handle, reinterpret_cast(static_cast(uid)), - [](const char* name, void* param, HAL_SimValueHandle handle, - HAL_Bool readonly, const HAL_Value* value) { - int32_t uid = reinterpret_cast(param); - CallbackJNI::GetInstance().SendValue( - uid, ValueInfo{name, handle, static_cast(readonly), *value}); - }, - initialNotify); - store->SetCancel([cuid] { HALSIM_CancelSimValueChangedCallback(cuid); }); - return uid; + return AllocateValueCallback(env, static_cast(handle), + callback, initialNotify, + &HALSIM_RegisterSimValueChangedCallback); } /* @@ -520,7 +531,7 @@ JNIEXPORT void JNICALL Java_edu_wpi_first_hal_simulation_SimDeviceDataJNI_cancelSimValueChangedCallback (JNIEnv* env, jclass, jint uid) { - CallbackJNI::GetInstance().FreeCallback(env, uid); + FreeValueCallback(env, uid, &HALSIM_CancelSimValueChangedCallback); } /* diff --git a/hal/src/main/native/include/hal/simulation/SimDeviceData.h b/hal/src/main/native/include/hal/simulation/SimDeviceData.h index e702c5c81b..a95db1c2b5 100644 --- a/hal/src/main/native/include/hal/simulation/SimDeviceData.h +++ b/hal/src/main/native/include/hal/simulation/SimDeviceData.h @@ -31,8 +31,9 @@ int32_t HALSIM_RegisterSimDeviceCreatedCallback( void HALSIM_CancelSimDeviceCreatedCallback(int32_t uid); -int32_t HALSIM_RegisterSimDeviceFreedCallback( - const char* prefix, void* param, HALSIM_SimDeviceCallback callback); +int32_t HALSIM_RegisterSimDeviceFreedCallback(const char* prefix, void* param, + HALSIM_SimDeviceCallback callback, + HAL_Bool initialNotify); void HALSIM_CancelSimDeviceFreedCallback(int32_t uid); diff --git a/hal/src/main/native/sim/mockdata/SimDeviceData.cpp b/hal/src/main/native/sim/mockdata/SimDeviceData.cpp index 6c0a7f2972..e70c42e81f 100644 --- a/hal/src/main/native/sim/mockdata/SimDeviceData.cpp +++ b/hal/src/main/native/sim/mockdata/SimDeviceData.cpp @@ -204,8 +204,10 @@ int32_t SimDeviceData::RegisterDeviceCreatedCallback( // initial notifications if (initialNotify) { - for (auto&& device : m_devices) - callback(device->name.c_str(), param, device->handle); + for (auto&& device : m_devices) { + if (wpi::StringRef{device->name}.startswith(prefix)) + callback(device->name.c_str(), param, device->handle); + } } return index; @@ -383,8 +385,9 @@ void HALSIM_CancelSimDeviceCreatedCallback(int32_t uid) { SimSimDeviceData->CancelDeviceCreatedCallback(uid); } -int32_t HALSIM_RegisterSimDeviceFreedCallback( - const char* prefix, void* param, HALSIM_SimDeviceCallback callback) { +int32_t HALSIM_RegisterSimDeviceFreedCallback(const char* prefix, void* param, + HALSIM_SimDeviceCallback callback, + HAL_Bool initialNotify) { return SimSimDeviceData->RegisterDeviceFreedCallback(prefix, param, callback); } diff --git a/simulation/halsim_ws_core/src/main/native/cpp/WSProvider_SimDevice.cpp b/simulation/halsim_ws_core/src/main/native/cpp/WSProvider_SimDevice.cpp index ee8fdedc96..8ccfc0a588 100644 --- a/simulation/halsim_ws_core/src/main/native/cpp/WSProvider_SimDevice.cpp +++ b/simulation/halsim_ws_core/src/main/native/cpp/WSProvider_SimDevice.cpp @@ -170,7 +170,7 @@ void HALSimWSProviderSimDevices::Initialize(wpi::uv::Loop& loop) { m_deviceCreatedCbKey = HALSIM_RegisterSimDeviceCreatedCallback( "", this, HALSimWSProviderSimDevices::DeviceCreatedCallbackStatic, 1); m_deviceFreedCbKey = HALSIM_RegisterSimDeviceFreedCallback( - "", this, HALSimWSProviderSimDevices::DeviceFreedCallbackStatic); + "", this, HALSimWSProviderSimDevices::DeviceFreedCallbackStatic, false); m_exec = UvExecFn::Create(loop, [](auto out, LoopFn func) { func(); 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 713c112f7e..54af515096 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 @@ -93,8 +93,8 @@ public class SimDeviceSim { return new CallbackStore(uid, SimDeviceDataJNI::cancelSimDeviceCreatedCallback); } - public static CallbackStore registerDeviceFreedCallback(String prefix, SimDeviceCallback callback) { - int uid = SimDeviceDataJNI.registerSimDeviceFreedCallback(prefix, callback); + public static CallbackStore registerDeviceFreedCallback(String prefix, SimDeviceCallback callback, boolean initialNotify) { + int uid = SimDeviceDataJNI.registerSimDeviceFreedCallback(prefix, callback, initialNotify); return new CallbackStore(uid, SimDeviceDataJNI::cancelSimDeviceFreedCallback); } 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 6b9fe61408..2c0c3ee266 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 @@ -12,20 +12,77 @@ import org.junit.jupiter.api.Test; import edu.wpi.first.hal.SimBoolean; import edu.wpi.first.hal.SimDevice; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; class SimDeviceSimTest { @Test void testBasic() { - SimDevice dev = SimDevice.create("test"); - SimBoolean devBool = dev.createBoolean("bool", false, false); + try (SimDevice dev = SimDevice.create("test")) { + SimBoolean devBool = dev.createBoolean("bool", false, false); - SimDeviceSim sim = new SimDeviceSim("test"); - SimBoolean simBool = sim.getBoolean("bool"); + SimDeviceSim sim = new SimDeviceSim("test"); + SimBoolean simBool = sim.getBoolean("bool"); - assertFalse(simBool.get()); - simBool.set(true); - assertTrue(devBool.get()); + assertFalse(simBool.get()); + simBool.set(true); + assertTrue(devBool.get()); + } + } + + @Test + void testDeviceCreatedCallback() { + AtomicInteger callback1Counter = new AtomicInteger(0); + AtomicInteger callback2Counter = new AtomicInteger(0); + + try (SimDevice otherdev = SimDevice.create("testnotDC"); + SimDevice dev1 = SimDevice.create("testDC1")) { + SimDeviceSim sim = new SimDeviceSim("testDC1"); + try ( + CallbackStore callback1 = sim.registerDeviceCreatedCallback("testDC", (name, handle) -> { + callback1Counter.addAndGet(1); + }, false); + CallbackStore callback2 = sim.registerDeviceCreatedCallback("testDC", (name, handle) -> { + 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"); + + SimDevice dev2 = SimDevice.create("testDC2"); + dev2.close(); + + 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"); + } + + SimDevice dev3 = SimDevice.create("testDC3"); + dev3.close(); + + assertEquals(1, callback1Counter.get(), "Callback 1 called after closure"); + assertEquals(2, callback2Counter.get(), "Callback 2 called after closure"); + } + } + + @Test + void testDeviceFreedCallback() { + AtomicInteger counter = new AtomicInteger(0); + + SimDevice dev1 = SimDevice.create("testDF1"); + SimDeviceSim sim = new SimDeviceSim("testDF1"); + try (CallbackStore callback = sim.registerDeviceFreedCallback("testDF", (name, handle) -> { + counter.addAndGet(1); + }, false)) { + assertEquals(0, counter.get(), "Callback called early"); + dev1.close(); + assertEquals(1, counter.get(), "Callback called either more than once or not at all"); + } + + SimDevice dev2 = SimDevice.create("testDF2"); + dev2.close(); + + assertEquals(1, counter.get(), "Callback called after closure"); } }