A bug in ofSerial::readByte()

Hi,

I’m sorry if this is a duplicate report, but I want to report here just to make sure.

In ofSerial.cpp, ofSerial::readByte() (MaxOSX/Linux version) are implemented as follows:

  
  
int ofSerial::readByte(){  
    ...  
    unsigned char tmpByte[10];  
  
    int nRead = read(fd, tmpByte, 1);	  
    if(nRead == 0){  
        printf("ofSerial: trouble reading from port\n");  
        return -1;  
    }  
  
    return (int)(tmpByte[0]);  
}  
  

But as far as I tried, read() returns -1 when there are no bytes to read. So I believe that the line of ‘if’ statement should be something like this:

  
  
    if(nRead != 1){  
  

Best,
Shigeru

[edit: cut-n-paste fail.]

Actually, I think I have a fix for this.

The problem is related to the fact that (under linux/osx) the ports are being opened non-blocking. This is actually a good thing, as it keeps of from locking up and waiting on input or waiting to write.

The actual bug is caused by a failure to check errno(), if the read (or write) fails and errno == 11, then it’s actually not really an error.

Here’s how I patched it on my copy:

sliver-u> diff -w ofSerial.cpp-dist ofSerial.cpp
3a4,6

#if defined( TARGET_OSX ) || defined( TARGET_LINUX )
#include “errno.h”
#endif
164a168,169
// we don’t use this again, so close it JET
closedir(dir);
472a478,482
if (errno == 11) {
ofLog(OF_VERBOSE,“ofSerial: non-blocking write failed”);
return 0;
}
else {
473a484
perror(“writeBytes”);
475a487
}
509c521,529
< ofLog(OF_ERROR,“ofSerial: trouble reading from port”);


// we opened fd non-blocking, so if there’s no data, we get
// errno 11. which, in this case, isn’t really an error, just
// a statement that there was no data ready on a non-blocking
// fd
if (errno == 11) {
return OF_SERIAL_NO_DATA;
}
else {
ofLog(OF_ERROR,“ofSerial: trouble reading from port: fd %d, errno %d”, fd, errno);
511a532
}
545a567,571
if (errno == 11) {
ofLog(OF_VERBOSE,“ofSerial: non-blocking write failed”);
return 0;
}
else {
546a573
perror(“writeByte”);
548a576
}
587c615,623
< ofLog(OF_ERROR,“ofSerial: trouble reading from port”);


// we opened fd non-blocking, so if there’s no data, we get
// errno 11. which, in this case, isn’t really an error, just
// a statement that there was no data ready on a non-blocking
// fd
if (errno == 11) {
return OF_SERIAL_NO_DATA;
}
else {
ofLog(OF_ERROR,“ofSerial: trouble reading from port: fd %d, errno %d”, fd, errno);
589a626
}

at the very least, I think this should be returned as a LOG_WARNING v. a LOG_ERROR, as jet has said its not really an error…for example, if you mod the ofFirmata app and take out everything but a mouseClick LED reaction, console will still output a steady stream of serial errors to you.

HI

I am having problems with the ofArduino library too.

this is what is printed out:
OF_ERROR: ofSerial: trouble reading from port

the program does not really do anything other than trying to start communications with the arduino (duemilanove).

i am using the the StandardFirmata that came with arduino 016.
i tried both baurates 115200 and 57600. i am running of006.

what can it be.

thanks,
stephan.

testApp.h

  
#ifndef _TEST_APP  
#define _TEST_APP  
  
  
#include "ofMain.h"  
#include "ofAddons.h"  
  
  
class testApp : public ofSimpleApp{  
	  
public:  
	  
	void setup();  
	void update();  
	void draw();  
	  
	void keyPressed(int key);  
	void keyReleased(int key);  
	  
	void mouseMoved(int x, int y );  
	void mouseDragged(int x, int y, int button);  
	void mousePressed(int x, int y, int button);  
	void mouseReleased();  
	  
	void setupArduino();  
	void updateArduino();  
	  
	ofArduino	ard;  
	  
	bool		bSetupArduino;			// flag variable for setting up arduino once  
	  
	int data;  
	long timer;  
  
};  
  
#endif	  
  
  

testApp.cpp

  
#include "testApp.h"  
#include "ofMain.h"  
  
  
//--------------------------------------------------------------  
void testApp::setup(){	   
	  
	ofSetVerticalSync(true);  
	ofBackground(255,0,130);  
	  
	ard.connect("/dev/cu.usbserial-A9005ddA", 115200); //57600);	  
	  
	bSetupArduino	= false;							// flag so we setup arduino when its ready, you don't need to touch this :)  
}  
  
//--------------------------------------------------------------  
void testApp::update(){  
	  
	  
	if ( ard.isArduinoReady()){  
		  
		// 1st: setup the arduino if haven't already:  
		if (bSetupArduino == false){  
			setupArduino();  
			bSetupArduino = true;	// only do this once  
		}		  
		  
		// 2nd do the update of the arduino  
		updateArduino();  
	}  
  
}  
  
//--------------------------------------------------------------  
void testApp::setupArduino(){  
	  
	// this is where you setup all the pins and pin modes, etc  
	// it's a good idea to set all pins to output, to avoid noise  
	for (int i = 0; i < 13; i++){  
		ard.sendDigitalPinMode(i, ARD_OUTPUT);  
	}  
  
}  
  
//--------------------------------------------------------------  
void testApp::updateArduino(){  
	  
	// update the arduino, get any data or messages:   
	ard.update();  
  
	  
}  
  
  
//--------------------------------------------------------------  
void testApp::draw(){  
	  
	  
	  
	  
}  
  
  

the error seems to come from here int the ofSerial.cpp file:

  
int ofSerial::readByte(){  
  
	if (!bInited){  
		ofLog(OF_LOG_ERROR,"ofSerial: serial not inited");  
		return OF_SERIAL_ERROR;  
	}  
  
	unsigned char tmpByte[1];  
	memset(tmpByte, 0, 1);  
  
	//---------------------------------------------  
	#if defined( TARGET_OSX ) || defined( TARGET_LINUX )  
		int nRead = read(fd, tmpByte, 1);  
		if(nRead < 0){  
			ofLog(OF_LOG_ERROR,"ofSerial: trouble reading from port");  
            return OF_SERIAL_ERROR;  
		}  
		if(nRead == 0)  
			return OF_SERIAL_NO_DATA;  
    #endif  
    //---------------------------------------------  
  
    //---------------------------------------------  
	#ifdef TARGET_WIN32  
		DWORD nRead;  
		if (!ReadFile(hComm, tmpByte, 1, &nRead, 0)){  
			ofLog(OF_LOG_ERROR,"ofSerial: trouble reading from port");  
			return OF_SERIAL_ERROR;  
		}  
	#endif  
	//---------------------------------------------  
  
	return (int)(tmpByte[0]);  
}  
  

hi stephan

what platform are you using?

osx 10.5 usinf xcode 3.1.3

In my quest to get the arduino working for me, i noticed that of006 comes with is a firmata example.

it work. great.

the console is still printing an error though.

OF_ERROR: ofSerial: trouble reading from port

i wonder why?

stephan.

kotobuki, your solution brings only another error.

I use Ubuntu 9.10 Karmic and the Code::Blocks from the installation script.

I was wondering: does there need to be code on the arduino to support the Firmata library? In the Arduino-0017 software, there is under File -> Examples -> Firmata a few examples. I tried using the “StandardFirmata” on the arduino, but I still get the “ofSerial: trouble reading from port” on the “firmataExample” in OF.

So yeah, how do I get the example code running?

and jet, your diff file seems nice, can you just post your ofSerial.cpp file?

Thanks all

hey all,

after user Jet’s patch, i’ve fixed this, and it’s available on my github:
http://github.com/damiannz/openFrameworks/blob/2adecb73ca07f675599feeb39052aa85caa45082/libs/openFrameworks/communication/ofSerial.cpp

you need to check the return value of serial readBytes(): if it returns OF_SERIAL_NO_DATA then you should retry the call; moreover if you try to call readBytes() when available() is returning 0 you’ll get OF_SERIAL_ERROR.

same goes for writeBytes(): you need to check the return value to make sure what you expect is written. if you call writeBytes with 8 bytes of data and writeBytes() returns 5 then you still have to call writeBytes() again to write the remaining 3 bytes (and check the return value, and call again…)

the way i do it for reading is this:

  
  
// this is how many bytes we want to read - change this if you want something other  
// than 8 bytes  
int bytes_required = 8;  
unsigned char bytes[bytes_required];  
int bytes_remaining = bytes_required;  
while ( bytes_remaining > 0 )  
{  
  // check for data  
  if ( serial.available() > 0 )  
  {  
    // try to read - note offset into the bytes[] array, this is so that we   
    // don't overwrite the bytes we already have  
    int result = serial.readBytes( &bytes[bytes_required-bytes_remaining], bytes_remaining );  
    // check for error code  
    if ( result == OF_SERIAL_ERROR )  
    {  
      ofLog( OF_LOG_ERROR, "unrecoverable error reading serial" );  
      break;  
    }  
    else if ( result == OF_SERIAL_NO_DATA )  
    {  
      // nothing read  
    }  
    else  
    {  
      // we read some data!  
      // maybe not all..  
      bytes_remaining -= result;  
    }  
  }  
}  
// read data is now in bytes[]  
  

hope this helps someone,
d

Thanks Damin, Jet, and everyone working on fixing this. Had a quick moment of panic when I saw streams of OF_ERROR messages streaming down my console, despite the fact that as far as I could tell the arduino was working just fine with my app.

Arturo, somehow I can’t get your fix to work. I’ve updated the ofSerial.cpp file and it has stopped giving me “trouble reading port” errors, but as soon as I run the update() of the ofArduino class, my application crashes.

The weird thing is, the first time I start it up it works fine. But if I compile it after that, it crashes after a second or two. It looks like it crashes as soon as oF starts communicating with the Arduino.

The error I’m getting is the following:

  
Program received signal:  “EXC_BAD_ACCESS”.  
sharedlibrary apply-load-rules all  
warning: Could not find object file "/Users/theo/Documents/CODE/__OPENFRAMEWORKS/SANDBOX/COMPILE_LIBRARIES/buildGlutFramework/libForeground.a(macx_foreground.o)" - no debug information available for "/Users/mcast/Code/GLUT-ToPost/macx_foreground.m".  
  
warning: Could not find object file "/Developer/usr/lib/gcc/i686-apple-darwin9/4.0.1/libgcc.a(_eprintf.o)" - no debug information available for "/var/tmp/gcc/gcc-5493~1/src/gcc/libgcc2.c".  

Has someone experienced something similar? If it helps I can post the whole code.