Revision 59:593fb493172f, committed 2012-10-24

Comitter:

donatien

Date:

Wed Oct 24 14:13:49 2012 +0000

Parent:

58:f1965f3f4504

Child:

60:763eefc845b1

Commit message:

- Disabling/Re-enabling of unsollicited response codes between commands to avoid having unsollicited result codes mixed up with commands responses; - Suppressed 100ms guard time in ATCommandsInterface; - Fixed race condition in ATCommandsInterface

Changed in this revision

--- a/VodafoneUSBModem.cpp	Mon Oct 22 15:47:01 2012 +0000
+++ b/VodafoneUSBModem.cpp	Wed Oct 24 14:13:49 2012 +0000
@@ -512,14 +512,14 @@
   {
     return OK;
   }
-
+  
   DBG("Starting AT thread if needed");
   int ret = m_at.open();
   if(ret)
   {
     return ret;
   }
-
+  
   DBG("Sending initialisation commands");
   ret = m_at.init();
   if(ret)

--- a/at/ATCommandsInterface.cpp	Mon Oct 22 15:47:01 2012 +0000
+++ b/at/ATCommandsInterface.cpp	Wed Oct 24 14:13:49 2012 +0000
@@ -34,7 +34,7 @@
 ATCommandsInterface::ATCommandsInterface(IOStream* pStream) :
    m_pStream(pStream), m_open(false), m_env2AT(), m_AT2Env(), m_processingMtx(),
    m_processingThread(&ATCommandsInterface::staticCallback, this, (osPriority)AT_THREAD_PRIORITY, 4*192),
-   m_eventsMtx()
+   m_eventsMgmtMtx(), m_eventsProcessingMtx()
 {
   memset(m_eventsHandlers, 0, MAX_AT_EVENTS_HANDLERS * sizeof(IATEventsHandler*));
 
@@ -57,32 +57,25 @@
 
   m_open = true;
 
-  //Advertize this to events handlers
-  m_eventsMtx.lock();
-  for(int i = 0; i < MAX_AT_EVENTS_HANDLERS; i++) //Find a free slot
-  {
-    if( m_eventsHandlers[i] != NULL )
-    {
-      m_eventsHandlers[i]->onDispatchStart();
-    }
-  }
-  m_eventsMtx.unlock();
-
   DBG("AT interface opened");
-
+  
   return OK;
 }
 
