0

I have created a wrapper library around QSerialPort. I want to communicate with my device. First, I send list command to my device and it should return list of commands supported by that device. However, while debugging my code, i observed that list command is being send to the device and device returns the proper response to it (i debugged using serial traffic sniffer Linux tools). However, i am not getting any response from my device using QSerialPort (while serial traffic sniffer tool was disable). I am unable to get it work after testing it several times.

My Serial.h:

class Serial : public Print {

public:
    Serial();
    explicit Serial(const char *dev_path);
    ~Serial();

    int begin(unsigned long baudrate);
    int begin(unsigned long baudrate, uint8_t cfg);
    void end(void);

    int available(void) const;
    bool availableForWrite(void) const;
    void flush(void);
    bool isError(void) const;
    void reset(void);

    unsigned long write(uint8_t c);
    unsigned long write(uint8_t *p_data, unsigned long maxSize);
    int read(void);

    void close();

    QSerialPort &getPort()
    {
        return *_p_port;
    }

public slots:
    void readyBe(void);

private:
    QSerialPort *_p_port;
    unsigned long _baudrate;
};

My Serial.cpp:

Serial::Serial()
{
    _p_port = new QSerialPort();
    if (_p_port == nullptr)
        throw std::runtime_error("Can't allocate memory");
}

Serial::Serial(const char *dev_path)
{
    _p_port = new QSerialPort(QString(dev_path), QApplication::instance());
    if (_p_port == nullptr)
        throw std::runtime_error("Can't allocate memory");
    // _p_port->setPortName(QString(dev_path));
    if (_p_port->open(QIODevice::ReadWrite) == false) {
        throw std::runtime_error("Can't open the serial _p_port");
        delete _p_port;
    }
    _p_port->setBaudRate(QSerialPort::Baud9600);
    _p_port->setDataBits(QSerialPort::Data8);
    _p_port->setParity(QSerialPort::NoParity);
    _p_port->setStopBits(QSerialPort::OneStop);
    _p_port->setFlowControl(QSerialPort::NoFlowControl);
}

Serial::~Serial()
{
    if (_p_port != nullptr) {
        end();
        delete _p_port;
    }
}

int Serial::begin(unsigned long baudrate)
{
    if (_p_port->setBaudRate(baudrate, QSerialPort::AllDirections) == false)
        return -1;
    _baudrate = baudrate;
    return 0;
}

void Serial::end()
{
    if (_p_port->isOpen())
        _p_port->close();
}

int Serial::available(void) const
{
    int num_bytes = _p_port->bytesAvailable();
    return num_bytes;
}

bool Serial::availableForWrite(void) const
{
    if (_p_port->isWritable())
        return true;
    return false;
}

void Serial::flush()
{
    _p_port->flush();
}

unsigned long Serial::write(uint8_t c)
{
    if (_p_port->putChar(c))
        return 1;
    return 0;
}

unsigned long Serial::write(uint8_t *p_data, unsigned long maxSize)
{
    return _p_port->write(reinterpret_cast<const char *>(p_data), (qint64)maxSize);
}

int Serial::read(void)
{
    char c;
    _p_port->getChar(&c);
    return c;
}

void Serial::reset(void)
{
    _p_port->clear(QSerialPort::AllDirections);
    _p_port->clearError();
}

bool Serial::isError(void) const
{
    if (_p_port->error() ==  QSerialPort::NoError)
        return false;
    return true;
}

And my main.cpp:

int main(int argc, char *argv[])
{
    QApplication a(argc, argv);
    MainWindow w;
    // w.show();

    Serial serial("ttyACM0");
    if (serial.begin(115200))
        std::cout << "Failed to set Baud rate" << std::endl;
    std::cout << "Sending data" << std::endl;
    // QObject::connect(&(serial.getPort()), SIGNAL(readyRead()), &serial, SLOT(readyBe()));
    serial.print("list\r");
    serial.flush();

    while (true) {
        while (true) {
            while (serial.available() == 0) {
                if (serial.isError()) {
                    std::cout << "Error" << std::endl;
                    // serial.reset();
                }
            }

            char c = serial.read();
            std::cout << c;
            if (c == '\n')
                break;
        }
        std::cout << std::endl;
    }

    return a.exec();

}
abhiarora
  • 9,743
  • 5
  • 32
  • 57
  • "Can't read anything using QSerialPort" is not a question, it is a statement. The fact that you appended a question mark at the end of it does not make it a question. It is a highly unintellectual americanism, and it saddens me to see people from other parts of the world use it. – Mike Nakis May 20 '17 at 19:46
  • 1
    @MikeNakis: I have edited the title. – abhiarora May 20 '17 at 19:50
  • You don't appear to do any error checking on the `QSerialPort` itself -- you should `connect` to the various signals available to check what's happening. Also note that in your `Serial` constructor, if `_p_port->open` fails you `throw` an exception immediately *before* deleting `_p_port`. – G.M. May 20 '17 at 19:57
  • @G.M. : I know that i should delete _p_port before throwing an exception. I just want to make sure everything is working before i proceed any further. – abhiarora May 20 '17 at 19:59
  • You *need* to add some basic error checking. Also, what does `serial.print(...)` do? Where is it defined -- in the `Print` class? – G.M. May 21 '17 at 08:28
  • Yes. `Print` class defines some `print` methods using `write(char c)` functions. – abhiarora May 21 '17 at 08:30
  • @G.M.: Do you think i have missed something other than error checking? – abhiarora May 21 '17 at 08:30

1 Answers1

1

You've pretty much missed everything needed for this code to work: the event loop. I/O in real life is asynchronous. You can't just "read" from the port without having some means of getting informed when the data is available, and actually letting the asynchronous I/O requests get processed. Yes, there are some legacy APIs that let you do that, but they mostly lead to spaghetti code, wasted threads, and poor performance.

The while (serial.available() == 0) loop is a no-op. It does nothing to actually let the available() return any other value. All that available() does internally is read an integer member of a class. You never run any code that could update the value stored in that member. Even if you would convert this to serial.waitForReadyRead(), which does update the number of available bytes, you're still not spinning an event loop, and thus you won't be able to process timeouts, or react to any other events an application might need to react to. QIODevice::waitForReadyRead is only meant to do one thing: return when a readyRead signal would fire. It won't process any other events, and it's a crutch used to port blocking code and is not really meant for production use.

You should redesign your code to be asynchronous and driven by signals from QSerialPort. The work will then be done from within QCoreApplication::exec - you won't have a loop of your own. This inversion of control is critical for getting async I/O working.

See e.g. this answer for a very simple example of an async approach, and this answer for a more complete one.

Kuba hasn't forgotten Monica
  • 95,931
  • 16
  • 151
  • 313