AVR serielle Kommunikation

[Da H. (darko91)]

Liebe Leute,
Ich hoffe, jemand von Euch kann mir bei meinem Problem helfen
Ich programmiere das ArduinoUno Board (ATmega328p-µC) in Atmel. Bei der 
seriellen Kommunikation hänge ich jedoch.

Code:

#include <avr/io.h>
#include <avr/interrupt.h>

#define cbi(sfr, bit) (_SFR_BYTE(sfr) &= ~_BV(bit))
#define sbi(sfr, bit) (_SFR_BYTE(sfr) |= _BV(bit))

#define F_CPU  16000000
#define BAUD  9600
#define BRC    ((F_CPU/16/BAUD) - 1)


#define RX_BUFFER_SIZE 128

char rxBuffer[RX_BUFFER_SIZE];
uint8_t rxReadPos = 0;
uint8_t rxWritePos = 0;

char getChar(void);
char peekChar(void);

int main( void )
{
  UBRR0H = (BRC >> 8);
  UBRR0L = BRC;
  
  UCSR0B = (1 << RXEN0) | (1 << RXCIE0);
  UCSR0C = (1 << UCSZ01) | (1 << UCSZ00);
  
  DDRB = (1 << PORTB3);
  
  sei();
  
  
  while(1)
  {
    char c = getChar();
    
    if(c == 'a')
    {
      sbi(PORTB, PORTB3);
    }
    else if(c == 'b')
    {
      cbi(PORTB, PORTB3);
    }
  }
}  
char peekChar(void)
{
  char ret = '\0';
  
  if(rxReadPos != rxWritePos)
  {
    ret = rxBuffer[rxReadPos];
  }
  return ret;
}
char getChar(void)
{
  char ret = '\0';
  
  if(rxReadPos != rxWritePos)
  {
    ret = rxBuffer[rxReadPos];
    
    rxReadPos++;
    
    if(rxReadPos >= RX_BUFFER_SIZE)
    {
      rxReadPos = 0;
    }
  }
  return ret;
}
ISR(USART_RX_vect)
{
  rxBuffer[rxWritePos] = UDR0;
  
  rxWritePos++;
  
  if(rxWritePos >= RX_BUFFER_SIZE)
  {
    rxWritePos = 0;
  }
}

Nach dem Builden und danach Draufspielen aufs Arduino öffne ich das Tera 
Term, welches mir ein Signal ans Board schicken soll.
Die RX-LED am Board blinkt kurz, was bedeutet, dass das Signal 
angekommen ist. Die an PORTB3 angeschlossene LED leuchtet aber nicht.
Woran kann das Problem liegen? Tipps?

MfG

[Peter II (Gast)]

rxReadPos und rxWritePos  müssen volatile sein.

[Uwe (de0508)]

Hallo,

bei deinen globalen Variable, mit ISR Zugriff, muss noch das Wort 
"volatile" ergänzt werden.

[Da H. (darko91)]

Peter II schrieb:
> rxReadPos und rxWritePos  müssen volatile sein.

Stimmt, danke!

volatile uint8_t rxReadPos = 0;
volatile uint8_t rxWritePos = 0;

