I have a compass that connects to my PC through RS232 @ 38400 BAUD. the compass outputs different "sentences" to send different kinds of data. Each sentence is a string that begins with a special header and ends with <CR><LF>. The string that I'm looking for is
"$IIHDG,A,B,C,D,E*hh<CR><LF>

where A, B, C, D, and E are numbers that change i.e. heading.

I have a basic serial class that reads a specified amount from the comm port and stores it in buffer. My code works most of the time but occasionally it errors out. To get around that I added an if statement that lets it return 400 if it errors out (bad I know but it's just a temporary workaround). Does anyone have any ideas how I can speed this up?

char buffer[5000];
	char* ptr;
	char* subStr = "$IIHDG,";

	do	{
		port.read(buffer, 5000, true);
		ptr = strstr(buffer, subStr);
	} while(ptr == NULL);

	ptr = strtok(ptr,",");
	ptr = strtok(NULL,",");
	if (ptr != NULL)
		return strtod(ptr,NULL);
	else
		return 400;

What exactly does port.read() do?

Things I can think of include
- it doesn't append a \0, so all your string functions are working on a prayer.

- it doesn't do "framing", so if you happen to lose sync with the transmitter, you end up with messages like this

> "$IIHD
> G,A,B,C,D
> ,E*hh<CR><LF>
None of which match what you're looking for.

It's only when you happen to "luck" onto the start of the message and manage to receive most of it in a single call that you finally "succeed".

Try

int readCompass ( ) {
  int   result = 400;
  char  buffer[5000];
  int   nBuff = 0;

  do {
    char  c;
    // If this can timeout, or return error, deal with it
    port.read(&c, 1, true);
    buffer[nBuff++] = c;
    if ( c == '\n' ) break; // That's a frame!
  } while ( nBuff < sizeof(buffer)-1 );
  buffer[nBuff] = '\0';

  if ( strstr( buffer, "$IIHDG," ) ) {
    char *ptr = strtok( buffer, "," );
    if ( ptr != NULL ) {
      ptr = strtok( ptr, NULL );
      if ( ptr != NULL ) {
        result = strtod(ptr,NULL);
      }
    }
  }
  return result;
}

Sorry, I should have given the function declaration for port.read(). Here it is:

int Serial::read(char *buffer, int buffLen, bool nullTerminate);

I tried your code but it only returns 400. I've got that function on a timer.

System::Void tmrGet_Tick(System::Object^  sender, System::EventArgs^  e) {
			heading = compass->getHeading();
			textBox1->Text = heading.ToString();
		 }
}

I put your code inside the getHeading() function. I've also tried this:

do	{
		for (int i = 0; c[0] != static_cast<char>(10); i++)	{
			port.read(c, 1, false);
			buffer[i] = c[0];
		}
		c[0] = NULL;
		ptr = strstr(buffer, subStr);
	} while (ptr == NULL);

	ptr = strtok(ptr,",");
	ptr = strtok(NULL,",");
	num = strtod(ptr,NULL);
	return num;

C / C++ / M$-mung-ware.

Pick a language and stick to it.

> I tried your code but it only returns 400. I've got that function on a timer.
Well yeah, you would if you tried to \0 terminate a single character.
Instant buffer overrun.

Again, you're posting code without explaining how some of your variables are declared.
What is c[0] all about?

This is all written in c++ so I'm not sure what you mean by M$-mung-ware. I'm was using c[0] just like buffer[] before that. I thought that was obvious. In that last segment, I was trying to simulate the Readline() function from some other languages. So I would take in one character at a time into c[0] and then if it isn't the line feed character (ASCII 0x0A) to store it to buffer and get the next character until it is the Line Feed. Then check to see if what is in buffer is the correct sentence that I need.

Well yeah, you would if you tried to \0 terminate a single character.
Instant buffer overrun.

port.read(c, 1, false);

I'm not null terminating a single character. That is what the third parameter in the function means. False means no null terminate. The code from my last post actually works every time but it lags way too much. Example, if I move the compass slowly, there is no noticeable problem, but if I move the compass very quickly it takes about 5 seconds for the program to catch up. It displays every reading that the compass passed through when it should be able to display exactly what the compass reads at that any given instant. I'm displaying the readings in a text box on a Visual C++ form.

Again, I'm sorry if this is too vague. I'm just not sure how to explain the problem very well.

> System::Void tmrGet_Tick(System::Object^ sender, System::EventArgs^ e)
Those ^ are some M$ extension to the C++ language.

Example, if I move the compass slowly, there is no noticeable problem, but if I move the compass very quickly it takes about 5 seconds for the program to catch up. It displays every reading that the compass passed through when it should be able to display exactly what the compass reads at that any given instant.

OIC, this is an entirely different problem to the one you presented.

The problem would seem to be that your text rendering is taking a lot longer than it takes to send a message.
Or rather, your timeout is a lot longer than the maximum message rate.

Does your serial::read function block if there is no data? It would be a big help if you could arrange for it to be non-blocking.

> tmrGet_Tick
How often is this triggered?
If it's anything quicker than say 250mS (4Hz), then you're going to be hard-pushed to read the text string being redrawn with rapidly changing values. All you're really going to be interested in is where it stops at.

To that end, I would suggest that your getHeading() function should attempt to read ALL the values present on the serial line (until read() indicates there is no more data), and then return the last conversion value.

What you will need to do is detect when you have a partial message when read() does timeout, and store that message fragment for when you next call getHeading()

I don't think it blocks it if there is no data. The timer is set to 100ms but I've tried slowing it down and it still has the same problem. I tried flushing the com port but I haven't had any luck with that either. I can't have the one function call read all the data because it is unlimited. As long as the compass is on it constantly sends new data.

Have you tried splitting the problem in two?

Does just reading the serial port exhibit any lag effects?

What about reading it, triggered by the timer (again, without displaying it or anything).

Does the GUI have any lag effects if your readCompass() just returns immediately with a number (say return 42; or return rand()%360; )

Have you tried splitting the problem in two?

Does just reading the serial port exhibit any lag effects?

What about reading it, triggered by the timer (again, without displaying it or anything).

Does the GUI have any lag effects if your readCompass() just returns immediately with a number (say return 42; or return rand()%360; )

Yeah. I tried that. The program seems to run fine, but it's like it builds up a bunch of the numbers then tries to display them after it's a few seconds behind.

I tried splitting the getHeading() and display on two different timers, but that didn't help.d I put getHeading() on a timer set for 100 ms and the display on a timer for 500 ms. It still lagged.

I made a video of all my code and how it runs. I also included a hyper terminal view of the data coming in from the compass. Hope this helps.

http://www.youtube.com/watch?v=W5PXKHD9kB8

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.