From c57a7f0a41e4f92dfc80fdd8b4f5ba0cd6cbf7f0 Mon Sep 17 00:00:00 2001 From: Thad House Date: Fri, 30 Jun 2017 18:58:58 -0700 Subject: [PATCH] Switches all notifiers created with the external API to be threaded (#546) Testing showed this wasn't an issue with timing, and allows for more safety in user code making mistakes. Places where the extra thread wouldn't help have been kept non threaded, using a new internal API. --- hal/include/HAL/Notifier.h | 2 -- hal/include/HAL/cpp/NotifierInternal.h | 15 +++++++++++++++ hal/lib/athena/HAL.cpp | 4 +++- hal/lib/athena/Notifier.cpp | 13 +++++++------ wpilibj/src/athena/cpp/lib/NotifierJNI.cpp | 4 +++- 5 files changed, 28 insertions(+), 10 deletions(-) create mode 100644 hal/include/HAL/cpp/NotifierInternal.h diff --git a/hal/include/HAL/Notifier.h b/hal/include/HAL/Notifier.h index 4df181965d..e7628760b1 100644 --- a/hal/include/HAL/Notifier.h +++ b/hal/include/HAL/Notifier.h @@ -20,8 +20,6 @@ typedef void (*HAL_NotifierProcessFunction)(uint64_t currentTime, HAL_NotifierHandle HAL_InitializeNotifier(HAL_NotifierProcessFunction process, void* param, int32_t* status); -HAL_NotifierHandle HAL_InitializeNotifierThreaded( - HAL_NotifierProcessFunction process, void* param, int32_t* status); void HAL_CleanNotifier(HAL_NotifierHandle notifierHandle, int32_t* status); void* HAL_GetNotifierParam(HAL_NotifierHandle notifierHandle, int32_t* status); void HAL_UpdateNotifierAlarm(HAL_NotifierHandle notifierHandle, diff --git a/hal/include/HAL/cpp/NotifierInternal.h b/hal/include/HAL/cpp/NotifierInternal.h new file mode 100644 index 0000000000..eb0326f156 --- /dev/null +++ b/hal/include/HAL/cpp/NotifierInternal.h @@ -0,0 +1,15 @@ +/*----------------------------------------------------------------------------*/ +/* Copyright (c) FIRST 2017. 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. */ +/*----------------------------------------------------------------------------*/ + +#pragma once + +#include "HAL/Types.h" + +extern "C" { +HAL_NotifierHandle HAL_InitializeNotifierNonThreadedUnsafe( + HAL_NotifierProcessFunction process, void* param, int32_t* status); +} diff --git a/hal/lib/athena/HAL.cpp b/hal/lib/athena/HAL.cpp index af7967b187..5cb35cb2fc 100644 --- a/hal/lib/athena/HAL.cpp +++ b/hal/lib/athena/HAL.cpp @@ -25,6 +25,7 @@ #include "HAL/DriverStation.h" #include "HAL/Errors.h" #include "HAL/Notifier.h" +#include "HAL/cpp/NotifierInternal.h" #include "HAL/cpp/priority_mutex.h" #include "HAL/handles/HandlesInternal.h" #include "ctre/ctre.h" @@ -306,7 +307,8 @@ int32_t HAL_Initialize(int32_t mode) { HAL_BaseInitialize(&status); if (!rolloverNotifier) - rolloverNotifier = HAL_InitializeNotifier(timerRollover, nullptr, &status); + rolloverNotifier = HAL_InitializeNotifierNonThreadedUnsafe( + timerRollover, nullptr, &status); if (status == 0) { uint64_t curTime = HAL_GetFPGATime(&status); if (status == 0) diff --git a/hal/lib/athena/Notifier.cpp b/hal/lib/athena/Notifier.cpp index fc33c44c42..83e6bb0f55 100644 --- a/hal/lib/athena/Notifier.cpp +++ b/hal/lib/athena/Notifier.cpp @@ -17,6 +17,7 @@ #include "HAL/ChipObject.h" #include "HAL/Errors.h" #include "HAL/HAL.h" +#include "HAL/cpp/NotifierInternal.h" #include "HAL/cpp/make_unique.h" #include "HAL/cpp/priority_mutex.h" #include "HAL/handles/UnlimitedHandleResource.h" @@ -173,8 +174,8 @@ static void threadedNotifierHandler(uint64_t currentTimeInt, extern "C" { -HAL_NotifierHandle HAL_InitializeNotifier(HAL_NotifierProcessFunction process, - void* param, int32_t* status) { +HAL_NotifierHandle HAL_InitializeNotifierNonThreadedUnsafe( + HAL_NotifierProcessFunction process, void* param, int32_t* status) { if (!process) { *status = NULL_PARAMETER; return 0; @@ -211,14 +212,14 @@ HAL_NotifierHandle HAL_InitializeNotifier(HAL_NotifierProcessFunction process, return handle; } -HAL_NotifierHandle HAL_InitializeNotifierThreaded( - HAL_NotifierProcessFunction process, void* param, int32_t* status) { +HAL_NotifierHandle HAL_InitializeNotifier(HAL_NotifierProcessFunction process, + void* param, int32_t* status) { NotifierThreadOwner* notify = new NotifierThreadOwner; notify->Start(); notify->SetFunc(process, param); - auto notifierHandle = - HAL_InitializeNotifier(threadedNotifierHandler, notify, status); + auto notifierHandle = HAL_InitializeNotifierNonThreadedUnsafe( + threadedNotifierHandler, notify, status); if (notifierHandle == HAL_kInvalidHandle || *status != 0) { delete notify; diff --git a/wpilibj/src/athena/cpp/lib/NotifierJNI.cpp b/wpilibj/src/athena/cpp/lib/NotifierJNI.cpp index e318669fe9..16886dbb4c 100644 --- a/wpilibj/src/athena/cpp/lib/NotifierJNI.cpp +++ b/wpilibj/src/athena/cpp/lib/NotifierJNI.cpp @@ -18,6 +18,7 @@ #include "HAL/cpp/Log.h" #include "edu_wpi_first_wpilibj_hal_NotifierJNI.h" #include "support/SafeThread.h" +#include "HAL/cpp/NotifierInternal.h" using namespace frc; @@ -147,7 +148,8 @@ Java_edu_wpi_first_wpilibj_hal_NotifierJNI_initializeNotifier( notify->Start(); notify->SetFunc(env, func, mid); int32_t status = 0; - HAL_NotifierHandle notifierHandle = HAL_InitializeNotifier(notifierHandler, notify, &status); + HAL_NotifierHandle notifierHandle = + HAL_InitializeNotifierNonThreadedUnsafe(notifierHandler, notify, &status); NOTIFIERJNI_LOG(logDEBUG) << "Notifier Handle = " << notifierHandle; NOTIFIERJNI_LOG(logDEBUG) << "Status = " << status;