Reading from UART

Using Arduino Programming Questions
1

Hi, I try to read data from the UART.
When I hook up my logic analyzer I get the expected Responses, but when I read this Response with my arduino, I only get the first, second and last Byte of the ~ 20 Byte Long Response.
Is this due to the fact I dont read fast enought from the UART?

int MDBSerial::GetResponse()
{
	while (available())
	{
		read();

	}
        return 1;
}

int MDBSerial::read()
{
	unsigned char resh, resl;
	int count = 0;
	

	resh = UCSR1B;
	resl = UDR1;
	
	return 1;
}

bool MDBSerial::available()
{
	return (UCSR1A & (1 << RXC));
}

Tarcontar

2 
int MDBSerial::read()
{
	unsigned char resh, resl;
	int count = 0;
	

	resh = UCSR1B;
	resl = UDR1;
	
	return 1;
}

This can't be right. Read the data into two local variables, then return 1 no matter what was read and the two variables get destroyed when they go out of scope at the end. So the data that you read will always be lost. All you get from this function is a 1 letting you know that it did in fact read something.

I think that if you want help with this you're going to have to post a lot more of the code. Like all of it. Or at least a complete example that compiles and demonstrates the issue.

Same with this function:

int MDBSerial::GetResponse()
{
	while (available())
	{
		read();

	}
        return 1;
}

You read the data and throw it away and then return a 1 no matter what you read. How is that supposed to get the response? Perhaps a better name for this function would have been, throwResponseAway().

3

Sorry I just wanted to Keep it simple and not post too much Code.

#include "MDBSerial.h"
#include <Arduino.h>

MDBSerial::MDBSerial()
{
	m_TXn = 1;
	m_UDRn = &(UDR0);
	m_RXENn = RXEN0;
	m_TXENn = TXEN0;
	m_UBRRnH = &(UBRR0H);
	m_UBRRnL = &(UBRR0L);
	m_UCSRnA = &(UCSR0A);
	m_UCSRnB = &(UCSR0B);
	m_UCSRnC = &(UCSR0C);
	m_UCSZn0 = UCSZ00;
	m_UCSZn1 = UCSZ01;
	m_UCSZn2 = UCSZ02;

	init();
}

void MDBSerial::SetSerial(HardwareSerial &print)
{
	serial = &print;
	serial->begin(9600);
}

void MDBSerial::hardReset()
{
	pinMode(m_TXn, OUTPUT);
	digitalWrite(m_TXn, HIGH);
	delay(TIME_SETUP);
	digitalWrite(m_TXn, LOW);
	delay(TIME_BREAK);
	digitalWrite(m_TXn, HIGH);
}

void MDBSerial::Ack()
{
	write(ACK, DATA);
}

void MDBSerial::Nak()
{
	write(NAK, DATA);
}

void MDBSerial::Ret()
{
	write(RET, DATA);
}

void MDBSerial::SendCommand(int address, int cmd, int *data, int dataCount)
{
	unsigned char sum = 0;
	write(address | cmd, ADDRESS);
	sum += address | cmd;

	for (unsigned int i = 0; i < dataCount; i++)
	{
		write(data[i], DATA);
		sum += data[i];
	}

	//send the checksum
	write(sum, DATA);
}

int MDBSerial::GetResponse(int *count, int **data)
{
	int mode = 0;
	int sum = 0;
	*count = 0;

	while (available() /*&& !mode /*&& *count < DATA_MAX*/)
	{
		read(&input_buffer[*count], &mode);
		(*count)++;
	}

	//TODO: check if checksum is correct
	*data = &input_buffer[0];
	return ACK;
}

void MDBSerial::init()
{
	UCSR1B = (1 << RXEN1) | (1 << TXEN1) | (1 << RXCIE1);
	UBRR1H = 0;
	UBRR1L = 103; // 9600baud
	UCSR1A &= ~(1 << U2X1); //disable rate doubler //should be U2Xn ??
	UCSR1C |= (1 << UCSZ11) | (1 << UCSZ10); //9bit mode
	UCSR1B |= (1 << UCSZ12); //also for 9 bit mode
	UCSR1C = UCSR1C | 0b00000100; //one stop bit
	UCSR1C = UCSR1C | 0b00000000; //no parity bit
}

void MDBSerial::write(int cmd, int mode)
{
	while (!(UCSR1A & (1 << UDRE)));
	if (mode)
		UCSR1B |= (1 << TXB8);
	else
		UCSR1B &= ~(1 << TXB8);
	UDR1 = cmd;
	m_commandSentTime = millis();
}

int MDBSerial::read(int *data, int *mode)
{
	unsigned char status, resh, resl;
	int count = 0;

	status = *m_UCSRnA;

	if (status & (1 << FE))
	{
		serial->println("NO STOP BIT");
		return 0;
	}
	else if (status & (1 << DOR))
	{
		serial->println("BUFFER FULL");
		return 0;
	}
	else if (status & (1 << UPE))
	{
		serial->println("INVALID PARITY");
		return 0;
	}

	resh = *m_UCSRnB;
	resl = *m_UDRn;

	serial->println(resl);

	*mode = (resh >> 1) & 0x01;
	*data = resl;
	
	return 1;
}

