From 1fe3c48771244456fbc3aa24928dc52af07017ef Mon Sep 17 00:00:00 2001 From: Simon Newton Date: Sun, 7 Dec 2014 16:01:28 -0800 Subject: [PATCH 1/2] Move GPIO writes to a separate thread. Fix some bugs. --- plugins/gpio/GPIODriver.cpp | 61 ++++++++++++++++++++++++++++--- plugins/gpio/GPIODriver.h | 11 +++++- plugins/gpio/GPIOPlugin.cpp | 2 +- plugins/gpio/GPIOPlugin.h | 3 +- plugins/opendmx/OpenDmxThread.cpp | 1 - 5 files changed, 68 insertions(+), 10 deletions(-) diff --git a/plugins/gpio/GPIODriver.cpp b/plugins/gpio/GPIODriver.cpp index f6a3430d2f..0e4a4c7e24 100644 --- a/plugins/gpio/GPIODriver.cpp +++ b/plugins/gpio/GPIODriver.cpp @@ -34,6 +34,7 @@ #include "ola/io/IOUtils.h" #include "ola/Logging.h" +#include "ola/thread/Mutex.h" namespace ola { namespace plugin { @@ -41,16 +42,24 @@ namespace gpio { const char GPIODriver::GPIO_BASE_DIR[] = "/sys/class/gpio/gpio"; -using std::vector; - +using ola::thread::MutexLocker; using std::string; using std::vector; GPIODriver::GPIODriver(const Options &options) - : m_options(options) { + : m_options(options), + m_term(false), + m_dmx_changed(false) { } GPIODriver::~GPIODriver() { + { + MutexLocker locker(&m_mutex); + m_term = true; + } + m_cond.Signal(); + Join(); + CloseGPIOFDs(); } @@ -59,11 +68,51 @@ bool GPIODriver::Init() { return false; } - return true; + return Start(); } bool GPIODriver::SendDmx(const DmxBuffer &dmx) { - return UpdateGPIOPins(dmx); + { + MutexLocker locker(&m_mutex); + // avoid the reference counting + m_buffer.Set(dmx); + m_dmx_changed = true; + } + m_cond.Signal(); + return true; +} + +void *GPIODriver::Run() { + Clock clock; + DmxBuffer output; + + while (true) { + bool update_pins = false; + + TimeStamp wake_up; + clock.CurrentTime(&wake_up); + wake_up += TimeInterval(1, 0); + + // Wait for one of: i) termination ii) DMX changed iii) timeout + m_mutex.Lock(); + if (!m_term && !m_dmx_changed) { + m_cond.TimedWait(&m_mutex, wake_up); + } + + if (m_term) { + m_mutex.Unlock(); + break; + } else if (m_dmx_changed) { + output.Set(m_buffer); + m_dmx_changed = false; + update_pins = true; + } + m_mutex.Unlock(); + if (update_pins) { + UpdateGPIOPins(output); + } + } + return NULL; } bool GPIODriver::SetupGPIO() { @@ -79,7 +128,7 @@ bool GPIODriver::SetupGPIO() { std::ostringstream str; str << GPIO_BASE_DIR << static_cast(*iter) << "/value"; int pin_fd; - if (ola::io::Open(str.str(), O_RDWR, &pin_fd)) { + if (!ola::io::Open(str.str(), O_RDWR, &pin_fd)) { failed = true; break; } diff --git a/plugins/gpio/GPIODriver.h b/plugins/gpio/GPIODriver.h index 47c1ec4858..5fc83a3d93 100644 --- a/plugins/gpio/GPIODriver.h +++ b/plugins/gpio/GPIODriver.h @@ -24,6 +24,7 @@ #include #include #include +#include #include @@ -34,7 +35,7 @@ namespace gpio { /** * @brief Uses data in a DMXBuffer to drive GPIO pins. */ -class GPIODriver { +class GPIODriver : private ola::thread::Thread { public: /** * @brief The Options. @@ -94,6 +95,8 @@ class GPIODriver { */ bool SendDmx(const DmxBuffer &dmx); + void *Run(); + private: enum GPIOState { ON, @@ -112,6 +115,12 @@ class GPIODriver { const Options m_options; GPIOPins m_gpio_pins; + DmxBuffer m_buffer; + bool m_term; // GUARDED_BY(m_mutex); + bool m_dmx_changed; // GUARDED_BY(m_mutex); + ola::thread::Mutex m_mutex; + ola::thread::ConditionVariable m_cond; + bool SetupGPIO(); bool UpdateGPIOPins(const DmxBuffer &dmx); void CloseGPIOFDs(); diff --git a/plugins/gpio/GPIOPlugin.cpp b/plugins/gpio/GPIOPlugin.cpp index 16fd4fe1e0..c47d809e99 100644 --- a/plugins/gpio/GPIOPlugin.cpp +++ b/plugins/gpio/GPIOPlugin.cpp @@ -103,7 +103,7 @@ bool GPIOPlugin::StartHook() { bool GPIOPlugin::StopHook() { if (m_device) { m_plugin_adaptor->UnregisterDevice(m_device); - return m_device->Stop(); + m_device->Stop(); delete m_device; m_device = NULL; } diff --git a/plugins/gpio/GPIOPlugin.h b/plugins/gpio/GPIOPlugin.h index 1bc0ddc2a0..b66ea568e5 100644 --- a/plugins/gpio/GPIOPlugin.h +++ b/plugins/gpio/GPIOPlugin.h @@ -39,7 +39,8 @@ class GPIOPlugin: public ola::Plugin { * @param plugin_adaptor the PluginAdaptor to use */ explicit GPIOPlugin(class ola::PluginAdaptor *plugin_adaptor) - : Plugin(plugin_adaptor) {} + : Plugin(plugin_adaptor), + m_device(NULL) {} std::string Name() const { return PLUGIN_NAME; } std::string Description() const; diff --git a/plugins/opendmx/OpenDmxThread.cpp b/plugins/opendmx/OpenDmxThread.cpp index dab20ef90a..31582e8d4a 100644 --- a/plugins/opendmx/OpenDmxThread.cpp +++ b/plugins/opendmx/OpenDmxThread.cpp @@ -89,7 +89,6 @@ void *OpenDmxThread::Run() { m_term_cond.TimedWait(&m_term_mutex, wake_up); m_term_mutex.Unlock(); - ola::io::Open(m_path, O_WRONLY, &m_fd); } else { From 49e673ba85ff2aee068e3654bfab581485ad45d0 Mon Sep 17 00:00:00 2001 From: Simon Newton Date: Sun, 7 Dec 2014 16:22:04 -0800 Subject: [PATCH 2/2] Another GPIO fix. --- plugins/gpio/GPIODriver.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/gpio/GPIODriver.cpp b/plugins/gpio/GPIODriver.cpp index 0e4a4c7e24..2214e4bb5a 100644 --- a/plugins/gpio/GPIODriver.cpp +++ b/plugins/gpio/GPIODriver.cpp @@ -133,6 +133,8 @@ bool GPIODriver::SetupGPIO() { break; } + GPIOPin pin = {pin_fd, UNDEFINED, false}; + // Set dir str.str(""); str << GPIO_BASE_DIR << static_cast(*iter) << "/direction"; @@ -148,7 +150,6 @@ bool GPIODriver::SetupGPIO() { } close(fd); - GPIOPin pin = {fd, UNDEFINED, false}; m_gpio_pins.push_back(pin); }