signal/slot not working

mzimmers

How do you know it is called? Is the code containing the emit yours/debug-breakpointable by you?
Is m_worker another thread?

Yes, and yes. The emit is in my first post.

Add a brand new pair of slots, which do nothing, and attach them too to the signals. Preferably first. Do they behave the same?

    QObject::connect(m_worker, &Worker::wifiSetupAckReceived, this, &WifiSetup::testSlot1);
    QObject::connect(m_worker, &Worker::wifiSetupAckReceived, this, &WifiSetup::testSlot2);
    QObject::connect(m_worker, &Worker::wifiSetupAckReceived, this, &WifiSetup::onAckReceived);
void WifiSetup::testSlot1()
{
    qDebug() << "testSlot1 reached.";
}
void WifiSetup::testSlot2()
{
    qDebug() << "testSlot2 reached.";
}

I put breakpoints on all three slots...none are hit. And it's not some debugger error; no action by the slot(s) is taken.

JonB

@mzimmers
Earlier you said:

When mt == MSG_DISCOVERY_ACK, the signal and slot work correctly.

When mt == MSG_WIFI_SETUP_ACK, the signal is called, but the slot is never reached.

Good grief! I meant, a new pair, one on newSerialData, one on wifiSetupAckReceived. Not 3 on one and one on the other! Wanted to check the two signals each behaved the same as you reported (one good, one bad) on new slots.

Dunno, bet it's a thread thing :)

mzimmers

@JonB said in signal/slot not working:

Good grief! I meant, a new pair, one on newSerialData, one on wifiSetupAckReceived. Not 3 on one and one on the other! Wanted to check the two signals each behaved the same as you reported (one good, one bad) on new slots.

Sigh...my brain's still in low gear.

    QObject::connect(m_worker, &Worker::newSerialData, this, &WifiSetup::testSlot1);
    QObject::connect(m_worker, &Worker::newSerialData, this, &WifiSetup::onMsgReceived);
    QObject::connect(m_worker, &Worker::wifiSetupAckReceived, this, &WifiSetup::testSlot2);
    QObject::connect(m_worker, &Worker::wifiSetupAckReceived, this, &WifiSetup::onAckReceived);

Both slots for newSerialData get hit; neither of the ones for wifiSetupAckReceived get hit.

Dunno, bet it's a thread thing :)

What's confounding about this is that one works, and the other doesn't. Seemingly identical.

JonB

@mzimmers
Send me $5 if it turns out you're doing something else in your code causing this behaviour....

mzimmers

@JonB define "something else."

Hell, I should send you a bottle for all the help you've given me over the years...

Of course, by an extension of that reasoning, I'd have to buy SGaist his own distillery.

SGaist

Hi,

@mzimmers said in signal/slot not working:

mt = m_msg.getType();
switch (mt)

When is m_msg updated ? You could be processing the same value twice.

mzimmers

void Worker::processSerial(QByteArray buff)
{
    MsgType mt;
    string s;

    s.assign(buff);

    m_msg.decodeXml(s);
    mt = m_msg.getType();
    switch (mt)
    {
    case MSG_DISCOVERY_ACK:
        emit newSerialData(&m_msg);
        break;
    case MSG_WIFI_SETUP_ACK:
        emit wifiSetupAckReceived(&m_msg);
        break;
...

Besides, the signal is being emitted (at least according to the debugger).

sierdzio

Insert a qDebug() before the emit to be 100% sure.

One more idea... this method isn't called once, is it? Because if it is called once, and your have a break; in your switch then only the first ack will trigger MSG_DISCOVERY_ACK, then it will break and never bother to check if MSG_WIFI_SETUP_ACK is satisfied or not.

JonB

Let's be 100% clear for everyone: because of your separate threads, these are queued-connection signals, right? So the slots won't be called during the emits, only later on, and we're all clear about this, right?

@sierdzio said in signal/slot not working:

Insert a qDebug() before the emit to be 100% sure.

Damn right, I'd have expected you to do this to make sure already!

sierdzio

@JonB said in signal/slot not working:

Let's be 100% clear for everyone: because of your separate threads, these are queued-connection signals, right? So the slots won't be called during the emits, only later on, and we're all clear about this, right?

Yes, they will be queued. Good point, that adds one, although rare, possibility: the event loop may be too busy to process the queue.

mzimmers

Sigh.

I really should never have become a programmer.

This was pure cockpit error on my part. Everyone feel free to negrate me.

And Jon, email me your postal address; I'll send you a bottle of the good stuff.

Sorry, everyone.

J.Hilk

@mzimmers So, what was it, not be shy, we won't judge :D

I was about to suggest changing the argument of your signals to actual objects (not pointers or references) to force the compiler the acknowledge that copying has to be done, and that the lifetime of the objects do not interfere.

sierdzio

Do not despair :-) I'm pretty sure all of us here had spent a whole day (or 3...) looking for some nasty bug only to discover a missing semicolon :D

J.Hilk

@sierdzio said in signal/slot not working:

Do not despair :-) I'm pretty sure all of us here had spent a whole day (or 3...) looking for some nasty bug only to discover a missing semicolon :D

very true, my personal favorite:

if(conditionA == true);
     callFunctionA();

and callFunctionA() always being executed. Now days QtC warns you about it, but that wasn't always the case. Long days of debugging ....

mzimmers

@J-Hilk said in signal/slot not working:

@mzimmers So, what was it, not be shy, we won't judge :D

Ridiculously stupid. My Wifisetup object had:

this->done(0);

at the end of a slot for a button push. Once upon a time, this was correct, but the protocol changed, so now we wait for a confirmation before exiting. (The code that I posted was part of this.) That's where the new done() is. Unfortunately, some doofus who shall remain nameless, forgot to remove the original one. So, the signal fired, but the object containing the slot had already been destroyed.

I guess I'm a little surprised that I don't get some kind of error/warning, but I guess Qt's OK with signals just disappearing into the bit bucket.

I was about to suggest changing the argument of your signals to actual objects (not pointers or references) to force the compiler the acknowledge that copying has to be done, and that the lifetime of the objects do not interfere.

This is probably a good idea for future reference. As an embedded software engineer, I often find resources to be somewhat limited, so I'm accustomed to rather parsimonious programming. I used pointers to save stack space, which is probably silly on a desktop.

fcarney

Do this:

//this->done(0);  // memory management test 001

Now its a feature!

shaan7

@mzimmers said in signal/slot not working:

I guess I'm a little surprised that I don't get some kind of error/warning, but I guess Qt's OK with signals just disappearing into the bit bucket.

Yeah, except in cases where it bites (like this), it is quite convenient that you can delete QObjects and signal/slot connections are automatically removed.

JonB

@mzimmers said in signal/slot not working:

I guess I'm a little surprised that I don't get some kind of error/warning, but I guess Qt's OK with signals just disappearing into the bit bucket.

Not sure what you're surprised about? What do you want a warning for? (And I'm glad there isn't one.) As @shaan7 says, in Qt deleting the object used as the source of a signal or the destination of a slot in a connect() means that connection is automatically removed with it, and thank goodness for that!

If you ask me to give you a phone call when I wake up in the morning, and I have a heart attack and die during the night, or you die, either way no phone call gets made and we are all happy about it, without anyone sending a warning in the post :)