zu Blöd zum Programmieren: Brauche Hilfe für UART Zeichenkette

[Christoph T. (tentone)]

Hallo liebe Forenteilnehmer,

dieses Thema wurde schon X-mal diskutiert und ich bin
a) zu blöd um deutsch zu verstehen
b) zu blöd zum programmieren

Ziel ist eine Motorsteuerung über den Computer zu machen (programmiert 
in C#, funktioniert).
Es wird über einen Drehknopf die Umlaufgeschwindigkeit übertragen als 
String.
Also z.B. 11, 12 oder 5.
Dieser String bedient im uC den delay_befehl.
Ich weis nicht schön und nicht professionell, dies wird sobald die 
Kommunikation funktioniert auf Interrupt umgebaut.
Und genau in diesem delay_ms liegt das Problem. Funktioniert nicht!

Im Prinzip funktioniert alles.

Aber ich kann aus der eingelesenen Zeichenkette kein strcpy auf 
Empfangen_Buffer setzen, warum auch immer.
Und ich kann, wenn ich das Control_Center (myAvr) nehme kein Stringende 
vom Terminal feststellen, es sei denn ich behelfe mir mit einem ";" am 
schluss.
Das nächste Rätsel ist warum kann ich den String nicht in eine Zahl 
umwandeln.

Hier mein Source-Code:
#define F_CPU 16000000
#include <avr\io.h>
#include <util\delay.h>
#include <avr\interrupt.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define   BAUD   9600

#define UBRR_VAL F_CPU/16/BAUD-1
#define MaxLength 40
volatile char ReceivedByte;
int Empfangen_Flag=0;
int Flag_Letztes_Byte=0;
int Counter_Length=0;
char Empfange_String[MaxLength];
char *Empfangen_Buffer;

void init_uart(unsigned int ubrr) {
       UBRR1H = (unsigned char)(ubrr>>8);
       UBRR1L = (unsigned char)(ubrr);
       UCSR1B |= (1<<TXEN1 | 1<<RXEN1 | 1<<RXCIE1);
}
void uartPutChar(uint8_t data)
{
  //de:  warten bis Senderegister leer ist
  //eng: wait until
  while ((UCSR1A & 0x20)==0)
  {}
  //de:  senden der Daten
  //eng: send Data
  UDR1=data;
}

void uartPutString(const char* buffer)
{
  uint8_t x;
  //de:  je Zeichen
  //eng  for each character
  while (x=buffer[0])
  {
    //de:  Zeichen senden
    //eng: send character
    uartPutChar(x);
    //de:  nächstes Zeichen
    //eng: next character
    buffer++;
  }
}

char uartGetChar()
{
  char data=0;
  //de:  warte bis Empfang fertig
  //eng: wait until receive is complete
  while (!(UCSR1A&128));
  //de:  empfangen
  //eng: receive
  data=UDR1;
  return data;
}
//  && !( UCSR1A & (1<<UDRE1))

ISR(USART1_RX_vect) // receive incoming bytes
{
  ReceivedByte=UDR1;
  if (Empfangen_Flag==0)
  {
  if (ReceivedByte != '\n' && ReceivedByte !=';'  && Counter_Length < 
MaxLength-1 )
  {
    Empfange_String[Counter_Length]=ReceivedByte;



    // uartPutChar(Empfange_String[Counter_Length]);
    Counter_Length++;
  }
  else
  {
    Empfange_String[Counter_Length] = 0x00;
    strcpy(Empfangen_Buffer,Empfange_String);
    Counter_Length=0;
    Empfangen_Flag=1;
    Flag_Letztes_Byte=0;
    // PORTL =~(1<<7);
    UCSR1A ^= (1<<RXCIE1) ;

  }
  }

}




//---------------------------------------------------------------------- 
-------
//de:  Hauptfunktion
//eng: mainfunction
int main (void)
{


  DDRJ=0xFF;
  DDRL = 0xFF;
  int zahl=1;
  init_uart(UBRR_VAL);
  //de:  zu zählender Wert, mit Null vorbelegen
  // int Anfang=3000;
  uint8_t Schritt[4]={5,9,10,6};
  uint16_t schritt_zaehler=0;
  // Die PWM
  DDRG = (1 << DDG5); // output direction
  TCCR0A |= (1 << WGM02) | (1 << WGM01) | (1 << WGM00); // Fast-PWM Mode
  TCCR0A |= (1 << COM0B1) | (0 << COM0B0); // non-inverting PWM
  TCCR0B |= (1 << CS02); // init prescaler

  uartPutString("Hallo Test\n");
  sei();

  // Mainloop
  while (true)
  {
    OCR0B = 2.55*50;
    PORTJ=Schritt[schritt_zaehler&03];
    if (Empfangen_Flag==1)
    {

      uartPutString(Empfange_String);
      uartPutChar('\n');
      zahl=atoi(Empfange_String);

      Empfangen_Flag=0;
      // PORTL =~(1<<7);
      // PORTL |= (1<<7);
      uartPutString("Hallo Test\n");
      itoa(zahl, Empfangen_Buffer,10);
      uartPutString(Empfangen_Buffer);

      UCSR1A |= (1<<RXCIE1);
    }
    schritt_zaehler++;
    strcat(Empfange_String,";");

    if (zahl >  11)
      PORTL |= (1<<7);
    if (zahl < 5)
      PORTL &=~(1<<7);
    _delay_ms(zahl*1000);


  }
  return 0;
}

Ich Danke euch für eure Hilfe, ich verzweifle schon seit ein paar Tagen.

Grüße und viel spass einem Programmierdelitaten das Problem erklären :)

