Make HAL_Initialize timeout configurable, makes result a bool, and makes Java an exception rather than assert. (#557)

hal/include/HAL/HAL.h

@@ -68,7 +68,7 @@ HAL_PortHandle HAL_GetPortWithModule(int32_t module, int32_t channel);
 
 uint64_t HAL_GetFPGATime(int32_t* status);
 
-int32_t HAL_Initialize(int32_t mode);
+HAL_Bool HAL_Initialize(int32_t timeout, int32_t mode);
 
 // ifdef's definition is to allow for default parameters in C++.
 #ifdef __cplusplus

hal/lib/athena/HAL.cpp

@@ -287,15 +287,61 @@ void HAL_BaseInitialize(int32_t* status) {
   initialized = true;
 }
 
+static bool killExistingProgram(int timeout, int mode) {
+  // Kill any previous robot programs
+  std::fstream fs;
+  // By making this both in/out, it won't give us an error if it doesnt exist
+  fs.open("/var/lock/frc.pid", std::fstream::in | std::fstream::out);
+  if (fs.bad()) return false;
+
+  pid_t pid = 0;
+  if (!fs.eof() && !fs.fail()) {
+    fs >> pid;
+    // see if the pid is around, but we don't want to mess with init id=1, or
+    // ourselves
+    if (pid >= 2 && kill(pid, 0) == 0 && pid != getpid()) {
+      llvm::outs() << "Killing previously running FRC program...\n";
+      kill(pid, SIGTERM);  // try to kill it
+      std::this_thread::sleep_for(std::chrono::milliseconds(timeout));
+      if (kill(pid, 0) == 0) {
+        // still not successfull
+        if (mode == 0) {
+          llvm::outs() << "FRC pid " << pid << " did not die within " << timeout
+                       << "ms. Aborting\n";
+          return 0;              // just fail
+        } else if (mode == 1) {  // kill -9 it
+          kill(pid, SIGKILL);
+        } else {
+          llvm::outs() << "WARNING: FRC pid " << pid << " did not die within "
+                       << timeout << "ms.\n";
+        }
+      }
+    }
+  }
+  fs.close();
+  // we will re-open it write only to truncate the file
+  fs.open("/var/lock/frc.pid", std::fstream::out | std::fstream::trunc);
+  fs.seekp(0);
+  pid = getpid();
+  fs << pid << std::endl;
+  fs.close();
+  return true;
+}
+
 /**
  * Call this to start up HAL. This is required for robot programs.
  */
-int32_t HAL_Initialize(int32_t mode) {
+HAL_Bool HAL_Initialize(int32_t timeout, int32_t mode) {
   setlinebuf(stdin);
   setlinebuf(stdout);
 
   prctl(PR_SET_PDEATHSIG, SIGTERM);
 
+  // Return false if program failed to kill an existing program
+  if (!killExistingProgram(timeout, mode)) {
+    return false;
+  }
+
   FRC_NetworkCommunication_Reserve(nullptr);
 
   std::atexit([]() {
@@ -311,52 +357,21 @@ int32_t HAL_Initialize(int32_t mode) {
         timerRollover, nullptr, &status);
   if (status == 0) {
     uint64_t curTime = HAL_GetFPGATime(&status);
-    if (status == 0)
+    if (status == 0) {
       HAL_UpdateNotifierAlarm(rolloverNotifier, curTime + 0x80000000ULL,
                               &status);
-  }
-
-  // Kill any previous robot programs
-  std::fstream fs;
-  // By making this both in/out, it won't give us an error if it doesnt exist
-  fs.open("/var/lock/frc.pid", std::fstream::in | std::fstream::out);
-  if (fs.bad()) return 0;
-
-  pid_t pid = 0;
-  if (!fs.eof() && !fs.fail()) {
-    fs >> pid;
-    // see if the pid is around, but we don't want to mess with init id=1, or
-    // ourselves
-    if (pid >= 2 && kill(pid, 0) == 0 && pid != getpid()) {
-      llvm::outs() << "Killing previously running FRC program...\n";
-      kill(pid, SIGTERM);  // try to kill it
-      std::this_thread::sleep_for(std::chrono::milliseconds(100));
-      if (kill(pid, 0) == 0) {
-        // still not successfull
-        if (mode == 0) {
-          llvm::outs() << "FRC pid " << pid
-                       << " did not die within 110ms. Aborting\n";
-          return 0;              // just fail
-        } else if (mode == 1) {  // kill -9 it
-          kill(pid, SIGKILL);
-        } else {
-          llvm::outs() << "WARNING: FRC pid " << pid
-                       << " did not die within 110ms.\n";
-        }
-      }
+    } else {
+      // return false if status failed.
+      return false;
     }
+  } else {
+    // return false if status failed.
+    return false;
   }
-  fs.close();
-  // we will re-open it write only to truncate the file
-  fs.open("/var/lock/frc.pid", std::fstream::out | std::fstream::trunc);
-  fs.seekp(0);
-  pid = getpid();
-  fs << pid << std::endl;
-  fs.close();
 
   HAL_InitializeDriverStation();
 
-  return 1;
+  return true;
 }
 
 int64_t HAL_Report(int32_t resource, int32_t instanceNumber, int32_t context,

wpilibc/athena/include/RobotBase.h

@@ -19,7 +19,7 @@ class DriverStation;
 
 #define START_ROBOT_CLASS(_ClassName_)                                  \
   int main() {                                                          \
-    if (!HAL_Initialize(0)) {                                           \
+    if (!HAL_Initialize(500, 0)) {                                      \
       llvm::errs() << "FATAL ERROR: HAL could not be initialized\n";    \
       return -1;                                                        \
     }                                                                   \

wpilibcIntegrationTests/src/TestEnvironment.cpp

@@ -26,7 +26,7 @@ class TestEnvironment : public testing::Environment {
     if (m_alreadySetUp) return;
     m_alreadySetUp = true;
 
-    if (!HAL_Initialize(0)) {
+    if (!HAL_Initialize(500, 0)) {
       llvm::errs() << "FATAL ERROR: HAL could not be initialized\n";
       std::exit(-1);
     }

wpilibj/src/athena/cpp/lib/HAL.cpp

@@ -34,11 +34,11 @@ extern "C" {
 /*
  * Class: edu_wpi_first_wpilibj_hal_HAL
  * Method:    Initialize
- * Signature: (I)I
+ * Signature: (Z)II
  */
-JNIEXPORT jint JNICALL
-Java_edu_wpi_first_wpilibj_hal_HAL_initialize(JNIEnv*, jclass, jint mode) {
-  return HAL_Initialize(mode);
+JNIEXPORT jboolean JNICALL
+Java_edu_wpi_first_wpilibj_hal_HAL_initialize(JNIEnv*, jclass, jint timeout, jint mode) {
+  return HAL_Initialize(timeout, mode);
 }
 
 /*

wpilibj/src/athena/java/edu/wpi/first/wpilibj/RobotBase.java

@@ -168,8 +168,9 @@ public abstract class RobotBase {
    * Common initialization for all robot programs.
    */
   public static void initializeHardwareConfiguration() {
-    int rv = HAL.initialize(0);
-    assert rv == 1;
+    if (!HAL.initialize(500, 0)) {
+      throw new IllegalStateException("Failed to initialize. Terminating");
+    }
 
     // Set some implementations so that the static methods work properly
     Timer.SetImplementation(new HardwareTimer());

wpilibj/src/athena/java/edu/wpi/first/wpilibj/hal/HAL.java

@@ -16,7 +16,7 @@ import java.nio.ByteBuffer;
 public class HAL extends JNIWrapper {
   public static native void waitForDSData();
 
-  public static native int initialize(int mode);
+  public static native boolean initialize(int timeout, int mode);
 
   public static native void observeUserProgramStarting();