Fehler nach Einfügen einer Else-Anweisung

[Jonas E. (jonas_e43)]

Hallo,
nachdem ich in meinem Programm eine weitere Else-Anweisung eingefügt 
habe,
funktioniert es nicht mehr vernünftig.

Zum Programm:
Das uart_receive_complete bool wird auf true gesetzt, wenn ein String 
über eine UART-Interruptroutine empfangen wurde. Dann wird damit 
begonnen, den String zu verarbeiten. Das funktioniert auch, solange kein 
weiteres Else verwendet wird.

if(uart_receive_complete)
    {
      lcd_clear();
      char cmd = uart_string[0];
      if ( cmd == CMD_HEADER )
      {
        strcpy(g_Header, uart_string+1);
      }
      else if ( cmd == CMD_TAG )
      {
        timer1_disable();
        timer1_reset();
        char accepted = uart_string[1];
        
        if ( accepted == UART_TRUE )
        {
          lcd_stringCenter("Welcome", 1);
          char* name = ( uart_string + 2);
          lcd_stringCenter(name, 2);
          led_toggle(LED_ACCEPT);
          delay(3000);
        }
        else if ( accepted == UART_FALSE )
        {
          lcd_stringCenter("ID unknown.", 1);
          lcd_stringCenter("Access denied.", 2);
          led_toggle(LED_DENIED);
          delay(3000);
        }
      /*  else
        {
          lcd_stringCenter("ID transmitted.", 1);
          delay(2500);
        }
      */
        rfid_enable();
      }
      else
      {
        lcd_stringCenter("Unknown command:", 1);
        lcd_stringCenter(uart_string, 2);
        led_toggle(LED_ACCEPT);
        led_toggle(LED_DENIED);
      }

      free(uart_string);
      uart_receive_complete = false;
      LED_PORT &= ~(1<<LED_ACCEPT);
      LED_PORT &= ~(1<<LED_DENIED);
      renderHeader();
    }

Wenn ich aber nun die Else-Anweisung ausklammere um eine weitere 
Verarbeitungsmöglichkeit hinzuzufügen, wird g_header nicht mehr 
vollkommen erkannt. Es wird lediglich, der erste Buchstabe kopiert.
Ich flashe mit diesem Programm einen Attiny 4313. Der Flashspeicher ist 
dann zu etwa 95% voll. Woran könnte das Problem liegen.
So langsam bin ich am verzweifeln.
Wenn Jemandem etwas auffällt würde ich mich über Hinweise oder Lösungen 
freuen.
MfG Jonas

[Dr. Sommer (Gast)]

Jonas E. schrieb:
> free(uart_string);
Du verwendest dynamische Speicherverwaltung auf einem Controller mit 256 
Bytes RAM?! Du verwendest fröhlich ewig lange Delays während derer das 
Programm auf keine Eingaben reagiert? Eventuell klappt das, eventuell 
generiert das aber auch schwierig zu findende Probleme...

[holger (Gast)]

>Du verwendest dynamische Speicherverwaltung auf einem Controller mit 256
>Bytes RAM?! Du verwendest fröhlich ewig lange Delays

und Strings die vermutlich auch im RAM liegen.

z.B.         lcd_stringCenter("Unknown command:", 1);

Das else

          lcd_stringCenter("ID transmitted.", 1);


hat dem Stack vermutlich nur noch den Rest gegeben. Aus die Maus.

[holger (Gast)]

>Du verwendest dynamische Speicherverwaltung auf einem Controller mit 256
>Bytes RAM?

Es sind nur 128 Byte;)

[Thomas E. (thomase)]

holger schrieb:
>>Du verwendest dynamische Speicherverwaltung auf einem Controller mit 256
>>Bytes RAM?
>
> Es sind nur 128 Byte;)
Nee. Das sind 256. Aber trotzdem...

mfg.

[Jonas E. (jonas_e43)]

Wie kann aus dem Ram die nicht mehr benötigten Strings entfernen, um 
wieder Speicherplatz zu erlangen?
Gruß Jonas

[Dr. Sommer (Gast)]

Jonas E. schrieb:
> Wie kann aus dem Ram die nicht mehr benötigten Strings entfernen, um
> wieder Speicherplatz zu erlangen?
Gar nicht. Allerdings sollte der GCC clever genug sein, die Strings in 
den Flash zu packen. Mach dich mal schlau wie man Pointer auf 
PROGMEM-Arrays (aka. String-Konstanten) mit dem AVR-GCC an Funktionen 
übergibt. Und nimm alle malloc&free aus dem Programm...

[holger (Gast)]

>Wie kann aus dem Ram die nicht mehr benötigten Strings entfernen, um
>wieder Speicherplatz zu erlangen?

Überflüssige Ausgaben gar nicht erst machen.

          lcd_stringCenter("ID unknown.", 1);
          lcd_stringCenter("Access denied.", 2);

Da reicht doch auch:

          lcd_stringCenter("ID unknown", 1);
          lcd_stringCenter("Access denied", 2);

Ich hab jetzt mal die beiden '.' weggelassen. Spart zwei Byte.

Oder

          lcd_stringCenter("ID?", 1);
          lcd_stringCenter("Access denied", 2);

Nochmal 7 Byte weniger.

Oder einfach nur

         lcd_stringCenter("Access denied", 1);

Es gibt immer einen Weg RAM zu sparen.

[Dr. Sommer (Gast)]

holger schrieb:
> Es gibt immer einen Weg RAM zu sparen.
Wieso RAM?! String-Literale gehören in den Flash! Und können auch direkt 
von da byteweise gelesen&verarbeitet werden, braucht 0 Bytes RAM.

[holger (Gast)]