Tentone

[Klaus W. (mfgkw)]

Christoph Tenten schrieb:
> ich verzweifle schon seit ein paar Tagen.

Dann wäre die Zeit für eine vernünftige Formatierung und ein einerseits 
vollständiges Programm andererseits ohne überflüssigen Kram nicht mehr 
drin gewesen?

[Nickelodien (Gast)]

Abtippfehler?

> while (x=buffer[0])

[Tentone (Gast)]

Hallo Nickelodien,

nein das ist kein Tippfehler.
Darf man das so nicht machen?

Gruß und schonmal Danke
Tentone

[Thomas L. (thomasblue)]

x = buffer[0] ist eine zuweisung und daher immer TRUE

du meintest wohl x == buffer[0]

:-)

[Shzui (Gast)]

Thomas L. schrieb:
> x = buffer[0] ist eine zuweisung und daher immer TRUE

Wo hast Du denn das gelernt?

kauf Dir besser ein anderes Buch.


Shzui

[Stefan E. (sternst)]

Thomas L. schrieb:
> x = buffer[0] ist eine zuweisung und daher immer TRUE

Nein. Das Ergebnis einer Zuweisung ist das, was zugewiesen wurde. Die 
Schleife bricht daher bei der Null-Terminierung ab.

Thomas L. schrieb:
> du meintest wohl x == buffer[0]

Nein. Die Schleife mag zwar ungewohnt aussehen, ist aber absolut korrekt 
so, wie sie ist.

[Thomas L. (thomasblue)]

Stefan Ernst schrieb:
> Nein. Das Ergebnis einer Zuweisung ist das, was zugewiesen wurde. Die
> Schleife bricht daher bei der Null-Terminierung ab.

Jau stimmt, hast recht. Diesen Fall hab ich nicht bedacht, weil ich das 
normaler Weise anders programmieren würde.

"_delay_ms(zahl*1000);" ist vllt das Problem. So weit ich weiß, darf man 
bei dieser Funktion nur eine Konstante benutzen. Gab dazu glaube schon 
ein paar Einträge hier..

[Michael H. (michael_h45)]

"dürfen" ist hier nicht richtig.
Die Funktion wird als always_inline definiert und wird dann nicht für 
jede Zahl angesprungen, sondern für jede Zahl einzeln in den Speicher 
übernommen.
Das Programm wird also unnötig groß.

Umgangen wird das "Problem" so:

void delay_ms(double ms)
{
   for(;ms;ms--)
      _delay_ms(1);
}

@Christoph:
Du müsstest eine handvoll warnings erhalten. Arbeite die alle erst mal 
durch.

[Jim M. (turboj)]

Mal 'ne dumme Frage: Sind die Stringfunktionen wie strcpy() überhaupt 
reentrant? Darf man die im Interrupt so einfach verwenden?


