Infinite loop running in the background Topic is solved

If you are using the main C++ distribution of wxWidgets, Feel free to ask any question related to wxWidgets development here. This means questions regarding to C++ and wxWidgets, not compile problems.
Post Reply
Ksawery
Experienced Solver
Experienced Solver
Posts: 83
Joined: Thu Jul 25, 2019 12:31 pm

Infinite loop running in the background

Post by Ksawery »

I'm building a GUI which will use data continuously received via a serial port. I'd like to setup a loop that will always run in the background, poll for new data (via a Modbus protocol) and update the text in the main GUI window. What would be the best way to implement this? Do I need to create a new thread? I'm still pretty new to wxWidgets.

Many thanks,
Ksawery
User avatar
doublemax
Moderator
Moderator
Posts: 19115
Joined: Fri Apr 21, 2006 8:03 pm
Location: $FCE2

Re: Infinite loop running in the background

Post by doublemax »

For anything that could lead to a timeout, it's recommended to put it into a secondary thread, so that there is no chance to block the gui. Communication with the main thread will happen by sending an event.

wxThreadHelper makes this a little easier. The documentation shows complete code including sending the event.

https://docs.wxwidgets.org/trunk/classw ... elper.html
Use the source, Luke!
Ksawery
Experienced Solver
Experienced Solver
Posts: 83
Joined: Thu Jul 25, 2019 12:31 pm

Re: Infinite loop running in the background

Post by Ksawery »

Thank you, I'll give it a read.
PB
Part Of The Furniture
Part Of The Furniture
Posts: 4193
Joined: Sun Jan 03, 2010 5:45 pm

Re: Infinite loop running in the background

Post by PB »

Just for comparison, here is a very simple complete example using just wxThread, where instead of Sleep()ing, one could communicate with the device

Code: Select all

#include <wx/wx.h>
#include <wx/thread.h>

class MyThread : public wxThread
{
public:
    MyThread(wxEvtHandler* sink) 
        : wxThread(wxTHREAD_JOINABLE), m_sink(sink) 
    {}    
protected:
    wxEvtHandler* m_sink;        
    
    ExitCode Entry() override
    {                        
        size_t count = 0;    
        wxThreadEvent evt;
        
        evt.SetString("Thread started.");
        wxQueueEvent(m_sink, evt.Clone()); 
        
        while( !TestDestroy() ) 
        {                    
            Sleep(1000);
            evt.SetString(wxString::Format("Thread counting: count = %zu", ++count));
            wxQueueEvent(m_sink, evt.Clone()); 
        }
        
        evt.SetString("Thread finished.");
        wxQueueEvent(m_sink, evt.Clone()); 
        
        return static_cast<wxThread::ExitCode>(nullptr);
    }    
};

class MyFrame : public wxFrame
{
public:
    MyFrame() : wxFrame(nullptr, wxID_ANY, "Test", wxDefaultPosition, wxSize(800, 600))          
    {  
        wxPanel* mainPanel = new wxPanel(this);        
        wxBoxSizer* mainPanelSizer = new wxBoxSizer(wxVERTICAL);
        wxButton* button = nullptr;

        button = new wxButton(mainPanel, wxID_ANY, "&Start Thread");
        button->Bind(wxEVT_COMMAND_BUTTON_CLICKED, &MyFrame::OnStartThread, this);
        mainPanelSizer->Add(button, wxSizerFlags().Expand().DoubleBorder());

        button = new wxButton(mainPanel, wxID_ANY, "S&top Thread");
        button->Bind(wxEVT_COMMAND_BUTTON_CLICKED, &MyFrame::OnStopThread, this);
        mainPanelSizer->Add(button, wxSizerFlags().Expand().DoubleBorder());
        
        wxTextCtrl* logCtrl = new wxTextCtrl(mainPanel, wxID_ANY, wxEmptyString, wxDefaultPosition, wxDefaultSize, 
            wxTE_MULTILINE | wxTE_READONLY | wxTE_RICH2);     
        wxLog::SetActiveTarget(new wxLogTextCtrl(logCtrl));                                         
        mainPanelSizer->Add(logCtrl, wxSizerFlags().Expand().DoubleBorder().Proportion(5));

        Bind(wxEVT_CLOSE_WINDOW, &MyFrame::OnClose, this);
        Bind(wxEVT_THREAD, &MyFrame::OnThread, this);                    
                
        mainPanel->SetSizer(mainPanelSizer);
    }   
protected:        
    MyThread* m_thread{nullptr};

