Custom Event Isn't Being Caught Topic is solved

[Deluge]

I'm reworking an old project & have moved some code around. My MainWindow class used to post a custom event, when a secondary thread finished, to the main thread. Example:

abc.cpp (catching event) (source):

Code: Select all

Connect(wxID_ANY, ID_KEY, wxCommandEventHandler(MainWindow::ChangeLetter), 0, this);

abc.cpp (MainWindow::KeyThread) (source):

Code: Select all

Connect(wxID_ANY, ID_KEY, wxCommandEventHandler(MainWindow::ChangeLetter), 0, this);

...

void *MainWindow::KeyThread(void *arg) {
    wxEvtHandler *obj = wxDynamicCast(arg, wxEvtHandler);
    if (obj) {
        wxSound sound(cur_sound);
        sound.Play(wxSOUND_SYNC);
        wxCommandEvent KeyEvent(ID_KEY, wxID_ANY);
        wxPostEvent(obj, KeyEvent);
    }
    pthread_exit(NULL);
}

...

void MainWindow::ChangeLetter(wxCommandEvent& event) {
    ...
}

This was working fine. But I have recently moved secondary threading into a different source file that handles all my sound playing. But I can't seem to catch my custom event anymore:

sound.cpp (source):

Code: Select all

/** cleans up for thread exit */
static void exitThreadEvent(void* arg) {
	unloadChunks();
	thread_is_active = false;

	wxEvtHandler *source_window = wxDynamicCast(arg, wxEvtHandler);
	if (source_window) {
		// DEBUG:
		logMessage("Sending SoundFinishEvent ...");

		// event to send to main thread
		wxCommandEvent SoundFinishEvent(wxEVT_NULL, ID_SOUNDEND);
		wxPostEvent(source_window, SoundFinishEvent); // FIXME: event not caught
	}
}

abc.cpp (source):

Code: Select all

Connect(ID_SOUNDEND, wxEVT_NULL, wxCommandEventHandler(MainWindow::OnSoundFinish), 0, this);

abc.cpp (MainWindow::OnSoundFinish) (source):

Code: Select all

void MainWindow::OnSoundFinish(wxCommandEvent& event) {
	// DEBUG:
	logMessage("SoundFinishEvent");
}

In my log, I see the message "Sending SoundFinishEvent ...", then the event is supposed to to be posted to the main thread but MainWindow::OnSoundFinish is never called.

Whenever the method SoundPlayer::play is called, my MainWindow instance is passed as a parameter. This passes the instance to a new thread which plays a sound so that an event can be sent back to it as the thread finishes (NOTE: My SoundPlayer class handles playing all sounds with SDL, previously I used wxSound but wanted support for OGG/Vorbis audio).

The reason I am using pthreads is because that is traditionally what I used & am familiar with it. I have thought about making the change to wxThread, & only just recently learned about std::thread. But this should work with pthreads.

I have been trying to figure out for about 24 hours now, why this isn't working. Can anyone see my mistake(s) in my code?

[doublemax]

Code: Select all

wxCommandEvent SoundFinishEvent(wxEVT_NULL, ID_SOUNDEND);

ID and event type are in the wrong order here.

FWIW: I don't think it's a good idea to use wxEVT_NULL, as it could shadow mistakes in the event handling code. Make first tries with something like wxEVT_BUTTON, if that works define a custom event type.

[Deluge]

Code: Select all

wxCommandEvent SoundFinishEvent(wxEVT_NULL, ID_SOUNDEND);

ID and event type are in the wrong order here.

Switching them doesn't fix it:

Code: Select all

wxCommandEvent SoundFinishEvent(ID_SOUNDEND, wxEVT_NULL);

According to the docs, wxEventType is the first argument, & id is the second.

Here is some test code that does work:

Code: Select all

#include <iostream>
#include <pthread.h>
#include <unistd.h>
#include <wx/wx.h>

using namespace std;


const int ID_THREADEND = wxNewId();
pthread_t thread_id;

// prototype thread function
void* threadIt(void* arg);


class MainWindow : public wxFrame {
public:
	MainWindow();
	wxButton* getButton() { return button; }
private:
	void ChangeColor();
	void OnButton(wxCommandEvent& event);
	void OnThreadFinish(wxCommandEvent& event);

	wxPanel* bg;
	wxButton* button;
} *frame;


class App : public wxApp {
public:
	virtual bool OnInit();
};


