[rs232.h] ComRd(,,) slow execution

[Spaghetto]

Dear Mr.Bozzolo,

thanks for the answer, I understand perfectly what OVR_CZ said, but, according to my previous discussion with ebalci some posts ago, the delays used to wait the rs232 answer to at commands are useless, ebalci said

"You don't need the 0.2 second delay after calling ComWrt.

 You can directly call ComRd, the time it takes for the GSM modem to process your command and answer is actually tolerated by the timeout value of the port, which you can set by SetCommTime function."

suggesting me:

"I would give a timeout of 1 second and remove the Delay (0.2) line, if I were you."

Even if the logic is fully correct, it didn't work to my code, and if I remove the delays the ComRd(,,) don't catch the answer. And I'm still asking why...

Here's the function. (  previously used: SetComTime (ComPort, 1); )

******************************************************************************

unsigned char sendat(const char A[], unsigned char port, float sec)
{
   char writeBuf[50] = {0},
        readBuf[200] = {0};
   unsigned char indexBuf;
    
    FlushInQ (port);
    strcpy(writeBuf,A);
    ComWrt (port, writeBuf, strlen(writeBuf));
    DelayWithEventProcessing (sec);                      //<cr><lf>OK<cr><lf>
    ComRdTerm (port, readBuf, GetInQLen(port), '\r'); //OK<cr><lf>
    ComRdTerm (port, readBuf, GetInQLen(port), '\r'); //OK
    if ( findstr(readBuf,"OK",&indexBuf) ) return 1;
    else return 0;
}

******************************************************************************

Could you help me to remove the delay and let the ComRdTerm wait for the answer automatically (as it should do)?

Thanks

Ps. Laureato in ingegneria elettronica all'unina? Spero di esserle presto collega! 😄 Questo è proprio il progetto per la mia tesi per il dipartimento di misure (secondo me conoscerà anche il mio prof)!

[RobertoBozzolo]

What is happening will be clear to you once you figure out how limits for ComRdTerm are evaluated:

• Termination character is met, OR
• # of characters is found, OR
• timeout occurs

That is, your code:

ComWrt (port, writeBuf, strlen(writeBuf));
ComRdTerm (port, readBuf, GetInQLen(port), '\r');

 is executed this way:

1. Message is sent to the modem: it will take some time for the modem to elaborate it and respond, but:
2. you immediately issue a ComRdTerm with GetInQLen embedded
3. GetInQLen returns 0 characters in queue, which is used as a terminating condition, so
4. ComRdTerm terminates having read nothing!

To better use ComRdTerm try setting 'count' parameter to an appropriate, fixed value depending on the expected response lenght. In your example, 2 characters on the first read and 4 on the second.

Or even better, use a simple ComRd (port, readBuf, 6); that will terminate only after reading the full <cr><lf>OK<cr><lf> response.

[OVR_CZ]

as Roberto Bozzolo say, i do not mean not use any wait function but something like this:
(it is example, i do not test this code, it is only for ilustration)

//Send data
	//FlushInQ (port);//warning: this can possibly flush unsolicited message from modem, or worse, part of it, which can cause problem when parse response data.
     ComWrt (port, writeBuf, strlen(writeBuf));	//send command
     //DelayWithEventProcessing (sec);			//wait to data is send, this can take some time depend on big sizeof data and slow baudrate, but int most cases this is not need. it can be also done by GetOutQLen() in loop with DelayWithEventProcessing

//Read response
     int timecount=0;
     #define MAXSIZE 128
     char InData[MAXSIZE+1];//incoming data(command response)
     int InDataCount=0;//number of data susceffuly read

     timecount=0;
     InDataCount=0

     while ((timecount<10)&&(InDataCount<MAXSIZE))//each cykle takes cca 100ms so this is 1*100=1sec, also check if ther is enougth space in data buffer 
     {
		int InQlen;//work variable to get number of characters in queue
		int len;   //work variable to get number of characters truly read to buffer(in this loop step)
          timecount++;
          DelayWithEventProcessing (0.1);                      //wait to incomming data cca 100ms
          InQlen=GetInQLen(port);
		  if (InQlen<0)return -1;//error when data read;
          if (InQlen>0)//if data in queue
          {
		   if (InQlen+InDataCount<MAXSIZE)InQlen=MAXSIZE-InDataCount;//limit to buffer size
           len=ComRd(port, &InData[InDataCount],InQlen);//read data to actual end of buffer
		   if (len<0)return -1;//error when data read;
		   InDataCount+=len;//move actual end of buffer by truly read data count
           InData[InDataCount]=0;//endup string with 0
		   //FilterUnsolicitedMessage(InData,&InDataCount); if need filter unsolicited messages torouhth data read(maybe this can remove some compexity from isResponseCompleted)
           if (isResponseCompleted(InData,InDataCount)==1)break;    //not nesscessary but can speedup communication by terminate waiting ////if ( findstr(readBuf,"OK",&indexBuf) ) return 1;
		   if (timecount>2)timecount=2;//this cause to not wait more than 2*0.1sec after susceffuly read data (i.e. some commands have delay before response, but there is not big delay between massage characters itself)
		   //if (InDataCount>=MAXSIZE-1)timecount=MAXSIZE-2;//this cause to wait at least 2*0.1sec after susceffuly read data(sometimes nesscessary for very long response like SMS list)
          }
     }
	 //FilterUnsolicitedMessage(InData,&InDataCount); if need filter unsolicited messages here
	 result=GetResponseResultCode(InData,InDataCount);//Check for OK,Error,....