Ich kenne die AVR-Libc nicht so gut, aber beim 8051 wäre das i.d.R. 
nämlich NICHT der Fall. Aber Du hast auch die Länge des Strings schon 
parat - da kannst Du dann auch memcpy() verwenden oder einfach mit einer 
Schleife "zu Fuß" kopieren.

[Thomas L. (thomasblue)]

Hab ich das jetzt falsch gelesen, oder ist haust du in

char *Empfangen_Buffer;

mit dem

strcpy( Empfangen_Buffer, Empfange_String );

auf irgendeinen Speicherbereich, den du dir nie reserviert hast?

[Peter II (Gast)]

Jim Meba schrieb:
> Mal 'ne dumme Frage: Sind die Stringfunktionen wie strcpy() überhaupt
> reentrant? Darf man die im Interrupt so einfach verwenden?
diese auf jeden fall, bei strtok könnte es probleme geben weil dort 
intern sich der Status gemerkt wird. strcpy muss sicht nichts merken.

[Thomas L. (thomasblue)]

Da er aber eh das "Empfangen_Flag" hat, könnte er (auch um die ISR 
kürzer zu machen) das strcpy in die main verfrachten und so zugleich 
diese potentielle Fehlerquelle umgehen.

[Tentone (Gast)]

Hallo Leute,

Danke mal soweit für eure Antworten.
Habe jetzt den Code soweit, dass nur noch ein Fehler auftaucht:
 UBRR1H =(uint8_t)(UBRR_VAL>>8); :warning: suggest parentheses around + 
or - inside shift
was das heißen soll weis ich nicht :)  ....
Auf jeden Fall habe ich das Datenblatt durchgelesen mein Problem soweit 
behoben, d.h. der Empfang und das Senden klappt.

Jetzt habe ich den Code stark reduziert, damit nur mein Problem zum 
tragen kommt.
Und zwar wenn ich z.B. über das ControlCenter "Das ist ein Test" sende,
dann wird nur unter 2 Bedingungen der Interrupt beendet:
1. wenn der Text mit einem ";" beendet wird
2. wenn die MaxLength voll ist

Ziel: Erkennen, dass der Text zueende ist ohne Punkt 1. und 2.

#include <avr\io.h>
#include <util\delay.h>
#include <avr\interrupt.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>


#define UART_MAXSTRLEN 40
volatile uint8_t uart_str_complete = 0;     // 1 .. String komplett empfangen
volatile uint8_t uart_str_count = 0;
volatile char uart_string[UART_MAXSTRLEN + 1] = "";       

 // *******************************************************
// #define F_CPU 16000000
#define BAUD1 9600UL   // Baudrate
 
// Berechnungen
#define UBRR_VAL (F_CPU/(BAUD1*16))-1

void USART_Init()
{
  
       UBRR1H =(uint8_t)(UBRR_VAL>>8);
       UBRR1L = (unsigned char)(UBRR_VAL);
       UCSR1B |= (1<<TXEN1 | 1<<RXEN1 | 1<<RXCIE1);
}

void uart1PutChar(char data)
{
   //warte bis UDR1 leer ist
   while (!(UCSR1A&32));
   //sende
   UDR1=data;
} 
 
 
/* puts ist unabhaengig vom Controllertyp */
void uart_puts (const char *buffer)
{
    for (int i=0; buffer[i] !=0;i++) uart1PutChar (buffer[i]); 
}

ISR(USART1_RX_vect)
{
   unsigned char nextChar;
 
  // Daten aus dem Puffer lesen
  nextChar = UDR1;
  if( uart_str_complete == 0 ) {  // wenn uart_string gerade in Verwendung, neues Zeichen verwerfen
 
    // Daten werden erst in uart_string geschrieben, wenn nicht String-Ende/max Zeichenlänge erreicht ist/string gerade verarbeitet wird
    if( nextChar != '\n' &&
        nextChar != '\r' &&
    nextChar != ';'&& 
        uart_str_count < UART_MAXSTRLEN - 1 ) {
      uart_string[uart_str_count] = nextChar;
      uart_str_count++;
    }
    else {
      uart_string[uart_str_count] = '\0';
      uart_str_count = 0;
      uart_str_complete = 1;
    }
  }
}