>> Es gibt immer einen Weg RAM zu sparen.
>Wieso RAM?! String-Literale gehören in den Flash!

Das weiss ich selber, ich bin ja nicht blöd. Im Flash
stehen die sowieso. Woher sollen die sonst ins RAM kommen?

Schreib ihm seine Routine um Daten aus dem Flash zu nehmen
doch einfach. Da hatte ich jetzt im Moment keinen Bock drauf.

Ich hab ihm nur einen Weg gezeigt wie er da ohne grosse
Verrenkungen evtl. noch mal so davonkommt.

[Jonas E. (jonas_e43)]

Achso jetzt versteh ich das :D.
Würde mich trotzdem über eine Routine freuen.
Gruß Jonas

[holger (Gast)]

>Achso jetzt versteh ich das :D.
>Würde mich trotzdem über eine Routine freuen.

Dann zeig doch mal deinen Code für lcd_stringCenter().

[Jonas E. (jonas_e43)]

lcd_StringCenter():

void lcd_stringCenter(const char* data, int row)
{
  if ( sizeof(data) > 0 )
  {
    int length = strlen(data);
    int column = 0;
    
    if ( length <= LCD_COLOUMS )
    {
      column = LCD_COLOUMS / 2 - ( length + ( length % 2 ) ) / 2;
    }
    
    lcd_stringAt(data, column, row);
  }
}

lcd_stringAt(data, column, row):

void lcd_stringAt( const char *data, int coloumn, int row )
{
  lcd_setcursor(coloumn, row);
  
  while( *data != '\0' )
  {
    if ( coloumn >= LCD_COLOUMS )
    {
      coloumn = 0;
      row++;
      lcd_setcursor(coloumn, row);
    }
    lcd_data( *data++ );

    coloumn++;
  }
}

[holger (Gast)]

>lcd_StringCenter():

Das jetzt auf PROGMEM umzubauen sieht nicht so einfach aus.

Aber mal ein paar andere Tips vor dem schlafen gehen:

>void lcd_stringCenter(const char* data, int row)

int row? Wozu? Kann row negativ werden? Dein Display hat ja
auch nur zwei Zeilen. Wozu brauchst du da einen 16 Bit Wert?

void lcd_stringCenter(const char* data, uint8_t row)

ist doch besser oder nicht?

Der gleiche Müll:

    int length = strlen(data);
    int column = 0;

Besser

    uint8_t length = strlen(data);
    uint8_t column = 0;

Noch son Ding. Ausser int scheinst du ja nichts anderes zu kennen.

>void lcd_stringAt( const char *data, int coloumn, int row )

void lcd_stringAt( const char *data, uint8_t coloumn, uint8_t row )

Das gleiche vermutlich mit   lcd_setcursor(coloumn, row);

sizeof(data) ist immer grösser 0.

  if ( data != 0 )

usw.

Bring erstmal diesen Unsinn in Ordnung.
Das wird dein Programm schon mal ordentlich eindampfen.

[Jonas E. (jonas_e43)]

Alles klar.
Ich bin noch an das Programmieren mit C# gewöhnt wo man nicht so auf 
Speicher achten braucht. Nun habe ich alle 16 bit Ints zu uint8_t 
"verwandelt". Spart einen guten Teil an Speicher ein. Nur löst das 
leider nicht mein grundlegendes Problem...
Gruß Jonas

[Jonas E. (jonas_e43)]

Hallo Holger,
ist es möglich, dass du mir deine E-Mail Adresse per PN schreibst, 
sodass ich dir das komplette Programm mal schicken könnte und du es dann 
einmal ansehen könntest? Was könnte z.B. verbessert werden?
Würde mich echt sehr darüber freuen.
Gruß Jonas

[holger (Gast)]

>Hallo Holger,
>ist es möglich, dass du mir deine E-Mail Adresse per PN schreibst,

Nö;)

Ich hab mich mal an deiner Funktion vergangen. Nur rein
theoretisch, könnten also noch Fehler drin sein.

So müsste das mit Strings aus dem Flash klappen.
Ohne Gewehr!

void lcd_stringCenter_P(const char* data, uint8_t row)
{
  char c;

  if ( data > 0 )
  {
    uint8_t length = strlen_P(data);
    uint8_t column = 0;
    
    if ( length <= LCD_COLOUMS )
    {
      column = LCD_COLOUMS / 2 - ( length + ( length % 2 ) ) / 2;
    }
    
    lcd_setcursor(coloumn, row);
  
    while((c = pgm_read_byte(data++)))
    {
      if ( coloumn >= LCD_COLOUMS )
      {
        coloumn = 0;
        row++;
        lcd_setcursor(coloumn, row);
      }
      lcd_data(c);

      coloumn++;
    }
  }
}

Du ersetzt jetzt aber nicht deine void lcd_stringCenter() mit dieser
Funktion. Du nimmst dies als eine neue Funktion mit rein.

Da wo du z.B

          lcd_stringCenter("ID unknown.", 1);

schreibst musst du jetzt

          lcd_stringCenter_P((PSTR)"ID unknown.", 1);

draus machen.

Strings aus dem RAM kannst du damit nicht ausgeben! Also

          lcd_stringCenter(name, 2);

bleibt so wie es war.

Viel Spass damit;)

[Jonas E. (jonas_e43)]

Wenn ich diese Funktion verwende:

lcd_stringCenter_P((PSTR)"ID unknown.", 1);

Er schmeißt dann aber bei mir einen Feher raus:
Error  6  too few arguments to function 'lcd_stringCenter_P'

[Fabian O. (xfr)]

Es muss heißen:

lcd_stringCenter_P(PSTR("ID unknown."), 1);