0

Issue :
Using While loop to check condition and use timer if doesnt get response from server within specified time.

OS: Linux
SDK : QT 5.5

Description:

I have implemenetd a client side and in code there is while loop which continously checks for some condition( "check machine has started") to be true. This condition changes when it get some message from machine Server . When I implemented while loop it get stuck in it and doesnt come out. I fired Question in this forum and someone was kind enough to point me my mistake and he suggested I should use QStateMachine as my while loop is eating all my resources.

So while googling more about state machine I bumped into QCoreApplication::processEvents(). When i added it in my code everything works as expected but the timer part still timesout. Now my question is

1) What is difference between QStateMachine and QCoreApplication::processEvents() & which one is better.

2) How to correctly use QTimer so that if condition in while loop doesnt become true within stipulated time just timeout and skip while loop.

void timerStart( QTimer* timer, int timeMillisecond )
    {
      timer = new QTimer( this );
      connect( timer, SIGNAL( timeout() ), this, SLOT( noRespFrmMachine( ) ) ) ;
      timer->start( timeMillisecond );
    }

    void timerStop( QTimer* timer )
    {
      if( timer )
        {
          timer->stop();
        }
    }

    void noRespFrmMachine( )
    {
      vecCmd.clear();
    }

    void prepareWriteToMachine()
    {
         // Some other code
          //  check if Machine has started we will get response in MainWindow and
          // it will update isMachineStarted so we can check that
          QTimer *timer ;
          timerStart( timer, TIMEOUT_START ); // IS THIS RIGHT??
          while( isMachineStarted() == false  )  // getMachineRunning-> return isMachineStarted ??
            {
              // do nothing wiat for machine server to send machine_started signal/msg
         QCoreApplication::processEvents(); // added this one and my aplication is responsive and get correct result

            }
          if( isMachineStarted() == true )
            {
              // if we are here it mean timer has not expired & machine is runing
              // now check for another command and execute
              timerStop( timer );
              while( vecCmd.size() > 0 )
                {
                  QString strTemp = returnFrontFrmVec();
                  setStoreMsgFrmCliReq( strTemp );
                  reconnectToServer();
                }
            }
        }
    }
samprat
  • 2,150
  • 8
  • 39
  • 73
  • 1
    Can you make your isMachineStarted() into a signal, which is emitted when the thing is started? This way do would not need to have busy waits calling processEvents(). – Archie Aug 27 '15 at 11:32
  • if I use signal , it will skip while loop condition and execute other commands which I dont want unless it is executed. I dont have problem with process Event all I want to knwo difference between Qprocess Event and QState machine – samprat Aug 27 '15 at 11:37
  • Well, QStateMachine is an implementation of a state machine, where state change can be triggered by signals. ProcessEvents() is a part of Qt's event loop mechanisms, it dispatches the pending events to their destinations (slots and event handlers). So these are two different things and have little in common except being a part of Qt framework. – Archie Aug 27 '15 at 11:43
  • so getting response from machine server is also state change right? so I can implement above problem with QState Machine? – samprat Aug 27 '15 at 11:47
  • 1
    Getting response indeed can be a signal to trigger the state change. But currently your machine is questioned by polling in a loop (by calling isMachineStarted() or checking the vecCmd.size), you have to make them into signals, so that the machine part will emit started() when started or dataAvailable() when there is a command to handle, for example. – Archie Aug 27 '15 at 11:51
  • Problem is bad design and missing MVC pattern. You should have some data model which is updated by your network service. UI part should observe changes of this model and update itself on those changes. Use of `QState` is most probably not needed here. This is useful, in case complex UI which changes on reaction of data model changes. – Marek R Aug 27 '15 at 12:20
  • @Marek, I want to know why it is bad design so that in future I can improve. Can you please elaborate more – samprat Aug 27 '15 at 12:26
  • This a big topic. Just read about MVC for Qt. At beginning it is hard to understand and applied this but as you will gain experience it will became clear and useful. – Marek R Aug 27 '15 at 20:06

1 Answers1

0

The line timerStop( timer ); should cause segmentation fault. The function timerStart does not change the local variable timer of prepareWriteToMachine(), so that variable contains undefined junk.

If you want to change external variable timer in timerStart you should pass pointer to pointer to QTimer into that function:

void timerStart(QTimer**timer, int timeMillisecond)
{
    *timer = new QTimer(this);
    ...
}

By the way, the check if( timer ) in timerStop() does not make sense in any case, since timer is never set to zero.

Orest Hera
  • 6,706
  • 2
  • 21
  • 35
  • @ Orest, "timerStart does not change the local variable timer of prepareWriteToMachine" My question is why it would change as I am passing varibale to a function by reference? Why you think its segmentation fault? I am asking to clarify my doubts and to learn new concept. – samprat Sep 01 '15 at 10:36
  • @samprat, currently you pass `timer` variable to `timerStart` by value. So, `timer` of `prepareWriteToMachine` is not changed by `timerStart`. As a result, the variable `timer` of `prepareWriteToMachine` is never initialized leading to crash. Indeed you can pass it by reference. To do that the function `timerStart` should be defined with ampersand symbol `void timerStart(QTimer*& timer, int timeMillisecond)`. In that case the function `timerStart` will work directly with external `timer` variable. – Orest Hera Sep 03 '15 at 08:39