int main (void)
{
char* Hier_Soll_ES_REIN;
  USART_Init();
  sei();
  while (true) // Mainloop
  {
    
  if (uart_str_complete==1)
  {
    uart_puts("Geschafft\n");
    // strcpy(Hier_Soll_ES_REIN, (const char*)uart_string);
    uart_puts((const char*)uart_string);
    uart_str_complete = 0;
  }
  
    
  }
  return 0;
}

Wenn noch grobe Fehler im Code sein sollten, wären Hinweise ganz nett.
Der erste Code hat nur durch zufall funktioniert, der scheint jetzt 
stabil zu sein :) ....

Gruß und Danke für eure Hilfe
Tentone

PS:: Hoffe es ist jetzt besser formatiert.

[Klaus W. (mfgkw)]

Wesentlich besser!
Jetzt könnte man darin auch vernünftig Fehler suchen :-)

[Thomas L. (thomasblue)]

Drei Ideen:

1) Versuch doch ein wenig mehr zu debuggen. Also z.B. weißt du, wie 
viele Zeichen denn empfangen werden? Könntest ja den counter zu 
Testzwecken erst in der Main zurücksetzen und vorher Stand ausgeben 
lassen.

2) Würde ich denken, dass die ISR o.k. ist. Was im Umkehrschluss ja fast 
nur bedeuten kann, dass dein C#-Programm gar kein Stringende mitsendet? 
Hast du dir schon mal über ein Terminalprogramm angeschaut, was wirklich 
gesendet wird? "hterm" ist dafür ganz gut geeignet.

3)

char* Hier_Soll_ES_REIN;

ist kein Array / Speicherbereich.

[Christoph T. (tentone)]

Hallo Thomas

Thomas L. schrieb:

> 2) Würde ich denken, dass die ISR o.k. ist. Was im Umkehrschluss ja fast
> nur bedeuten kann, dass dein C#-Programm gar kein Stringende mitsendet?
> Hast du dir schon mal über ein Terminalprogramm angeschaut, was wirklich
> gesendet wird? "hterm" ist dafür ganz gut geeignet.

1. Ich habe das Programm zu Testzwecken von myavr verwendet. 
(ControlTerminal)
2. Die Ausgabe auf den Schirm passt. Nur erkennt er kein Stringende.
Ich vermute auch, dass es an der Terminierung von dem ControlTerminal 
liegt.

Ich werde es mal mit hterm probieren.

Wie funktioniert, das eigentlich mit dem strcpy?

Meine Hardware: Atmega640 auf dem MK3 board.

Gruß
Tentone

[Karl H. (kbuchegg)]

Tentone schrieb:

> Habe jetzt den Code soweit, dass nur noch ein Fehler auftaucht:
>  UBRR1H =(uint8_t)(UBRR_VAL>>8); :warning: suggest parentheses around +
> or - inside shift
> was das heißen soll weis ich nicht :)


Darüber solltest du aber nachdenken!

Suggest                       Ich schlage vor
parentheses                  Klammern
around + or - inside shift   wenn du ein + oder ein - verwendest


Der Compiler warnt dich also davor, dass du Klammern verwenden sollst, 
weil da höchst wahrscheinlich etwas nicht so läuft, wie du dir das 
vorstellst.

Was könnte es sein?
Der Compiler markiert dir diesen Ausdruck

   (uint8_t)(UBRR_VAL>>8)

Der SChlüssel zum ganzen findet sich darin, dass UBRR_VAL ein Makro ist.

#define UBRR_VAL (F_CPU/(BAUD1*16))-1

und das wir uns daran erinnern, dass ein #define einfach nur eien 
Textersetzung veranlasst.

Also mach das mal. Ersetze in der Anweisung den Text von UBRR_VAL durch 
den Text für den das Makro steht.

Aus

(uint8_t)(UBRR_VAL>>8)

wird

(F_CPU/(BAUD1*16))-1 >> 8

Und jetzt analysiere mal, worauf sich jetzt das >> 8 bezieht. Und dann 
überleg mal worauf es sich beziehen sollte und was du an der 
Klammersetzung verändern musst, damit es das auch tut.