Connecting 2 different classes together

GunkutA

Hello, I am trying to send a signal from one class to another class. For this, I am using a connect line such as this:

USB *USBInstance = new USB();
 connect(this,SIGNAL(sendChosenComPort(QString)),USBInstance,SLOT(receiveChosenComPort()));

But after I add these lines, when I run, "The program has unexpectedly finished." error appears.
Then I tried to add

USB* USBInstance ;

to the header file and

USBInstance = new USB();

to cpp. file. But again that gives an error. How can I connect 2 different classes in order to send a signal? Thanks beforehand.
P.S: I am a total newbie to QT and also to C++, so please explain it accordingly . :(

sierdzio

@GunkutA said in Connecting 2 different classes together:

@sierdzio Well the thing is, it needs to know the selected COM Port from the GUI. So there will be 3 COM Ports for example and when the button pressed, USB class will know the which COM Port is selected and then it will connect to it. So I believe I need to send COM Port info from GUI to USB class.

Yes, do it by argument. USB class does not need to know anything more. So, a minimal example:

// UI
UiClass : public QWidget
{
  Q_OBJECT

  USB *usb = nullptr;

signals:
  void openPort(const int port) const;

public:
  UiClass(QWidget *parent = nullptr) : public QWidget(parent) {
    usb = new USB;
    connect(this, &UiClass::openPort, usb, &USB::onOpenPort);
  }
};

// USB class
class USB : public QObject
{
  Q_OBJECT

public slots:
  void onOpenPort(const int port) {
    // Do something with the port
  }

};

sierdzio

Is USB a subclass of QObject? It has to be in order for signals and slots to work. It would be good to give it a parent, too - so it will be deleted when parent is deleted (to avoid a memory leak).

Run your app with debugger attached (F5 in Qt Creator) - it will show you exactly where it crashes.

It may be because you have used parentheses for empty constructor. Try this:

USB *USBInstance = new USB;

BTW. Use the new connection syntax if you can. It's way better and checked at compile time.

GunkutA

Thank you, USB is a QObject ( if that means it is a sub class) and protocolform is a QWidget. I am not sure if I can or should make one of these a parent and other one is a child. So that means I cannot connect QObject to a QWidget? If yes how can I solve that? Should I add Qobject line to the header file of the QWidget too?

sierdzio

@GunkutA said in Connecting 2 different classes together:

Thank you, USB is a QObject ( if that means it is a sub class) and protocolform is a QWidget. I am not sure if I can or should make one of these a parent and other one is a child.

QObjects can live without parents, and it is possible to connect signals and slots between them in this case - so don't worry. This is only about object cleanup when deleting objects or closing app.

So that means I cannot connect QObject to a QWidget?

You can.

If yes how can I solve that?

Run in debugger and see where it crashes. Observe the Application Output, too, for any warnings that Qt may print out.

Should I add Qobject line to the header file of the QWidget too?

If you mean the Q_OBJECT macro - yes, absolutely you should.

GunkutA

@sierdzio Thanks for the all clear answers. The thing is, I included "protocolform.h" file in the header of usb. But I couldn't include the header file of USB into the protocolform. So I just added

class USB;

line to the header of protocolform.h. Does that sound like a proper way of doing it? I am not sure which one gets started first unfortunately.

sierdzio

@GunkutA said in Connecting 2 different classes together:

"protocolform.h"

Does your USB class need that protocolform anywhere? Is it used there at all? If no, remove it.

Include the header to USB in the file where you create the object (USB *USBInstance = new USB;), that's where compiler needs the information.

If you are in a situation where both of these classes need to include each other - that's called a circular dependency. It's usually an error in class design, but in rare cases where it's really the only solution you can use forward declaration (as you did) to get around it.

But this is not causing your crash. If a header include was missing, your compiler would throw an error during compilation, before you could even try running the app.

GunkutA

I have checked it up with a debugger and I think it crashes in USB* USBInstance = new USB; line. Is there another way I can create an instance of USB class so that I can use it in connect? such as:

 connect(this,SIGNAL(sendChosenComPort(QString)),USBInstance,SLOT(receiveChosenComPort()));

JonB

@GunkutA
Your understanding of what needs to be added when two .h files need to reference each other seems correct. One of them can include the other's .h; the other .h must instead just go class Other. That second .h file can then have methods etc. referencing only a pointer to the first class, i.e. it can have Other * references but not plain Other references. Both header files can be #included into both .cpp files.

A couple of observations:

USB *USBInstance = new USB();

connect(this,SIGNAL(sendChosenComPort(QString)),USBInstance,SLOT(receiveChosenComPort()));

But after I add these lines, when I run, "The program has unexpectedly finished." error appears.

Does it crash on that connect() line, or do you mean it crashes later on when that signal is emitted?

Note that your code must have executed the USB *USBInstance = new USB(); before it executes the connect() line. Else it may crash, and won't be right anyway.

Should I add Qobject line to the header file of the QWidget too?

If you mean the Q_OBJECT macro - yes, absolutely you should.

Note that if you add Q_OBJECT macro to a .h file after you have created it, you should force a complete rebuild of your project, else things may go wrong, in an unpredictable/inexplicable way. I usually do that by deleting all files in the debug/release directory where you're compiling all your .o./.obj/executable into.

Earlier @sierdzio referred you to the new connection syntax. You should take the time to read this and change over your syntax to using that, as for example it provides more compile-time type checking etc. I know a lot of old examples around the web still show SIGNAL() & SLOT() macros, but you would benefit from weaning yourself off them.

sierdzio

@GunkutA said in Connecting 2 different classes together:

I have checked it up with a debugger and I think it crashes in USB* USBInstance = new USB; line. Is there another way I can create an instance of USB class so that I can use it in connect? such as:
 connect(this,SIGNAL(sendChosenComPort(QString)),USBInstance,SLOT(receiveChosenComPort()));

Then look into constructor of your USB class, apparently that is what crashes.

As far as I can tell, this has nothing to do with the connect() statement. And the way you wrote it seems correct. Don't worry :-)

JonB

@GunkutA said in Connecting 2 different classes together:

I have checked it up with a debugger and I think it crashes in USB* USBInstance = new USB; line. Is there another way I can create an instance of USB class so that I can use it in connect? such as:
connect(this,SIGNAL(sendChosenComPort(QString)),USBInstance,SLOT(receiveChosenComPort()));

There are two separate things here: creating a new USB and connect()ing the signal to the slot. If your code is crashing inside new USB() then you need to sort that out before you can go anywhere near signal/slot connections, they are not related.

If you have it crashing in the debugger, there should be a "stack trace" window where you can see the actual line of code inside the USB() constructor it is crashing on.

Trust me, you need to get rid of those SIGNAL() & SLOT() macros and change to the new syntax. It is an investment of time/understanding which will benefit you in the long run.

GunkutA

@JonB Ah I think I got it now. Let's say I have 2 different classes that linked each other with including header file and forward declaration. What I should have done is: Using connects in one of that classes' cpp file. What I tried to do is using connects in the both of the cpp files which was incorrect I guess. Using connects in only one of these might work I guess. But does it matter in which class I use connects? The one included other's header file in it, or the one that has a forward declaration in it?

GunkutA

@JonB I am starting to learn them right now. Thanks!

JonB

@GunkutA
You certainly do not want two connects for one signal-slot connection.

You should never connect a signal in class A to a slot in class B from within class A. In principle, signallers never know about slotters, but slotters can know about signallers. You may do the connect from the class B slot class, or you may do it from elsewhere which knows about both A & B.

It might be that after this you no longer need that "circular dependency", maybe you had that to try to do the incorrect double-connect()? It may well be that the slot class need to include the details of the signal class, to achieve the connect(), but from what you describe why does the signal class need to know anything at all about the slot class?

However, for your current issue. Comment out the connect()s completely, temporarily! Do the new USB. Does that crash?

GunkutA

Also I forgot to mention that, there is also a connect and declaration in the usb.cpp file like this:

 protocolFormInstance = new protocolForm();
   connect(this,SIGNAL(sendCOMPort(QString)),protocolFormInstance , SLOT(receiveCOMPort(QString)));

And the header file of protocolForm there is this line:

USB* USBInstance;

Now when I commented out these all lines from USB header and cpp file. The connect and declaration line the protocolform.cpp file do not crash anymore. So I believe I should transfer the connect line I commented out from the USB.cpp file to protocolform.cpp.
So at the end protocol.cpp file should include both of the connects. However, when I tried that I cannot use the sendComPort() signal which is declared in the USB header. Maybe I should make USB class a child class of the protocolform class.

JonB

@GunkutA
First, not surprisingly, I now see you have changed what your connect() looks like for the slot side. Using the SLOT() macro hides incorrect parameter passing. Your new-style connect really should look like:

 connect(this, &ThisClassName::sendCOMPort, protocolFormInstance , &ProtocolFormClassName::receiveCOMPort);

But looking at this, it seems likely it's the wrong way round. If this is USB, and that is the signaller, then the above should be done within the ProtocolForm slot class:

 connect(USBinstance, &USB::sendCOMPort, this, &ProtocolFormClassName::receiveCOMPort);

Maybe I should make USB class a child class of the protocolform class.

Absolutely not!

So at the end protocol.cpp file should include both of the connects.

What both connect()s? You have shown one connect(). What is the second connect()? I want to see both the connect()s you have.

GunkutA

@JonB Okay I tried all possibilities and found what causes the crash ( I hope). So when I declare USB in protocolform and protocol form in USB.cpp , at the same time program gets crashed :

USB.h

private:
    protocolForm* protocolFormInstance;

USB.ccp

protocolFormInstance = new protocolForm;

Protocolform.h

private:
    Ui::protocolForm *ui;
    USB* USBInstance;

Protocolform.cpp

 USBInstance = new USB;

Eventough all the connect lines are commented out, that declarations cause crash I believe. ( Btw I can send all the headers and cpp files in here if it wouldn't be too long.)

sierdzio

It matters where these instantiation lines are. If you have both news in constructors then it will surely crash (infinite loop!). If you separate them so that one object is create later, it will work.

JonB

@GunkutA
Protocolform is some kind of UI widget/form, right? While, at a guess, USB is not a GUI element?

Then I get why Protocolform needs to know about USB, to connect a slot to its signal(s). That sounds OK.

But, if that is so, why does USB class need to know anything about Protocolform UI class?? Why does USB.h/cpp need those references to protocolForm? It sounds like it should not, and then you would get of your circular dependency....

GunkutA

Because I was planning to know button presses from GUI ( which is protocolForm) in the USB class. Then I would connect to the USB port.

GunkutA

@sierdzio I believe they are in constructors :(
Complete USB.cpp code ( as an example) looks like this:

#include "usb.h"
#include <QtSerialPort>
#include <QSerialPortInfo>
#include <QtSerialPort/QSerialPortInfo>
#include <QtSerialPort/QSerialPortInfo>
#include <QSerialPort>
#include "mainwindow.h"
#include "protocolform.h"
#include <QDebug>


USB::USB(QObject *parent) : QObject(parent)
{
   QSerialPort *serial;
   timer = new QTimer(this);
   serial = new QSerialPort(this);
   connect(timer, SIGNAL(timeout()), this, SLOT(timerTimeoutSLOT()));
   timer->start(1000);
    protocolFormInstance = new protocolForm;
  //  protocolFormInstance = new protocolForm();
   connect(this,&USB::sendCOMPort(QString),protocolFormInstance , &protocolForm::receiveCOMPort(QString));



}



void USB::timerTimeoutSLOT()
{
   qDebug()<<"Timed out...";
   emit sendCOMPort("begin");
  // Q_FOREACH(QSerialPortInfo port, QSerialPortInfo::availablePorts()) {
     // emit sendCOMPort(port.portName());
  // }

}

void USB::receiveChosenComPort(const QString &ChosenComPort)
{

}

and the header of the USB:

#ifndef USB_H
#define USB_H
#include "protocolform.h"

#include <QObject>

class USB : public QObject
{
    Q_OBJECT
public:
    explicit USB(QObject *parent = nullptr);
    QTimer *timer;

public slots:
    void timerTimeoutSLOT();
    void receiveChosenComPort(const QString &ChosenComPort);

signals:
    void sendCOMPort(const QString &COMPort);

private:
    //protocolForm* protocolFormInstance;
};

#endif // USB_H