wxIMPLEMENT_APP(App);


MainWindow::MainWindow() :
		wxFrame(NULL, wxID_ANY, _T("Catch Event from Thread")) {

	bg = new wxPanel(this, wxID_ANY);

	button = new wxButton(bg, wxID_ANY, _T("Enter Thread"));
	button->Connect(wxEVT_BUTTON, wxCommandEventHandler(MainWindow::OnButton), 0, this);
	Connect(ID_THREADEND, wxEVT_NULL, wxCommandEventHandler(MainWindow::OnThreadFinish), 0, this);

	wxBoxSizer* sizer = new wxBoxSizer(wxHORIZONTAL);
	sizer->Add(button, 1, wxEXPAND);

	bg->SetSizer(sizer);
	bg->SetAutoLayout(true);
	bg->Layout();
}

void MainWindow::ChangeColor() {
	wxColour color = button->GetBackgroundColour();

	if (color == wxColour("red")) {
		cout << "Setting color to green ...\n";
		button->SetBackgroundColour("green");
	} else {
		cout << "Setting color to red ...\n";
		button->SetBackgroundColour("red");
	}
}

void MainWindow::OnButton(wxCommandEvent& event) {
	cout << "Button pressed\n";

	pthread_create(&thread_id, NULL, threadIt, this);
}

void MainWindow::OnThreadFinish(wxCommandEvent& event) {
	cout << "Thread finished\n";

	ChangeColor();
}


bool App::OnInit() {
	MainWindow* frame = new MainWindow();
	SetTopWindow(frame);
	frame->Show();

	return true;
}


void* threadIt(void* arg) {
	cout << "Entered thread ...\n";

	sleep(3);

	cout << "Exiting thread ...\n";

	// MainWindow instance
	wxEvtHandler* eh = wxDynamicCast(arg, wxEvtHandler);
	if (eh) {
		wxCommandEvent testEvent(wxEVT_NULL, ID_THREADEND);
		wxPostEvent(eh, testEvent);
	}

	return (void*) 0;
}

--- Edit ---

Thanks for the advice about wxEVT_NULL. I plan to create a custom event type.

--- Edit ---

Note: wxEventType & id arguments order is reversed in the wxEvtHandler::Connect method.

--- Edit ---

Created custom wxEventType:

event.h:

Code: Select all

...
static const wxEventType EVT_SOUND_FINISH = wxNewEventType();

