From b65fce86bf8e38ab093a3da88362ea1918c1e13a Mon Sep 17 00:00:00 2001
From: Tyler Veness <calcmogul@gmail.com>
Date: Fri, 24 Sep 2021 16:01:13 -0700
Subject: [PATCH] [wpilib] Remove Timer lock in wpilibj and update docs (#3602)

Timer reports a negative duration if the sim timing is restarted. This
can be worked around by calling Reset(). Other options included:

1. Have RestartTiming() call Timer::Reset() on a list of instantiated
   Timers.
2. Have Timer::Get() reset the timer if it notices time went backwards.
   This requires dropping const qualification though, which is a
   breaking change that only fixes a minor edge case.

Closes #2732.
---
 wpilibc/src/main/native/include/frc/Timer.h   |  5 +-
 .../java/edu/wpi/first/wpilibj/Timer.java     | 64 ++++++++-----------
 2 files changed, 32 insertions(+), 37 deletions(-)

diff --git a/wpilibc/src/main/native/include/frc/Timer.h b/wpilibc/src/main/native/include/frc/Timer.h
index a1a8bc7758..499fbe50cb 100644
--- a/wpilibc/src/main/native/include/frc/Timer.h
+++ b/wpilibc/src/main/native/include/frc/Timer.h
@@ -29,7 +29,10 @@ void Wait(units::second_t seconds);
 units::second_t GetTime();
 
 /**
- * A wrapper for the frc::Timer class that returns unit-typed values.
+ * A timer class.
+ *
+ * Note that if the user calls frc::sim::RestartTiming(), they should also reset
+ * the timer so Get() won't return a negative duration.
  */
 class Timer {
  public:
diff --git a/wpilibj/src/main/java/edu/wpi/first/wpilibj/Timer.java b/wpilibj/src/main/java/edu/wpi/first/wpilibj/Timer.java
index cead2ce4d0..939e255c3b 100644
--- a/wpilibj/src/main/java/edu/wpi/first/wpilibj/Timer.java
+++ b/wpilibj/src/main/java/edu/wpi/first/wpilibj/Timer.java
@@ -4,6 +4,12 @@
 
 package edu.wpi.first.wpilibj;
 
+/**
+ * A timer class.
+ *
+ * <p>Note that if the user calls SimHooks.restartTiming(), they should also reset the timer so
+ * get() won't return a negative duration.
+ */
 public class Timer {
   /**
    * Return the system clock time in seconds. Return the time from the FPGA hardware clock in
@@ -49,9 +55,6 @@ public class Timer {
   private double m_accumulatedTime;
   private boolean m_running;
 
-  /** Lock for synchronization. */
-  private final Object m_lock = new Object();
-
   @SuppressWarnings("MissingJavadocMethod")
   public Timer() {
     reset();
@@ -69,24 +72,21 @@ public class Timer {
    * @return Current time value for this timer in seconds
    */
   public double get() {
-    synchronized (m_lock) {
-      if (m_running) {
-        return m_accumulatedTime + (getMsClock() - m_startTime) / 1000.0;
-      } else {
-        return m_accumulatedTime;
-      }
+    if (m_running) {
+      return m_accumulatedTime + (getMsClock() - m_startTime) / 1000.0;
+    } else {
+      return m_accumulatedTime;
     }
   }
 
   /**
-   * Reset the timer by setting the time to 0. Make the timer startTime the current time so new
-   * requests will be relative now
+   * Reset the timer by setting the time to 0.
+   *
+   * <p>Make the timer startTime the current time so new requests will be relative now.
    */
   public void reset() {
-    synchronized (m_lock) {
-      m_accumulatedTime = 0;
-      m_startTime = getMsClock();
-    }
+    m_accumulatedTime = 0;
+    m_startTime = getMsClock();
   }
 
   /**
@@ -95,11 +95,9 @@ public class Timer {
    * already running.
    */
   public void start() {
-    synchronized (m_lock) {
-      if (!m_running) {
-        m_startTime = getMsClock();
-        m_running = true;
-      }
+    if (!m_running) {
+      m_startTime = getMsClock();
+      m_running = true;
     }
   }
 
@@ -109,10 +107,8 @@ public class Timer {
    * clock.
    */
   public void stop() {
-    synchronized (m_lock) {
-      m_accumulatedTime = get();
-      m_running = false;
-    }
+    m_accumulatedTime = get();
+    m_running = false;
   }
 
   /**
@@ -122,9 +118,7 @@ public class Timer {
    * @return Whether the period has passed.
    */
   public boolean hasElapsed(double seconds) {
-    synchronized (m_lock) {
-      return get() >= seconds;
-    }
+    return get() >= seconds;
   }
 
   /**
@@ -150,15 +144,13 @@ public class Timer {
    * @return Whether the period has passed.
    */
   public boolean advanceIfElapsed(double seconds) {
-    synchronized (m_lock) {
-      if (get() >= seconds) {
-        // Advance the start time by the period.
-        // Don't set it to the current time... we want to avoid drift.
-        m_startTime += seconds * 1000;
-        return true;
-      } else {
-        return false;
-      }
+    if (get() >= seconds) {
+      // Advance the start time by the period.
+      // Don't set it to the current time... we want to avoid drift.
+      m_startTime += seconds * 1000;
+      return true;
+    } else {
+      return false;
     }
   }
 }