How to minimize serial communication read errors from a arduino

Question

I am trying to stablish a serial communication channel between my computer and my arduino. When I look at ArduinoIDE, I get a perfect message sent from arduino - 3 identical numbers. Now, I am trying to create a c++ app to read that data on a computer running Ubuntu, however I get a lot of garbage on the string. I have been reading and searching all around without success. Could any one help me find the source of my problem?

Code:

SerialComm.h:

#ifndef SERIALCOMM_HPP
#define SERIALCOMM_HPP

#include <fstream>
#include <string>

#include <stdio.h> // standard input / output functions
#include <string.h> // string function definitions
#include <unistd.h> // UNIX standard function definitions
#include <fcntl.h> // File control definitions
#include <errno.h> // Error number definitions
#include <termios.h> // POSIX terminal control definitionss

class SerialComm {
public:

    SerialComm() noexcept {
    }

    virtual ~SerialComm() noexcept {
        tcsetattr(fd, TCSANOW, &port_settings);
        close(fd);
    }

    void begin(std::string port, speed_t baudrate);
    std::string read_data();

private:
    int fd;
    speed_t _baudrate;
    std::string _port;
    static constexpr int BUFFER_SIZE = 256;
    char buffer[BUFFER_SIZE];
    termios port_settings;
};

SerialComm.cpp

#include "SerialComm.hpp"

#include <iostream>

using namespace std;

void SerialComm::begin(string porta, speed_t baudrate) {
    _port = porta;
    _baudrate = baudrate;

    // abre a porta
    fd = open(_port.c_str(), O_RDWR | O_NOCTTY | O_NDELAY);

    if (fd == -1) {
        printf((string("Unable to open port ") + _port).c_str());
    } else {
        fcntl(fd, F_SETFL, 0);
        printf("port is open.\n");
    }

    cfsetispeed(&port_settings, _baudrate); // set baud rates
    cfsetospeed(&port_settings, _baudrate);

    port_settings.c_cflag &= ~PARENB; // set no parity, stop bits, data bits
    port_settings.c_cflag &= ~CSTOPB;
    port_settings.c_cflag &= ~CSIZE;
    port_settings.c_cflag |= CS8;

    tcsetattr(fd, TCSANOW, &port_settings); // apply the settings to the port   
}

string SerialComm::read_data() {
    int state = 1;

    while (true) {
        state = read(fd, buffer, BUFFER_SIZE);
        if (state > 0) 
        {
            return string( buffer );
        }
    }
}

main.ccp

int main(int argc, char* argv[])
{

    SerialComm serial;

    serial.begin("/dev/ttyACM0", B115200);

    for(auto i = 0; i < 100; ++i)
    {
        cout << serial.read_data() << endl;
    }
}

serial.ino:

double sinal = 0;

void setup()
{
  Serial.begin( 115200 );
}

void loop()
{
  sinal = analogRead( A0 ) * ( 5.0 / 1024.0 );
  Serial.print( "$" );
  Serial.print( sinal, 5 );
  Serial.print( "," );
  Serial.print( sinal, 5 );
  Serial.print( "," );
  Serial.print( sinal, 5 );
  Serial.print( "#\n" );
}

Arduino IDE output:

$2.24121,2.24121,2.24121#
$2.24609,2.24609,2.24609#
$2.24121,2.24121,2.24121#
$2.24121,2.24121,2.24121#
$2.24609,2.24609,2.24609#

computer output:

$2.24609,2.24?�̯m#
$2.
09375#
$2.2412109375,2.2412109937500#
$2.2460937500,2.2460937500,2.2460937500#
375#
$2.2460937500,2.2460937500,2.2460937500#
$2.
375,2.2412109375#
$2.241210937937500#
$2.2460937500,2.2460937500,2.2460937500#

PS: the above was the prettiest output I could get.

Answer 1

Besides the problem with the unterminated string buffer, you also can't say that you will receive a complete message in a single call to read. Instead you have to read in a loop until you have the end of the message (the newline you send).

This of course present you with another problem, that you can receive the end of one message, and the beginning of the next message in the same read call. This means you have to save the beginning of the next message, and put it in the buffer before next call to read.

Answer 2

I think this is your error:

string SerialComm::read_data() {
    int state = 1;
    int receivedbyte = 0;  // never used!

    while (true) {
        state = read(fd, buffer, BUFFER_SIZE);
        if (state > 0) 
        {
            return string( buffer );
        }
    }
    buffer[receivedbyte + 1] = '\0';  // never reached!  And "off-by-one" if it were...
}

This might work better:

string SerialComm::read_data() {
    int receivedbyte = 0;

    while (true) {
        receivedbyte = read(fd, buffer, BUFFER_SIZE - 1);
        if (receivedbyte > 0) 
        {
            buffer[receivedbyte] = '\0';
            return string( buffer );
        }
    }
}

That should eliminate any garbage that you were seeing due to unterminated strings. That said, to get nice newline-terminated strings, you probably need an outer loop to look for those boundaries and divide the stream properly that way.

One way to do that might be this: Declare a string received in your class to hold all buffered input that hasn't yet been returned to the caller. Then, rewrite read_data() as follows:

string SerialComm::read_data()
{
    while (true)
    {
        size_t pos = received.find_first_of('\n', 0);
        if (pos != string::npos)
        {
            string result = received.substr(0, pos);
            received.erase(0, pos);
            return result;
        }

        int receivedbytes;
        do
        {
            receivedbytes = read(fd, buffer, BUFFER_SIZE - 1);
        } while (receivedbytes == 0);

        if (receivedbytes < 0)
            abort();  // error... you might want to handle it more cleanly, though

        buffer[receivedbytes] = 0;
        received += string( buffer );
    }
}            

If you'd like a version that returns an empty string when there no complete line to see, as opposed to waiting forever for data, you can use this version of code. Note: If there's buffered data with no terminating newline, it will hold onto it until it does see the terminating newline. You might want to add a separate flush method to make that data visible.

string SerialComm::read_data()
{
    while (true)
    {
        size_t pos = received.find_first_of('\n', 0);
        if (pos != string::npos)
        {
            string result = received.substr(0, pos);
            received.erase(0, pos);
            return result;
        }

        int receivedbytes = read(fd, buffer, BUFFER_SIZE - 1);

        if (receivedbytes < 0)
            abort();  // error... you might want to handle it more cleanly, though

        if (receivedbytes == 0)
            return string();  // nothing to see yet

        // Add received data to buffer and loop to see if we have a newline yet.           
        buffer[receivedbytes] = 0;
        received += string( buffer );
    }
}