From 2356008d8cf2c5c588696a97e9a7d6933f549a8a Mon Sep 17 00:00:00 2001 From: Thomas Clark Date: Wed, 6 Aug 2014 09:46:50 -0400 Subject: [PATCH] Fixed interrupt freeing in C++ When interrupts are cancelled on any interruptable class, the resource is now freed. Previously, the resource was only freed if the object is destructed before CancelInterrupts() is called, so it was impossible to create and destruct more than 8 interrupts. The interrupts resource object is now in InterruptableSensorBase instead of SensorBase. A synchronous interrupt integration test was added. Change-Id: I0806176340cecd4c1480dd8f043474cc05919f24 --- .../include/InterruptableSensorBase.h | 3 ++ wpilibc/wpilibC++/include/SensorBase.h | 2 -- .../wpilibC++/lib/InterruptableSensorBase.cpp | 4 +++ wpilibc/wpilibC++/lib/SensorBase.cpp | 3 -- .../src/DIOLoopTest.cpp | 28 ++++++++++++++++++- 5 files changed, 34 insertions(+), 6 deletions(-) diff --git a/wpilibc/wpilibC++/include/InterruptableSensorBase.h b/wpilibc/wpilibC++/include/InterruptableSensorBase.h index 1e04243e53..1c4f161bc2 100644 --- a/wpilibc/wpilibC++/include/InterruptableSensorBase.h +++ b/wpilibc/wpilibC++/include/InterruptableSensorBase.h @@ -7,6 +7,7 @@ #include "HAL/HAL.hpp" #include "SensorBase.h" +#include "Resource.h" class InterruptableSensorBase : public SensorBase { @@ -24,4 +25,6 @@ protected: void* m_interrupt; uint32_t m_interruptIndex; void AllocateInterrupts(bool watcher); + + static Resource *m_interrupts; }; diff --git a/wpilibc/wpilibC++/include/SensorBase.h b/wpilibc/wpilibC++/include/SensorBase.h index 08ba1d25b3..a629e1864b 100644 --- a/wpilibc/wpilibC++/include/SensorBase.h +++ b/wpilibc/wpilibC++/include/SensorBase.h @@ -8,7 +8,6 @@ #include "ErrorBase.h" #include #include "Base.h" -#include "Resource.h" /** * Base class for all sensors. @@ -52,7 +51,6 @@ protected: static void* m_digital_ports[kDigitalChannels]; static void* m_relay_ports[kRelayChannels]; static void* m_pwm_ports[kPwmChannels]; - static Resource *m_interrupts; private: DISALLOW_COPY_AND_ASSIGN(SensorBase); diff --git a/wpilibc/wpilibC++/lib/InterruptableSensorBase.cpp b/wpilibc/wpilibC++/lib/InterruptableSensorBase.cpp index 30b8a5305f..02f5713684 100644 --- a/wpilibc/wpilibC++/lib/InterruptableSensorBase.cpp +++ b/wpilibc/wpilibC++/lib/InterruptableSensorBase.cpp @@ -7,9 +7,12 @@ #include "InterruptableSensorBase.h" #include "Utility.h" +Resource *InterruptableSensorBase::m_interrupts = NULL; + InterruptableSensorBase::InterruptableSensorBase() { m_interrupt = NULL; + Resource::CreateResourceObject(&m_interrupts, interrupt_kNumSystems); } InterruptableSensorBase::~InterruptableSensorBase() @@ -37,6 +40,7 @@ void InterruptableSensorBase::CancelInterrupts() cleanInterrupts(m_interrupt, &status); wpi_setErrorWithContext(status, getHALErrorMessage(status)); m_interrupt = NULL; + m_interrupts->Free(m_interruptIndex); } /** diff --git a/wpilibc/wpilibC++/lib/SensorBase.cpp b/wpilibc/wpilibC++/lib/SensorBase.cpp index 1a5fbbbbd3..a7942b8013 100644 --- a/wpilibc/wpilibC++/lib/SensorBase.cpp +++ b/wpilibc/wpilibC++/lib/SensorBase.cpp @@ -17,7 +17,6 @@ const uint32_t SensorBase::kPwmChannels; const uint32_t SensorBase::kRelayChannels; const uint32_t SensorBase::kPDPChannels; const uint32_t SensorBase::kChassisSlots; -Resource *SensorBase::m_interrupts = NULL; SensorBase *SensorBase::m_singletonList = NULL; static bool portsInitialized = false; @@ -55,8 +54,6 @@ SensorBase::SensorBase() wpi_setErrorWithContext(status, getHALErrorMessage(status)); } } - - Resource::CreateResourceObject(&m_interrupts, interrupt_kNumSystems); } /** diff --git a/wpilibc/wpilibC++IntegrationTests/src/DIOLoopTest.cpp b/wpilibc/wpilibC++IntegrationTests/src/DIOLoopTest.cpp index 507023a0b7..aadfeb395f 100644 --- a/wpilibc/wpilibC++IntegrationTests/src/DIOLoopTest.cpp +++ b/wpilibc/wpilibC++IntegrationTests/src/DIOLoopTest.cpp @@ -11,6 +11,9 @@ static const double kDelayTime = 0.1; +static const double kSynchronousInterruptTime = 2.0; +static const double kSynchronousInterruptTimeTolerance = 0.01; + /** * A fixture with a digital input and a digital output physically wired * together. @@ -79,7 +82,7 @@ static void InterruptHandler(uint32_t interruptAssertedMask, void *param) { *(int *)param = 12345; } -TEST_F(DIOLoopTest, AsynchronusInterruptWorks) { +TEST_F(DIOLoopTest, AsynchronousInterruptWorks) { int param = 0; // Given an interrupt handler that sets an int to 12345 @@ -95,3 +98,26 @@ TEST_F(DIOLoopTest, AsynchronusInterruptWorks) { Wait(kDelayTime); EXPECT_EQ(12345, param) << "The interrupt did not run."; } + +static void *InterruptTriggerer(void *data) { + DigitalOutput *output = static_cast(data); + output->Set(false); + Wait(kSynchronousInterruptTime); + output->Set(true); + return NULL; +} + +TEST_F(DIOLoopTest, SynchronousInterruptWorks) { + // Given a synchronous interrupt + m_input->RequestInterrupts(); + + // If we have another thread trigger the interrupt in a few seconds + pthread_t interruptTriggererLoop; + pthread_create(&interruptTriggererLoop, NULL, InterruptTriggerer, m_output); + + // Then this thread should pause and resume after that number of seconds + Timer timer; + timer.Start(); + m_input->WaitForInterrupt(kSynchronousInterruptTime + 1.0); + EXPECT_NEAR(kSynchronousInterruptTime, timer.Get(), kSynchronousInterruptTimeTolerance); +}