bool MDBSerial::available()
{
	return (UCSR1A & (1 << RXC));
}
4

And that is what you call a complete program that compiles and demonstrates the issue? I get undefined reference to setup and undefined reference to loop at the very least.

Complete code. Compiles. Demonstrates the problem. If you find yourself about to post anything less than that, just stop because you are wasting your time.

5

If you find yourself about to post anything less than that, just stop because you are wasting your time.

And ours. While I don't mind you wasting your time, I do mind you wasting mine.

6

Ok I see so here is my Code...

#include <CoinChanger.h>
#include <MDBSerial.h>

MDBSerial mdb(1);
CoinChanger changer(mdb);

void setup()
{
  mdb.SetSerial(Serial);
  changer.SetSerial(Serial);
  //changer.Reset();
  Serial.println("VMC###############");
}

void loop()
{
  changer.setup();
  //changer.Enable();
  delay(50);
}

Im not sure if a Loop etc is the correct way, since it seems way too slow.

CoinChanger.cpp (4.59 KB)

CoinChanger.h (616 Bytes)

MDBSerial.cpp (4.41 KB)

MDBSerial.h (1.4 KB)

7

I tried to understand your code, but failed. In loop(), you call CoinChanger::setup() which doesn't set anything up.

The setup() method passes an uninitilialized pointer to the MDBSerial::GetResponse() method, which doesn't use the argument at all.

Why does GetResponse() take an argument it does not use?

Why is input_buffer[] volatile?

Why doesn't GetResponse do what its name implies?

8

I only get the first, second and last Byte of the ~ 20 Byte Long Response.
Is this due to the fact I dont read fast enought from the UART?

Probably. The UART only contains two bytes of buffering. If you try to use the same "wait long enough for the whole command to be received, and then read it one byte at a time", it will fail with symptoms similar to what you describe.

Your loop in getResponse:

	while (available() /*&& !mode /*&& *count < DATA_MAX*/)
	{
		read(&input_buffer[*count], &mode);
		(*count)++;
	}

should never read more than two bytes, since your "available" function is only accessing the hardware status.

The Arduino code (which you can look at, don't forget) uses interrupts to fill a ~64byte software buffer, and available() returns the number of characters in the software buffer...

9

westfw:
The Arduino code (which you can look at, don't forget) uses interrupts to fill a ~64byte software buffer, and available() returns the number of characters in the software buffer...

Speaking of, why not use the already built in libraries for doing this?

10

Hi, thanks for your answers.
So what is the right way to test if there is data available on the UART? there must be a way to check that.

I dont use an existing solution, since they suck at my data Format, which is 1 start, 9 data and 1 stop bit.
Found nothing that would work for this specific Setup.

Sorry for the messy code, but this is a very early testing library for my project, that was also a reason why I kept the code in the start post as short as possible.

Tarcontar

11

If I recall correctly, this thread from the beginning of the year contains a solution for 9 databits communication.

12

So what is the right way to test if there is data available on the UART?

The WAY that you're testing is fine. But if you're receiving more than 2 bytes of response with no interrupts, you need to do it OFTEN.

Now that you've posted CoinChanger.cpp, and I've looked at it, I'm not sure how

I only get the first, second and last Byte of the ~ 20 Byte Long Response

can be an accurate description of the behavior.
As far as I can tell, you don't have any inappropriate code that would be preventing you from reading the UART fast enough. Instead, you have what I'll call "too fast" code; it looks like you're thinking that once a message starts to arrive, you can read the whole message with that "while(available())" loop, and wind up with the whole message in your buffer. But in fact the message is dribbling in slowly at about 1 byte per millisecond (at 9600bps), and the loop runs MUCH faster than that. So the code should be aborting out of GetResponse() after only the first character or so, rather than waiting for the entire message. (or is there echoing of the SendCommand() data involved?)
But that doesn't explain the symptom you describe...

I think you need "idle timeout" code in GetResponse(); it should continue to receive characters and not return until several byte-times have gone by without seeing any additional traffic. It'd be easier if there was a definite end-of-message character... I found some other MDB coin-changer arduino code that looks like:

// wait for the answer
        case 5:
            // Wait for 24 bytes to be received
            if(Serial1.available() < 24) {
                return;
        }

You can't delay() or wait for 24 bytes to be available (that requires the ISR-based Arduino code), but if there are always 24bytes in the response, you can certainly wait until 24 bytes have been received!
Or you can do the timeout, something like:

   starttime= millis();
   while (starttime  + 50 > millis()) {  // wait for a 50ms gap
     if (available()) {
        buffer[*count++] = read();
        starttime = millis();  // reset timeout
      }
    }

Or ... something else entirely is wrong.

13
14