Search code examples
c++wxwidgets

wxWidgets wxThreadHelper Example Question

I'm relatively new to wxWidgets and trying to get a grasp on how to properly implement threading and events and all that good stuff. I was able to get a simple frame running in my projects directory just fine, but as I ran into issues when trying to add in threading.

I ran the Thread demo given in the "samples" of the repo without issue but as I understand it, the more recent preferred method is to use wxThreadHelper because you don't need to pass around frame pointers.

I'm essentially starting off in the wxThreadHelper example that's given here. All I really did was separate things into header and src.

Header:

// For compilers that support precompilation, includes "wx/wx.h".
#include "wx/wxprec.h"

// for all others, include the necessary headers (this file is usually all you
// need because it includes almost all "standard" wxWidgets headers)
#ifndef WX_PRECOMP
    #include "wx/wx.h"
#endif

#if !wxUSE_THREADS
    #error "This sample requires thread support!"
#endif // wxUSE_THREADS


wxDECLARE_EVENT(EVT_COMMS_UPDATE, wxThreadEvent);

enum {LAYOUT_TEST_PROPORTIONS = wxID_HIGHEST+1,
        LAYOUT_TEST_SIZER,
        LAYOUT_TEST_NB_SIZER
    };


class wxMainFrame : public wxFrame, public wxThreadHelper, private wxLog {
private:

public:
    wxMainFrame();
    ~wxMainFrame(){
        // it's better to do any thread cleanup in the OnClose()
        // event handler, rather than in the destructor.
        // This is because the event loop for a top-level window is not
        // active anymore when its destructor is called and if the thread
        // sends events when ending, they won't be processed unless
        // you ended the thread from OnClose.
        // See @ref overview_windowdeletion for more info.
    }
    void onCommsUpdate_t();
    void OnClose(wxCloseEvent& evt);
    
    wxTextCtrl * m_txtctrl;
    
    
protected:
    virtual wxThread::ExitCode Entry();
    
    //fields update from threads
    double data_t;
    wxCriticalSection m_dataCS; // protects field above - i don't really know how this thing scopes the CS
    
    wxDECLARE_EVENT_TABLE();

};

SRC:

#include "wxMainFrame.h"

wxDEFINE_EVENT(EVT_COMMS_UPDATE, wxThreadEvent)
wxBEGIN_EVENT_TABLE(wxMainFrame, wxFrame)
    EVT_THREAD(EVT_COMMS_UPDATE, wxMainFrame::onCommsUpdate_t)
    EVT_CLOSE(wxMainFrame::OnClose)
wxEND_EVENT_TABLE()

wxMainFrame::wxMainFrame() : wxFrame(NULL, wxID_ANY, "wxWidgets Layout Demo") {
    
    //Enable thread
    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;
    }
    
    this->data_t = 0;
    
    // Make a menubar
    wxMenu *file_menu = new wxMenu;

    file_menu->Append(LAYOUT_TEST_PROPORTIONS, "&Proportions demo...\tF1");
    file_menu->Append(LAYOUT_TEST_SIZER, "Test wx&FlexSizer...\tF2");
    file_menu->Append(LAYOUT_TEST_NB_SIZER, "Test &notebook sizers...\tF3");
    wxMenuBar *menu_bar = new wxMenuBar;

    menu_bar->Append(file_menu, "&File");

    // Associate the menu bar with the frame
    SetMenuBar(menu_bar);
    
    

    
    // create the logging text control and a header showing the meaning of the
    // different columns
    wxTextCtrl *header = new wxTextCtrl(this, wxID_ANY, "",
                                        wxDefaultPosition, wxDefaultSize,
                                        wxTE_READONLY);
    //DoLogLine(header, "  Time", " Thread", "Message");
    m_txtctrl = new wxTextCtrl(this, wxID_ANY, "",
                               wxDefaultPosition, wxDefaultSize,
                               wxTE_MULTILINE | wxTE_READONLY); 
    
    wxLog::SetActiveTarget(this);

    // use fixed width font to align output in nice columns
    wxFont font(wxFontInfo().Family(wxFONTFAMILY_TELETYPE));
    header      ->SetFont(font);
    m_txtctrl   ->SetFont(font);

    m_txtctrl->SetFocus();  //Field to have focus on startu

    // layout and show the frame
    wxBoxSizer *sizer = new wxBoxSizer(wxVERTICAL);
    sizer->Add(header, wxSizerFlags().Expand());
    sizer->Add(m_txtctrl, wxSizerFlags(1).Expand());    
    SetSizer(sizer);

    SetSize(600, 350);
    
}

