[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.
This commit is contained in:
Tyler Veness
2021-09-24 16:01:13 -07:00
committed by GitHub
parent 3b8d3bbcbf
commit b65fce86bf
2 changed files with 32 additions and 37 deletions

View File

@@ -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:

View File

@@ -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;
}
}
}