funktioniert leider trotzdem nicht =(

[Da H. (darko91)]

Uwe S. schrieb:
> Hallo,
>
> bei deinen globalen Variable, mit ISR Zugriff, muss noch das Wort
> "volatile" ergänzt werden.

habs ergänzt, funktioniert leider noch immer nicht :(

volatile uint8_t rxReadPos = 0;
volatile uint8_t rxWritePos = 0;

[Karl H. (kbuchegg)]

Darko Jen schrieb:
> Uwe S. schrieb:
>> Hallo,
>>
>> bei deinen globalen Variable, mit ISR Zugriff, muss noch das Wort
>> "volatile" ergänzt werden.
>
> habs ergänzt, funktioniert leider noch immer nicht :(


Eine recht gute Strategie ist es, zuerst mal die Kommunikation vom Board 
zum Terminal in Betrieb zu nehmen, damit du dein Programm in die Lage 
versetzt dir bei der Fehleranalyse zu helfen.

Also vergiss erst mal die Übertragung vom PC zum AVR, und kümmere dich 
um die umgekehrte Richtung.

Du willst, dass ein

....
int main( void )
{
  UBRR0H = (BRC >> 8);
  UBRR0L = BRC;
  
  UCSR0B = (1 << TXEN0);
  UCSR0C = (1 << UCSZ01) | (1 << UCSZ00);
  
  while(1)
  {
    putChar( 'x' );
  }
}

dir lauter x aufs Terminal zaubert.
Die nächste Funktion, die du haben willst, ist die putString.

void putString( const char* s )
{
  while( *s )
    putChar( *s++ );
}

denn damit kannst du dann

int main()
{
   ...
  while( 1 )
  {
    putString( "Hallo\n\r" );
  }
}

schon Texte aufs Terminal ausgeben.

Und mit diesen beiden Hilfsmitteln bist du dann schon in der Lage, 
deinen Empfangscode so zu pimpen, dass er dir mitteilt, was über die 
UART reinkommt

ISR(USART_RX_vect)
{
  rxBuffer[rxWritePos] = UDR0;
  putChar( rxBuffer[rxWritePos] );
  
  rxWritePos++;

und du kannst zum ersten Mal deinem Programm am Terminal dabei zusehen, 
welche Zeichen es empfängt.

Wenn die stimmen, dann kann man natürlich auch in der main mal eine 
Testausgabe machen lassen.

int main()
{
   ...

  while(1)
  {
    char c = getChar();

    putString( "Empfangen: " );
    putChar( c );
    
    if(c == 'a')
....

Also: Prioritäten ändern!
Zuerst musst du dein Programm in die Lage versetzen sich bemerkbar zu 
machen. Und dann überträgst du vom PC zum AVR. Denn in dieser 
Reihenfolge kann dir der AVR helfen. Anders rum eher nicht, weil du 
keine Möglichkeit hast, eindeutig festzustellen, was aus der UART 
rauskommt. Wenn überhaupt.

[holger (Gast)]

>Die an PORTB3 angeschlossene LED leuchtet aber nicht.
>Woran kann das Problem liegen? Tipps?

Falsche Baudrate.

F_CPU im Code zu definieren heisst noch lange nicht
das die CPU auch mit diesem Takt läuft.
Falsch gesetze Fuses machen dir schnell einen
Strich durch die Rechnung.

[Leo C. (rapid)]

Karl Heinz schrieb:
> Also vergiss erst mal die Übertragung vom PC zum AVR, und kümmere dich
> um die umgekehrte Richtung.

Und wenn das Senden vom AVR funktioniert, kannst Du Dich wieder ums 
Empfangen kümmern.

Darko Jen schrieb:
> uint8_t rxReadPos = 0;
> uint8_t rxWritePos = 0;

volatile wurde schon erwähnt. Außerderm kannst Du Dir die 
Initialisierung mit 0 sparen, da sie sowieso Default ist.

> char peekChar(void)
> {
>   char ret = '\0';
>
>   if(rxReadPos != rxWritePos)
>   {
>     ret = rxBuffer[rxReadPos];
>   }
>   return ret;
> }

Die Funktion sollte so funktionieren. Da rxReadPos volatile ist, muß der 
Compiler die Variable 2 mal aus dem RAM laden. Mit einer Hilfsvariablen 
kann ein RAM-Zugriff wegoptimiert werden. Bei dieser einfachen Funktion 
bringt das wenig, aber das Prinzip ist nützlich für komplexere 
Funktionen.
[/c]
char peekChar(void)
{
  char ret = '\0';
  uint8_t ri = rxReadPos;

  if(ri != rxWritePos)
  {
    ret = rxBuffer[ri];
  }
  return ret;
}
[/c]

> char getChar(void)
> {
>   char ret = '\0';
>
>   if(rxReadPos != rxWritePos)
>   {
>     ret = rxBuffer[rxReadPos];
>     rxReadPos++;
>     if(rxReadPos >= RX_BUFFER_SIZE)
>     {
>       rxReadPos = 0;
>     }
>   }
>   return ret;
> }

Hier bringt die Optimierung schon mehr, da rxReadPos sonst 4 mal gelesen 
und 2 mal geschrieben wird.

char getChar(void)
{
  char ret = '\0';
  uint8_t ri = rxReadPos;

  if(ri != rxWritePos)
  {
    ret = rxBuffer[ri];
    ri++;
    if(ri >= RX_BUFFER_SIZE)
    {
      ri = 0;
    }
    rxReadPos = ri;
  }
  return ret;
}

> ISR(USART_RX_vect)
> {
>   rxBuffer[rxWritePos] = UDR0;
>   rxWritePos++;
>   if(rxWritePos >= RX_BUFFER_SIZE)
>   {
>     rxWritePos = 0;
>   }
> }

Hier fehlt die Überprüfung ob der Puffer voll ist.
Da Du '\0' als "Puffer leer" wertest, würde ich dieses Zeichen auch 
nicht in den Puffer schreiben.

ISR(USART_RX_vect)
{
  uint8_t wi;
  char c = UDR0;

  if (c != '\0')
  {
    wi = rxWritePos;
    /* Eine Position im Puffer ist garantiert immer frei. */
    rxBuffer[wi] = c;
    wi++;
    if(wi >= RX_BUFFER_SIZE)
    {
      wi = 0;
    }
    /* Der Write-Index darf den Read-Index nicht überholen */
    /* Deshalb WritePos nur speichern, wenn ReadPos noch nicht eingeholt wurde */
    if (wi != rxReadPos)
    {
      rxWritePos = wi;
    }
  }
}

[Da H. (darko91)]

...
...

> Hier fehlt die Überprüfung ob der Puffer voll ist.
> Da Du '\0' als "Puffer leer" wertest, würde ich dieses Zeichen auch
> nicht in den Puffer schreiben.
>

> ISR(USART_RX_vect)
> {
>   uint8_t wi;
>   char c = UDR0;
> 
>   if (c != '\0')
>   {
>     wi = rxWritePos;
>     /* Eine Position im Puffer ist garantiert immer frei. */
>     rxBuffer[wi] = c;
>     wi++;
>     if(wi >= RX_BUFFER_SIZE)
>     {
>       wi = 0;
>     }
>     /* Der Write-Index darf den Read-Index nicht überholen */
>     /* Deshalb WritePos nur speichern, wenn ReadPos noch nicht eingeholt 
> wurde */
>     if (wi != rxReadPos)
>     {
>       rxWritePos = wi;
>     }
>   }
> }
>

Danke Leo für deine Optimierung vom Code.
Leider ändert das nichts an der Funktion. Die LED leuchtet nicht. 
Lediglich die RX-LED am Board selber leuchtet jedes Mal beim Tippen 
eines Characters, sonst nichts. Hast noch einen Vorschlag?

[Walter S. (avatar)]

Darko Jen schrieb:
> Hast noch einen Vorschlag?

die anderen Tipps beherzigen, z.B. von Karlheinz

[Da H. (darko91)]

Karl Heinz schrieb:
> Darko Jen schrieb:
>> Uwe S. schrieb:

> ....
> int main( void )
> {
>   UBRR0H = (BRC >> 8);
>   UBRR0L = BRC;
>
>   UCSR0B = (1 << TXEN0);
>   UCSR0C = (1 << UCSZ01) | (1 << UCSZ00);
>
>   while(1)
>   {
>     putChar( 'x' );
>   }
> }
> [/c]
>
> dir lauter x aufs Terminal zaubert.
> Die nächste Funktion, die du haben willst, ist die putString.
>
>

> void putString( const char* s )
> {
>   while( *s )
>     putChar( *s++ );
> }
>

> denn damit kannst du dann
>

> int main()
> {
>    ...
>   while( 1 )
>   {
>     putString( "Hallo\n\r" );
>   }
> }
>

> schon Texte aufs Terminal ausgeben.
>
> Und mit diesen beiden Hilfsmitteln bist du dann schon in der Lage,
> deinen Empfangscode so zu pimpen, dass er dir mitteilt, was über die
> UART reinkommt
>

> ISR(USART_RX_vect)
> {
>   rxBuffer[rxWritePos] = UDR0;
>   putChar( rxBuffer[rxWritePos] );
> 
>   rxWritePos++;
>

>
> und du kannst zum ersten Mal deinem Programm am Terminal dabei zusehen,
> welche Zeichen es empfängt.
>
> Wenn die stimmen, dann kann man natürlich auch in der main mal eine
> Testausgabe machen lassen.
>
>

> int main()
> {
>    ...
> 
>   while(1)
>   {
>     char c = getChar();
> 
>     putString( "Empfangen: " );
>     putChar( c );
> 
>     if(c == 'a')
> ....
>

>
> Also: Prioritäten ändern!
> Zuerst musst du dein Programm in die Lage versetzen sich bemerkbar zu
> machen. Und dann überträgst du vom PC zum AVR. Denn in dieser
> Reihenfolge kann dir der AVR helfen. Anders rum eher nicht, weil du
> keine Möglichkeit hast, eindeutig festzustellen, was aus der UART
> rauskommt. Wenn überhaupt.

Danke Karl Heinz für deine Hilfe,
den 1. Schritt (Character-Ausgabe) habe ich somit geschafft.
Bei der Stringausgabe hänge ich leider - anstatt im Sekundentakt "Hallo" 
auszugeben, werden jeweils 2xDs ausgegeben ("DD" "DD"...)
Wie kommt es dazu?
LG

char putChar(char c)
{
  UDR0 = 'D';
  return UDR0;
}
void putString( const char* s )
{
  while( *s )
    putChar( *s++ );
}
int main( void )
{
  UBRR0H = (BRC >> 8);
  UBRR0L = BRC;
  
  UCSR0B = (1 << TXEN0);
  UCSR0A = (1 << UDRE0);
  UCSR0C = (1 << UCSZ01) | (1 << UCSZ00);
  
  while( 1 )
  {
    putString( "Hallo\n\r" );
    _delay_ms(1000);
  }
}

[Bernhard S. (dl9rdw)]

Na dann schau mal was Deine putchar Funktion macht, dann weißt auch 
warum das so ist.

Grüße Bernhard...

[Leo C. (rapid)]

> Bei der Stringausgabe hänge ich leider - anstatt im Sekundentakt "Hallo"
> auszugeben, werden jeweils 2xDs ausgegeben ("DD" "DD"...)
> Wie kommt es dazu?

Die Frage müßte lauten: "Wieso werden nur jeweils zwei 'D' 
ausgegeben?"
"Hallo\n\r" hat schließlich 7 Zeichen.

Hint: UDRE0

[Da H. (darko91)]

Bernhard Schröcker schrieb:
> Na dann schau mal was Deine putchar Funktion macht, dann weißt auch
> warum das so ist.
>
> Grüße Bernhard...

Ich weiß schon, was die putchar Funktion macht, leider weiß ich aber 
nicht, wie ich das Problem behebe. Daher bitte ich auch um Hilfe, 
wodurch ich dann einiges dazulernen würde

[Da H. (darko91)]

Leo C. schrieb:
>> Bei der Stringausgabe hänge ich leider - anstatt im Sekundentakt "Hallo"
>> auszugeben, werden jeweils 2xDs ausgegeben ("DD" "DD"...)
>> Wie kommt es dazu?
>
> Die Frage müßte lauten: "Wieso werden nur jeweils zwei 'D'
> ausgegeben?"
> "Hallo\n\r" hat schließlich 7 Zeichen.
>
> Hint: UDRE0

Richtig übersetzt
Ins UDR0 wird 'D' reingeschrieben. Durch die putString Funktion (s++) 
wird dann 2x 'D' ausgegeben.
> "Hallo\n\r" hat schließlich 7 Zeichen.
schon klar, wie soll ich die nun ausgeben?

[Leo C. (rapid)]

1. Schau mal nach, wie putString() funktioniert. Welche Funktion ruft 
sie auf?

> Ins UDR0 wird 'D' reingeschrieben. Durch die putString Funktion (s++)
> wird dann 2x 'D' ausgegeben.

Nein, nicht 2x, es wird 7x 'D' ins UDR0 geschrieben. Daß Du nur 2 davon 
zu sehen bekommst, liegt daran, daß Du die Zeichen schneller in UDR0 
schreibst, als sie gesendet werden können.

2. UDRE0 != UDR0

[Da H. (darko91)]

Liebe Leute,
Ich bedanke mich für eure Hilfe. Der Code funktioniert nun
Was ich eigenartig finde ist, dass die LED nur dann leuchtet, wenn die 
UART Initialisierung in einer eigenen Funktion steht.

Nun die Frage: Was muss hier am Code noch genau verändert werden, um 
eine Bluetooth Kommunikation bereitzustellen statt dem Terminal?
LED soll quasi per Smartphone ein und ausgeschaltet werden

http://www.seeedstudio.com/wiki/index.php?title=Bluetooth_Shield
Verwendeter Bluetooth-Chip: BC417
TX des Moduls wird mit PORTD7 des Boards verbunden
RX mit PORTD6
wie geht es nun weiter? Konfigurationen?
Jeder Vorschlag ist willkommen!!

Code:

#include <avr/io.h>
#include <avr/interrupt.h>
#include <stdint.h>
#define F_CPU  16000000UL
#include <util/delay.h>

#define cbi(sfr, bit) (_SFR_BYTE(sfr) &= ~_BV(bit))    // clear bit
#define sbi(sfr, bit) (_SFR_BYTE(sfr) |= _BV(bit))    // set bit

#define FOSC  16000000    // 16MHz
#define BAUD  9600      // Baudrate 9600
#define BRC    FOSC/16/BAUD-1  // Prescaler

#define RX_BUFFER_SIZE 128      // buffer 128 bits
char rxBuffer[RX_BUFFER_SIZE];

volatile uint8_t rxReadPos;
volatile uint8_t rxWritePos;

char getChar(void);

void USART_Init(unsigned int ubrr)    // wenn direkt in Main, funktioniert nicht
{
  //Set baud rate
  UBRR0H = (ubrr>>8);
  UBRR0L = ubrr;
  
  UCSR0B = (1 << RXEN0) | (1 << TXEN0) | (1<<RXCIE0);      // Enable receiver and transmitter
  UCSR0A = (1 << UDRE0);  //richtiger Empfang ansonsten nur mit BAUD 19200 bei Terminal möglich
  UCSR0C = (1 << UCSZ01) | (1 << UCSZ00);    // Set frame: 8 bit data
}

int main( void )
{
  USART_Init(BRC);
  
  DDRB = (1 << PORTB0);  // PORTB3 is OUTPUT
    
  sei();  // set interrupt
  
  while(1)  
  {
    char c = getChar();
    
    if(c == '0')          // if sent character = "0"..
    {
      cbi(PORTB, PORTB0);      // clear bit - set LED to 0
      //PORTB &= ~(1<<PORTB0);
    }
    else if(c == '1')        // if sent character = "1"..
    {
      sbi(PORTB, PORTB0);      // set bit - set LED to 1
      //PORTB |= (1<<PORTB0);
    }
  }
  
  /*while(1)  // DDDD... übertragen
  {
    UDR0 = 'D';
    _delay_ms(500);
  }*/  

}
char getChar(void)
{
  char ret = '\0';
  
  if(rxReadPos != rxWritePos)
  {
    ret = rxBuffer[rxReadPos];
    
    rxReadPos++;
    
    if(rxReadPos >= RX_BUFFER_SIZE)
    {
      rxReadPos = 0;
    }
  }
  return ret;
}
ISR(USART_RX_vect)
{
  rxBuffer[rxWritePos] = UDR0;
  
  rxWritePos++;
  
  if(rxWritePos >= RX_BUFFER_SIZE)
  {
    rxWritePos = 0;
  }
}

[Leo C. (rapid)]

Warum die Init in main() nicht gehen sollte, ist mir ein Rätsel.
Ansonsten:

>   //Set baud rate
>   UBRR0H = (ubrr>>8);
>   UBRR0L = ubrr;

Der Compiler kennt auch UBRR0 als 16 bit Register.

   //Set baud rate
   UBRR0 = ubrr;

Der generierte Code ist der Gleiche. Aber: weniger Sourcecode == weniger 
(Tipp-)Fehler-(möglichkeiten).

>   UCSR0B = (1 << RXEN0) | (1 << TXEN0) | (1<<RXCIE0);      // Enable

Besser den USART erst enablen, wenn er komplett initialisiert ist. Diese 
Zeile also ans Ende setzen.

>   UCSR0A = (1 << UDRE0);  //richtiger Empfang ansonsten nur mit BAUD

Was soll das?


>     UDR0 = 'D';
>     _delay_ms(500);

Also, das willt Du wirklich nicht!
sondern:

     while ((UCSR0A & (1<<UDRE0)) == 0)
         ; /* warte bis TX Data Register empty */
     UDR0 = 'D';

Hätte nicht gedacht, daß man den Hinweis auf UDRE0 so mißverstehen kann.

[Da H. (darko91)]

Leo C. schrieb:
> Was soll das?
>
>     UDR0 = 'D';
>     _delay_ms(500);
>
> Also, das willt Du wirklich nicht!
> sondern:
>

>      while ((UCSR0A & (1<<UDRE0)) == 0)
>          ; /* warte bis TX Data Register empty */
>      UDR0 = 'D';
>

>
> Hätte nicht gedacht, daß man den Hinweis auf UDRE0 so mißverstehen kann.

der Abschnitt des Codes wurde Anfangs verwendet, um ein 'D' ans Terminal 
zu schicken. Jetzt wurde er nicht rausgelöscht, sondern auskommentiert. 
Danke trotzdem

[Leo C. (rapid)]

Meine Frage "Was soll das?" bezog sich auf diese Zeile:

>   UCSR0A = (1 << UDRE0);  //richtiger Empfang ansonsten nur mit BAUD