fixed lots of race conditions in the error reporting framework
Conflicts:
aos/externals/WPILib/WPILib/Error.cpp
diff --git a/aos/externals/WPILib/WPILib/AnalogChannel.cpp b/aos/externals/WPILib/WPILib/AnalogChannel.cpp
index 834837f..26708c1 100644
--- a/aos/externals/WPILib/WPILib/AnalogChannel.cpp
+++ b/aos/externals/WPILib/WPILib/AnalogChannel.cpp
@@ -38,9 +38,9 @@
}
snprintf(buf, 64, "Analog Input %d (Module: %d)", channel, moduleNumber);
- if (channels->Allocate((moduleNumber - 1) * kAnalogChannels + channel - 1, buf) == ~0ul)
+ if (channels->Allocate((moduleNumber - 1) * kAnalogChannels +
+ channel - 1, buf, this) == ~0ul)
{
- CloneError(channels);
return;
}
m_channel = channel;
@@ -86,7 +86,8 @@
*/
AnalogChannel::~AnalogChannel()
{
- channels->Free((m_module->GetNumber() - 1) * kAnalogChannels + m_channel - 1);
+ channels->Free((m_module->GetNumber() - 1) * kAnalogChannels + m_channel - 1,
+ this);
}
/**
diff --git a/aos/externals/WPILib/WPILib/AnalogTrigger.cpp b/aos/externals/WPILib/WPILib/AnalogTrigger.cpp
index ffe45c1..2deeb54 100644
--- a/aos/externals/WPILib/WPILib/AnalogTrigger.cpp
+++ b/aos/externals/WPILib/WPILib/AnalogTrigger.cpp
@@ -21,10 +21,9 @@
void AnalogTrigger::InitTrigger(UINT8 moduleNumber, UINT32 channel)
{
Resource::CreateResourceObject(&triggers, tAnalogTrigger::kNumSystems);
- UINT32 index = triggers->Allocate("Analog Trigger");
+ UINT32 index = triggers->Allocate("Analog Trigger", this);
if (index == ~0ul)
{
- CloneError(triggers);
return;
}
m_index = (UINT8)index;
@@ -74,7 +73,7 @@
AnalogTrigger::~AnalogTrigger()
{
- triggers->Free(m_index);
+ triggers->Free(m_index, this);
delete m_trigger;
}
diff --git a/aos/externals/WPILib/WPILib/Counter.cpp b/aos/externals/WPILib/WPILib/Counter.cpp
index a11c268..e7c17ad 100644
--- a/aos/externals/WPILib/WPILib/Counter.cpp
+++ b/aos/externals/WPILib/WPILib/Counter.cpp
@@ -20,10 +20,9 @@
void Counter::InitCounter(Mode mode)
{
Resource::CreateResourceObject(&counters, tCounter::kNumSystems);
- UINT32 index = counters->Allocate("Counter");
+ UINT32 index = counters->Allocate("Counter", this);
if (index == ~0ul)
{
- CloneError(counters);
return;
}
m_index = index;
@@ -183,7 +182,7 @@
}
delete m_counter;
m_counter = NULL;
- counters->Free(m_index);
+ counters->Free(m_index, this);
}
/**
diff --git a/aos/externals/WPILib/WPILib/DigitalModule.cpp b/aos/externals/WPILib/WPILib/DigitalModule.cpp
index 3e7f728..e29f841 100644
--- a/aos/externals/WPILib/WPILib/DigitalModule.cpp
+++ b/aos/externals/WPILib/WPILib/DigitalModule.cpp
@@ -248,7 +248,8 @@
{
char buf[64];
snprintf(buf, 64, "DIO %d (Module %d)", channel, m_moduleNumber);
- if (DIOChannels->Allocate(kDigitalChannels * (m_moduleNumber - 1) + channel - 1, buf) == ~0ul) return false;
+ if (DIOChannels->Allocate(kDigitalChannels * (m_moduleNumber - 1) +
+ channel - 1, buf, this) == ~0ul) return false;
tRioStatusCode localStatus = NiFpga_Status_Success;
{
Synchronized sync(m_digitalSemaphore);
@@ -276,7 +277,8 @@
*/
void DigitalModule::FreeDIO(UINT32 channel)
{
- DIOChannels->Free(kDigitalChannels * (m_moduleNumber - 1) + channel - 1);
+ DIOChannels->Free(kDigitalChannels * (m_moduleNumber - 1) + channel - 1,
+ this);
}
/**
@@ -429,7 +431,7 @@
{
char buf[64];
snprintf(buf, 64, "DO_PWM (Module: %d)", m_moduleNumber);
- return DO_PWMGenerators[(m_moduleNumber - 1)]->Allocate(buf);
+ return DO_PWMGenerators[(m_moduleNumber - 1)]->Allocate(buf, this);
}
/**
@@ -440,7 +442,7 @@
void DigitalModule::FreeDO_PWM(UINT32 pwmGenerator)
{
if (pwmGenerator == ~0ul) return;
- DO_PWMGenerators[(m_moduleNumber - 1)]->Free(pwmGenerator);
+ DO_PWMGenerators[(m_moduleNumber - 1)]->Free(pwmGenerator, this);
}
/**
diff --git a/aos/externals/WPILib/WPILib/DigitalOutput.cpp b/aos/externals/WPILib/WPILib/DigitalOutput.cpp
index a571066..4f45ee7 100644
--- a/aos/externals/WPILib/WPILib/DigitalOutput.cpp
+++ b/aos/externals/WPILib/WPILib/DigitalOutput.cpp
@@ -214,10 +214,9 @@
void DigitalOutput::RequestInterrupts(tInterruptHandler handler, void *param)
{
if (StatusIsFatal()) return;
- UINT32 index = interruptsResource->Allocate("Sync Interrupt");
+ UINT32 index = interruptsResource->Allocate("Sync Interrupt", this);
if (index == ~0ul)
{
- CloneError(interruptsResource);
return;
}
m_interruptIndex = index;
@@ -245,10 +244,9 @@
void DigitalOutput::RequestInterrupts()
{
if (StatusIsFatal()) return;
- UINT32 index = interruptsResource->Allocate("Sync Interrupt");
+ UINT32 index = interruptsResource->Allocate("Sync Interrupt", this);
if (index == ~0ul)
{
- CloneError(interruptsResource);
return;
}
m_interruptIndex = index;
diff --git a/aos/externals/WPILib/WPILib/DigitalSource.cpp b/aos/externals/WPILib/WPILib/DigitalSource.cpp
index 3c1e3a8..862b439 100644
--- a/aos/externals/WPILib/WPILib/DigitalSource.cpp
+++ b/aos/externals/WPILib/WPILib/DigitalSource.cpp
@@ -24,7 +24,7 @@
{
delete m_manager;
delete m_interrupt;
- interruptsResource->Free(m_interruptIndex);
+ interruptsResource->Free(m_interruptIndex, this);
}
}
@@ -39,10 +39,9 @@
void DigitalSource::RequestInterrupts(tInterruptHandler handler, void *param)
{
if (StatusIsFatal()) return;
- UINT32 index = interruptsResource->Allocate("Async Interrupt");
+ UINT32 index = interruptsResource->Allocate("Async Interrupt", this);
if (index == ~0ul)
{
- CloneError(interruptsResource);
return;
}
m_interruptIndex = index;
@@ -70,10 +69,9 @@
void DigitalSource::RequestInterrupts()
{
if (StatusIsFatal()) return;
- UINT32 index = interruptsResource->Allocate("Sync Interrupt");
+ UINT32 index = interruptsResource->Allocate("Sync Interrupt", this);
if (index == ~0ul)
{
- CloneError(interruptsResource);
return;
}
m_interruptIndex = index;
diff --git a/aos/externals/WPILib/WPILib/DoubleSolenoid.cpp b/aos/externals/WPILib/WPILib/DoubleSolenoid.cpp
index d05c7ff..4b3e473 100644
--- a/aos/externals/WPILib/WPILib/DoubleSolenoid.cpp
+++ b/aos/externals/WPILib/WPILib/DoubleSolenoid.cpp
@@ -37,15 +37,15 @@
Resource::CreateResourceObject(&m_allocated, tSolenoid::kNumDO7_0Elements * kSolenoidChannels);
snprintf(buf, 64, "Solenoid %d (Module %d)", m_forwardChannel, m_moduleNumber);
- if (m_allocated->Allocate((m_moduleNumber - 1) * kSolenoidChannels + m_forwardChannel - 1, buf) == ~0ul)
+ if (m_allocated->Allocate((m_moduleNumber - 1) * kSolenoidChannels +
+ m_forwardChannel - 1, buf, this) == ~0ul)
{
- CloneError(m_allocated);
return;
}
snprintf(buf, 64, "Solenoid %d (Module %d)", m_reverseChannel, m_moduleNumber);
- if (m_allocated->Allocate((m_moduleNumber - 1) * kSolenoidChannels + m_reverseChannel - 1, buf) == ~0ul)
+ if (m_allocated->Allocate((m_moduleNumber - 1) * kSolenoidChannels +
+ m_reverseChannel - 1, buf, this) == ~0ul)
{
- CloneError(m_allocated);
return;
}
m_forwardMask = 1 << (m_forwardChannel - 1);
@@ -90,11 +90,10 @@
*/
DoubleSolenoid::~DoubleSolenoid()
{
- if (CheckSolenoidModule(m_moduleNumber))
- {
- m_allocated->Free((m_moduleNumber - 1) * kSolenoidChannels + m_forwardChannel - 1);
- m_allocated->Free((m_moduleNumber - 1) * kSolenoidChannels + m_reverseChannel - 1);
- }
+ m_allocated->Free((m_moduleNumber - 1) * kSolenoidChannels +
+ m_forwardChannel - 1, this);
+ m_allocated->Free((m_moduleNumber - 1) * kSolenoidChannels +
+ m_reverseChannel - 1, this);
}
/**
diff --git a/aos/externals/WPILib/WPILib/Encoder.cpp b/aos/externals/WPILib/WPILib/Encoder.cpp
index cdf1dba..22be8fe 100644
--- a/aos/externals/WPILib/WPILib/Encoder.cpp
+++ b/aos/externals/WPILib/WPILib/Encoder.cpp
@@ -31,10 +31,9 @@
{
case k4X:
Resource::CreateResourceObject(&quadEncoders, tEncoder::kNumSystems);
- UINT32 index = quadEncoders->Allocate("4X Encoder");
+ UINT32 index = quadEncoders->Allocate("4X Encoder", this);
if (index == ~0ul)
{
- CloneError(quadEncoders);
return;
}
if (m_aSource->StatusIsFatal())
@@ -195,7 +194,7 @@
}
else
{
- quadEncoders->Free(m_index);
+ quadEncoders->Free(m_index, this);
delete m_encoder;
}
}
diff --git a/aos/externals/WPILib/WPILib/Error.cpp b/aos/externals/WPILib/WPILib/Error.cpp
index 30f0cbc..b817e20 100644
--- a/aos/externals/WPILib/WPILib/Error.cpp
+++ b/aos/externals/WPILib/WPILib/Error.cpp
@@ -25,8 +25,30 @@
Error::~Error()
{}
-void Error::Clone(Error &error)
+/**
+ * Clones another error into this if this is currently clear. If not, does
+ * nothing.
+ * This is necessary because just using "if (!IsClear()) Clone(error)" has a
+ * race condition.
+ */
+void Error::CloneIfClear(const Error &error) {
+ Synchronized sync(m_semaphore);
+ if (IsClear()) {
+ DoClone(error);
+ }
+}
+
+/**
+ * Clones another error into this object.
+ */
+void Error::Clone(const Error &error) {
+ Synchronized sync(m_semaphore);
+ DoClone(error);
+}
+
+void Error::DoClone(const Error &error)
{
+ Synchronized sync(error.m_semaphore);
m_code = error.m_code;
m_message = error.m_message;
m_filename = error.m_filename;
@@ -36,17 +58,19 @@
m_timestamp = error.m_timestamp;
}
+bool Error::IsClear() const { return GetCode() == 0; }
+
Error::Code Error::GetCode() const
{ return m_code; }
-const char * Error::GetMessage() const
-{ return m_message.c_str(); }
+std::string Error::GetMessage() const
+{ return m_message; }
-const char * Error::GetFilename() const
-{ return m_filename.c_str(); }
+std::string Error::GetFilename() const
+{ return m_filename; }
-const char * Error::GetFunction() const
-{ return m_function.c_str(); }
+std::string Error::GetFunction() const
+{ return m_function; }
UINT32 Error::GetLineNumber() const
{ return m_lineNumber; }
@@ -57,8 +81,10 @@
double Error::GetTime() const
{ return m_timestamp; }
-void Error::Set(Code code, const char* contextMessage, const char* filename, const char* function, UINT32 lineNumber, const ErrorBase* originatingObject)
-{
+void Error::Set(Code code, const char* contextMessage, const char* filename,
+ const char* function, UINT32 lineNumber,
+ const ErrorBase* originatingObject) {
+ Synchronized sync(m_semaphore);
m_code = code;
m_message = contextMessage;
m_filename = filename;
@@ -69,25 +95,30 @@
Report();
- if (m_suspendOnErrorEnabled) taskSuspend(0);
+ if (m_suspendOnErrorEnabled) taskSuspend(0 /*self*/);
}
-void Error::Report()
+void Error::Report() const
{
// Error string buffers
char *error = new char[256];
char *error_with_code = new char[256];
// Build error strings
- if (m_code != -1)
+ if (m_code != -1 && m_code != 1)
{
- snprintf(error, 256, "%s: status = %d (0x%08X) %s ...in %s() in %s at line %d\n",
- m_code < 0 ? "ERROR" : "WARNING", (INT32)m_code, (UINT32)m_code, m_message.c_str(),
- m_function.c_str(), m_filename.c_str(), m_lineNumber);
- sprintf(error_with_code,"<Code>%d %s", (INT32)m_code, error);
+ snprintf(error, sizeof(error),
+ "%s: status = %d (0x%08X) %s ...in %s() in %s at line %d\n",
+ m_code < 0 ? "ERROR" : "WARNING", (INT32)m_code,
+ (UINT32)m_code, m_message.c_str(),
+ m_function.c_str(), m_filename.c_str(), m_lineNumber);
+ snprintf(error_with_code, sizeof(error_with_code),
+ "<Code>%d %s", (INT32)m_code, error);
} else {
- snprintf(error, 256, "ERROR: %s ...in %s() in %s at line %d\n", m_message.c_str(),
- m_function.c_str(), m_filename.c_str(), m_lineNumber);
+ snprintf(error, sizeof(error),
+ "%s: %s ...in %s() in %s at line %d\n",
+ m_code < 0 ? "ERROR" : "WARNING", m_message.c_str(),
+ m_function.c_str(), m_filename.c_str(), m_lineNumber);
strcpy(error_with_code, error);
}
// TODO: Add logging to disk
@@ -111,6 +142,7 @@
void Error::Clear()
{
+ Synchronized sync(m_semaphore);
m_code = 0;
m_message = "";
m_filename = "";
diff --git a/aos/externals/WPILib/WPILib/Error.h b/aos/externals/WPILib/WPILib/Error.h
index a80fe06..545b23b 100644
--- a/aos/externals/WPILib/WPILib/Error.h
+++ b/aos/externals/WPILib/WPILib/Error.h
@@ -10,37 +10,54 @@
#include "Base.h"
#include "ChipObject/NiRio.h"
#include <string>
-#include <vxWorks.h>
+#include "Synchronized.h"
// Forward declarations
class ErrorBase;
/**
- * Error object represents a library error.
+ * Represents an error or warning.
+ *
+ * All methods that can change instance variables are protected by a lock so
+ * that it is safe to call any methods from multiple tasks at the same time.
*/
class Error
{
public:
+ // -1 is other error, 1 is other warning.
typedef tRioStatusCode Code;
Error();
~Error();
- void Clone(Error &error);
- Code GetCode() const;
- const char *GetMessage() const;
- const char *GetFilename() const;
- const char *GetFunction() const;
- UINT32 GetLineNumber() const;
- const ErrorBase* GetOriginatingObject() const;
- double GetTime() const;
+
+ void CloneIfClear(const Error &error);
+ void Clone(const Error &error);
void Clear();
void Set(Code code, const char* contextMessage, const char* filename,
const char *function, UINT32 lineNumber, const ErrorBase* originatingObject);
+
+ bool IsClear() const;
+ Code GetCode() const;
+ // Have to return by value to avoid race conditions using the result.
+ std::string GetMessage() const;
+ std::string GetFilename() const;
+ std::string GetFunction() const;
+ UINT32 GetLineNumber() const;
+ const ErrorBase* GetOriginatingObject() const;
+ double GetTime() const;
+
+ // Enable or disable printing out a stack trace on the console whenever there
+ // is an error.
static void EnableStackTrace(bool enable) { m_stackTraceEnabled=enable; }
+ // Enable or disable having any task that gets an error suspend itself.
static void EnableSuspendOnError(bool enable) { m_suspendOnErrorEnabled=enable; }
private:
- void Report();
+ // Deals with notifying other code of this error.
+ void Report() const;
+ // Actually implements cloning.
+ // Does not lock m_semaphore, so callers must.
+ void DoClone(const Error &error);
Code m_code;
std::string m_message;
@@ -49,6 +66,7 @@
UINT32 m_lineNumber;
const ErrorBase* m_originatingObject;
double m_timestamp;
+ ReentrantSemaphore m_semaphore;
static bool m_stackTraceEnabled;
static bool m_suspendOnErrorEnabled;
diff --git a/aos/externals/WPILib/WPILib/ErrorBase.cpp b/aos/externals/WPILib/WPILib/ErrorBase.cpp
index 2722963..3fde5b1 100644
--- a/aos/externals/WPILib/WPILib/ErrorBase.cpp
+++ b/aos/externals/WPILib/WPILib/ErrorBase.cpp
@@ -14,8 +14,8 @@
#include <symLib.h>
#include <sysSymTbl.h>
-SEM_ID ErrorBase::_globalErrorMutex = semMCreate(SEM_Q_PRIORITY | SEM_DELETE_SAFE | SEM_INVERSION_SAFE);
Error ErrorBase::_globalError;
+
/**
* @brief Initialize the instance status to 0 for now.
*/
@@ -48,7 +48,9 @@
}
/**
- * @brief Set error information associated with a C library call that set an error to the "errno" global variable.
+ * @brief Set error information associated with a C library call that set an
+ * error to the "errno" "global variable" (it's really a macro that calls a
+ * function so that it's thread safe).
*
* @param contextMessage A custom message from the code that set the error.
* @param filename Filename of the error source
@@ -71,20 +73,16 @@
SYM_TYPE ptype;
symFindByValue(statSymTbl, errNo, statName, &pval, &ptype);
if (pval != errNo)
- snprintf(err, 256, "Unknown errno 0x%08X: %s", errNo, contextMessage);
+ snprintf(err, sizeof(err), "Unknown errno 0x%08X: %s", errNo, contextMessage);
else
- snprintf(err, 256, "%s (0x%08X): %s", statName, errNo, contextMessage);
+ snprintf(err, sizeof(err), "%s (0x%08X): %s", statName, errNo, contextMessage);
delete [] statName;
}
// Set the current error information for this object.
m_error.Set(-1, err, filename, function, lineNumber, this);
- // Update the global error if there is not one already set.
- Synchronized mutex(_globalErrorMutex);
- if (_globalError.GetCode() == 0) {
- _globalError.Clone(m_error);
- }
+ _globalError.CloneIfClear(m_error);
}
/**
@@ -106,11 +104,7 @@
// Set the current error information for this object.
m_error.Set(imaqGetLastError(), err, filename, function, lineNumber, this);
- // Update the global error if there is not one already set.
- Synchronized mutex(_globalErrorMutex);
- if (_globalError.GetCode() == 0) {
- _globalError.Clone(m_error);
- }
+ _globalError.CloneIfClear(m_error);
}
}
@@ -131,11 +125,7 @@
// Set the current error information for this object.
m_error.Set(code, contextMessage, filename, function, lineNumber, this);
- // Update the global error if there is not one already set.
- Synchronized mutex(_globalErrorMutex);
- if (_globalError.GetCode() == 0) {
- _globalError.Clone(m_error);
- }
+ _globalError.CloneIfClear(m_error);
}
}
@@ -157,14 +147,10 @@
// Set the current error information for this object.
m_error.Set(-1, err, filename, function, lineNumber, this);
- // Update the global error if there is not one already set.
- Synchronized mutex(_globalErrorMutex);
- if (_globalError.GetCode() == 0) {
- _globalError.Clone(m_error);
- }
+ _globalError.CloneIfClear(m_error);
}
-void ErrorBase::CloneError(ErrorBase *rhs) const
+void ErrorBase::CloneError(const ErrorBase *rhs) const
{
m_error.Clone(rhs->GetError());
}
@@ -182,13 +168,8 @@
void ErrorBase::SetGlobalError(Error::Code code, const char *contextMessage,
const char* filename, const char* function, UINT32 lineNumber)
{
- // If there was an error
- if (code != 0) {
- Synchronized mutex(_globalErrorMutex);
-
- // Set the current error information for this object.
- _globalError.Set(code, contextMessage, filename, function, lineNumber, NULL);
- }
+ // Set the current error information for this object.
+ _globalError.Set(code, contextMessage, filename, function, lineNumber, NULL);
}
void ErrorBase::SetGlobalWPIError(const char *errorMessage, const char *contextMessage,
@@ -197,19 +178,14 @@
char err[256];
sprintf(err, "%s: %s", errorMessage, contextMessage);
- Synchronized mutex(_globalErrorMutex);
- if (_globalError.GetCode() != 0) {
- _globalError.Clear();
- }
_globalError.Set(-1, err, filename, function, lineNumber, NULL);
}
/**
* Retrieve the current global error.
*/
-Error& ErrorBase::GetGlobalError()
+const Error& ErrorBase::GetGlobalError()
{
- Synchronized mutex(_globalErrorMutex);
return _globalError;
}
diff --git a/aos/externals/WPILib/WPILib/ErrorBase.h b/aos/externals/WPILib/WPILib/ErrorBase.h
index f967562..9091f05 100644
--- a/aos/externals/WPILib/WPILib/ErrorBase.h
+++ b/aos/externals/WPILib/WPILib/ErrorBase.h
@@ -10,59 +10,86 @@
#include "Base.h"
#include "ChipObject/NiRio.h"
#include "Error.h"
-#include <semLib.h>
-#include <vxWorks.h>
-#define wpi_setErrnoErrorWithContext(context) (this->SetErrnoError((context), __FILE__, __FUNCTION__, __LINE__))
-#define wpi_setErrnoError() (wpi_setErrnoErrorWithContext(""))
-#define wpi_setImaqErrorWithContext(code, context) (this->SetImaqError((code), (context), __FILE__, __FUNCTION__, __LINE__))
-#define wpi_setErrorWithContext(code, context) (this->SetError((code), (context), __FILE__, __FUNCTION__, __LINE__))
+// Helper macros to fill in the context information for you. See the
+// documentation for the methods that they call for details.
+#define wpi_setErrnoErrorWithContext(context) \
+ (this->SetErrnoError((context), __FILE__, __FUNCTION__, __LINE__))
+#define wpi_setErrnoError() \
+ (wpi_setErrnoErrorWithContext(""))
+#define wpi_setImaqErrorWithContext(code, context) \
+ (this->SetImaqError((code), (context), __FILE__, __FUNCTION__, __LINE__))
+#define wpi_setErrorWithContext(code, context) \
+ (this->SetError((code), (context), __FILE__, __FUNCTION__, __LINE__))
#define wpi_setError(code) (wpi_setErrorWithContext(code, ""))
-#define wpi_setStaticErrorWithContext(object, code, context) (object->SetError((code), (context), __FILE__, __FUNCTION__, __LINE__))
-#define wpi_setStaticError(object, code) (wpi_setStaticErrorWithContext(object, code, ""))
-#define wpi_setGlobalErrorWithContext(code, context) (ErrorBase::SetGlobalError((code), (context), __FILE__, __FUNCTION__, __LINE__))
+#define wpi_setStaticErrorWithContext(object, code, context) \
+ (object->SetError((code), (context), __FILE__, __FUNCTION__, __LINE__))
+#define wpi_setStaticError(object, code) \
+ (wpi_setStaticErrorWithContext(object, code, ""))
+#define wpi_setGlobalErrorWithContext(code, context) \
+ (ErrorBase::SetGlobalError((code), (context), \
+ __FILE__, __FUNCTION__, __LINE__))
#define wpi_setGlobalError(code) (wpi_setGlobalErrorWithContext(code, ""))
-#define wpi_setWPIErrorWithContext(error, context) (this->SetWPIError((wpi_error_s_##error), (context), __FILE__, __FUNCTION__, __LINE__))
+#define wpi_setWPIErrorWithContext(error, context) \
+ (this->SetWPIError((wpi_error_s_##error), (context), \
+ __FILE__, __FUNCTION__, __LINE__))
#define wpi_setWPIError(error) (wpi_setWPIErrorWithContext(error, ""))
-#define wpi_setStaticWPIErrorWithContext(object, error, context) (object->SetWPIError((wpi_error_s_##error), (context), __FILE__, __FUNCTION__, __LINE__))
-#define wpi_setStaticWPIError(object, error) (wpi_setStaticWPIErrorWithContext(object, error, ""))
-#define wpi_setGlobalWPIErrorWithContext(error, context) (ErrorBase::SetGlobalWPIError((wpi_error_s_##error), (context), __FILE__, __FUNCTION__, __LINE__))
-#define wpi_setGlobalWPIError(error) (wpi_setGlobalWPIErrorWithContext(error, ""))
+#define wpi_setStaticWPIErrorWithContext(object, error, context) \
+ (object->SetWPIError((wpi_error_s_##error), (context), \
+ __FILE__, __FUNCTION__, __LINE__))
+#define wpi_setStaticWPIError(object, error) \
+ (wpi_setStaticWPIErrorWithContext(object, error, ""))
+#define wpi_setGlobalWPIErrorWithContext(error, context) \
+ (ErrorBase::SetGlobalWPIError((wpi_error_s_##error), (context), \
+ __FILE__, __FUNCTION__, __LINE__))
+#define wpi_setGlobalWPIError(error) \
+ (wpi_setGlobalWPIErrorWithContext(error, ""))
/**
* Base class for most objects.
* ErrorBase is the base class for most objects since it holds the generated error
* for that object. In addition, there is a single instance of a global error object
+ *
+ * BE AWARE: This does include a mutable instance variable! This means that even
+ * if you make an object const it's not really. However, all modification to
+ * that instance variable is protected by a semaphore, so it does not create
+ * more thread safety issues.
+ *
+ * All of the Set*Error methods will update the global error if there is nothing
+ * there already.
*/
class ErrorBase
{
-//TODO: Consider initializing instance variables and cleanup in destructor
public:
+ ErrorBase();
virtual ~ErrorBase();
- virtual Error& GetError();
- virtual const Error& GetError() const;
- virtual void SetErrnoError(const char *contextMessage,
+
+ Error& GetError();
+ const Error& GetError() const;
+ void SetErrnoError(const char *contextMessage,
const char* filename, const char* function, UINT32 lineNumber) const;
- virtual void SetImaqError(int success, const char *contextMessage,
+ void SetImaqError(int success, const char *contextMessage,
const char* filename, const char* function, UINT32 lineNumber) const;
- virtual void SetError(Error::Code code, const char *contextMessage,
+ void SetError(Error::Code code, const char *contextMessage,
const char* filename, const char* function, UINT32 lineNumber) const;
- virtual void SetWPIError(const char *errorMessage, const char *contextMessage,
+ void SetWPIError(const char *errorMessage, const char *contextMessage,
const char* filename, const char* function, UINT32 lineNumber) const;
- virtual void CloneError(ErrorBase *rhs) const;
- virtual void ClearError() const;
- virtual bool StatusIsFatal() const;
+ void CloneError(const ErrorBase *rhs) const;
+ void ClearError() const;
+ bool StatusIsFatal() const;
static void SetGlobalError(Error::Code code, const char *contextMessage,
const char* filename, const char* function, UINT32 lineNumber);
static void SetGlobalWPIError(const char *errorMessage, const char *contextMessage,
const char* filename, const char* function, UINT32 lineNumber);
- static Error& GetGlobalError();
+ static const Error& GetGlobalError();
+
protected:
+ // This mutable is safe because Error guarantees that all modifications are
+ // protected with an internal lock.
mutable Error m_error;
// TODO: Replace globalError with a global list of all errors.
- static SEM_ID _globalErrorMutex;
static Error _globalError;
- ErrorBase();
+
private:
DISALLOW_COPY_AND_ASSIGN(ErrorBase);
};
diff --git a/aos/externals/WPILib/WPILib/PWM.cpp b/aos/externals/WPILib/WPILib/PWM.cpp
index 05bf16d..7ae5370 100644
--- a/aos/externals/WPILib/WPILib/PWM.cpp
+++ b/aos/externals/WPILib/WPILib/PWM.cpp
@@ -42,9 +42,9 @@
}
snprintf(buf, 64, "PWM %d (Module: %d)", channel, moduleNumber);
- if (allocated->Allocate((moduleNumber - 1) * kPwmChannels + channel - 1, buf) == ~0ul)
+ if (allocated->Allocate((moduleNumber - 1) * kPwmChannels + channel - 1,
+ buf, this) == ~0ul)
{
- CloneError(allocated);
return;
}
m_channel = channel;
@@ -92,7 +92,8 @@
if (m_module)
{
m_module->SetPWM(m_channel, kPwmDisabled);
- allocated->Free((m_module->GetNumber() - 1) * kPwmChannels + m_channel - 1);
+ allocated->Free((m_module->GetNumber() - 1) * kPwmChannels + m_channel - 1,
+ this);
}
}
diff --git a/aos/externals/WPILib/WPILib/Relay.cpp b/aos/externals/WPILib/WPILib/Relay.cpp
index dd59d52..9fb3a2d 100644
--- a/aos/externals/WPILib/WPILib/Relay.cpp
+++ b/aos/externals/WPILib/WPILib/Relay.cpp
@@ -43,9 +43,9 @@
if (m_direction == kBothDirections || m_direction == kForwardOnly)
{
snprintf(buf, 64, "Forward Relay %d (Module: %d)", m_channel, moduleNumber);
- if (relayChannels->Allocate(((moduleNumber - 1) * kRelayChannels + m_channel - 1) * 2, buf) == ~0ul)
+ if (relayChannels->Allocate(((moduleNumber - 1) * kRelayChannels +
+ m_channel - 1) * 2, buf, this) == ~0ul)
{
- CloneError(relayChannels);
return;
}
@@ -54,9 +54,9 @@
if (m_direction == kBothDirections || m_direction == kReverseOnly)
{
snprintf(buf, 64, "Reverse Relay %d (Module: %d)", m_channel, moduleNumber);
- if (relayChannels->Allocate(((moduleNumber - 1) * kRelayChannels + m_channel - 1) * 2 + 1, buf) == ~0ul)
+ if (relayChannels->Allocate(((moduleNumber - 1) * kRelayChannels
+ + m_channel - 1) * 2 + 1, buf, this) == ~0ul)
{
- CloneError(relayChannels);
return;
}
@@ -105,11 +105,13 @@
if (m_direction == kBothDirections || m_direction == kForwardOnly)
{
- relayChannels->Free(((m_module->GetNumber() - 1) * kRelayChannels + m_channel - 1) * 2);
+ relayChannels->Free(((m_module->GetNumber() - 1) *
+ kRelayChannels + m_channel - 1) * 2, this);
}
if (m_direction == kBothDirections || m_direction == kReverseOnly)
{
- relayChannels->Free(((m_module->GetNumber() - 1) * kRelayChannels + m_channel - 1) * 2 + 1);
+ relayChannels->Free(((m_module->GetNumber() - 1) *
+ kRelayChannels + m_channel - 1) * 2 + 1, this);
}
}
diff --git a/aos/externals/WPILib/WPILib/Resource.cpp b/aos/externals/WPILib/WPILib/Resource.cpp
index ec35eb3..ef7d15d 100644
--- a/aos/externals/WPILib/WPILib/Resource.cpp
+++ b/aos/externals/WPILib/WPILib/Resource.cpp
@@ -6,7 +6,6 @@
#include "Resource.h"
#include "WPIErrors.h"
-#include "ErrorBase.h"
ReentrantSemaphore Resource::m_createLock;
@@ -28,6 +27,7 @@
/**
* Factory method to create a Resource allocation-tracker *if* needed.
+ * Handles the necessary synchronization internally.
*
* @param r -- address of the caller's Resource pointer. If *r == NULL, this
* will construct a Resource and make *r point to it. If *r != NULL, i.e.
@@ -59,7 +59,7 @@
* When a resource is requested, mark it allocated. In this case, a free resource value
* within the range is located and returned after it is marked allocated.
*/
-UINT32 Resource::Allocate(const char *resourceDesc)
+UINT32 Resource::Allocate(const char *resourceDesc, const ErrorBase *error)
{
Synchronized sync(m_allocateLock);
for (UINT32 i=0; i < m_size; i++)
@@ -70,7 +70,7 @@
return i;
}
}
- wpi_setWPIErrorWithContext(NoAvailableResources, resourceDesc);
+ wpi_setStaticWPIErrorWithContext(error, NoAvailableResources, resourceDesc);
return ~0ul;
}
@@ -79,17 +79,18 @@
* The user requests a specific resource value, i.e. channel number and it is verified
* unallocated, then returned.
*/
-UINT32 Resource::Allocate(UINT32 index, const char *resourceDesc)
+UINT32 Resource::Allocate(UINT32 index, const char *resourceDesc,
+ const ErrorBase *error)
{
Synchronized sync(m_allocateLock);
if (index >= m_size)
{
- wpi_setWPIErrorWithContext(ChannelIndexOutOfRange, resourceDesc);
+ wpi_setStaticWPIErrorWithContext(error, ChannelIndexOutOfRange, resourceDesc);
return ~0ul;
}
if ( m_isAllocated[index] )
{
- wpi_setWPIErrorWithContext(ResourceAlreadyAllocated, resourceDesc);
+ wpi_setStaticWPIErrorWithContext(error, ResourceAlreadyAllocated, resourceDesc);
return ~0ul;
}
m_isAllocated[index] = true;
@@ -102,18 +103,18 @@
* After a resource is no longer needed, for example a destructor is called for a channel assignment
* class, Free will release the resource value so it can be reused somewhere else in the program.
*/
-void Resource::Free(UINT32 index)
+void Resource::Free(UINT32 index, const ErrorBase *error)
{
Synchronized sync(m_allocateLock);
if (index == ~0ul) return;
if (index >= m_size)
{
- wpi_setWPIError(NotAllocated);
+ wpi_setStaticWPIError(error, NotAllocated);
return;
}
if (!m_isAllocated[index])
{
- wpi_setWPIError(NotAllocated);
+ wpi_setStaticWPIError(error, NotAllocated);
return;
}
m_isAllocated[index] = false;
diff --git a/aos/externals/WPILib/WPILib/Resource.h b/aos/externals/WPILib/WPILib/Resource.h
index dbe1936..11faaec 100644
--- a/aos/externals/WPILib/WPILib/Resource.h
+++ b/aos/externals/WPILib/WPILib/Resource.h
@@ -19,15 +19,18 @@
* The Resource class does not allocate the hardware channels or other
* resources; it just tracks which indices were marked in use by
* Allocate and not yet freed by Free.
+ *
+ * Several methods require an ErrorBase* so that they can report any errors.
*/
-class Resource : public ErrorBase
+class Resource
{
public:
virtual ~Resource();
static void CreateResourceObject(Resource **r, UINT32 elements);
- UINT32 Allocate(const char *resourceDesc);
- UINT32 Allocate(UINT32 index, const char *resourceDesc);
- void Free(UINT32 index);
+ UINT32 Allocate(const char *resourceDesc, const ErrorBase *error);
+ UINT32 Allocate(UINT32 index, const char *resourceDesc,
+ const ErrorBase *error);
+ void Free(UINT32 index, const ErrorBase *error);
private:
explicit Resource(UINT32 size);
diff --git a/aos/externals/WPILib/WPILib/Solenoid.cpp b/aos/externals/WPILib/WPILib/Solenoid.cpp
index 35813af..7a56a09 100644
--- a/aos/externals/WPILib/WPILib/Solenoid.cpp
+++ b/aos/externals/WPILib/WPILib/Solenoid.cpp
@@ -30,9 +30,10 @@
Resource::CreateResourceObject(&m_allocated, tSolenoid::kNumDO7_0Elements * kSolenoidChannels);
snprintf(buf, 64, "Solenoid %d (Module: %d)", m_channel, m_moduleNumber);
- if (m_allocated->Allocate((m_moduleNumber - 1) * kSolenoidChannels + m_channel - 1, buf) == ~0ul)
+ if (m_allocated->Allocate((m_moduleNumber - 1) * kSolenoidChannels
+ + m_channel - 1,
+ buf, this) == ~0ul)
{
- CloneError(m_allocated);
return;
}
@@ -72,7 +73,8 @@
{
if (CheckSolenoidModule(m_moduleNumber))
{
- m_allocated->Free((m_moduleNumber - 1) * kSolenoidChannels + m_channel - 1);
+ m_allocated->Free((m_moduleNumber - 1) * kSolenoidChannels + m_channel - 1,
+ this);
}
}
diff --git a/aos/externals/WPILib/WPILib/Synchronized.cpp b/aos/externals/WPILib/WPILib/Synchronized.cpp
index eb17318..fd133b5 100644
--- a/aos/externals/WPILib/WPILib/Synchronized.cpp
+++ b/aos/externals/WPILib/WPILib/Synchronized.cpp
@@ -20,7 +20,7 @@
semTake(m_semaphore, WAIT_FOREVER);
}
-Synchronized::Synchronized(ReentrantSemaphore& semaphore)
+Synchronized::Synchronized(const ReentrantSemaphore& semaphore)
{
m_semaphore = semaphore.m_semaphore;
semTake(m_semaphore, WAIT_FOREVER);
diff --git a/aos/externals/WPILib/WPILib/Synchronized.h b/aos/externals/WPILib/WPILib/Synchronized.h
index 28cfbc3..4e44b0e 100644
--- a/aos/externals/WPILib/WPILib/Synchronized.h
+++ b/aos/externals/WPILib/WPILib/Synchronized.h
@@ -27,11 +27,15 @@
*
* This class is safe to use in static variables because it does not depend on
* any other C++ static constructors or destructors.
+ *
+ * The instance methods are marked const because using this class from multiple
+ * threads at once is safe and they don't actually modify the value of any of
+ * the instance variables.
*/
class ReentrantSemaphore
{
public:
- explicit ReentrantSemaphore() {
+ ReentrantSemaphore() {
m_semaphore = semMCreate(SEM_Q_PRIORITY | SEM_DELETE_SAFE);
}
~ReentrantSemaphore() {
@@ -42,7 +46,7 @@
* Lock the semaphore, blocking until it's available.
* @return 0 for success, -1 for error. If -1, the error will be in errno.
*/
- int take() {
+ int take() const {
return semTake(m_semaphore, WAIT_FOREVER);
}
@@ -50,7 +54,7 @@
* Unlock the semaphore.
* @return 0 for success, -1 for error. If -1, the error will be in errno.
*/
- int give() {
+ int give() const {
return semGive(m_semaphore);
}
@@ -83,8 +87,8 @@
{
public:
explicit Synchronized(SEM_ID);
- explicit Synchronized(ReentrantSemaphore&);
- virtual ~Synchronized();
+ explicit Synchronized(const ReentrantSemaphore&);
+ ~Synchronized();
private:
SEM_ID m_semaphore;