From 5dc5ed83b3d0a0eb583d22d57301f6b4f76d0f58 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Thu, 17 Dec 2015 16:19:44 -0800 Subject: [PATCH] Rewrite Java Notifier and update Interrupt JNI. Notifier takes advantage of the multi-notifier support now in HAL. Each Notifier is now handled by a separate thread at the JNI level, so one notifier taking longer to process (or being breakpointed) does not stop the other notifiers from running. These threads are configured as daemon threads. In both Notifier and Interrupt JNI, the JNI thread attachment no longer repeatedly calls AttachCurrentThread(). This improves performance but more importantly avoids impacting the Eclipse debugger, which attempts to track each call to AttachCurrentThread() as a separate Java thread. Note: There is currently no way to free an interrupt handler. Repeatedly calling attachInterruptHandler() will result in leaking previous handlers. Change-Id: Ib12e3df88943c03e0269d3906e5b153767139391 --- hal/include/HAL/Notifier.hpp | 5 +- hal/lib/Athena/Notifier.cpp | 18 +- wpilibj/src/athena/cpp/lib/InterruptJNI.cpp | 336 ++++++++++-------- wpilibj/src/athena/cpp/lib/NotifierJNI.cpp | 203 +++++++---- wpilibj/src/athena/cpp/lib/SafeThread.cpp | 31 ++ wpilibj/src/athena/cpp/lib/SafeThread.h | 93 +++++ .../java/edu/wpi/first/wpilibj/Notifier.java | 289 +++++---------- .../wpi/first/wpilibj/hal/NotifierJNI.java | 18 +- 8 files changed, 564 insertions(+), 429 deletions(-) create mode 100644 wpilibj/src/athena/cpp/lib/SafeThread.cpp create mode 100644 wpilibj/src/athena/cpp/lib/SafeThread.h diff --git a/hal/include/HAL/Notifier.hpp b/hal/include/HAL/Notifier.hpp index f16572ff8b..1ec7ec952b 100644 --- a/hal/include/HAL/Notifier.hpp +++ b/hal/include/HAL/Notifier.hpp @@ -4,8 +4,9 @@ extern "C" { - void* initializeNotifier(void (*ProcessQueue)(uint32_t, void*), void* param, int32_t *status); + void* initializeNotifier(void (*process)(uint32_t, void*), void* param, int32_t *status); void cleanNotifier(void* notifier_pointer, int32_t *status); - + void* getNotifierParam(void* notifier_pointer, int32_t *status); void updateNotifierAlarm(void* notifier_pointer, uint32_t triggerTime, int32_t *status); + void stopNotifierAlarm(void* notifier_pointer, int32_t *status); } diff --git a/hal/lib/Athena/Notifier.cpp b/hal/lib/Athena/Notifier.cpp index c2c6c4c202..f7f69c3d02 100644 --- a/hal/lib/Athena/Notifier.cpp +++ b/hal/lib/Athena/Notifier.cpp @@ -59,9 +59,9 @@ static void cleanupNotifierAtExit() { notifierManager = nullptr; } -void* initializeNotifier(void (*ProcessQueue)(uint32_t, void*), void *param, int32_t *status) +void* initializeNotifier(void (*process)(uint32_t, void*), void *param, int32_t *status) { - if (!ProcessQueue) { + if (!process) { *status = NULL_PARAMETER; return nullptr; } @@ -85,7 +85,7 @@ void* initializeNotifier(void (*ProcessQueue)(uint32_t, void*), void *param, int notifier->next = notifiers; if (notifier->next) notifier->next->prev = notifier; notifier->param = param; - notifier->process = ProcessQueue; + notifier->process = process; notifiers = notifier; return notifier; } @@ -119,6 +119,11 @@ void cleanNotifier(void* notifier_pointer, int32_t *status) } } +void* getNotifierParam(void* notifier_pointer, int32_t *status) +{ + return ((Notifier*)notifier_pointer)->param; +} + void updateNotifierAlarm(void* notifier_pointer, uint32_t triggerTime, int32_t *status) { std::lock_guard sync(notifierMutex); @@ -141,3 +146,10 @@ void updateNotifierAlarm(void* notifier_pointer, uint32_t triggerTime, int32_t * notifierInterruptMutex.unlock(); } + +void stopNotifierAlarm(void* notifier_pointer, int32_t *status) +{ + std::lock_guard sync(notifierMutex); + Notifier* notifier = (Notifier*)notifier_pointer; + notifier->triggerTime = UINT32_MAX; +} diff --git a/wpilibj/src/athena/cpp/lib/InterruptJNI.cpp b/wpilibj/src/athena/cpp/lib/InterruptJNI.cpp index 25ce6cfe41..e01f3ea2cc 100644 --- a/wpilibj/src/athena/cpp/lib/InterruptJNI.cpp +++ b/wpilibj/src/athena/cpp/lib/InterruptJNI.cpp @@ -1,10 +1,15 @@ #include #include +#include +#include +#include +#include #include "Log.hpp" #include "edu_wpi_first_wpilibj_hal_InterruptJNI.h" #include "HAL/Interrupts.hpp" #include "HALUtil.h" +#include "SafeThread.h" TLogLevel interruptJNILogLevel = logERROR; @@ -12,6 +17,94 @@ TLogLevel interruptJNILogLevel = logERROR; if (level > interruptJNILogLevel) ; \ else Log().Get(level) +// Thread where callbacks are actually performed. +// +// JNI's AttachCurrentThread() creates a Java Thread object on every +// invocation, which is both time inefficient and causes issues with Eclipse +// (which tries to keep a thread list up-to-date and thus gets swamped). +// +// Instead, this class attaches just once. When a hardware notification +// occurs, a condition variable wakes up this thread and this thread actually +// makes the call into Java. +// +// We don't want to use a FIFO here. If the user code takes too long to +// process, we will just ignore the redundant wakeup. +class InterruptThreadJNI : public SafeThread { + public: + void Main(); + + bool m_notify = false; + uint32_t m_mask = 0; + jobject m_func = nullptr; + jmethodID m_mid; + jobject m_param = nullptr; +}; + +class InterruptJNI : public SafeThreadOwner { + public: + void SetFunc(JNIEnv* env, jobject func, jmethodID mid, jobject param); + + void Notify(uint32_t mask) { + auto thr = GetThread(); + if (!thr) return; + thr->m_notify = true; + thr->m_mask = mask; + thr->m_cond.notify_one(); + } +}; + +void InterruptJNI::SetFunc(JNIEnv* env, jobject func, jmethodID mid, + jobject param) { + auto thr = GetThread(); + if (!thr) return; + // free global references + if (thr->m_func) env->DeleteGlobalRef(thr->m_func); + if (thr->m_param) env->DeleteGlobalRef(thr->m_param); + // create global references + thr->m_func = env->NewGlobalRef(func); + thr->m_param = param ? env->NewGlobalRef(param) : nullptr; + thr->m_mid = mid; +} + +void InterruptThreadJNI::Main() { + JNIEnv *env; + JavaVMAttachArgs args; + args.version = JNI_VERSION_1_2; + args.name = const_cast("Interrupt"); + args.group = nullptr; + jint rs = jvm->AttachCurrentThreadAsDaemon((void**)&env, &args); + if (rs != JNI_OK) return; + + std::unique_lock lock(m_mutex); + while (m_active) { + m_cond.wait(lock, [&] { return !m_active || m_notify; }); + if (!m_active) break; + m_notify = false; + if (!m_func) continue; + jobject func = m_func; + jmethodID mid = m_mid; + uint32_t mask = m_mask; + jobject param = m_param; + lock.unlock(); // don't hold mutex during callback execution + env->CallVoidMethod(func, mid, (jint)mask, param); + if (env->ExceptionCheck()) { + env->ExceptionDescribe(); + env->ExceptionClear(); + } + lock.lock(); + } + + // free global references + if (m_func) env->DeleteGlobalRef(m_func); + if (m_param) env->DeleteGlobalRef(m_param); + + jvm->DetachCurrentThread(); +} + +void interruptHandler(uint32_t mask, void* param) { + ((InterruptJNI*)param)->Notify(mask); +} + extern "C" { /* @@ -22,18 +115,18 @@ extern "C" { JNIEXPORT jlong JNICALL Java_edu_wpi_first_wpilibj_hal_InterruptJNI_initializeInterrupts (JNIEnv * env, jclass, jint interruptIndex, jboolean watcher) { - INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI initializeInterrupts"; - INTERRUPTJNI_LOG(logDEBUG) << "interruptIndex = " << interruptIndex; - INTERRUPTJNI_LOG(logDEBUG) << "watcher = " << (bool) watcher; + INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI initializeInterrupts"; + INTERRUPTJNI_LOG(logDEBUG) << "interruptIndex = " << interruptIndex; + INTERRUPTJNI_LOG(logDEBUG) << "watcher = " << (bool)watcher; - int32_t status = 0; - void* interrupt = initializeInterrupts(interruptIndex, watcher, &status); + int32_t status = 0; + void* interrupt = initializeInterrupts(interruptIndex, watcher, &status); - INTERRUPTJNI_LOG(logDEBUG) << "Interrupt Ptr = " << interrupt; - INTERRUPTJNI_LOG(logDEBUG) << "Status = " << status; + INTERRUPTJNI_LOG(logDEBUG) << "Interrupt Ptr = " << interrupt; + INTERRUPTJNI_LOG(logDEBUG) << "Status = " << status; - CheckStatus(env, status); - return (jlong)interrupt; + CheckStatus(env, status); + return (jlong)interrupt; } @@ -45,15 +138,15 @@ JNIEXPORT jlong JNICALL Java_edu_wpi_first_wpilibj_hal_InterruptJNI_initializeIn JNIEXPORT void JNICALL Java_edu_wpi_first_wpilibj_hal_InterruptJNI_cleanInterrupts (JNIEnv * env, jclass, jlong interrupt_pointer) { - INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI cleanInterrupts"; - INTERRUPTJNI_LOG(logDEBUG) << "Interrupt Ptr = " << (void*)interrupt_pointer; + INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI cleanInterrupts"; + INTERRUPTJNI_LOG(logDEBUG) << "Interrupt Ptr = " << (void*)interrupt_pointer; - int32_t status = 0; - cleanInterrupts((void*)interrupt_pointer, &status); + int32_t status = 0; + cleanInterrupts((void*)interrupt_pointer, &status); - INTERRUPTJNI_LOG(logDEBUG) << "Status = " << status; + INTERRUPTJNI_LOG(logDEBUG) << "Status = " << status; - CheckStatus(env, status); + CheckStatus(env, status); } /* @@ -64,16 +157,17 @@ JNIEXPORT void JNICALL Java_edu_wpi_first_wpilibj_hal_InterruptJNI_cleanInterrup JNIEXPORT int JNICALL Java_edu_wpi_first_wpilibj_hal_InterruptJNI_waitForInterrupt (JNIEnv * env, jclass, jlong interrupt_pointer, jdouble timeout, jboolean ignorePrevious) { - INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI waitForInterrupt"; - INTERRUPTJNI_LOG(logDEBUG) << "Interrupt Ptr = " << (void*)interrupt_pointer; + INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI waitForInterrupt"; + INTERRUPTJNI_LOG(logDEBUG) << "Interrupt Ptr = " << (void*)interrupt_pointer; - int32_t status = 0; - int result = waitForInterrupt((void*)interrupt_pointer, timeout, ignorePrevious, &status); + int32_t status = 0; + int result = waitForInterrupt((void*)interrupt_pointer, timeout, + ignorePrevious, &status); - INTERRUPTJNI_LOG(logDEBUG) << "Status = " << status; + INTERRUPTJNI_LOG(logDEBUG) << "Status = " << status; - CheckStatus(env, status); - return result; + CheckStatus(env, status); + return result; } /* @@ -84,15 +178,15 @@ JNIEXPORT int JNICALL Java_edu_wpi_first_wpilibj_hal_InterruptJNI_waitForInterru JNIEXPORT void JNICALL Java_edu_wpi_first_wpilibj_hal_InterruptJNI_enableInterrupts (JNIEnv * env, jclass, jlong interrupt_pointer) { - INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI enableInterrupts"; - INTERRUPTJNI_LOG(logDEBUG) << "Interrupt Ptr = " << (void*)interrupt_pointer; + INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI enableInterrupts"; + INTERRUPTJNI_LOG(logDEBUG) << "Interrupt Ptr = " << (void*)interrupt_pointer; - int32_t status = 0; - enableInterrupts((void*)interrupt_pointer, &status); + int32_t status = 0; + enableInterrupts((void*)interrupt_pointer, &status); - INTERRUPTJNI_LOG(logDEBUG) << "Status = " << status; + INTERRUPTJNI_LOG(logDEBUG) << "Status = " << status; - CheckStatus(env, status); + CheckStatus(env, status); } /* @@ -103,15 +197,15 @@ JNIEXPORT void JNICALL Java_edu_wpi_first_wpilibj_hal_InterruptJNI_enableInterru JNIEXPORT void JNICALL Java_edu_wpi_first_wpilibj_hal_InterruptJNI_disableInterrupts (JNIEnv * env, jclass, jlong interrupt_pointer) { - INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI disableInterrupts"; - INTERRUPTJNI_LOG(logDEBUG) << "Interrupt Ptr = " << (void*)interrupt_pointer; + INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI disableInterrupts"; + INTERRUPTJNI_LOG(logDEBUG) << "Interrupt Ptr = " << (void*)interrupt_pointer; - int32_t status = 0; - disableInterrupts((void*)interrupt_pointer, &status); + int32_t status = 0; + disableInterrupts((void*)interrupt_pointer, &status); - INTERRUPTJNI_LOG(logDEBUG) << "Status = " << status; + INTERRUPTJNI_LOG(logDEBUG) << "Status = " << status; - CheckStatus(env, status); + CheckStatus(env, status); } /* @@ -122,15 +216,15 @@ JNIEXPORT void JNICALL Java_edu_wpi_first_wpilibj_hal_InterruptJNI_disableInterr JNIEXPORT jdouble JNICALL Java_edu_wpi_first_wpilibj_hal_InterruptJNI_readRisingTimestamp (JNIEnv * env, jclass, jlong interrupt_pointer) { - INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI readRisingTimestamp"; - INTERRUPTJNI_LOG(logDEBUG) << "Interrupt Ptr = " << (void*)interrupt_pointer; + INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI readRisingTimestamp"; + INTERRUPTJNI_LOG(logDEBUG) << "Interrupt Ptr = " << (void*)interrupt_pointer; - int32_t status = 0; - jdouble timeStamp = readRisingTimestamp((void*)interrupt_pointer, &status); + int32_t status = 0; + jdouble timeStamp = readRisingTimestamp((void*)interrupt_pointer, &status); - INTERRUPTJNI_LOG(logDEBUG) << "Status = " << status; - CheckStatus(env, status); - return timeStamp; + INTERRUPTJNI_LOG(logDEBUG) << "Status = " << status; + CheckStatus(env, status); + return timeStamp; } /* @@ -141,15 +235,15 @@ JNIEXPORT jdouble JNICALL Java_edu_wpi_first_wpilibj_hal_InterruptJNI_readRising JNIEXPORT jdouble JNICALL Java_edu_wpi_first_wpilibj_hal_InterruptJNI_readFallingTimestamp (JNIEnv * env, jclass, jlong interrupt_pointer) { - INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI readFallingTimestamp"; - INTERRUPTJNI_LOG(logDEBUG) << "Interrupt Ptr = " << (void*)interrupt_pointer; + INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI readFallingTimestamp"; + INTERRUPTJNI_LOG(logDEBUG) << "Interrupt Ptr = " << (void*)interrupt_pointer; - int32_t status = 0; - jdouble timeStamp = readFallingTimestamp((void*)interrupt_pointer, &status); + int32_t status = 0; + jdouble timeStamp = readFallingTimestamp((void*)interrupt_pointer, &status); - INTERRUPTJNI_LOG(logDEBUG) << "Status = " << status; - CheckStatus(env, status); - return timeStamp; + INTERRUPTJNI_LOG(logDEBUG) << "Status = " << status; + CheckStatus(env, status); + return timeStamp; } /* @@ -160,70 +254,18 @@ JNIEXPORT jdouble JNICALL Java_edu_wpi_first_wpilibj_hal_InterruptJNI_readFallin JNIEXPORT void JNICALL Java_edu_wpi_first_wpilibj_hal_InterruptJNI_requestInterrupts (JNIEnv * env, jclass, jlong interrupt_pointer, jbyte routing_module, jint routing_pin, jboolean routing_analog_trigger) { - INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI requestInterrupts"; - INTERRUPTJNI_LOG(logDEBUG) << "Interrupt Ptr = " << (void*)interrupt_pointer; - INTERRUPTJNI_LOG(logDEBUG) << "routing module = " << (jint) routing_module; - INTERRUPTJNI_LOG(logDEBUG) << "routing pin = " << routing_pin; - INTERRUPTJNI_LOG(logDEBUG) << "routing analog trigger = " << (jint) routing_analog_trigger; + INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI requestInterrupts"; + INTERRUPTJNI_LOG(logDEBUG) << "Interrupt Ptr = " << (void*)interrupt_pointer; + INTERRUPTJNI_LOG(logDEBUG) << "routing module = " << (jint)routing_module; + INTERRUPTJNI_LOG(logDEBUG) << "routing pin = " << routing_pin; + INTERRUPTJNI_LOG(logDEBUG) << "routing analog trigger = " << (jint)routing_analog_trigger; - int32_t status = 0; - requestInterrupts((void*)interrupt_pointer, (uint8_t) routing_module, (uint32_t) routing_pin, routing_analog_trigger, &status); + int32_t status = 0; + requestInterrupts((void*)interrupt_pointer, (uint8_t)routing_module, + (uint32_t)routing_pin, routing_analog_trigger, &status); - INTERRUPTJNI_LOG(logDEBUG) << "Status = " << status; - CheckStatus(env, status); -} - - - -struct InterruptHandlerParam { - /* - * The object edu/wpi/first/wpilibj/hal/InterruptJNI/InterruptHandlerFunction - * that contains the callback method [code]mid[/code]. - */ - jobject handler_obj; - - //The method id for the callback method - jmethodID mid; - - //The params passed to the function - jobject param; - - ~InterruptHandlerParam(){ - INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI InterruptHandlerParam Destructor"; - JNIEnv *env; - jint rs = jvm->AttachCurrentThread((void**)&env, NULL); - assert (rs == JNI_OK); - - env->DeleteGlobalRef(handler_obj); - env->DeleteGlobalRef(param); - rs = jvm->DetachCurrentThread(); - assert (rs == JNI_OK); - INTERRUPTJNI_LOG(logDEBUG) << "Leaving INTERRUPTJNI InterruptHandlerParam Destructor"; - } -}; - -void interruptHandler(uint32_t mask, void *data) { - INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI interruptHandler"; - InterruptHandlerParam *param = static_cast(data); - - INTERRUPTJNI_LOG(logDEBUG) << "InterruptHandlerParam Ptr = " << param; - INTERRUPTJNI_LOG(logDEBUG) << "InterruptHandlerParam->obj = " << param->handler_obj; - INTERRUPTJNI_LOG(logDEBUG) << "InterruptHandlerParam->param = " << param->param; - - //Because this is a callback in a new thread we must attach it to the JVM - JNIEnv *env; - jint rs = jvm->AttachCurrentThread((void**)&env, NULL); - assert (rs == JNI_OK); - INTERRUPTJNI_LOG(logDEBUG) << "Attached to thread"; - - env->CallVoidMethod(param->handler_obj, param->mid, mask, param->param); - if (env->ExceptionCheck()) { - env->ExceptionDescribe(); - } - - rs = jvm->DetachCurrentThread(); - assert (rs == JNI_OK); - INTERRUPTJNI_LOG(logDEBUG) << "Leaving INTERRUPTJNI interruptHandler"; + INTERRUPTJNI_LOG(logDEBUG) << "Status = " << status; + CheckStatus(env, status); } /* @@ -234,43 +276,36 @@ void interruptHandler(uint32_t mask, void *data) { JNIEXPORT void JNICALL Java_edu_wpi_first_wpilibj_hal_InterruptJNI_attachInterruptHandler (JNIEnv * env, jclass, jlong interrupt_pointer, jobject handler, jobject param) { - INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI attachInterruptHandler"; - INTERRUPTJNI_LOG(logDEBUG) << "Interrupt Ptr = " << (void*)interrupt_pointer; + INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI attachInterruptHandler"; + INTERRUPTJNI_LOG(logDEBUG) << "Interrupt Ptr = " << (void*)interrupt_pointer; - //Store the interrupt callback paramaters - InterruptHandlerParam *interruptHandlerParam = new InterruptHandlerParam(); - //Stores the object that contains the callback - interruptHandlerParam->handler_obj = env->NewGlobalRef(handler); - //The parameter that will be passed back to the JVM when the method is called - interruptHandlerParam->param = env->NewGlobalRef(param); + jclass cls = env->GetObjectClass(handler); + INTERRUPTJNI_LOG(logDEBUG) << "class = " << cls; + if (cls == 0) { + INTERRUPTJNI_LOG(logERROR) << "Error getting java class"; + assert(false); + return; + } + jmethodID mid = env->GetMethodID(cls, "apply", "(ILjava/lang/Object;)V"); + INTERRUPTJNI_LOG(logDEBUG) << "method = " << mid; + if (mid == 0) { + INTERRUPTJNI_LOG(logERROR) << "Error getting java method ID"; + assert(false); + return; + } - jclass cls = env->GetObjectClass(handler); - INTERRUPTJNI_LOG(logDEBUG) << "class = " << cls; - if (cls == 0) { - INTERRUPTJNI_LOG(logERROR) << "Error getting java class"; - assert (false); - return; - } + InterruptJNI* intr = new InterruptJNI; + intr->Start(); + intr->SetFunc(env, handler, mid, param); - jmethodID mid = env->GetMethodID(cls, "apply", "(ILjava/lang/Object;)V"); - INTERRUPTJNI_LOG(logDEBUG) << "method = " << mid; - if (mid == 0) { - INTERRUPTJNI_LOG(logERROR) << "Error getting java method ID"; - assert (false); - return; - } - interruptHandlerParam->mid = mid; + INTERRUPTJNI_LOG(logDEBUG) << "InterruptThreadJNI Ptr = " << intr; - INTERRUPTJNI_LOG(logDEBUG) << "InterruptHandlerParam Ptr = " << interruptHandlerParam; - INTERRUPTJNI_LOG(logDEBUG) << "InterruptHandlerParam->obj (handler) = " << interruptHandlerParam->handler_obj; - INTERRUPTJNI_LOG(logDEBUG) << "InterruptHandlerParam->mid = " << interruptHandlerParam->mid; - INTERRUPTJNI_LOG(logDEBUG) << "InterruptHandlerParam->param = " << interruptHandlerParam->param; + int32_t status = 0; + attachInterruptHandler((void*)interrupt_pointer, interruptHandler, intr, + &status); - int32_t status = 0; - attachInterruptHandler((void*)interrupt_pointer, interruptHandler, interruptHandlerParam, &status); - - INTERRUPTJNI_LOG(logDEBUG) << "Status = " << status; - CheckStatus(env, status); + INTERRUPTJNI_LOG(logDEBUG) << "Status = " << status; + CheckStatus(env, status); } /* @@ -281,16 +316,17 @@ JNIEXPORT void JNICALL Java_edu_wpi_first_wpilibj_hal_InterruptJNI_attachInterru JNIEXPORT void JNICALL Java_edu_wpi_first_wpilibj_hal_InterruptJNI_setInterruptUpSourceEdge (JNIEnv * env, jclass, jlong interrupt_pointer, jboolean risingEdge, jboolean fallingEdge) { - INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI setInterruptUpSourceEdge"; - INTERRUPTJNI_LOG(logDEBUG) << "Interrupt Ptr = " << (void*)interrupt_pointer; - INTERRUPTJNI_LOG(logDEBUG) << "Rising Edge = " << (bool) risingEdge; - INTERRUPTJNI_LOG(logDEBUG) << "Falling Edge = " << (bool) fallingEdge; + INTERRUPTJNI_LOG(logDEBUG) << "Calling INTERRUPTJNI setInterruptUpSourceEdge"; + INTERRUPTJNI_LOG(logDEBUG) << "Interrupt Ptr = " << (void*)interrupt_pointer; + INTERRUPTJNI_LOG(logDEBUG) << "Rising Edge = " << (bool)risingEdge; + INTERRUPTJNI_LOG(logDEBUG) << "Falling Edge = " << (bool)fallingEdge; - int32_t status = 0; - setInterruptUpSourceEdge((void*)interrupt_pointer, risingEdge, fallingEdge, &status); + int32_t status = 0; + setInterruptUpSourceEdge((void*)interrupt_pointer, risingEdge, fallingEdge, + &status); - INTERRUPTJNI_LOG(logDEBUG) << "Status = " << status; - CheckStatus(env, status); + INTERRUPTJNI_LOG(logDEBUG) << "Status = " << status; + CheckStatus(env, status); } } // extern "C" diff --git a/wpilibj/src/athena/cpp/lib/NotifierJNI.cpp b/wpilibj/src/athena/cpp/lib/NotifierJNI.cpp index 43fe78acf5..34c73aa853 100644 --- a/wpilibj/src/athena/cpp/lib/NotifierJNI.cpp +++ b/wpilibj/src/athena/cpp/lib/NotifierJNI.cpp @@ -1,11 +1,16 @@ #include #include +#include +#include #include +#include +#include #include #include "Log.hpp" #include "edu_wpi_first_wpilibj_hal_NotifierJNI.h" #include "HAL/Notifier.hpp" #include "HALUtil.h" +#include "SafeThread.h" // set the logging level TLogLevel notifierJNILogLevel = logWARNING; @@ -14,44 +19,86 @@ TLogLevel notifierJNILogLevel = logWARNING; if (level > notifierJNILogLevel) ; \ else Log().Get(level) -// These two are used to pass information to the notifierHandler without using -// up function parameters. -// See below for more information. -static jobject func_global; -static jmethodID mid_global; +// Thread where callbacks are actually performed. +// +// JNI's AttachCurrentThread() creates a Java Thread object on every +// invocation, which is both time inefficient and causes issues with Eclipse +// (which tries to keep a thread list up-to-date and thus gets swamped). +// +// Instead, this class attaches just once. When a hardware notification +// occurs, a condition variable wakes up this thread and this thread actually +// makes the call into Java. +// +// We don't want to use a FIFO here. If the user code takes too long to +// process, we will just ignore the redundant wakeup. +class NotifierThreadJNI : public SafeThread { + public: + void Main(); + + bool m_notify = false; + jobject m_func = nullptr; + jmethodID m_mid; + uint32_t m_currentTime; +}; + +class NotifierJNI : public SafeThreadOwner { + public: + void SetFunc(JNIEnv* env, jobject func, jmethodID mid); + + void Notify(uint32_t currentTime) { + auto thr = GetThread(); + if (!thr) return; + thr->m_currentTime = currentTime; + thr->m_notify = true; + thr->m_cond.notify_one(); + } +}; + +void NotifierJNI::SetFunc(JNIEnv* env, jobject func, jmethodID mid) { + auto thr = GetThread(); + if (!thr) return; + // free global reference + if (thr->m_func) env->DeleteGlobalRef(thr->m_func); + // create global reference + thr->m_func = env->NewGlobalRef(func); + thr->m_mid = mid; +} + +void NotifierThreadJNI::Main() { + JNIEnv *env; + JavaVMAttachArgs args; + args.version = JNI_VERSION_1_2; + args.name = const_cast("Notifier"); + args.group = nullptr; + jint rs = jvm->AttachCurrentThreadAsDaemon((void**)&env, &args); + if (rs != JNI_OK) return; + + std::unique_lock lock(m_mutex); + while (m_active) { + m_cond.wait(lock, [&] { return !m_active || m_notify; }); + if (!m_active) break; + m_notify = false; + if (!m_func) continue; + jobject func = m_func; + jmethodID mid = m_mid; + uint32_t currentTime = m_currentTime; + lock.unlock(); // don't hold mutex during callback execution + env->CallVoidMethod(func, mid, (jint)currentTime); + if (env->ExceptionCheck()) { + env->ExceptionDescribe(); + env->ExceptionClear(); + } + lock.lock(); + } + + // free global reference + if (m_func) env->DeleteGlobalRef(m_func); + + jvm->DetachCurrentThread(); +} -// The arguments are unused by the HAL Notifier; they just satisfy a particular -// function signature. void notifierHandler(uint32_t currentTimeInt, void* param) { - jobject handler_obj = func_global; - jmethodID mid = mid_global; - - NOTIFIERJNI_LOG(logDEBUG) << "Calling NOTIFIERJNI notifierHandler"; - - //Because this is a callback in a new thread we must attach it to the JVM. - JNIEnv *env; - jint rs; - // Check to see if we are already part of a JVM thread or if we need to attach - // ourselves. - int getEnvStat = jvm->GetEnv((void **)&env, JNI_VERSION_1_8); - if (getEnvStat == JNI_EDETACHED) { - rs = jvm->AttachCurrentThread((void**)&env, NULL); - assert (rs == JNI_OK); - } - NOTIFIERJNI_LOG(logDEBUG) << "Attached to thread. Object is: " << handler_obj; - - // Actuall call the user function. - env->CallVoidMethod(handler_obj, mid); - if (env->ExceptionCheck()) { - env->ExceptionDescribe(); - } - - // Only detach if we needed to attach oursleves in the first place. - if (getEnvStat == JNI_EDETACHED) { - rs = jvm->DetachCurrentThread(); - assert (rs == JNI_OK); - } - NOTIFIERJNI_LOG(logDEBUG) << "Leaving NOTIFIERJNI notifierHandler"; + ((NotifierJNI*)param)->Notify(currentTimeInt); } extern "C" { @@ -64,31 +111,38 @@ extern "C" { JNIEXPORT jlong JNICALL Java_edu_wpi_first_wpilibj_hal_NotifierJNI_initializeNotifier (JNIEnv *env, jclass, jobject func) { - NOTIFIERJNI_LOG(logDEBUG) << "Calling NOTIFIERJNI initializeNotifier"; + NOTIFIERJNI_LOG(logDEBUG) << "Calling NOTIFIERJNI initializeNotifier"; jclass cls = env->GetObjectClass(func); - jmethodID mid = env->GetMethodID(cls, "run", "()V"); + if (cls == 0) { + NOTIFIERJNI_LOG(logERROR) << "Error getting java class"; + assert(false); + return 0; + } + jmethodID mid = env->GetMethodID(cls, "apply", "(I)V"); + if (mid == 0) { + NOTIFIERJNI_LOG(logERROR) << "Error getting java method ID"; + assert(false); + return 0; + } - // In order to pass the user's Runnable to the notifierHandler, we have to use - // something other than the function arguments (because the function arguments - // are dictated by the callback format). As such, we instead use a couple of - // global variables to pass around the object reference. - // This is not ideal, but the only other option that came to mind was to use - // lambda function captures, but, unfortunately, it turns out that using - // captures in a lambda changes the function signature such that it can no - // long be used as a standared C-style function pointer. - // Need to set as global ref to avoid seg faults when referring to it later. - func_global = env->NewGlobalRef(func); - mid_global = mid; + // each notifier runs in its own thread; this is so if one takes too long + // to execute, it doesn't keep the others from running + NotifierJNI* notify = new NotifierJNI; + notify->Start(); + notify->SetFunc(env, func, mid); + int32_t status = 0; + void *notifierPtr = initializeNotifier(notifierHandler, notify, &status); - int32_t status = 0; - void *notifierPtr = initializeNotifier(notifierHandler, nullptr, &status); + NOTIFIERJNI_LOG(logDEBUG) << "Notifier Ptr = " << notifierPtr; + NOTIFIERJNI_LOG(logDEBUG) << "Status = " << status; - NOTIFIERJNI_LOG(logDEBUG) << "Notifier Ptr = " << notifierPtr; - NOTIFIERJNI_LOG(logDEBUG) << "Status = " << status; + if (!notifierPtr || !CheckStatus(env, status)) { + // something went wrong in HAL, clean up + delete notify; + } - CheckStatus(env, status); - return (jlong)notifierPtr; + return (jlong)notifierPtr; } /* @@ -99,14 +153,17 @@ JNIEXPORT jlong JNICALL Java_edu_wpi_first_wpilibj_hal_NotifierJNI_initializeNot JNIEXPORT void JNICALL Java_edu_wpi_first_wpilibj_hal_NotifierJNI_cleanNotifier (JNIEnv *env, jclass, jlong notifierPtr) { - NOTIFIERJNI_LOG(logDEBUG) << "Calling NOTIFIERJNI cleanNotifier"; + NOTIFIERJNI_LOG(logDEBUG) << "Calling NOTIFIERJNI cleanNotifier"; - NOTIFIERJNI_LOG(logDEBUG) << "Notifier Ptr = " << (void*)notifierPtr; + NOTIFIERJNI_LOG(logDEBUG) << "Notifier Ptr = " << (void *)notifierPtr; int32_t status = 0; + NotifierJNI* notify = + (NotifierJNI*)getNotifierParam((void*)notifierPtr, &status); cleanNotifier((void*)notifierPtr, &status); - NOTIFIERJNI_LOG(logDEBUG) << "Status = " << status; - CheckStatus(env, status); + NOTIFIERJNI_LOG(logDEBUG) << "Status = " << status; + CheckStatus(env, status); + delete notify; } /* @@ -117,16 +174,34 @@ JNIEXPORT void JNICALL Java_edu_wpi_first_wpilibj_hal_NotifierJNI_cleanNotifier JNIEXPORT void JNICALL Java_edu_wpi_first_wpilibj_hal_NotifierJNI_updateNotifierAlarm (JNIEnv *env, jclass cls, jlong notifierPtr, jint triggerTime) { - NOTIFIERJNI_LOG(logDEBUG) << "Calling NOTIFIERJNI updateNotifierAlarm"; + NOTIFIERJNI_LOG(logDEBUG) << "Calling NOTIFIERJNI updateNotifierAlarm"; - NOTIFIERJNI_LOG(logDEBUG) << "Notifier Ptr = " << (void*)notifierPtr; + NOTIFIERJNI_LOG(logDEBUG) << "Notifier Ptr = " << (void *)notifierPtr; - NOTIFIERJNI_LOG(logDEBUG) << "triggerTime Ptr = " << &triggerTime; + NOTIFIERJNI_LOG(logDEBUG) << "triggerTime = " << triggerTime; int32_t status = 0; updateNotifierAlarm((void*)notifierPtr, (uint32_t)triggerTime, &status); - NOTIFIERJNI_LOG(logDEBUG) << "Status = " << status; - CheckStatus(env, status); + NOTIFIERJNI_LOG(logDEBUG) << "Status = " << status; + CheckStatus(env, status); +} + +/* + * Class: edu_wpi_first_wpilibj_hal_NotifierJNI + * Method: stopNotifierAlarm + * Signature: (J)V + */ +JNIEXPORT void JNICALL Java_edu_wpi_first_wpilibj_hal_NotifierJNI_stopNotifierAlarm + (JNIEnv *env, jclass cls, jlong notifierPtr) +{ + NOTIFIERJNI_LOG(logDEBUG) << "Calling NOTIFIERJNI stopNotifierAlarm"; + + NOTIFIERJNI_LOG(logDEBUG) << "Notifier Ptr = " << (void *)notifierPtr; + + int32_t status = 0; + stopNotifierAlarm((void*)notifierPtr, &status); + NOTIFIERJNI_LOG(logDEBUG) << "Status = " << status; + CheckStatus(env, status); } } // extern "C" diff --git a/wpilibj/src/athena/cpp/lib/SafeThread.cpp b/wpilibj/src/athena/cpp/lib/SafeThread.cpp new file mode 100644 index 0000000000..b77299e6e8 --- /dev/null +++ b/wpilibj/src/athena/cpp/lib/SafeThread.cpp @@ -0,0 +1,31 @@ +/*----------------------------------------------------------------------------*/ +/* Copyright (c) FIRST 2015. All Rights Reserved. */ +/* Open Source Software - may be modified and shared by FRC teams. The code */ +/* must be accompanied by the FIRST BSD license file in the root directory of */ +/* the project. */ +/*----------------------------------------------------------------------------*/ + +#include "SafeThread.h" + +//using namespace nt; + +void detail::SafeThreadOwnerBase::Start(SafeThread* thr) { + SafeThread* curthr = nullptr; + SafeThread* newthr = thr; + if (!m_thread.compare_exchange_strong(curthr, newthr)) { + delete newthr; + return; + } + std::thread([=]() { + newthr->Main(); + delete newthr; + }).detach(); +} + +void detail::SafeThreadOwnerBase::Stop() { + SafeThread* thr = m_thread.exchange(nullptr); + if (!thr) return; + std::lock_guard lock(thr->m_mutex); + thr->m_active = false; + thr->m_cond.notify_one(); +} diff --git a/wpilibj/src/athena/cpp/lib/SafeThread.h b/wpilibj/src/athena/cpp/lib/SafeThread.h new file mode 100644 index 0000000000..5af6f76e06 --- /dev/null +++ b/wpilibj/src/athena/cpp/lib/SafeThread.h @@ -0,0 +1,93 @@ +/*----------------------------------------------------------------------------*/ +/* Copyright (c) FIRST 2015. All Rights Reserved. */ +/* Open Source Software - may be modified and shared by FRC teams. The code */ +/* must be accompanied by the FIRST BSD license file in the root directory of */ +/* the project. */ +/*----------------------------------------------------------------------------*/ + +#ifndef NT_SAFETHREAD_H_ +#define NT_SAFETHREAD_H_ + +#include +#include +#include +#include + +//namespace nt { + +// Base class for SafeThreadOwner threads. +class SafeThread { + public: + virtual ~SafeThread() = default; + virtual void Main() = 0; + + std::mutex m_mutex; + bool m_active = true; + std::condition_variable m_cond; +}; + +namespace detail { + +// Non-template proxy base class for common proxy code. +class SafeThreadProxyBase { + public: + SafeThreadProxyBase(SafeThread* thr) : m_thread(thr) { + if (!m_thread) return; + std::unique_lock(m_thread->m_mutex).swap(m_lock); + if (!m_thread->m_active) { + m_lock.unlock(); + m_thread = nullptr; + return; + } + } + explicit operator bool() const { return m_thread != nullptr; } + std::unique_lock& GetLock() { return m_lock; } + + protected: + SafeThread* m_thread; + std::unique_lock m_lock; +}; + +// A proxy for SafeThread. +// Also serves as a scoped lock on SafeThread::m_mutex. +template +class SafeThreadProxy : public SafeThreadProxyBase { + public: + SafeThreadProxy(SafeThread* thr) : SafeThreadProxyBase(thr) {} + T& operator*() const { return *static_cast(m_thread); } + T* operator->() const { return static_cast(m_thread); } +}; + +// Non-template owner base class for common owner code. +class SafeThreadOwnerBase { + public: + void Stop(); + + protected: + SafeThreadOwnerBase() { m_thread = nullptr; } + SafeThreadOwnerBase(const SafeThreadOwnerBase&) = delete; + SafeThreadOwnerBase& operator=(const SafeThreadOwnerBase&) = delete; + ~SafeThreadOwnerBase() { Stop(); } + + void Start(SafeThread* thr); + SafeThread* GetThread() { return m_thread.load(); } + + private: + std::atomic m_thread; +}; + +} // namespace detail + +template +class SafeThreadOwner : public detail::SafeThreadOwnerBase { + public: + void Start() { Start(new T); } + void Start(T* thr) { detail::SafeThreadOwnerBase::Start(thr); } + + using Proxy = typename detail::SafeThreadProxy; + Proxy GetThread() { return Proxy(detail::SafeThreadOwnerBase::GetThread()); } +}; + +//} // namespace nt + +#endif // NT_SAFETHREAD_H_ diff --git a/wpilibj/src/athena/java/edu/wpi/first/wpilibj/Notifier.java b/wpilibj/src/athena/java/edu/wpi/first/wpilibj/Notifier.java index 492a9abadc..502fb2727e 100644 --- a/wpilibj/src/athena/java/edu/wpi/first/wpilibj/Notifier.java +++ b/wpilibj/src/athena/java/edu/wpi/first/wpilibj/Notifier.java @@ -9,228 +9,118 @@ import edu.wpi.first.wpilibj.Utility; public class Notifier { - static private class ProcessQueue implements Runnable { - public void run() { - Notifier current; - while (true) { - Notifier.queueLock.lock(); + private static class Process implements NotifierJNI.NotifierJNIHandlerFunction { + // The lock for the process information. + private ReentrantLock m_processLock = new ReentrantLock(); + // The C pointer to the notifier object. We don't use it directly, it is + // just passed to the JNI bindings. + private long m_notifier; + // The time, in microseconds, at which the corresponding handler should be + // called. Has the same zero as Utility.getFPGATime(). + private double m_expirationTime = 0; + // The handler passed in by the user which should be called at the + // appropriate interval. + private Runnable m_handler; + // Whether we are calling the handler just once or periodically. + private boolean m_periodic = false; + // If periodic, the period of the calling; if just once, stores how long it + // is until we call the handler. + private double m_period = 0; + // Lock on the handler so that the handler is not called before it has + // completed. This is only relevant if the handler takes a very long time + // to complete (or the period is very short) and when everything is being + // destructed. + private ReentrantLock m_handlerLock = new ReentrantLock(); - double currentTime = Utility.getFPGATime() * 1e-6; - current = Notifier.timerQueueHead; - - if (current == null || current.m_expirationTime > currentTime) { - Notifier.queueLock.unlock(); - break; - } - - Notifier.timerQueueHead = current.m_nextEvent; - - if (current.m_periodic) { - current.insertInQueue(true); - } else { - current.m_queued = false; - } - - current.m_handlerLock.lock(); - Notifier.queueLock.unlock(); - - current.m_handler.run(); - current.m_handlerLock.unlock(); - } - - - Notifier.queueLock.lock(); - Notifier.updateAlarm(); - Notifier.queueLock.unlock(); + public Process(Runnable run) { + m_handler = run; + m_notifier = NotifierJNI.initializeNotifier(this); } + @Override + protected void finalize() { + NotifierJNI.cleanNotifier(m_notifier); + m_handlerLock.lock(); + m_handlerLock = null; + } + + /** + * Update the alarm hardware to reflect the next alarm. + */ + private void updateAlarm() { + NotifierJNI.updateNotifierAlarm(m_notifier, (int) (m_expirationTime * 1e6)); + } + + /** + * Handler which is called by the HAL library; it handles the subsequent + * calling of the user handler. + */ + @Override + public void apply(int time) { + m_processLock.lock(); + if (m_periodic) { + m_expirationTime += m_period; + updateAlarm(); + } + + m_handlerLock.lock(); + m_processLock.unlock(); + + m_handler.run(); + m_handlerLock.unlock(); + } + + public void start(double period, boolean periodic) { + synchronized (m_processLock) { + m_periodic = periodic; + m_period = period; + m_expirationTime = Utility.getFPGATime() * 1e-6 + m_period; + updateAlarm(); + } + } + + public void stop() { + NotifierJNI.stopNotifierAlarm(m_notifier); + + // Wait for a currently executing handler to complete before returning + // from stop() + m_handlerLock.lock(); + m_handlerLock.unlock(); + } } - // Maximum time, in seconds, that the FPGA returns before rolling over to 0. - static private final double kRolloverTime = (1l << 32) / 1e6; - // Number of instances of Notifier classes created, so that we can call - // cleanNotifier() after all the Notifiers are stopped. - static private int refcount = 0; - // The next Notifier instance which needs to be called. - static private Notifier timerQueueHead = null; - // The C pointer to the notifier object. We don't use it directly, it is just - // passed to the JNI bindings. - private static long m_notifier; - // The lock for the queue information (namely, timerQueueHead and the - // m_nextEvent members). - private static ReentrantLock queueLock = new ReentrantLock(); - // The handler which is called by the HAL library; it handles the subsequent - // calling of the user handlers. - // This is the only Runnable actually passed to the JNI bindings. - private static ProcessQueue m_processQueue; - - // The next Notifier whose handler will need to be called after this one. - private Notifier m_nextEvent = null; - // The time, in microseconds, at which the corresponding handler should be - // called. Has the same zero as Utility.getFPGATime(). - private double m_expirationTime = 0; - // The handler passed in by the user which should be called at the appropriate - // interval. - private Runnable m_handler; - // Whether we are calling the handler just once or periodically. - private boolean m_periodic = false; - // If periodic, the period of the calling; if just once, stores how long it - // is until we call the handler. - private double m_period = 0; - // Whether we are currently queued to be called at m_expirationTime. - private boolean m_queued = false; - // Lock on the handler so that the handler is not called before it has - // completed. This is only relevant if the handler takes a very long time to - // complete (or the period is very short) and when everything is being - // destructed. - private ReentrantLock m_handlerLock = new ReentrantLock(); + private Process m_process; /** * Create a Notifier for timer event notification. - *$ + * * @param run The handler that is called at the notification time which is set * using StartSingle or StartPeriodic. */ public Notifier(Runnable run) { - if (refcount == 0) { - init(); - } - refcount += 1; - m_handler = run; - } - - protected void finalize() { - queueLock.lock(); - - deleteFromQueue(); - - // If this was the last instance of a Notifier, clean up after ourselves. - if ((--refcount) == 0) { - NotifierJNI.cleanNotifier(m_notifier); - } - - queueLock.unlock(); - - m_handlerLock.lock(); - m_handlerLock = null; - } - - /** - * Update the alarm hardware to reflect the current first element in the - * queue. Compute the time the next alarm should occur based on the current - * time and the period for the first element in the timer queue. WARNING: this - * method does not do synchronization! It must be called from somewhere that - * is taking care of synchronizing access to the queue. - */ - static protected void updateAlarm() { - if (timerQueueHead != null) { - NotifierJNI.updateNotifierAlarm(m_notifier, (int) (timerQueueHead.m_expirationTime * 1e6)); - } - } - - /** - * Insert this Notifier into the timer queue in right place. WARNING: this - * method does not do synchronization! It must be called from somewhere that - * is taking care of synchronizing access to the queue. - *$ - * @param reschedule If false, the scheduled alarm is based on the current - * time and UpdateAlarm method is called which will enable the alarm if - * necessary. If true, update the time by adding the period (no drift) - * when rescheduled periodic from ProcessQueue. This ensures that the - * public methods only update the queue after finishing inserting. - */ - protected void insertInQueue(boolean reschedule) { - if (reschedule) { - m_expirationTime += m_period; - } else { - m_expirationTime = Utility.getFPGATime() * 1e-6 + m_period; - } - - if (m_expirationTime > kRolloverTime) { - m_expirationTime -= kRolloverTime; - } - - if (timerQueueHead == null || timerQueueHead.m_expirationTime >= this.m_expirationTime) { - // the queue is empty or greater than the new entry - // the new entry becomes the first element - this.m_nextEvent = timerQueueHead; - timerQueueHead = this; - - if (!reschedule) { - // since the first element changed, update alarm, unless we already plan - // to - updateAlarm(); - } - } else { - for (Notifier n = timerQueueHead;; n = n.m_nextEvent) { - if (n.m_nextEvent == null || n.m_nextEvent.m_expirationTime > this.m_expirationTime) { - this.m_nextEvent = n.m_nextEvent; - n.m_nextEvent = this; - break; - } - } - } - - m_queued = true; - } - - /** - * Delete this Notifier from the timer queue. WARNING: this method does not do - * synchronization! It must be called from somewhere that is taking care of - * synchronizing access to the queue. Remove this Notifier from the timer - * queue and adjust the next interrupt time to reflect the current top of the - * queue. - */ - private void deleteFromQueue() { - if (m_queued) { - m_queued = false; - assert (timerQueueHead != null); - if (timerQueueHead == this) { - // removing the first item in the list - update the alarm - timerQueueHead = this.m_nextEvent; - updateAlarm(); - } else { - for (Notifier n = timerQueueHead; n != null; n = n.m_nextEvent) { - if (n.m_nextEvent == this) { - // this element is the next element from *n from the queue - // Point n around this. - n.m_nextEvent = this.m_nextEvent; - } - } - } - } + m_process = new Process(run); } /** * Register for single event notification. A timer event is queued for a * single event after the specified delay. - *$ + * * @param delay Seconds to wait before the handler is called. */ public void startSingle(double delay) { - queueLock.lock(); - m_periodic = false; - m_period = delay; - deleteFromQueue(); - insertInQueue(false); - queueLock.unlock(); + m_process.start(delay, false); } /** * Register for periodic event notification. A timer event is queued for * periodic event notification. Each time the interrupt occurs, the event will * be immediately requeued for the same time interval. - *$ + * * @param period Period in seconds to call the handler starting one period * after the call to this method. */ public void startPeriodic(double period) { - queueLock.lock(); - m_periodic = true; - m_period = period; - deleteFromQueue(); - insertInQueue(false); - queueLock.unlock(); + m_process.start(period, true); } /** @@ -240,19 +130,6 @@ public class Notifier { * function will block until the handler call is complete. */ public void stop() { - queueLock.lock(); - deleteFromQueue(); - queueLock.unlock(); - - // Wait for a currently executing handler to complete before returning from - // stop() - m_handlerLock.lock(); - m_handlerLock.unlock(); - } - - // First time init. - protected static void init() { - m_processQueue = new ProcessQueue(); - m_notifier = NotifierJNI.initializeNotifier(m_processQueue); + m_process.stop(); } } diff --git a/wpilibj/src/athena/java/edu/wpi/first/wpilibj/hal/NotifierJNI.java b/wpilibj/src/athena/java/edu/wpi/first/wpilibj/hal/NotifierJNI.java index 8ac72daa4d..f3c3df2046 100644 --- a/wpilibj/src/athena/java/edu/wpi/first/wpilibj/hal/NotifierJNI.java +++ b/wpilibj/src/athena/java/edu/wpi/first/wpilibj/hal/NotifierJNI.java @@ -11,11 +11,16 @@ import java.lang.Runtime; */ public class NotifierJNI extends JNIWrapper { /** - * Initializes the notifier to call the run() function of a Runnable. - * - * Should be called after initializeNotifierJVM(). + * Callback function */ - public static native long initializeNotifier(Runnable func); + public interface NotifierJNIHandlerFunction { + void apply(int curTime); + } + + /** + * Initializes the notifier. + */ + public static native long initializeNotifier(NotifierJNIHandlerFunction func); /** * Deletes the notifier object when we are done with it. @@ -26,4 +31,9 @@ public class NotifierJNI extends JNIWrapper { * Sets the notifier to call the callback in another triggerTime microseconds. */ public static native void updateNotifierAlarm(long notifierPtr, int triggerTime); + + /** + * Tells the notifier to stop calling the callback. + */ + public static native void stopNotifierAlarm(long notifierPtr); }