Signal and slot problem

SGaist

Hi,

Just one thing, you have now a memory leak.

But in any case, I must say I don't see any reason for making all these methods static.

mrjj

@SGaist
Thank champ. In the static heat, i completely missed that.

@azravian
You should have a
delete GPIO in the end of the function or even better
just go back to using the stack again. (so it cleans it self)
void gpio::ISR()
{
gpio GPIO;

azravian

@mrjj @SGaist
Yes i noticed it later and have deleted it. thanks again sir

azravian

@SGaist thank you for it :) and the point od making these methods statics is that i am using wiringPi library which need a function as a parameter so i need make it static.

jsulm

@azravian In this case you could add a public static method to get the pointer to the gpio instance (basically you create a singleton):

class gpio : public QObject
{
    Q_OBJECT
public:
    gpio(): instancePtr(nullptr) {}
    static gpio* instance()
    {
        if (!instancePtr) instancePtr = new gpio();
        return instancePtr;
    }
private:
    gpio* instancePtr;

You can use this method in main to setup the connection and in static methods in gpio to get the pointer to the instance. Don't forget to delete the instance later.

SGaist

@jsulm you forgot to also make instancePtr static. However, I don't think the singleton pattern should be applied here.

jsulm

@SGaist Yes, I forgot, thanks for pointing :-)

azravian

@mrjj @SGaist
Now it's all working now but i cannot use ui memeber like label, lineEdit etc. For example

void MainWindow::inserted()
{
    qDebug()<<"Inserted...";
    ui->label->setText("Inserted");
}

i do get "Inserted..." but the label does not change Text :(

SGaist

Silly question but are you user the label is visible ? Do you have something on it before you call inserted ?

azravian

@SGaist yes for instance it has the default text "textLabel" and that's visible on GUI

SGaist

Then do you have something else that modifies that label ?

Things would be a bit easier if there was a possibility to see your code.

azravian

@SGaist
Sure Sir, it's the same code except i added just one line

#include "mainwindow.h"
#include "ui_mainwindow.h"
#include "gpio.h"
#include <QDebug>

bool firstTime=true;
MainWindow::MainWindow(QWidget *parent) :
    QMainWindow(parent),
    ui(new Ui::MainWindow)
{
    if(firstTime)
    {
        gpio::Initialize();
        firstTime=false;
    }
    gpio* GPIO=new gpio;
    connect(GPIO,SIGNAL(RisingEdge()),this,SLOT(inserted()));
    connect(GPIO,SIGNAL(FallingEdge()),this,SLOT(Removed()));
    delete GPIO;
    ui->setupUi(this);
}

MainWindow::~MainWindow()
{
    delete ui;
}
void MainWindow::inserted()
{
    qDebug()<<"Inserted...";
    ui->label->setText("Inserted");
}

void MainWindow::Removed()
{
    ui->label->setText("Inserted");
    qDebug()<<"Removed...";
}

SGaist

If I may, you really should first cleanup your gpio class. It looks wrongly implemented and is wrongly used.

Make it a normal object i.e. don't use any static in it and don't make it a singleton. Once you have that sorted, it will be easier to integrate it in your GUI.

azravian

@SGaist
if i don't use ISR() and Initialize() as static i get error with wiringPiISR()
as
wiringPiISR (BUTTON, INT_EDGE_BOTH,&ISR);
it gives an error
error: cannot convert 'void (gpio::)()' to 'void ()()' for argument '3' to 'int wiringPiISR(int, int, void (*)())'
wiringPiISR (BUTTON, INT_EDGE_BOTH,&ISR);
^

SGaist

Ok, and there's no way to pass any parameter so here is an implementation example using a singleton:
WARNING : not tested on a Pi.

rpigpio.h

#ifndef RPIGPIO_H
#define RPIGPIO_H

#include <QObject>

class RpiGPIO : public QObject
{
    Q_OBJECT

public:
    static RpiGPIO *instance();
    static void destroy();

    static void initialize(int buttonNumber);

signals:
    void edgeRaised();
    void edgeFalled();

private:
    RpiGPIO();

    static void raisingEdgeCallback();
    static void fallingEdgeCallback();

    void onRaisingEdge();
    void onFallingEdge();

private:
    static RpiGPIO *_gpio;
};

#endif

rpigpio.cpp


#include "gpio.h"

RpiGPIO *RpiGPIO::_gpio = nullptr;

RpiGPIO::RpiGPIO()
   : QObject(nullptr)
{}

RpiGPIO *RpiGPIO::instance()
{
    if (!_gpio) {
        _gpio = new RpiGPIO;
    }

    return _gpio;
}

void RpiGPIO::destroy()
{
    delete _gpio;
}

void RpiGPIO::initialize(int buttonNumber)
{
    wiringPiISR(buttonNumber, INT_EDGE_RAISING, RpiGPIO::raisingEdgeCallback);
    wiringPiISR(buttonNumber, INT_EDGE_FALLING, RpiGPIO::fallingEdgeCallback);
}

void RpiGPIO::raisingEdgeCallback()
{
    RpiGPIO::instance()->onRaisingEdge();
}

void RpiGPIO::fallingEdgeCallback()
{
    RpiGPIO::instance()->onFallingEdge();
}

void RpiGPIO::onRaisingEdge()
{
    emit edgeRaised();
}

void RpiGPIO::onFallingEdge()
{
    emit edgeFalled();
}

Then you can do in your MainWindow:

MainWindow::MainWindow(QWidget *parent)
    : QMainWindow(parent)
{
    RpiGPIO::initialize(numberOfTheButtonYouWantToMonitor);
    connect(RpiGPIO::instance(), &RpiGPIO::edgeRaised, [](){ qDebug("raising");});
    connect(RpiGPIO::instance(), &RpiGPIO::edgeFalled, [](){ qDebug("falling");});
}

MainWindow::~MainWindow()
{
    RpiGPIO::destroy();
}

If you want to be even cleaner, move the calls to initialize and destroy in your main.cpp:

int main(int argc, char *argv[])
{
    QApplication app(argc, argv);
    RpiGPIO::initialize(buttonYouWantToMonitor);
    MainWindow mw;
    mw.show();
    int ret = app.exec();
    RpiGPIO::destroy();
    return ret;
}