Mixing ComCallback and  Send/Wait/Read communication model need some sort of lock because ComCallback be process in time of DelayWithEventProcessingbeing processed

(also you can use Read Response routine in Timer callback instead of ComCallback.)

int UnsolicitedReadLock=0;
//use only one(ComCallback or CbReadunsolicitedTimer)

void ComCallback(int portNumber, int eventMask, void *callbackdata)
{
  if (UnsolicitedReadLock!=0)return;
  if (eventMask & LWRS_RXCHAR)ReadUnsolicitedResponse(...);
}

int CVICALLBACK CbReadunsolicitedTimer (int panel, int control, int event, void *callbackData, int eventData1, int eventData2)
{
	switch (event)
		{
		case EVENT_TIMER_TICK:
		if (UnsolicitedReadLock!=0)break;
		ReadUnsolicitedResponse(...);
		}
	return 0;
}
//Process AT command
UnsolicitedReadLock=1;//lock to not process unsolicited data form timer/callback
ComWrt(...);
ReadResponse(..);
UnsolicitedReadLock=0;//unlock

Also be sure that you read all data in ComCallback because it is called only when new character arrived to queue, not when there is character in queue.

for this reason i recomend to use solution with Timer callback instead of ComCallback even that it is less efficient.

i hope this helps 🙂

P.S. maybe this example can be modified by using ComRdTerm to be more simple efficient, but i do not have any experiences with it,and this solution can be easily modified to use with any protokol not only Modem Commands/Responses.

[carlox]

@OVR_CZ I would avoid suche a mixed approach, too tricky to avoid to lost characters here and there. In a (proprietary) applications of mine I ended in a totally event driven COM port management.

As soon as a valid reply from device is received, an event is generated (read: a callback is called).

Of course there is a timeout event, too.

In my whole application there isn't a single delay, being totally event driven.

My external device does not generate unsolicited messages (being an half duplex RS485 channel), but it would be relatively easy to add code to support them.

[OVR_CZ]

hi carlox,

    I react to Spaghetto post about use ConfigComCallback to catch unsolicited messages, so i suggest how to handle it with smallest impact to code.

    For the best eficiency and profesional code which not block thread/process with waiting/delay for data,the best approach is (i fully agree) use ComCallback with timeout checking.

   But this can be litle hard to Write/debug, specialy for one who learn how to use/work with modem, In this case i think this is acceptable solution.(And CVI is one of best tools for this Quickly writen app)

   However this is up to Spaghetto which style is closest to his mind.

   Have nice day

[carlox]

Sure I understood what you have in mind

I wanted only to give a hint to our young friend about a better, and - for sure - much harder to program, approach.

When I faced the same problem (or a very similar one) I was not so lucky to have any advice, and I had to discover by myself why my program was wasting so much CPU time and the whole machine were so unresponsive... modern CPUs are so forgiving... some users never become aware they have a core locked on a tight loop...

[Spaghetto]

Dear users,

sorry for my late reply; I needed time to modify my code.

The program it's divided in two parts, I need absolutely a different approach to them, so my solution was the following:

• In the first part I set the module, and all the at commands are sent\read with this function:

Spoiler

unsigned char sendat(const char A[])
{
   char writeBuf[50] = {0},
        readBuf[200] = {0};
   unsigned char indexBuf;
    
    FlushInQ (ComPort);
    strcpy(writeBuf,A);
    ComWrt (ComPort, writeBuf, strlen(writeBuf));
    ComRd(ComPort,readBuf,6); //<cr><lf>OK<cr><lf>
    if ( findstr(readBuf,"OK",&indexBuf) ) return 1;
    else return 0;
}

called many times in the setting routines:

Spoiler