-//Initialize AT link
+//Initialize AT link & start events processing
 int ATCommandsInterface::init()
 {
   DBG("Sending ATZ E1 V1");
+  
+  //Lock transaction mutex
+  m_transactionMtx.lock();
+  
   //Should we flush m_pStream at this point ???
   int err;
   int tries = 5;
   do
   {
-    err = executeSimple("ATZ E1 V1", NULL, 3000); //Enable echo and verbosity
+    err = executeInternal("ATZ E1 V1", this, NULL, 3000); //Enable echo and verbosity
     if(err && tries)
     {
       WARN("No response, trying again");
@@ -92,10 +85,17 @@
   if( err )
   {
     ERR("Sending ATZ E1 V1 returned with err code %d", err);
+    m_transactionMtx.unlock();
     return err;
   }
+  
+  //Enable events handling and execute events enabling commands
+  enableEvents();
 
   DBG("AT interface initialized");
+  
+  //Unlock transaction mutex
+  m_transactionMtx.unlock();
 
   return OK;
 }
@@ -124,16 +124,8 @@
   m_processingMtx.lock();
   m_open = false;
 
-  //Advertize this to events handlers
-  m_eventsMtx.lock();
-  for(int i = 0; i < MAX_AT_EVENTS_HANDLERS; i++) //Find a free slot
-  {
-    if( m_eventsHandlers[i] != NULL )
-    {
-      m_eventsHandlers[i]->onDispatchStop();
-    }
-  }
-  m_eventsMtx.unlock();
+  //Disable events handling and advertize this to the events handlers
+  disableEvents();
 
   DBG("AT interface closed");
   return OK;
@@ -151,7 +143,6 @@
 
 int ATCommandsInterface::execute(const char* command, IATCommandsProcessor* pProcessor, ATResult* pResult, uint32_t timeout/*=1000*/)
 {
-  DBG("Executing command %s", command);
   if(!m_open)
   {
     WARN("Interface is not open!");
@@ -160,6 +151,60 @@
 
   //Lock transaction mutex
   m_transactionMtx.lock();
+  
+  disableEvents(); //Disable unsollicited result codes
+  int ret = executeInternal(command, pProcessor, pResult, timeout);
+  enableEvents(); //Re-enable unsollicited result codes whatever the result of the command is
+  
+  //Unlock transaction mutex
+  m_transactionMtx.unlock();
+  
+  return ret;
+}
+
+int ATCommandsInterface::registerEventsHandler(IATEventsHandler* pHdlr)
+{
+  m_eventsMgmtMtx.lock();
+  m_eventsProcessingMtx.lock();
+  for(int i = 0; i < MAX_AT_EVENTS_HANDLERS; i++) //Find a free slot
+  {
+    if( m_eventsHandlers[i] == NULL )
+    {
+      m_eventsHandlers[i] = pHdlr;
+      m_eventsProcessingMtx.unlock();
+      m_eventsMgmtMtx.unlock();
+      return OK;
+    }
+  }
+  m_eventsProcessingMtx.unlock();
+  m_eventsMgmtMtx.unlock();
+  return NET_OOM; //No room left
+}
+
+int ATCommandsInterface::deregisterEventsHandler(IATEventsHandler* pHdlr)
+{
+  m_eventsMgmtMtx.lock();
+  m_eventsProcessingMtx.lock();
+  for(int i = 0; i < MAX_AT_EVENTS_HANDLERS; i++) //Find handler in list
+  {
+    if( m_eventsHandlers[i] == pHdlr )
+    {
+      m_eventsHandlers[i] = NULL;
+      m_eventsProcessingMtx.unlock();
+      m_eventsMgmtMtx.unlock();
+      return OK;
+    }
+  }
+  m_eventsProcessingMtx.unlock();
+  m_eventsMgmtMtx.unlock();
+  return NET_NOTFOUND; //Not found
+}
+
+//Private methods
+
+int ATCommandsInterface::executeInternal(const char* command, IATCommandsProcessor* pProcessor, ATResult* pResult, uint32_t timeout/*=1000*/)
+{
+  DBG("Executing command %s", command);
 
   //Discard previous result if it arrived too late
   osEvent evt = m_AT2Env.get(0);
@@ -181,8 +226,6 @@
     m_pTransactionProcessor = this; //Use default behaviour
   }
 
-  Thread::wait(100); //FIXME find stg else
-
   DBG("Sending command ready signal to AT thread & aborting current blocking read operation");
 
   //Produce command ready signal
@@ -209,7 +252,6 @@
     m_pStream->abortRead(); //This is thread-safe
 
     WARN("Command returned no message");
-    m_transactionMtx.unlock();
     return NET_TIMEOUT;
   }
   DBG("Command returned with message %d", *msg);
@@ -229,44 +271,10 @@
 
   DBG("Command returned successfully");
 
-  //Unlock transaction mutex
-  m_transactionMtx.unlock();
-
   return ret;
 }
-
-int ATCommandsInterface::registerEventsHandler(IATEventsHandler* pHdlr)
-{
-  m_eventsMtx.lock();
-  for(int i = 0; i < MAX_AT_EVENTS_HANDLERS; i++) //Find a free slot
-  {
-    if( m_eventsHandlers[i] == NULL )
-    {
-      m_eventsHandlers[i] = pHdlr;
-      m_eventsMtx.unlock();
-      return OK;
-    }
-  }
-  m_eventsMtx.unlock();
-  return NET_OOM; //No room left
-}
-
-int ATCommandsInterface::deregisterEventsHandler(IATEventsHandler* pHdlr)
-{
-  m_eventsMtx.lock();
-  for(int i = 0; i < MAX_AT_EVENTS_HANDLERS; i++) //Find handler in list
-  {
-    if( m_eventsHandlers[i] == pHdlr )
-    {
-      m_eventsHandlers[i] = NULL;
-      m_eventsMtx.unlock();
-      return OK;
-    }
-  }
-  m_eventsMtx.unlock();
-  return NET_NOTFOUND; //Not found
-}
-
+#include "mbed.h"
+DigitalOut led_test(LED4);
 
 int ATCommandsInterface::tryReadLine()
 {
@@ -274,12 +282,14 @@
 
   //Block on serial read or incoming command
   DBG("Trying to read a new line from stream");
+  led_test=1;
   int ret = m_pStream->waitAvailable(); //This can be aborted
   size_t readLen = 0;
   if(ret == OK)
   {
     ret = m_pStream->read((uint8_t*)m_inputBuf + m_inputPos, &readLen, AT_INPUT_BUF_SIZE - 1 - m_inputPos, 0); //Do NOT wait at this point
   }
+  led_test=0;
   if(ret == OK)
   {
     m_inputPos+=readLen;
@@ -597,7 +607,7 @@
       }
     }
     //Looks for a unsolicited result code; we can have m_transactionState == COMMAND_SENT as the code may have arrived just before we sent the command
-    m_eventsMtx.lock();
+    m_eventsProcessingMtx.lock();
     //Go through the list
     for(int i = 0; i < MAX_AT_EVENTS_HANDLERS; i++) //Find a free slot
     {
@@ -610,7 +620,7 @@
         }
       }
     }
