From 13fd6a6619b36bef2df2a126455f83776cc991b4 Mon Sep 17 00:00:00 2001 From: Lars Op den Kamp Date: Sat, 8 Oct 2011 01:39:35 +0200 Subject: [PATCH] cec: removed dupe m_bRunning properties. wait until a thread is started before returning from CreateThread(). shorter read times, so the comm mutex doesn't stay locked too long. --- src/lib/AdapterCommunication.cpp | 91 +++++++++++++++++++------------- src/lib/AdapterCommunication.h | 6 +-- src/lib/CECProcessor.cpp | 18 ++++--- src/lib/LibCEC.cpp | 14 ++--- src/lib/LibCEC.h | 1 - src/lib/platform/threads.cpp | 63 +++++++++++++--------- src/lib/platform/threads.h | 13 +++-- 7 files changed, 114 insertions(+), 92 deletions(-) diff --git a/src/lib/AdapterCommunication.cpp b/src/lib/AdapterCommunication.cpp index 84ec74c..627ce27 100644 --- a/src/lib/AdapterCommunication.cpp +++ b/src/lib/AdapterCommunication.cpp @@ -44,9 +44,7 @@ CAdapterCommunication::CAdapterCommunication(CLibCEC *controller) : m_controller(controller), m_inbuf(NULL), m_iInbufSize(0), - m_iInbufUsed(0), - m_bStarted(false), - m_bStop(false) + m_iInbufUsed(0) { m_port = new CSerialPort; } @@ -67,8 +65,17 @@ CAdapterCommunication::~CAdapterCommunication(void) bool CAdapterCommunication::Open(const char *strPort, uint16_t iBaudRate /* = 38400 */, uint32_t iTimeoutMs /* = 10000 */) { - if (m_bStarted || !m_port) + CLockObject lock(&m_mutex); + if (!m_port) + { + m_controller->AddLog(CEC_LOG_ERROR, "port is NULL"); return false; + } + + if (IsOpen()) + { + m_controller->AddLog(CEC_LOG_ERROR, "port is already open"); + } if (!m_port->Open(strPort, iBaudRate)) { @@ -88,13 +95,12 @@ bool CAdapterCommunication::Open(const char *strPort, uint16_t iBaudRate /* = 38 if (CreateThread()) { - m_controller->AddLog(CEC_LOG_DEBUG, "reader thread created"); - m_bStarted = true; + m_controller->AddLog(CEC_LOG_DEBUG, "communication thread created"); return true; } else { - m_controller->AddLog(CEC_LOG_DEBUG, "could not create a reader thread"); + m_controller->AddLog(CEC_LOG_DEBUG, "could not create a communication thread"); } return false; @@ -102,62 +108,63 @@ bool CAdapterCommunication::Open(const char *strPort, uint16_t iBaudRate /* = 38 void CAdapterCommunication::Close(void) { - m_bStop = true; m_rcvCondition.Broadcast(); + CLockObject lock(&m_mutex); StopThread(); - if (m_port) - m_port->Close(); + if (m_inbuf) + { + free(m_inbuf); + m_inbuf = NULL; + m_iInbufSize = 0; + m_iInbufUsed = 0; + } } void *CAdapterCommunication::Process(void) { m_controller->AddLog(CEC_LOG_DEBUG, "communication thread started"); - while (!m_bStop) + while (!IsStopped()) { - if (!ReadFromDevice(1000)) + bool bSignal(false); { - m_bStarted = false; - break; + CLockObject lock(&m_mutex, true); + if (lock.IsLocked()) + bSignal = ReadFromDevice(100); } - if (!m_bStop) + if (bSignal) + m_rcvCondition.Signal(); + + if (!IsStopped()) Sleep(50); } - m_bStarted = false; return NULL; } bool CAdapterCommunication::ReadFromDevice(uint32_t iTimeout) { int32_t iBytesRead; + uint8_t buff[1024]; + if (!m_port) + return false; + iBytesRead = m_port->Read(buff, sizeof(buff), iTimeout); + if (iBytesRead < 0 || iBytesRead > 256) { - CLockObject lock(&m_mutex); - - uint8_t buff[1024]; - if (!m_port) - return false; - - iBytesRead = m_port->Read(buff, sizeof(buff), iTimeout); - if (iBytesRead < 0 || iBytesRead > 256) - { - CStdString strError; - strError.Format("error reading from serial port: %s", m_port->GetError().c_str()); - m_controller->AddLog(CEC_LOG_ERROR, strError); - return false; - } - else if (iBytesRead > 0) - AddData(buff, (uint8_t) iBytesRead); + CStdString strError; + strError.Format("error reading from serial port: %s", m_port->GetError().c_str()); + m_controller->AddLog(CEC_LOG_ERROR, strError); + StopThread(false); + return false; } + else if (iBytesRead > 0) + AddData(buff, (uint8_t) iBytesRead); - if (iBytesRead > 0) - m_rcvCondition.Signal(); - - return true; + return iBytesRead > 0; } void CAdapterCommunication::AddData(uint8_t *data, uint8_t iLen) @@ -197,9 +204,12 @@ bool CAdapterCommunication::Read(cec_frame &msg, uint32_t iTimeout) CLockObject lock(&m_mutex); if (m_iInbufUsed < 1) - m_rcvCondition.Wait(&m_mutex, iTimeout); + { + if (!m_rcvCondition.Wait(&m_mutex, iTimeout)) + return false; + } - if (m_iInbufUsed < 1 || m_bStop) + if (m_iInbufUsed < 1 || IsStopped()) return false; //search for first start of message @@ -374,3 +384,8 @@ bool CAdapterCommunication::PingAdapter(void) // TODO check for pong return true; } + +bool CAdapterCommunication::IsOpen(void) const +{ + return !IsStopped() && m_port->IsOpen(); +} diff --git a/src/lib/AdapterCommunication.h b/src/lib/AdapterCommunication.h index 8ce9c5b..135b5c2 100644 --- a/src/lib/AdapterCommunication.h +++ b/src/lib/AdapterCommunication.h @@ -40,7 +40,7 @@ namespace CEC class CSerialPort; class CLibCEC; - class CAdapterCommunication : CThread + class CAdapterCommunication : private CThread { public: CAdapterCommunication(CLibCEC *controller); @@ -51,7 +51,7 @@ namespace CEC bool Write(const cec_frame &frame); bool PingAdapter(void); void Close(void); - bool IsOpen(void) const { return !m_bStop && m_bStarted; } + bool IsOpen(void) const; std::string GetError(void) const; void *Process(void); @@ -68,8 +68,6 @@ namespace CEC uint8_t* m_inbuf; int16_t m_iInbufSize; int16_t m_iInbufUsed; - bool m_bStarted; - bool m_bStop; CMutex m_mutex; CCondition m_rcvCondition; }; diff --git a/src/lib/CECProcessor.cpp b/src/lib/CECProcessor.cpp index 74f03b3..6a4faff 100644 --- a/src/lib/CECProcessor.cpp +++ b/src/lib/CECProcessor.cpp @@ -59,7 +59,10 @@ CCECProcessor::~CCECProcessor(void) bool CCECProcessor::Start(void) { if (!m_communication || !m_communication->IsOpen()) + { + m_controller->AddLog(CEC_LOG_ERROR, "connection is closed"); return false; + } if (!SetLogicalAddress(m_iLogicalAddress)) { @@ -79,7 +82,7 @@ void *CCECProcessor::Process(void) { m_controller->AddLog(CEC_LOG_DEBUG, "processor thread started"); - while (!m_bStop) + while (!IsStopped()) { bool bParseFrame(false); { @@ -87,18 +90,17 @@ void *CCECProcessor::Process(void) cec_frame msg; msg.clear(); - if (!m_bStop && m_communication->IsOpen() && m_communication->Read(msg, CEC_BUTTON_TIMEOUT)) - bParseFrame = ParseMessage(msg); + if (m_communication->IsOpen() && m_communication->Read(msg, CEC_BUTTON_TIMEOUT)) + bParseFrame = ParseMessage(msg) && !IsStopped(); } - if (!m_bStop && bParseFrame) + if (bParseFrame) ParseCurrentFrame(); - if (!m_bStop) - { - m_controller->CheckKeypressTimeout(); + m_controller->CheckKeypressTimeout(); + + if (!IsStopped()) Sleep(50); - } } return NULL; diff --git a/src/lib/LibCEC.cpp b/src/lib/LibCEC.cpp index 652d732..0f88470 100644 --- a/src/lib/LibCEC.cpp +++ b/src/lib/LibCEC.cpp @@ -42,7 +42,6 @@ using namespace std; using namespace CEC; CLibCEC::CLibCEC(const char *strDeviceName, cec_logical_address iLogicalAddress /* = CECDEVICE_PLAYBACKDEVICE1 */, uint16_t iPhysicalAddress /* = CEC_DEFAULT_PHYSICAL_ADDRESS */) : - m_bStarted(false), m_iCurrentButton(CEC_USER_CONTROL_CODE_UNKNOWN), m_buttontime(0) { @@ -90,17 +89,15 @@ bool CLibCEC::Open(const char *strPort, uint32_t iTimeoutMs /* = 10000 */) return false; } - m_bStarted = true; return true; } void CLibCEC::Close(void) { - if (m_comm) - m_comm->Close(); if (m_cec) m_cec->StopThread(); - m_bStarted = false; + if (m_comm) + m_comm->Close(); } int8_t CLibCEC::FindAdapters(cec_adapter *deviceList, uint8_t iBufSize, const char *strDevicePath /* = NULL */) @@ -182,7 +179,7 @@ bool CLibCEC::SetInactiveView(void) void CLibCEC::AddLog(cec_log_level level, const string &strMessage) { - if (m_bStarted) + if (m_cec) { cec_log_message message; message.level = level; @@ -193,7 +190,7 @@ void CLibCEC::AddLog(cec_log_level level, const string &strMessage) void CLibCEC::AddKey(void) { - if (m_bStarted && m_iCurrentButton != CEC_USER_CONTROL_CODE_UNKNOWN) + if (m_iCurrentButton != CEC_USER_CONTROL_CODE_UNKNOWN) { cec_keypress key; @@ -207,9 +204,6 @@ void CLibCEC::AddKey(void) void CLibCEC::AddCommand(cec_logical_address source, cec_logical_address destination, cec_opcode opcode, cec_frame *parameters) { - if (!m_bStarted) - return; - cec_command command; command.clear(); diff --git a/src/lib/LibCEC.h b/src/lib/LibCEC.h index 58c2262..2a84952 100644 --- a/src/lib/LibCEC.h +++ b/src/lib/LibCEC.h @@ -80,7 +80,6 @@ namespace CEC virtual void SetCurrentButton(cec_user_control_code iButtonCode); protected: - bool m_bStarted; cec_user_control_code m_iCurrentButton; int64_t m_buttontime; CCECProcessor *m_cec; diff --git a/src/lib/platform/threads.cpp b/src/lib/platform/threads.cpp index 4695316..31a0fb3 100644 --- a/src/lib/platform/threads.cpp +++ b/src/lib/platform/threads.cpp @@ -60,11 +60,11 @@ void CMutex::Unlock(void) pthread_mutex_unlock(&m_mutex); } -CLockObject::CLockObject(CMutex *mutex) : +CLockObject::CLockObject(CMutex *mutex, bool bTryLock /* = false */) : m_mutex(mutex) { if (m_mutex) - m_mutex->Lock(); + m_bLocked = bTryLock ? m_mutex->TryLock() : m_mutex->Lock(); } CLockObject::~CLockObject(void) @@ -75,14 +75,17 @@ CLockObject::~CLockObject(void) void CLockObject::Leave(void) { - if (m_mutex) + if (m_mutex && m_bLocked) + { + m_bLocked = false; m_mutex->Unlock(); + } } void CLockObject::Lock(void) { if (m_mutex) - m_mutex->Lock(); + m_bLocked = m_mutex->Lock(); } CCondition::CCondition(void) @@ -106,19 +109,26 @@ void CCondition::Signal(void) pthread_cond_signal(&m_cond); } -bool CCondition::Wait(CMutex *mutex, uint32_t iTimeout) +bool CCondition::Wait(CMutex *mutex, uint32_t iTimeout /* = 0 */) { bool bReturn(false); sched_yield(); if (mutex) { - struct timespec abstime; - struct timeval now; - gettimeofday(&now, NULL); - iTimeout += now.tv_usec / 1000; - abstime.tv_sec = now.tv_sec + (time_t)(iTimeout / 1000); - abstime.tv_nsec = (int32_t)((iTimeout % (uint32_t)1000) * (uint32_t)1000000); - bReturn = (pthread_cond_timedwait(&m_cond, &mutex->m_mutex, &abstime) == 0); + if (iTimeout > 0) + { + struct timespec abstime; + struct timeval now; + gettimeofday(&now, NULL); + iTimeout += now.tv_usec / 1000; + abstime.tv_sec = now.tv_sec + (time_t)(iTimeout / 1000); + abstime.tv_nsec = (int32_t)((iTimeout % (uint32_t)1000) * (uint32_t)1000000); + bReturn = (pthread_cond_timedwait(&m_cond, &mutex->m_mutex, &abstime) == 0); + } + else + { + bReturn = (pthread_cond_wait(&m_cond, &mutex->m_mutex) == 0); + } } return bReturn; @@ -133,8 +143,8 @@ void CCondition::Sleep(uint32_t iTimeout) } CThread::CThread(void) : - m_bRunning(false), - m_bStop(false) + m_bStop(false), + m_bRunning(false) { } @@ -143,7 +153,7 @@ CThread::~CThread(void) StopThread(); } -bool CThread::CreateThread(void) +bool CThread::CreateThread(bool bWait /* = true */) { bool bReturn(false); @@ -151,7 +161,8 @@ bool CThread::CreateThread(void) m_bStop = false; if (!m_bRunning && pthread_create(&m_thread, NULL, (void *(*) (void *))&CThread::ThreadHandler, (void *)this) == 0) { - m_bRunning = true; + if (bWait) + m_threadCondition.Wait(&m_threadMutex); bReturn = true; } @@ -163,26 +174,26 @@ void *CThread::ThreadHandler(CThread *thread) void *retVal = NULL; if (thread) + { + thread->m_bRunning = true; + thread->m_threadCondition.Broadcast(); retVal = thread->Process(); - thread->m_bRunning = false; + thread->m_bRunning = false; + } return retVal; } bool CThread::StopThread(bool bWaitForExit /* = true */) { - bool bReturn(false); + bool bReturn(true); m_bStop = true; m_threadCondition.Broadcast(); - if (m_bRunning) - { - void *retVal; - if (bWaitForExit) - bReturn = (pthread_join(m_thread, &retVal) == 0); - m_bRunning = false; - } + void *retVal; + if (bWaitForExit && m_bRunning) + bReturn = (pthread_join(m_thread, &retVal) == 0); return bReturn; } @@ -190,5 +201,5 @@ bool CThread::StopThread(bool bWaitForExit /* = true */) bool CThread::Sleep(uint32_t iTimeout) { CLockObject lock(&m_threadMutex); - return m_bStop ? false :m_threadCondition.Wait(&m_threadMutex, iTimeout); + return m_bStop ? false : m_threadCondition.Wait(&m_threadMutex, iTimeout); } diff --git a/src/lib/platform/threads.h b/src/lib/platform/threads.h index 9ba273c..4161de3 100644 --- a/src/lib/platform/threads.h +++ b/src/lib/platform/threads.h @@ -46,7 +46,7 @@ namespace CEC void Broadcast(void); void Signal(void); - bool Wait(CMutex *mutex, uint32_t iTimeout); + bool Wait(CMutex *mutex, uint32_t iTimeout = 0); static void Sleep(uint32_t iTimeout); private: @@ -69,7 +69,7 @@ namespace CEC class CLockObject { public: - CLockObject(CMutex *mutex); + CLockObject(CMutex *mutex, bool bTryLock = false); ~CLockObject(void); bool IsLocked(void) const { return m_bLocked; } @@ -88,18 +88,21 @@ namespace CEC virtual ~CThread(void); virtual bool IsRunning(void) const { return m_bRunning; } - virtual bool CreateThread(void); + virtual bool CreateThread(bool bWait = true); virtual bool StopThread(bool bWaitForExit = true); + virtual bool IsStopped(void) const { return m_bStop; }; virtual bool Sleep(uint32_t iTimeout); static void *ThreadHandler(CThread *thread); virtual void *Process(void) = 0; protected: + CCondition m_threadCondition; + + private: pthread_t m_thread; CMutex m_threadMutex; - CCondition m_threadCondition; - bool m_bRunning; bool m_bStop; + bool m_bRunning; }; }; -- 2.34.1