From 12e2043b7779fc3594dca4371d41c832bb324c2b Mon Sep 17 00:00:00 2001 From: Tyler Veness Date: Fri, 15 Sep 2023 19:59:03 -0700 Subject: [PATCH] [wpilib] Clean up Notifier (#5630) The user-facing docs were simplified, SetHandler() was renamed to SetCallback(), and the internal documentation was synchronized. --- wpilibc/src/main/native/cpp/Notifier.cpp | 51 +++++---- .../src/main/native/include/frc/Notifier.h | 102 +++++++++-------- .../java/edu/wpi/first/wpilibj/Notifier.java | 107 +++++++++++------- 3 files changed, 148 insertions(+), 112 deletions(-) diff --git a/wpilibc/src/main/native/cpp/Notifier.cpp b/wpilibc/src/main/native/cpp/Notifier.cpp index d8377527b7..147beb4003 100644 --- a/wpilibc/src/main/native/cpp/Notifier.cpp +++ b/wpilibc/src/main/native/cpp/Notifier.cpp @@ -17,11 +17,11 @@ using namespace frc; -Notifier::Notifier(std::function handler) { - if (!handler) { - throw FRC_MakeError(err::NullParameter, "handler"); +Notifier::Notifier(std::function callback) { + if (!callback) { + throw FRC_MakeError(err::NullParameter, "callback"); } - m_handler = handler; + m_callback = callback; int32_t status = 0; m_notifier = HAL_InitializeNotifier(&status); FRC_CheckErrorStatus(status, "InitializeNotifier"); @@ -38,32 +38,32 @@ Notifier::Notifier(std::function handler) { break; } - std::function handler; + std::function callback; { std::scoped_lock lock(m_processMutex); - handler = m_handler; + callback = m_callback; if (m_periodic) { m_expirationTime += m_period; UpdateAlarm(); } else { - // need to update the alarm to cause it to wait again + // Need to update the alarm to cause it to wait again UpdateAlarm(UINT64_MAX); } } - // call callback - if (handler) { - handler(); + // Call callback + if (callback) { + callback(); } } }); } -Notifier::Notifier(int priority, std::function handler) { - if (!handler) { - throw FRC_MakeError(err::NullParameter, "handler"); +Notifier::Notifier(int priority, std::function callback) { + if (!callback) { + throw FRC_MakeError(err::NullParameter, "callback"); } - m_handler = handler; + m_callback = callback; int32_t status = 0; m_notifier = HAL_InitializeNotifier(&status); FRC_CheckErrorStatus(status, "InitializeNotifier"); @@ -81,10 +81,10 @@ Notifier::Notifier(int priority, std::function handler) { break; } - std::function handler; + std::function callback; { std::scoped_lock lock(m_processMutex); - handler = m_handler; + callback = m_callback; if (m_periodic) { m_expirationTime += m_period; UpdateAlarm(); @@ -95,9 +95,9 @@ Notifier::Notifier(int priority, std::function handler) { } // call callback - if (handler) { + if (callback) { try { - handler(); + callback(); } catch (const frc::RuntimeError& e) { e.Report(); FRC_ReportError( @@ -123,7 +123,7 @@ Notifier::~Notifier() { HAL_StopNotifier(handle, &status); FRC_ReportError(status, "StopNotifier"); - // Join the thread to ensure the handler has exited. + // Join the thread to ensure the callback has exited. if (m_thread.joinable()) { m_thread.join(); } @@ -134,7 +134,7 @@ Notifier::~Notifier() { Notifier::Notifier(Notifier&& rhs) : m_thread(std::move(rhs.m_thread)), m_notifier(rhs.m_notifier.load()), - m_handler(std::move(rhs.m_handler)), + m_callback(std::move(rhs.m_callback)), m_expirationTime(std::move(rhs.m_expirationTime)), m_period(std::move(rhs.m_period)), m_periodic(std::move(rhs.m_periodic)) { @@ -145,7 +145,7 @@ Notifier& Notifier::operator=(Notifier&& rhs) { m_thread = std::move(rhs.m_thread); m_notifier = rhs.m_notifier.load(); rhs.m_notifier = HAL_kInvalidHandle; - m_handler = std::move(rhs.m_handler); + m_callback = std::move(rhs.m_callback); m_expirationTime = std::move(rhs.m_expirationTime); m_period = std::move(rhs.m_period); m_periodic = std::move(rhs.m_periodic); @@ -161,9 +161,14 @@ void Notifier::SetName(std::string_view name) { HAL_SetNotifierName(m_notifier, buf.data(), &status); } -void Notifier::SetHandler(std::function handler) { +void Notifier::SetHandler(std::function callback) { std::scoped_lock lock(m_processMutex); - m_handler = handler; + m_callback = callback; +} + +void Notifier::SetCallback(std::function callback) { + std::scoped_lock lock(m_processMutex); + m_callback = callback; } void Notifier::StartSingle(units::second_t delay) { diff --git a/wpilibc/src/main/native/include/frc/Notifier.h b/wpilibc/src/main/native/include/frc/Notifier.h index 4915aa1d9f..0bd54d46f7 100644 --- a/wpilibc/src/main/native/include/frc/Notifier.h +++ b/wpilibc/src/main/native/include/frc/Notifier.h @@ -20,7 +20,7 @@ namespace frc { /** - * Notifiers run a callback function on a separate thread at a specified period. + * Notifiers run a user-provided callback function on a separate thread. * * If StartSingle() is used, the callback will run once. If StartPeriodic() is * used, the callback will run repeatedly with the given period until stop() is @@ -29,21 +29,25 @@ namespace frc { class Notifier { public: /** - * Create a Notifier for timer event notification. + * Create a Notifier with the given callback. * - * @param handler The handler is called at the notification time which is set - * using StartSingle or StartPeriodic. + * Configure when the callback runs with StartSingle() or StartPeriodic(). + * + * @param callback The callback to run. */ - explicit Notifier(std::function handler); + explicit Notifier(std::function callback); - template - requires std::invocable - Notifier(Callable&& f, Arg&& arg, Args&&... args) - : Notifier(std::bind(std::forward(f), std::forward(arg), + template + Notifier(std::invocable auto&& callback, Arg&& arg, + Args&&... args) + : Notifier(std::bind(std::forward(callback), + std::forward(arg), std::forward(args)...)) {} /** - * Create a Notifier for timer event notification. + * Create a Notifier with the given callback. + * + * Configure when the callback runs with StartSingle() or StartPeriodic(). * * This overload makes the underlying thread run with a real-time priority. * This is useful for reducing scheduling jitter on processes which are @@ -52,17 +56,16 @@ class Notifier { * @param priority The FIFO real-time scheduler priority ([1..99] where a * higher number represents higher priority). See "man 7 * sched" for more details. - * @param handler The handler is called at the notification time which is set - * using StartSingle or StartPeriodic. + * @param callback The callback to run. */ - explicit Notifier(int priority, std::function handler); + explicit Notifier(int priority, std::function callback); - template - requires std::invocable - Notifier(int priority, Callable&& f, Arg&& arg, Args&&... args) - : Notifier(priority, - std::bind(std::forward(f), std::forward(arg), - std::forward(args)...)) {} + template + Notifier(int priority, std::invocable auto&& callback, + Arg&& arg, Args&&... args) + : Notifier(priority, std::bind(std::forward(callback), + std::forward(arg), + std::forward(args)...)) {} /** * Free the resources for a timer event. @@ -73,51 +76,54 @@ class Notifier { Notifier& operator=(Notifier&& rhs); /** - * Sets the name of the notifier. Used for debugging purposes only. + * Sets the name of the notifier. Used for debugging purposes only. * * @param name Name */ void SetName(std::string_view name); /** - * Change the handler function. + * Change the callback function. * - * @param handler Handler + * @param callback The callback function. + * @deprecated Use SetCallback() instead. */ - void SetHandler(std::function handler); + [[deprecated("Use SetCallback() instead.")]] + void SetHandler(std::function callback); /** - * Register for single event notification. + * Change the callback function. * - * A timer event is queued for a single event after the specified delay. + * @param callback The callback function. + */ + void SetCallback(std::function callback); + + /** + * Run the callback once after the given delay. * - * @param delay Amount of time to wait before the handler is called. + * @param delay Time to wait before the callback is called. */ void StartSingle(units::second_t delay); /** - * Register for periodic event notification. + * Run the callback periodically with the given period. * - * 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. + * The user-provided callback should be written so that it completes before + * the next time it's scheduled to run. * - * The user-provided callback should be written in a nonblocking manner so the - * callback can be recalled at the next periodic event notification. - * - * @param period Period to call the handler starting one period - * after the call to this method. + * @param period Period after which to to call the callback starting one + * period after the call to this method. */ void StartPeriodic(units::second_t period); /** - * Stop timer events from occurring. + * Stop further callback invocations. * - * Stop any repeating timer events from occurring. This will also remove any - * single notification events from the queue. + * No further periodic callbacks will occur. Single invocations will also be + * cancelled if they haven't yet occurred. * - * If a timer-based call to the registered handler is in progress, this - * function will block until the handler call is complete. + * If a callback invocation is in progress, this function will block until the + * callback is complete. */ void Stop(); @@ -154,22 +160,24 @@ class Notifier { // The thread waiting on the HAL alarm std::thread m_thread; - // Held while updating process information + // The mutex held while updating process information wpi::mutex m_processMutex; - // HAL handle, atomic for proper destruction + // HAL handle (atomic for proper destruction) std::atomic m_notifier{0}; - // Address of the handler - std::function m_handler; + // The user-provided callback + std::function m_callback; - // The absolute expiration time + // The time at which the callback should be called. Has the same zero as + // Timer::GetFPGATimestamp(). units::second_t m_expirationTime = 0_s; - // The relative time (either periodic or single) + // If periodic, stores the callback period; if single, stores the time until + // the callback call. units::second_t m_period = 0_s; - // True if this is a periodic event + // True if the callback is periodic bool m_periodic = false; }; diff --git a/wpilibj/src/main/java/edu/wpi/first/wpilibj/Notifier.java b/wpilibj/src/main/java/edu/wpi/first/wpilibj/Notifier.java index 25657b398f..7ed61621d5 100644 --- a/wpilibj/src/main/java/edu/wpi/first/wpilibj/Notifier.java +++ b/wpilibj/src/main/java/edu/wpi/first/wpilibj/Notifier.java @@ -11,7 +11,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.ReentrantLock; /** - * Notifiers run a callback function on a separate thread at a specified period. + * Notifiers run a user-provided callback function on a separate thread. * *

If startSingle() is used, the callback will run once. If startPeriodic() is used, the callback * will run repeatedly with the given period until stop() is called. @@ -19,23 +19,27 @@ import java.util.concurrent.locks.ReentrantLock; public class Notifier implements AutoCloseable { // The thread waiting on the HAL alarm. private Thread m_thread; - // The lock for the process information. + + // The lock held while updating process information. private final 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. + + // HAL handle passed to the JNI bindings (atomic for proper destruction). private final AtomicInteger m_notifier = new AtomicInteger(); - // The time, in seconds, at which the corresponding handler should be - // called. Has the same zero as RobotController.getFPGATime(). + + // The user-provided callback. + private Runnable m_callback; + + // The time, in seconds, at which the callback should be called. Has the same + // zero as RobotController.getFPGATime(). private double m_expirationTimeSeconds; - // 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; - // If periodic, the period of the calling; if just once, stores how long it - // is until we call the handler. + + // If periodic, stores the callback period; if single, stores the time until + // the callback call. private double m_periodSeconds; + // True if the callback is periodic + private boolean m_periodic; + @Override public void close() { int handle = m_notifier.getAndSet(0); @@ -43,7 +47,7 @@ public class Notifier implements AutoCloseable { return; } NotifierJNI.stopNotifier(handle); - // Join the thread to ensure the handler has exited. + // Join the thread to ensure the callback has exited. if (m_thread.isAlive()) { try { m_thread.interrupt(); @@ -75,15 +79,16 @@ public class Notifier implements AutoCloseable { } /** - * Create a Notifier for timer event notification. + * Create a Notifier with the given callback. * - * @param run The handler that is called at the notification time which is set using StartSingle - * or StartPeriodic. + *

Configure when the callback runs with startSingle() or startPeriodic(). + * + * @param callback The callback to run. */ - public Notifier(Runnable run) { - requireNonNullParam(run, "run", "Notifier"); + public Notifier(Runnable callback) { + requireNonNullParam(callback, "callback", "Notifier"); - m_handler = run; + m_callback = callback; m_notifier.set(NotifierJNI.initializeNotifier()); m_thread = @@ -99,23 +104,24 @@ public class Notifier implements AutoCloseable { break; } - Runnable handler; + Runnable threadHandler; m_processLock.lock(); try { - handler = m_handler; + threadHandler = m_callback; if (m_periodic) { m_expirationTimeSeconds += m_periodSeconds; updateAlarm(); } else { - // need to update the alarm to cause it to wait again + // Need to update the alarm to cause it to wait again updateAlarm((long) -1); } } finally { m_processLock.unlock(); } - if (handler != null) { - handler.run(); + // Call callback + if (threadHandler != null) { + threadHandler.run(); } } }); @@ -150,24 +156,39 @@ public class Notifier implements AutoCloseable { } /** - * Change the handler function. + * Change the callback function. * - * @param handler Handler + * @param callback The callback function. + * @deprecated Use setCallback() instead. */ - public void setHandler(Runnable handler) { + @Deprecated(forRemoval = true, since = "2024") + public void setHandler(Runnable callback) { m_processLock.lock(); try { - m_handler = handler; + m_callback = callback; } finally { m_processLock.unlock(); } } /** - * Register for single event notification. A timer event is queued for a single event after the - * specified delay. + * Change the callback function. * - * @param delaySeconds Seconds to wait before the handler is called. + * @param callback The callback function. + */ + public void setCallback(Runnable callback) { + m_processLock.lock(); + try { + m_callback = callback; + } finally { + m_processLock.unlock(); + } + } + + /** + * Run the callback once after the given delay. + * + * @param delaySeconds Time in seconds to wait before the callback is called. */ public void startSingle(double delaySeconds) { m_processLock.lock(); @@ -182,15 +203,13 @@ public class Notifier implements AutoCloseable { } /** - * 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. + * Run the callback periodically with the given period. * - *

The user-provided callback should be written in a nonblocking manner so the callback can be - * recalled at the next periodic event notification. + *

The user-provided callback should be written so that it completes before the next time it's + * scheduled to run. * - * @param periodSeconds Period in seconds to call the handler starting one period after the call - * to this method. + * @param periodSeconds Period in seconds after which to to call the callback starting one period + * after the call to this method. */ public void startPeriodic(double periodSeconds) { m_processLock.lock(); @@ -205,9 +224,13 @@ public class Notifier implements AutoCloseable { } /** - * Stop timer events from occurring. Stop any repeating timer events from occurring. This will - * also remove any single notification events from the queue. If a timer-based call to the - * registered handler is in progress, this function will block until the handler call is complete. + * Stop further callback invocations. + * + *

No further periodic callbacks will occur. Single invocations will also be cancelled if they + * haven't yet occurred. + * + *

If a callback invocation is in progress, this function will block until the callback is + * complete. */ public void stop() { m_processLock.lock();