-    m_eventsMtx.unlock();
+    m_eventsProcessingMtx.unlock();
     if(found)
     {
       return OK;
@@ -712,6 +722,54 @@
   return OK;
 }
 
+//This will be called on initialization & after the execution of a command
+void ATCommandsInterface::enableEvents()
+{
+  //Advertize this to events handlers
+  m_eventsMgmtMtx.lock();
+  for(int i = 0; i < MAX_AT_EVENTS_HANDLERS; i++) //Find a free slot
+  {
+    if( m_eventsHandlers[i] != NULL )
+    {
+      m_eventsHandlers[i]->onDispatchStart();
+      //Enable this kind of events
+      if(m_eventsHandlers[i]->getEventsEnableCommand() != NULL)
+      {
+        int ret = executeInternal(m_eventsHandlers[i]->getEventsEnableCommand(), this, NULL); //Execute enable command
+        if(ret)
+        {
+          WARN("Events enabling command failed");
+        }
+      }
+    }
+  }
+  m_eventsMgmtMtx.unlock();
+}
+
+//This will be called on de-initialization & before the execution of a command to prevent unsollicited result codes from polluting the results
+void ATCommandsInterface::disableEvents()
+{
+  //Advertize this to events handlers
+  m_eventsMgmtMtx.lock();
+  for(int i = 0; i < MAX_AT_EVENTS_HANDLERS; i++) //Find a free slot
+  {
+    if( m_eventsHandlers[i] != NULL )
+    {
+      m_eventsHandlers[i]->onDispatchStart();
+      //Disable this kind of events
+      if(m_eventsHandlers[i]->getEventsDisableCommand() != NULL)
+      {
+        int ret = executeInternal(m_eventsHandlers[i]->getEventsDisableCommand(), this, NULL); //Execute disable command
+        if(ret)
+        {
+          WARN("Events disabling command failed");
+        }
+      }
+    }
+  }
+  m_eventsMgmtMtx.unlock();
+}
+
 //Commands that can be called during onNewATResponseLine callback, additionally to close()
 //Access to this method is protected (can ONLY be called on processing thread during IATCommandsProcessor::onNewATResponseLine execution)
 int ATCommandsInterface::sendData(const char* data)
@@ -743,7 +801,8 @@
     if( memcmp(m_inputBuf, data + dataPos, readLen) != 0 )
     {
       //Echo does not match output
-      WARN("Echo does not match output");
+      m_inputBuf[readLen] = '\0';
+      WARN("Echo does not match output, got '%s' instead", m_inputBuf);
       m_inputPos = 0; //Reset input buffer state
       m_inputBuf[0] = '\0'; //Always have a null-terminating char at start of buffer
       return NET_DIFF;
@@ -809,10 +868,10 @@
     m_inputPos = 0; //Clear input buffer
     do
     {
+      DBG("Trying to send a pending command");
+      trySendCommand(); //This must be tried first as we discarded the buffer before and therefore would be blocking though there is a pending command
       DBG("Trying to read a new line");
       tryReadLine();
-      DBG("Trying to send a pending command");
-      trySendCommand();
     } while( m_processingThread.signal_wait(AT_SIG_PROCESSING_STOP, 0).status != osEventSignal ); //Loop until the process is interrupted
     m_processingMtx.unlock();
     DBG("AT Processing stopped");

--- a/at/ATCommandsInterface.h	Mon Oct 22 15:47:01 2012 +0000
+++ b/at/ATCommandsInterface.h	Wed Oct 24 14:13:49 2012 +0000
@@ -36,6 +36,8 @@
   virtual bool isATCodeHandled(const char* atCode) = 0; //Is this AT code handled
   virtual void onDispatchStart() = 0;
   virtual void onDispatchStop() = 0;
+  virtual char* getEventsEnableCommand() = 0;
+  virtual char* getEventsDisableCommand() = 0;
   virtual void onEvent(const char* atCode, const char* evt) = 0;
   friend class ATCommandsInterface;
 };