wxThread::ExitCode wxMainFrame::Entry(){
    // IMPORTANT: this function gets executed in the secondary thread context!
    while (!GetThread()->TestDestroy()) {
        // since this Entry() is implemented in MyFrame context we don't
        // need any pointer to access the m_data, m_processedData, m_dataCS
        // variables... very nice!
        wxCriticalSectionLocker lock(m_dataCS);
        
        this->data_t++;
        
        // VERY IMPORTANT: do not call any GUI function inside this
        //                 function; rather use wxQueueEvent():
        wxQueueEvent(this, new wxThreadEvent(EVT_COMMS_UPDATE));
    }
    
    // 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 wxMainFrame::onCommsUpdate_t(){
    //Do thread stuff
    //wxCriticalSectionLocker lock(m_dataCS);
    //std::cout << this->data_t;
}

void wxMainFrame::OnClose(wxCloseEvent& evt){
    // 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() &&      // DoStartALongTask() may have not been called
        GetThread()->IsRunning())
        GetThread()->Wait();
    Destroy();
}

Which results in this compilation error:

In file included from /usr/local/include/wx-3.1/wx/wx.h:24,
                 from includes/wxMainFrame.h:10,
                 from src/wxMainFrame.cpp:2:
/usr/local/include/wx-3.1/wx/event.h:4350:5: error: expected ‘,’ or ‘;’ before ‘                                                            const’
 4350 |     const wxEventTable theClass::sm_eventTable = \
      |     ^~~~~
src/wxMainFrame.cpp:5:1: note: in expansion of macro ‘wxBEGIN_EVENT_TABLE’
    5 | wxBEGIN_EVENT_TABLE(wxMainFrame, wxFrame)

As you can see this is a basic syntax error that I can see/fix if I go into /usr/local/include/wx-3.1/wx/event.h:4350

but this doesn't make sense, I can't be the first person to find a simple syntax error? Something must be wrong on my machine (ubuntu). This is the header that's added to my sys includes from make install right? How would a syntax error appear if the demos/Samples built from the source all work fine. Just thinking out loud I suppose.

I built the lib from source using the suggestions from here.

I also saw this post on wxWidget's forum here. Who didn't seem to have the same syntax issue I'm having. But also, I didn't really understand how to use the suggested macro because the wiki says the macro is actually wrapped by wx__DECLARE_EVT2. If this is the solution perhaps a simple example would be helpful to show me how to use it properly.

I appreciate any input. Thanks!

PS I've tried posting this question on the forum but was unable to register (tried with multiple browsers) so here I am.

Solution

  • There are many issues here. First, the compilation error you mention is due to a missing semicolon, the line:

    wxDEFINE_EVENT(EVT_COMMS_UPDATE, wxThreadEvent)

    should be

    wxDEFINE_EVENT(EVT_COMMS_UPDATE, wxThreadEvent);

    Second, the onCommsUpdate_t method needs to take a wxThreadEvent& parameter

    void onCommsUpdate_t(wxThreadEvent&);

    Obviously, the definition for the method needs to be updated too.

    Third, I honestly don't how the EVT_THREAD macro is supposed to work, but I know the line

    EVT_THREAD(EVT_COMMS_UPDATE, wxMainFrame::onCommsUpdate_t)

    doesn't work. The easiest way to handle a thread event is to use the Bind method. Add a line like this to the frame constructor to handle the event

    Bind(EVT_COMMS_UPDATE,&wxMainFrame::onCommsUpdate_t,this);

    Fourth, as written, your wxMainFrame::Entry method will be constantly spamming the event queue freezing up the application. In a real application, presumably the thread will do some work before queuing each event. Since in this example, no work is being done, you should add a line like

    wxMilliSleep(500);

    to the end of the while loop to make the thread sleep for a little bit before sending each event.

    Fifth, you never actually signal your thread to shutdown and it never shuts down on its own. Consequently the line GetThread()->Wait(); will be an infinite loop. There are many ways to signal the thread, but the easiest might be to add a bool member named something like m_shutdown to the frame class. Then

    a. initialize m_shutdown to false in the frame constructor.

    b. In the wxMainFrame::OnClose, use the critical section to set it to true before calling Wait:

    if (GetThread() &&      // DoStartALongTask() may have not been called
    GetThread()->IsRunning())
{
    {
        wxCriticalSectionLocker lock(m_dataCS);
        m_shutdown = true;
    }
    
    GetThread()->Wait();
}

    c. In the wxMainFrame::Entry method, add a check for the value of m_shutdown in the while loop (guarded by the critical section).

        while (!GetThread()->TestDestroy()) {
        // since this Entry() is implemented in MyFrame context we don't
        // need any pointer to access the m_data, m_processedData, m_dataCS
        // variables... very nice!
        {
            wxCriticalSectionLocker lock(m_dataCS);
            this->data_t++;
            if ( m_shutdown )
                break;
        }
...

    Sixth, I don't know what the line

    wxLog::SetActiveTarget(this);

    is supposed to do, but it causes a crash at program exit. If you need this, you'll need to store the initial log target and restore it either in the frame destructor or in the close event handler.