sound.cpp (switching wxCommandEvent parameters still doesn't fix it):

Code: Select all

// event to send to main thread
wxCommandEvent SoundFinishEvent(EVT_SOUND_FINISH, ID_SOUNDEND);
wxPostEvent(source_window, SoundFinishEvent); // FIXME: event not caught

abc.cpp:

Code: Select all

// sounds finish playing
Connect(EVT_SOUND_FINISH, wxCommandEventHandler(MainWindow::OnSoundFinish), 0, this);

Problem persists.

[evstevemd]

Not answering the question, but was wondering why don't you use Bind!

[Deluge]

Not answering the question, but was wondering why don't you use Bind!

I haven't studied the difference between Bind & Connect. Is one preferred for certain situations over the other? All I know is that "Bind" has something to do with "dynamically" connecting the event.

--- Edit ---

Oh, I guess the documentation uses the term "dynamically" for both.

[evstevemd]

Not answering the question, but was wondering why don't you use Bind!

I haven't studied the difference between Bind & Connect. Is one preferred for certain situations over the other? All I know is that "Bind" has something to do with "dynamically" connecting the event.

--- Edit ---

Oh, I guess the documentation uses the term "dynamically" for both.

Bind is preferred over Connect. Here is Vadim Zeitlin, wxWidgets core developer saying "Don't use Connect". Here I quote the answer:

First, don't use Connect() which was superseded by Bind() which is better in every way.

Second, both static (using event tables) and dynamic (using Bind()) methods of handling events work and you can use whichever you prefer. Personally, I recommend using Bind() because

It is much more flexible: can be used to connect the event on one object to any other object or even a free function or, in C++11, a lambda.
It is safer and catches most common errors such as using wrong event handler signature at compile time.
It is "dynamic", i.e. you can connect and disconnect the handler at any time.

The main advantages of the event tables are that

They are slightly shorter, especially in pre 3.0 versions.
They are much more common in documentation, examples, tutorials, ... just because they had a 15 year head start on Bind().

However they are clumsier to use because they require subclassing (deriving a new class from) an object in order to handle non-command events in it and they don't detect all errors at compile-time allowing you to write code that compiles fine but crashes at run-time.

[evstevemd]

Your code would be something like

Code: Select all

Bind(EVT_SOUND_FINISH, &MainWindow::OnSoundFinish, this);

and if you have specific ID

Code: Select all

Bind(EVT_SOUND_FINISH, &MainWindow::OnSoundFinish, this, MY_ID_HERE);

Binding to specific object

Code: Select all

object->Bind(EVT_SOUND_FINISH, &MainWindow::OnSoundFinish, this);

[Deluge]

Thank you for that information. If that is the case, why haven't they deprecated Connect yet?

[evstevemd]

Thank you for that information. If that is the case, why haven't they deprecated Connect yet?

That one I cannot answer, but my opinion is Bind is as of now is just better alternative to connect. You can ask that in wx-user list where core developers can answer.

[DavidHart]

Hi,

There are wxWiki pages for custom events that you might find helpful. Assuming that you don't need to stay compatible with wx2.8, see this page; it uses Bind() rather than Connect() but then so should you .

If you still support <wx3 then this is the corresponding page. However it's more confusing, having been written by several people over many years. I suggest you start here.

Regards,

David

[Deluge]

... If you still support <wx3 ...

Nope, I don't support wx 2.x anymore.

Changed my code to use Bind instead of Connect . Unfortunately, still doesn't solve my problem.

--- Edit ---

There are wxWiki pages for custom events that you might find helpful. Assuming that you don't need to stay compatible with wx2.8, see this page; it uses Bind() rather than Connect() but then so should you .

I can't seem to see anything in that which points out what I'm doing wrong. I tried using the wxDEFINE_EVENT macro. But no change:

event.h:

Code: Select all

//static const wxEventType EVT_SOUND_FINISH = wxNewEventType();
wxDEFINE_EVENT(EVT_SOUND_FINISH, wxCommandEvent);

And subclassing wxCommandEvent doesn't seem necessary in this scenario.

[alys666]

in your abc.h there are definitions

Code: Select all

const int ID_EXIT = wxNewId();
const int ID_ABC = wxNewId();
const int ID_FOOD = wxNewId();
const int ID_ANIMALS = wxNewId();
const int ID_MUSIC = wxNewId();
const int ID_TOYS = wxNewId();
const int ID_HELP = wxNewId();
const int ID_ABOUT = wxNewId();
const int ID_SPACE = wxNewId(); // Id used in threads for space
//const int ID_TAB = wxNewId(); // Id used in threads for tab
const int ID_KEY = wxNewId(); // Id used in threads for keypress
const int ID_OTHER = wxNewId(); // Id used in other threads

pls learn c++. it's not pascal or modula-2 or oberon...
header is just a piece of code included in your code via #include .
piece of code! like you wrote it manually.

when you included mentioned piece of code in different files, you have defined different objects with similar names, but different value because of wxNewId()...
when you moved code from one file to another, and something went wrong, the first idea - there are objects with same names but different meaning(values) in one file and another one.

[alys666]

and check other headers for this bug - your gnrcabt.h

Code: Select all

..
const int ID_INFO = wxNewId(); //<this is dynamic call, and values will be different in copies
const int ID_ART = wxNewId();
const int ID_LOG = wxNewId();

const int CREDIT_DEVELOPER = 100; //<this works because value will be the same everywhere
const int CREDIT_TRANSLATOR = 101;
const int CREDIT_ARTIST = 102;
const int CREDIT_PACKAGER = 103;
..

advice - to fix problem easy, forget about wxNewId in headers(it's just not working), but declare your IDs via enumeration:

Code: Select all

enum {
	Id1=5000, //get some values range, not used by wxWidgets
	Id2,
	Id3,
	....
}

[evstevemd]

advice - to fix problem easy, forget about wxNewId in headers(it's just not working), but declare your IDs via enumeration:

Code: Select all

enum {
	Id1=5000, //get some values range, not used by wxWidgets
	Id2,
	Id3,
	....
}

Good advice. id1 value can best be

Code: Select all

wxID_HIGHEST + 1

Also he can use static variables for IDs. Good catch!

[Deluge]

Thank you, but that is old code, & none of that helps me with the question of this topic.

--- Edit ---

I only linked to my old code to show the examples of how posting custom events was working.