    void OnStartThread(wxCommandEvent&)
    {
        if ( m_thread )
        {
            wxLogError("Thread is already running.");
            return;
        }
        
        m_thread = new MyThread(this);        
        if ( m_thread->Run() != wxTHREAD_NO_ERROR )
        {
            delete m_thread;
            m_thread = nullptr;
            wxLogError("Could not create the thread!");                    
        }
    }

    void OnStopThread(wxCommandEvent&)
    {                
        if ( !m_thread )
        {
            wxLogError("Thread is not running.");
            return;
        }

        StopThread();    
    }
        
    void MyFrame::OnClose(wxCloseEvent&)
    {        
        if ( m_thread )
        {
            if ( wxMessageBox("Thread is still runing, quit anyway?", 
                              "Question", wxYES_NO | wxNO_DEFAULT) != wxYES )
            {
                return;
            }
            StopThread();

        }
                
        Destroy();        
    }

    void OnThread(wxThreadEvent &evt)
    {    
        wxLogMessage(evt.GetString());
    }

    void StopThread()
    {
        wxCHECK_RET(m_thread, "Thread is not running");

        wxLogMessage("Stopping the thread....");

        m_thread->Delete();
        delete m_thread;
        m_thread = nullptr;    
    }
};

class MyApp : public wxApp
{
public:   
    bool OnInit() override
    {
        (new MyFrame)->Show();
        return true;
    }
}; wxIMPLEMENT_APP(MyApp);
And here is how to use a custom payload with a wxThreadEvent: viewtopic.php?f=1&t=45311&p=187832#p187836
Ksawery
Experienced Solver
Experienced Solver
Posts: 83
Joined: Thu Jul 25, 2019 12:31 pm

Re: Infinite loop running in the background

Post by Ksawery »

Thank you, I used the code template provided, and wxThreadHelper works pretty well. The only problem I have is that when I attempt to close the frame (which is my main GUI window), the application hangs and the window doesn't close. What's the issue here, do I need to call some code from the destructor?

Another issue I have is that wxCriticalSectionLocker lock(m_dataCS) caused the application to hang when called in OnThreadUpdate(), so I commented that line out. I'm not sure why it caused the app to hang?

My code is as follows:

cMain.h:

Code: Select all

#pragma once
#pragma warning(disable:4996)

#include <string.h>
#include "modbus.h"
#include "wx/wx.h"

class cMain : public wxFrame, public wxThreadHelper
{
public:
	cMain();

public:
	wxButton* m_btn1;
	wxStaticText* m_static_txt1;

	void OnButtonClicked(wxCommandEvent& evt);
	void StartModbusThread();
	void OnThreadUpdate(wxThreadEvent& evt);
	void OnClose(wxCloseEvent& evt);
	
protected:
	virtual wxThread::ExitCode Entry();
	uint16_t ADCReadings[5];
	wxCriticalSection m_ADCReadingsCS; // protects field above

private:
	modbus_t* ctx;
	wxFont myFont;
	std::string ADCReadingsString;

	wxDECLARE_EVENT_TABLE();
};

cMain.cpp:

Code: Select all

#include "cMain.h"

wxBEGIN_EVENT_TABLE(cMain, wxFrame)
	EVT_BUTTON(10001, OnButtonClicked)
	EVT_CLOSE(cMain::OnClose)
wxEND_EVENT_TABLE()

//Constructor
cMain::cMain() : wxFrame(nullptr, wxID_ANY, "Pomiar Wiązki w Linii Iniekcyjnej", wxPoint(100, 100), wxSize(800, 600))
{
	Bind(wxEVT_THREAD, &cMain::OnThreadUpdate, this);

	//Temporary button used to test MODBUS functionality
	m_btn1 = new wxButton(this, 10001, "Get Reading", wxPoint(10, 500), wxSize(150, 50));

	//Initialize static text
	m_static_txt1 = new wxStaticText(this, wxID_ANY, " ", wxPoint(300, 200), wxSize(50, 400));
	myFont = wxFont(12, wxFONTFAMILY_MODERN, wxFONTSTYLE_NORMAL, wxFONTWEIGHT_BOLD);
	m_static_txt1->SetFont(myFont);

	/* Initialize MODBUS */
	ctx = modbus_new_rtu("\\\\.\\COM4", 19200, 'E', 8, 1);
	modbus_set_slave(ctx, 1);
	modbus_rtu_set_serial_mode(ctx, MODBUS_RTU_RS232);
	modbus_connect(ctx);

	/* Start MODBUS communication*/
	StartModbusThread();
}

//Button used to test MODBUS functionality
void cMain::OnButtonClicked(wxCommandEvent& evt)
{
	modbus_read_input_registers(ctx, 3001, 5, ADCReadings);
	
	//Update static text with new ADC data
	ADCReadingsString = "ADC 1: " + std::to_string(ADCReadings[0]) + "\n\nADC 2: " +
		std::to_string(ADCReadings[1]) + "\n\nADC 3: " + std::to_string(ADCReadings[2]) +
		"\n\nADC 4: " + std::to_string(ADCReadings[3]) + "\n\nADC Status: " + std::to_string(ADCReadings[4]);

	m_static_txt1->SetLabel(ADCReadingsString);

	evt.Skip();
}

//Start of MODBUS helper thread
void cMain::StartModbusThread()
{
	// we want to start a long task, but we don't want our GUI to block
	// while it's executed, so we use a thread to do it.
	if (CreateThread(wxTHREAD_JOINABLE) != wxTHREAD_NO_ERROR)
	{
		wxLogError("Could not create the worker thread!");
		return;
	}
	// go!
	if (GetThread()->Run() != wxTHREAD_NO_ERROR)
	{
		wxLogError("Could not run the worker thread!");
		return;
	}
}

// VERY IMPORTANT: this function gets executed in the secondary thread context!
// Do not call any GUI function inside this function; rather use wxQueueEvent():
wxThread::ExitCode cMain::Entry()
{
	// here we do our long task, periodically calling TestDestroy():
	while (!GetThread()->TestDestroy())
	{		
		// ensure no one reads m_data while we write it
		wxCriticalSectionLocker lock(m_ADCReadingsCS);
		modbus_read_input_registers(ctx, 3001, 5, ADCReadings);

		// signal to main thread that new data has been received
		wxQueueEvent(GetEventHandler(), new wxThreadEvent());

		//delay next MODBUS message by 200ms
		wxThread::Sleep(200);
	}

	// TestDestroy() returned true (which means the main thread asked us
	// to terminate as soon as possible) or we ended the long task.
	return (wxThread::ExitCode)0;
}

void cMain::OnClose(wxCloseEvent&)
{
	// important: before terminating, we _must_ wait for our joinable
	// thread to end, if it's running; in fact it uses variables of this
	// instance and posts events to *this event handler
	if (GetThread() &&      // StartModbusThread() may have not been called
		GetThread()->IsRunning())
		GetThread()->Wait();
	Destroy();
}

void cMain::OnThreadUpdate(wxThreadEvent& evt)
{
	//wxCriticalSectionLocker lock(m_ADCReadingsCS);

	//Update static text with new ADC data
	ADCReadingsString = "ADC 1: " + std::to_string(ADCReadings[0]) + "\n\nADC 2: " +
		std::to_string(ADCReadings[1]) + "\n\nADC 3: " + std::to_string(ADCReadings[2]) +
		"\n\nADC 4: " + std::to_string(ADCReadings[3]) + "\n\nADC Status: " + std::to_string(ADCReadings[4]);

	m_static_txt1->SetLabel(ADCReadingsString);
}
Regards,
Ksawery
doublemax wrote: Fri Jul 26, 2019 12:10 pm For anything that could lead to a timeout, it's recommended to put it into a secondary thread, so that there is no chance to block the gui. Communication with the main thread will happen by sending an event.

wxThreadHelper makes this a little easier. The documentation shows complete code including sending the event.

https://docs.wxwidgets.org/trunk/classw ... elper.html
Last edited by Ksawery on Fri Jul 26, 2019 2:30 pm, edited 1 time in total.
Ksawery
Experienced Solver
Experienced Solver
Posts: 83
Joined: Thu Jul 25, 2019 12:31 pm

Re: Infinite loop running in the background

Post by Ksawery »

Thank you, I will try to use wxThreadHelper for now, but I will also take a look at the code you provided :)
PB wrote: Fri Jul 26, 2019 12:56 pm Just for comparison, here is a very simple complete example using just wxThread, where instead of Sleep()ing, one could communicate with the device

Code: Select all

#include <wx/wx.h>
#include <wx/thread.h>

class MyThread : public wxThread
{
public:
    MyThread(wxEvtHandler* sink) 
        : wxThread(wxTHREAD_JOINABLE), m_sink(sink) 
    {}    
protected:
    wxEvtHandler* m_sink;        
    
    ExitCode Entry() override
    {                        
        size_t count = 0;    
        wxThreadEvent evt;
        
        evt.SetString("Thread started.");
        wxQueueEvent(m_sink, evt.Clone()); 
        
        while( !TestDestroy() ) 
        {                    
            Sleep(1000);
            evt.SetString(wxString::Format("Thread counting: count = %zu", ++count));
            wxQueueEvent(m_sink, evt.Clone()); 
        }
        
        evt.SetString("Thread finished.");
        wxQueueEvent(m_sink, evt.Clone()); 
        
        return static_cast<wxThread::ExitCode>(nullptr);
    }    
};

class MyFrame : public wxFrame
{
public:
    MyFrame() : wxFrame(nullptr, wxID_ANY, "Test", wxDefaultPosition, wxSize(800, 600))          
    {  
        wxPanel* mainPanel = new wxPanel(this);        
        wxBoxSizer* mainPanelSizer = new wxBoxSizer(wxVERTICAL);
        wxButton* button = nullptr;

        button = new wxButton(mainPanel, wxID_ANY, "&Start Thread");
        button->Bind(wxEVT_COMMAND_BUTTON_CLICKED, &MyFrame::OnStartThread, this);
        mainPanelSizer->Add(button, wxSizerFlags().Expand().DoubleBorder());

        button = new wxButton(mainPanel, wxID_ANY, "S&top Thread");
        button->Bind(wxEVT_COMMAND_BUTTON_CLICKED, &MyFrame::OnStopThread, this);
        mainPanelSizer->Add(button, wxSizerFlags().Expand().DoubleBorder());
        
        wxTextCtrl* logCtrl = new wxTextCtrl(mainPanel, wxID_ANY, wxEmptyString, wxDefaultPosition, wxDefaultSize, 
            wxTE_MULTILINE | wxTE_READONLY | wxTE_RICH2);     
        wxLog::SetActiveTarget(new wxLogTextCtrl(logCtrl));                                         
        mainPanelSizer->Add(logCtrl, wxSizerFlags().Expand().DoubleBorder().Proportion(5));

        Bind(wxEVT_CLOSE_WINDOW, &MyFrame::OnClose, this);
        Bind(wxEVT_THREAD, &MyFrame::OnThread, this);                    
                
        mainPanel->SetSizer(mainPanelSizer);
    }   
protected:        
    MyThread* m_thread{nullptr};

    void OnStartThread(wxCommandEvent&)
    {
        if ( m_thread )
        {
            wxLogError("Thread is already running.");
            return;
        }
        
        m_thread = new MyThread(this);        
        if ( m_thread->Run() != wxTHREAD_NO_ERROR )
        {
            delete m_thread;
            m_thread = nullptr;
            wxLogError("Could not create the thread!");                    
        }
    }

    void OnStopThread(wxCommandEvent&)
    {                
        if ( !m_thread )
        {
            wxLogError("Thread is not running.");
            return;
        }

        StopThread();    
    }
        
    void MyFrame::OnClose(wxCloseEvent&)
    {        
        if ( m_thread )
        {
            if ( wxMessageBox("Thread is still runing, quit anyway?", 
                              "Question", wxYES_NO | wxNO_DEFAULT) != wxYES )
            {
                return;
            }
            StopThread();

        }
                
        Destroy();        
    }

    void OnThread(wxThreadEvent &evt)
    {    
        wxLogMessage(evt.GetString());
    }

    void StopThread()
    {
        wxCHECK_RET(m_thread, "Thread is not running");

        wxLogMessage("Stopping the thread....");

        m_thread->Delete();
        delete m_thread;
        m_thread = nullptr;    
    }
};

class MyApp : public wxApp
{
public:   
    bool OnInit() override
    {
        (new MyFrame)->Show();
        return true;
    }
}; wxIMPLEMENT_APP(MyApp);
And here is how to use a custom payload with a wxThreadEvent: viewtopic.php?f=1&t=45311&p=187832#p187836
User avatar
doublemax
Moderator
Moderator
Posts: 19115
Joined: Fri Apr 21, 2006 8:03 pm
Location: $FCE2

Re: Infinite loop running in the background

Post by doublemax »

Another issue I have is that wxCriticalSectionLocker lock(m_dataCS) caused the application to hang when called in OnThreadUpdate(), so I commented that line out. I'm not sure why it caused the app to hang?
In your version the lock is never released.

Add another block so that the wxCriticalSectionLocker gets destroyed.

Code: Select all

{
  // ensure no one reads m_data while we write it
  wxCriticalSectionLocker lock(m_ADCReadingsCS);
  modbus_read_input_registers(ctx, 3001, 5, ADCReadings);

  // signal to main thread that new data has been received
  wxQueueEvent(GetEventHandler(), new wxThreadEvent());
}

//delay next MODBUS message by 200ms
wxThread::Sleep(200);
Even better would be to transfer the actual data inside the event object.
Use the source, Luke!
Ksawery
Experienced Solver
Experienced Solver
Posts: 83
Joined: Thu Jul 25, 2019 12:31 pm

Re: Infinite loop running in the background

Post by Ksawery »

Thank you for your quick reply, that helped solve the problem of the application hanging on wxCriticalSectionLocker lock(m_dataCS), however I'm still unable to close the window (the window remains where it was and I have to manually terminate the application in the debugger). I'm still not sure why that is? If I don't enable the helper thread, the issue disappears.

Regards,
Ksawery
User avatar
doublemax
Moderator
Moderator
Posts: 19115
Joined: Fri Apr 21, 2006 8:03 pm
Location: $FCE2

Re: Infinite loop running in the background

Post by doublemax »

After testing the code myself, i think the demo code in the documentation is wrong. Where is says GetThread()->Wait(), it should be GetThread()->Delete()
Use the source, Luke!
Ksawery
Experienced Solver
Experienced Solver
Posts: 83
Joined: Thu Jul 25, 2019 12:31 pm

Re: Infinite loop running in the background

Post by Ksawery »

That makes sense, it now works, thank you!
Post Reply