@@ -91,6 +93,7 @@
 
   int executeSimple(const char* command, ATResult* pResult, uint32_t timeout=1000);
   int execute(const char* command, IATCommandsProcessor* pProcessor, ATResult* pResult, uint32_t timeout=1000);
+  
   int registerEventsHandler(IATEventsHandler* pHdlr);
   int deregisterEventsHandler(IATEventsHandler* pHdlr);
 
@@ -100,10 +103,15 @@
 
   static void staticCallback(void const* p);
 private:
+  int executeInternal(const char* command, IATCommandsProcessor* pProcessor, ATResult* pResult, uint32_t timeout=1000);
+  
   int tryReadLine();
   int trySendCommand();
   int processReadLine();
   int processEntryPrompt();
+  
+  void enableEvents();
+  void disableEvents();
 
   int ATResultToReturnCode(ATResult result); //Helper
 
@@ -138,7 +146,8 @@
   Mutex m_processingMtx;
   Thread m_processingThread;
 
-  Mutex m_eventsMtx;
+  Mutex m_eventsMgmtMtx; //Lock events use within the calling thread
+  Mutex m_eventsProcessingMtx; //Lock events use within the processing thread
 };
 
 #endif /* ATCOMMANDSINTERFACE_H_ */

--- a/sms/SMSInterface.cpp	Mon Oct 22 15:47:01 2012 +0000
+++ b/sms/SMSInterface.cpp	Wed Oct 24 14:13:49 2012 +0000
@@ -329,13 +329,22 @@
 
 /*virtual*/ void SMSInterface::onDispatchStart()
 {
-
-
+    
 }
 
 /*virtual*/ void SMSInterface::onDispatchStop()
 {
+    
+}
 
+/*virtual*/ char* SMSInterface::getEventsEnableCommand()
+{
+  return "AT+CNMI=2,1,0,0,0";
+}
+
+/*virtual*/ char* SMSInterface::getEventsDisableCommand()
+{
+  return "AT+CNMI=0,0,0,0,0"; //Indications will be buffered within the modem and flushed back when the former command is executed
 }
 
 /*virtual*/ void SMSInterface::onEvent(const char* atCode, const char* evt)

--- a/sms/SMSInterface.h	Mon Oct 22 15:47:01 2012 +0000
+++ b/sms/SMSInterface.h	Wed Oct 24 14:13:49 2012 +0000
@@ -76,6 +76,8 @@
   virtual bool isATCodeHandled(const char* atCode); //Is this AT code handled
   virtual void onDispatchStart();
   virtual void onDispatchStop();
+  virtual char* getEventsEnableCommand();
+  virtual char* getEventsDisableCommand();
   virtual void onEvent(const char* atCode, const char* evt);
 
   //Updates messages count/references

--- a/ussd/USSDInterface.cpp	Mon Oct 22 15:47:01 2012 +0000
+++ b/ussd/USSDInterface.cpp	Wed Oct 24 14:13:49 2012 +0000
@@ -144,6 +144,16 @@
 
 }
 
+/*virtual*/ char* USSDInterface::getEventsEnableCommand()
+{
+  return NULL; //No need to disable events here
+}
+
+/*virtual*/ char* USSDInterface::getEventsDisableCommand()
+{
+  return NULL; //No need to re-enable events here
+}
+
 /*virtual*/ void USSDInterface::onEvent(const char* atCode, const char* evt)
 {
   if( strcmp("+CUSD", atCode) != 0 )

--- a/ussd/USSDInterface.h	Mon Oct 22 15:47:01 2012 +0000
+++ b/ussd/USSDInterface.h	Wed Oct 24 14:13:49 2012 +0000
@@ -59,6 +59,8 @@
   virtual bool isATCodeHandled(const char* atCode); //Is this AT code handled
   virtual void onDispatchStart();
   virtual void onDispatchStop();
+  virtual char* getEventsEnableCommand();
+  virtual char* getEventsDisableCommand();
   virtual void onEvent(const char* atCode, const char* evt);
 
 private: