Running QProcess in a loop

goshantry

I want to monitor my GPIO using QT and check if a switch has been pressed or not. For that i have an API which i can run through the terminal and get the output.

I made the following function to check my GPIO

void myUser::check_gpio_status()
{
    audioClass audObj;
    QStringList list;
    int i = 0;
    qint64 id = 0;
    QProcess process;
    for(i=6;i>2;i--)
    {
        // The GPIO are at 6,5,4,2
        if(i == 3)
            i = 2;

        processStr = QString("/home/ubuntu/Desktop/DIO/gpio %1").arg(i);
        process.start(processStr);
        id = process.processId();
        qDebug() << "Process ID is: " << id << "Other is: " << process.pid();
        process.waitForFinished(-1); // will wait forever until finished

        QString stdout = process.readAllStandardOutput();
        QString stderr = process.readAllStandardError();

        if(stdout[13] == '1')
        {
            // Switch is not pressed
        }
        else if(stdout[13] == '0')
        {
            list.clear();
            audioStr = QString("/home/ubuntu/Sound/Audio/gpio%1.mp3").arg(i);
            list.append(audioStr);
            playDirectAudio(list,1, true);
        }

        
        process.kill();
        //process.close();
        //process.terminate();
        
    }
    process.deleteLater();

}

I call this function from my timer at every 200mS interval so that I can detect if any switch is pressed or not.
After a few minutes a get an error as too many files opened and the audio stops playing.

$ lsof | wc -l
typing the above command in terminal shows around 90000 files are opened.

Kindly help

jsulm

@goshantry said in Running QProcess in a loop:

process.deleteLater();

For what? It is allocated on the stack and will be deleted as soon as check_gpio_status() finishes. In fact this can lead to crashes because of double deletion.

Why

process.kill();

if you're waiting for process to finish?

What does playDirectAudio do?

Also, which files are open? You can check with lsof command.

goshantry

@jsulm the playAudio function is used to play the audio sounds

void myUser::playDirectAudio(QStringList list, int no_of_files, bool priority)
{
    int i;
    QString str;

    if ((player->state() == 0) || priority)       // No audio is playing
    {
        playlist.clear();           // Clear the playlist

        for(i=0;i<no_of_files;i++)      // Separate the stringlist and add them to playlist
        {
            str.clear();                // Clear the string
            str = list[i];              // Update the value of the list to string
            playlist.addMedia(QUrl::fromLocalFile(str));        // Add the list to media
        }

        player->setPlaylist(&playlist);                             // Set the playlist
        player->play();                                             // Play the audio sounds
    }
    else if(player->state() == 1)                    // If sound is already playing
    {                                   // Add media to playlist
        for(i=0;i<no_of_files;i++)      // Separate the stringlist and add them to playlist
        {
            str.clear();                // Clear the string
            str = list[i];              // Update the value of the list to string
            playlist.addMedia(QUrl::fromLocalFile(str));        // Add the list to media
        }
    }
}

If an audio is being played then it will add the audio to the playlist. Else it will play the audio file.

Also even if i remove the

process.deleteLater();
process.kill();

I am still getting the same error

JonB

@goshantry
Assuming your code is still as you showed in your first post:

You should not process.kill/terminate/deleteLater().
You must process.close() each time through the loop.

Separate issue:

for that i have an API which i can run through the terminal and get the output.

Are you not able/is it not convenient to access that API direct from your Qt program instead of having to run an external command?

goshantry

@JonB I used process.close() inside the loop
My debug output is

get GPO 4 is 1
0
Process ID is:  9830 Other is:  9830
get GPO 2 is 1
0
Process ID is:  9835 Other is:  9835
get GPO 6 is 1
0
Process ID is:  9840 Other is:  9840
get GPO 5 is 1
0
Process ID is:  9847 Other is:  9847
get GPO 4 is 1
0
Process ID is:  0 Other is:  0
get GPO 2 is 1
0
Process ID is:  0 Other is:  0
get GPO 6 is 1
0
QProcessPrivate::createPipe: Cannot create pipe 0x4b1b160: Too many open files
Process ID is:  0 Other is:  0
get GPO 5 is 1
0
QProcessPrivate::createPipe: Cannot create pipe 0x4b197b8: Too many open files
Process ID is:  0 Other is:  0
get GPO 4 is 1
0

I am still getting this error in my program

I am attaching the screenshots of accessing the GPIO of the system through terminal. I call this file from my qt program. Is there any better way to access this

0_1553314732680_Screenshot from 2019-03-23 09-46-54.png

0_1553314833531_Screenshot from 2019-03-23 09-47-14.png

0_1553314847886_Screenshot from 2019-03-23 09-47-33.png

JonB

@goshantry
I don't know what the problem/answer is, but if what you are showing is indeed the output you deffo have a problem where the PID reported is 0. It should never do that for your app, and there is some problem going on if it does....

You could insert waitForStarted:

        process.start(processStr);
        process.waitForStarted(-1);
        id = process.processId();

to see whether that changes the PID returned, though I'm not sure it will.

You should hook a slot to https://doc.qt.io/Qt-5/qprocess.html#errorOccurred to see whether that ever occurs.

You might try changing to put your QProcess on the heap (i.e. QProcess *process = new QProcess();), and create a new one each time through the loop, disposed with deleteLater(). It won't get to dispose unless you processEvents() or similar, but you could play with it to see if the behaviour is any different.

I'm also a touch concerned about what happens if you set off these gpio processes 4 times per 200ms and, for whatever reason, one of them does not finish within the 200ms. You should check to see whether your check_gpio_status() gets called again/re-entered while it still has not completed a previous invocation (e.g. use a static variable to check this)?

goshantry

@JonB I tried to change my function to the following

static int gpioCheckStatus = 6;

void myUser::check_gpio_status()
{
    audioClass audObj;
    QStringList list;
    QString audioStr, processStr, filePath = "/home/fiem/Desktop/DIO/Output.txt";
    int i = 0, lineNum = 0;
    qint64 id = 0;
    QFile file(filePath);

    {
        if(gpioCheckStatus == 3)
            gpioCheckStatus = 2;

        processStr = QString("/home/fiem/Desktop/DIO/gpio %1 > %2").arg(gpioCheckStatus).arg(filePath);

        system(processStr.toUtf8());


        if(!file.open(QIODevice::ReadOnly))
        {
            QMessageBox::information(this, "error", file.errorString());
        }

        QTextStream in(&file);

        // Read till end of file
        while(!in.atEnd())
        {
            // Read one line at a time
            QString line = in.readLine();
            qDebug() << line;

            if(line[13] == '0')
            {
                list.clear();
                audioStr = QString("/home/fiem/Sound/Audio/gpio%1.mp3").arg(gpioCheckStatus);
                list.append(audioStr);
                playDirectAudio(list,1, true);
            }

            lineNum++;
        }

        file.close();
        system("rm -rf '/home/fiem/Desktop/DIO/Output.txt'");

        gpioCheckStatus--;

        if(gpioCheckStatus < 2 || gpioCheckStatus > 6)
            gpioCheckStatus = 6;


    }

}

I tried to make a file while writing through system command and read the contents of the file with every iteration.
I have also removed the loop and placed a global variable to decrement whenever my 100mS timer triggers.

In this after a while I get an error stating
too many opened files

I am unable to understand what might be causing this and how to resolve this error

JonB

@goshantry
I don't understand what you are trying to achieve here with your static int gpioCheckStatus.

What I want you to do would be like (I'm not a full-time C++-er):

void myUser::check_gpio_status()
{ 
    static bool inCall = false;

    if (inCall)
        whoops("We have a problem: re-entered while previous call was still in progress :(").

    inCall = true;
    // do all code in your function here
    inCall = false;
}

goshantry

@JonB previously I was using the for loop. In that case when my timer triggered at every 100mS, I was checking the status of all the 4 GPIO's in one go. Now i took a variable gpioCheckStatus which check the status of 1 GPIO every time my timer triggers.

JonB

@goshantry
Nonetheless you should still just try my check for re-entrancy, whether you do 1 or 4 at a time. It should only take a few seconds to type in and test. If that gets hit you have an issue.

You have not addressed my other observations, e.g. in particular getting a PID of 0.

SGaist

Hi,

Do you know how that gpio works ?

It would likely be easier to implement that directly in your application.

JonB

@SGaist
I have already asked the OP whether he can't do it directly from his Qt app instead of via a shell command, but he did not answer....

@goshantry
Replace your call to gpio with one that calls lsof instead. You can also use the lsof to identify the PID of which processes have all these files open (if indeed they are still open once you do not invoke gpio). In particular, check whether that PID is actually the PID of your Qt app. If it is, the implication may be that elsewhere in your code you are opening files but not closing them as your timer ticks....

mrjj

Hi
Just as a note.
It seems to be http://wiringpi.com/
that gpio executables comes from - and its also a library
so if calling the command line utility turns out to be a no-go then
the library should be directly useable. ( and seems easy too)

goshantry

@SGaist @JonB I integrated the API directly in my application and now things work fine. I am able to read the GPIO values without needing QProcess.

Also I am trying to write the code as per the below function

static bool inCall = false;

    if (inCall)
        whoops("We have a problem: re-entered while previous call was still in progress :(").

    inCall = true;
    // do all code in your function here
    inCall = false;

but the thing is I check the GPIO every 100mS. If the switch is pressed then i play an audio which lasts about a few seconds, and if any other switch is pressed then that should play the audio as well.

@jsulm @JonB @SGaist @mrjj thanks a lot for your help.

SGaist

@JonB said in Running QProcess in a loop:

@SGaist
I have already asked the OP whether he can't do it directly from his Qt app instead of via a shell command, but he did not answer....

That's why I tried to push again your point with a different wording.

@goshantry since you are reading GPIOs, did you consider creating a dedicated class for that ?

By the way, what exactly are you using these GPIO for ?

goshantry

@SGaist I created a new class for the gpio. I am working on a project in which i have to monitor 4 different switches and play different sounds whenever any switch is pressed.

I am attaching the classes which i created

#ifndef F81866_H
#define F81866_H
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <fcntl.h>
#include <sys/time.h>
#include <sys/types.h>
#include <sys/stat.h>

#include <sys/io.h> /* linux-specific */

#ifdef __GLIBC__
#  include <sys/perm.h>
#endif

#define F81866_REG_LD		0x07
#define F81866_UNLOCK		0x87
#define F81866_LOCK             0xAA
#define F81866_CFG_GPIO		0x06
#define F81866_CFG_WDT		0x07
static unsigned short F81866_PORT_INDEX = 0x004eL;
static unsigned short F81866_PORT_DATA  = 0x004fL;

static unsigned int ChipIDTab[] = {
    0x19341010,             // F81866
    0x19340704,             // F81865
    0x19341502,             // F81964/F81962/F81966/F81967
};

class f81866
{
public:
    f81866();
    void f81866_unlock(void);
    void f81866_lock(void);
    unsigned char f81866_read(unsigned char bREG);
    void f81866_write(unsigned char bREG, unsigned char bVal);
    void f81866_set_logicdevice(unsigned char bDev);
    int f81866_init();
};

#endif // F81866_H

C file for the above header

#include "f81866.h"

f81866::f81866()
{

}

void f81866::f81866_unlock()
{
    outb(F81866_UNLOCK, F81866_PORT_INDEX);
    outb(F81866_UNLOCK, F81866_PORT_INDEX);
}

void f81866::f81866_lock()
{
    outb(F81866_LOCK, F81866_PORT_INDEX);
}

unsigned char f81866::f81866_read(unsigned char bREG)
{
    unsigned char bRES = (unsigned char)-1;
    f81866_unlock();
    outb(bREG, F81866_PORT_INDEX);
    bRES = inb(F81866_PORT_DATA);
    f81866_lock();
    return bRES;
}

void f81866::f81866_write(unsigned char bREG, unsigned char bVal)
{
    f81866_unlock();
    outb(bREG, F81866_PORT_INDEX);
    outb(bVal, F81866_PORT_DATA);
    f81866_lock();
}

void f81866::f81866_set_logicdevice(unsigned char bDev)
{
    f81866_write(F81866_REG_LD, bDev);
}

int f81866::f81866_init()
{
    int	nID = 0;
    int i;
    unsigned char *pbID = (unsigned char *)&nID;
    //fintek = 0x1934L, f81866 chip id = 0x10L
    pbID[1] = f81866_read(0x20L);
    pbID[0] = f81866_read(0x21L);
    pbID[3] = f81866_read(0x23L);
    pbID[2] = f81866_read(0x24L);
    for (i=0; i<(sizeof(ChipIDTab)/sizeof(ChipIDTab[0])); i++ )
        if ( nID == ChipIDTab[i] )
            return 0;

    F81866_PORT_INDEX = 0x002eL;
    F81866_PORT_DATA = 0x002fL;
    nID = 0;
    pbID[1] = f81866_read(0x20L);
    pbID[0] = f81866_read(0x21L);
    pbID[3] = f81866_read(0x23L);
    pbID[2] = f81866_read(0x24L);
    for (i=0; i<(sizeof(ChipIDTab)/sizeof(ChipIDTab[0])); i++ )
        if ( nID == ChipIDTab[i] )
            return 0;

    return -1;
}

The other class

#ifndef F81866_GPIO_H
#define F81866_GPIO_H

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <sys/io.h> /* linux-specific */

#define F81866_CFG_GPIO		0x06


class f81866_gpio
{
public:

    f81866_gpio();
    int getGPI(int nIndex);
    void setGPO(int nIndex, int nValue);
    void setDirection(int nIndex, int nValue);
    void initGPIO(void);
};

#endif // F81866_GPIO_H

c file for the above header

#include "f81866_gpio.h"
#include "f81866.h"


f81866_gpio::f81866_gpio()
{

}

int f81866_gpio::getGPI(int nIndex)
{
    f81866 classObj;
    unsigned char bRES = 0;
    nIndex &= 7;	//nIndex input = 0~3

    classObj.f81866_set_logicdevice(F81866_CFG_GPIO); //select GPIO logic-device
    bRES = classObj.f81866_read(0x8AL); 	//read state from input-state
    return (bRES & (1 << nIndex)) ? 1 : 0;
}

void f81866_gpio::setGPO(int nIndex, int nValue)
{
    unsigned char bRES = 0;
    f81866 classObj;
    nIndex &= 7;	//nIndex inut = 0~7
    nValue = nValue ? 1 : 0;
    classObj.f81866_set_logicdevice(F81866_CFG_GPIO); //select GPIO logic-device
    bRES = classObj.f81866_read(0x8AL); 	//read state from input-state
    bRES &= ~(1 << nIndex);		//clean old value
    bRES |= (nValue << nIndex);	//set new value
    classObj.f81866_write(0x89L, bRES); 	//write to output-data
}

void f81866_gpio::setDirection(int nIndex, int nValue)
{
    unsigned char bTMP;
    f81866 classObj;
    nIndex &= 7;
    nValue = nValue ? 1 : 0;
    classObj.f81866_set_logicdevice(F81866_CFG_GPIO); //select GPIO logic-device
    bTMP = classObj.f81866_read(0x88L);	//read current direction set
    bTMP &= ~(1 <<  nIndex);	//clean old direction set
    bTMP |= (nValue << nIndex);	//set new direction
    classObj.f81866_write(0x88L, bTMP);	//setvalue
}

void f81866_gpio::initGPIO()
{
    unsigned char bTMP;
    f81866 classObj;
    classObj.f81866_set_logicdevice(F81866_CFG_GPIO); //select GPIO logic-device
    bTMP = classObj.f81866_read(0x30L);
    bTMP |= 1;		//enable GPIO
    classObj.f81866_write(0x30L, bTMP);
    //default GPIO 0~3 is input.
    setDirection(4, 0);	//GPIO4 (IN0) set in
    setDirection(5, 0);	//GPIO5 (IN1) set in
    setDirection(6, 0);	//GPIO6 (IN2) set in
    setDirection(7, 0);	//GPIO7 (IN3) set in
}

Function to check the GPIO (I call this function from my timer)

void pisUser::check_gpio_status()
{
    int gpioVal = 0;
    f81866_gpio f818Obj;
    QStringList list;
    QString audioStr, processStr, filePath = "/home/fiem/Desktop/DIO/Output.txt";

    if(gpioCheckStatus == 3)
        gpioCheckStatus = 2;
    gpioVal = f818Obj.getGPI(gpioCheckStatus);
    if(gpioVal == 0)
    {
        processStr = QString("Emergency in room %1").arg(gpioCheckStatus);
        QFont engFont = QFont("Arial Black");
        engFont.setPointSize(80);
        ui->infoLabel->setFont(engFont);
        ui->infoLabel->setText(processStr);
        list.clear();
        audioStr = QString("/home/fiem/Sound/Audio/gpio%1.mp3").arg(gpioCheckStatus);
        list.append(audioStr);
        playDirectAudio(list,1, true);
    }

    gpioCheckStatus--;

    if(gpioCheckStatus < 2 || gpioCheckStatus > 6)
        gpioCheckStatus = 6;
}