// Impostazioni modem
                        if ( !sendat("AT+CLIP=1\r") )
                        {
                            //settingok=0;
                            notifymessage("Errore impostazioni (Warning): +CLIP. Possibile non ricezione delle chiamate");
                        }
                        if ( !sendat("AT+CMGF=1\r") )
                        {
                            settingok=0;
                            notifymessage("Errore impostazioni (Fatal): +CMGF");
                        }....

I discard all the unsolicited message, which in this part are irrilevant.

At the end I install a InstallComCallback (ComPort, LWRS_RXFLAG, 0, (int)'+' , CReceiveData, 0); catching the "+" character in every unsolicited message.

[Spaghetto]

• The second part, I catch that message with a function like this:

Spoiler

void CVICALLBACK CReceiveData(int portNumber,int eventMask, void *callbackData)  // Ricezione RS232 Interrupt
{   //void CVICALLBACK CallbackFunctionName (int portNumber, int eventMask, void *callbackData);

...

    if(interruptSet) //in questo modo posso mascherare la routine quando ricevo i messaggi
    {
            askstatus=0; // disattivo richiesta COPS e CSQ
            ComRdTerm (ComPort, readBuf, 100, '+'); // tolgo dalla queue tutto quello fino al +
            ComRdTerm (ComPort, readBuf, 100, '\r'); // salvo la queue nell'array
    
...
        // Sms ricevuto
            //+CMTI:"SM",1
            if( findstr(readBuf,"CMTI",&indexBuf) )
            {

....

   
        // Chiamata in arrivo
            //+CLIP: "+393339577700",145,,,,0
            else if( findstr(readBuf,"CLIP",&indexBuf) )
            {
                ComWrt (ComPort, "ATH\r", 4); // termino tutte le chiamate in ingresso
                
                indexBuf+=7;tempindex=0;
                if(readBuf[indexBuf]=='"') notifymessage("Chiamata in arrivo da Privato");
                else
                {
                    do { numbCall[tempindex++]=readBuf[indexBuf++];} while (readBuf[indexBuf]!='"');
                    numbCall[tempindex]=0;
                    strcpy(writeBuf,"Chiamata in arrivo da: ");
                    strcat(writeBuf,numbCall);
                    notifymessage(writeBuf);
                    FlushInQ (ComPort);
                    
                }       
            } //Fine CLIP

...

        // Impostazioni CNMI
            //+CNMI: (0-3),(0,1),(0,2,3),(0,2),(1) ed +CNMI: 1,1,0,0,1 [ relativamente chieste da AT+CNMI=? e AT+CNMI?
            else if( findstr(readBuf,"CNMI",&indexBuf) )
            {

....

and the other messages.

I read up to the "+" char, then i read upt to the cr, which in that answer they are all on the first row, so all the message are something like

---&%$&%$unsolicited_garbage<cr><lf> +MESS: ----------<cr><lf>OK<cr><lf>.

This way, to retrieve an answer, in the main, I just have to do something like this.

Spoiler

int CVICALLBACK C_CNMIquery (int panel, int control, int event,
        void *callbackData, int eventData1, int eventData2)
{
    switch (event)
    {
        case EVENT_COMMIT:
            int value;
            GetCtrlVal(mainPanel, MAIN_PANEL_GSM_ON, &value);
            if(value)
            {
                askstatus=0;
                simplesendat("AT+CNMI=?\r");    
                DelayWithEventProcessing (0.2);
                simplesendat("AT+CNMI?\r");
                askstatus=1;
            }
            else MessagePopup ("Attenzione", "Collegare prima il modulo");
            break;
    }
    return 0;
}

where

Spoiler

void simplesendat(const char A[])
{
   char writeBuf[50] = {0},
        readBuf[200] = {0};
   
    FlushInQ (ComPort);
    strcpy(writeBuf,A);
    ComWrt (ComPort, writeBuf, strlen(writeBuf));
}

I needed this mixed approach, so for setting istructions in the first part I haven't a "+" to catch the answer. And absolutely I needed to delete the Delay functions I wrongly used before.

Code now is working perfectly.

Thanks to everybody for all the kind and clever answers.

Redgards,

Luca Gallucci

[Spaghetto]

Maybe I've only to correct with something else that "DelayWithEventProcessing (0.2);" used between sending 2 AT commands to avoid to send a second command once the module is giving the answer to the first one (giving an error this way).

[carlox]

Spaghetto, you should keep track of state of your application, and not to trying to guess what's happening in external device with hand-crafted delays.

Specifically, you have to wait that previous command is over, successfully or not, before attempting to send other commands.

Otherwise Mr Murphy's Law sooner or later will hit you, and your program will undoubtely fail.