Optimierung (Optimization)<->Probleme

[Robert P. (Firma: ---) (ausmilkel)]

Hallo,

ich bin ein noch unerfahrener Programmierer in Bezug auf AVR. Habe 
Grundkentnisse in C und mir schon ein Programm zur Ansteuerung für ein 
Display implementiert und auch schon andere dazugehörige Programmabläufe 
um es dann zu visualisieren. Da die Code-größe jedoch sehr schnell 
"explodiert" probierte ich die Option zum Optimieren des Quellcodes 
(Optimization). Das Problem: vor der Optimierung verhielt sich das 
Programm ordnungsgemäß ( nach meinen Erwartungen ), wenn ich jedoch die 
verschiedenen Optimierungsgrade verwende kommen teiweise wirre Zeichen 
auf meinem Display zum Vorschein ( zusätzliche Buchstaben...). Teilweise 
wird auch der Programmablauf komplett gestört, beispielsweise nur zum 
Teil ausgeführt.

Hat jemand ähnliche Erfahrungen, Tipps oder Links zu Threads die mir 
helfen könnten?
Bei Bedarf kann ich auch den Quellcode schicken, ist halt nur "sehr 
viel", da ich viele eigene Headerdateien verwende.

mfg

Robert

[Berti (Gast)]

for schleifen

for(i=0;i<100;i++);

werden eventuell "wegoptimiert"
da hilft ein

asm"nop"

 in der for Schleife

[Falk B. (falk)]

@ Robert P. (Firma ---) (ausmilkel)

>(Optimization). Das Problem: vor der Optimierung verhielt sich das
>Programm ordnungsgemäß ( nach meinen Erwartungen ), wenn ich jedoch die
>verschiedenen Optimierungsgrade verwende kommen teiweise wirre Zeichen

Unsaubere Programmierung, selbstgebastelte Delays, etc.

>Bei Bedarf kann ich auch den Quellcode schicken, ist halt nur "sehr
>viel", da ich viele eigene Headerdateien verwende.

Pack es alles in ein ZIP und poste es hier.

MFG
Falk

[Robert P. (Firma: ---) (ausmilkel)]

Ok

hab hier im Anhang mal alles als ZIP-File angehangen. Ist doch nicht 
mehr so ivle Quellcode, hab nicht alles genutzt.

danke schon mal.

mfg
Robert

[Falk B. (falk)]

@ Robert P. (Firma ---) (ausmilkel)

>hab hier im Anhang mal alles als ZIP-File angehangen. Ist doch nicht
>mehr so ivle Quellcode, hab nicht alles genutzt.

Nun, das sieht man recht schnell.

//
//
// external clock 12Mhz
//
//
#include <avr/io.h>
#include <stdlib.h>
#include <util/delay.h>
#include <stdio.h>
#include <string.h>
[c]

Dein AVR läuft mit 12 MHz, aber wo ist das define für F_CPU?

Wenn das fehlt bzw. im AVR-Studio falsch angegeben ist, dann arbeiten die delay-Funktionen mit angenommenen 1 MHz. Dadurch werden aber deine Delays zu kurz (jetzt sind sie auf Grund der fehlenden Optimierung und der dadurch implementierten Fliesskommaberechnung wesentlich zu lang).

Also ein 

[c]
#define F_CPU 12000000L

VOR die Verwendnung von #include <util/delay.h> einfügen.

MFG
Falk

[Robert P. (Firma: ---) (ausmilkel)]

das mit der Frequenz kann man doch unter Project-> Configuration options 
und dann unter General bei frequency eintragen !? Hatte bisher 
funktioniert, jedenfalls laut Simulation. Ich trag es trotzdem noch in 
den Quellcode ein und probiere es.

mfg
Rober

[Karl H. (kbuchegg)]

Da haben wir schon mal den ersten

in lcdroutines.c findet sich

void lcd_data_2xint(unsigned int wert)
{
   int i=0;
   unsigned char temp22=0,temp12=0;
   char change[2]={0},temp[1]={0};

   if(wert>=10)  itoa(wert,change,10);

wenn wert größer als 10 ist, dann wird die ASCII
Repräsentierung davon logischerweise aus mehr als 2
Zeichen bestehen. Aus der Zahl 123 wird der String
"123". Das sieht auf den ersten Blick nach 3 Zeichen
aus. In Wirklichkeit braucht der String aber 4 Bytes,
weil jeder String in C mit einem '\0' Zeichen abgeschlossen
wird.

Das Feld, in welches du itoa schreiben lässt, ist aber
nur 2 Zeichen gross. Ergo-> die restlichen 2 Zeichen
landen im Speicher an Positionen die nicht mehr zu change
gehören.    Buum

Selbiges im einstelligen Fall. Wenn deine Zahl kleiner
9 ist, dann besteht der enstehende String aus 2 Zeichen.
Nämlich der '9' und dem obligatorischen abschliessenden
'\0' Zeichen.


Mir ist insgesammt nicht wirklich klar, was du
damit

void lcd_data_2xint(unsigned int wert)
{
   int i=0;
   unsigned char temp22=0,temp12=0;
   char change[2]={0},temp[1]={0};

   if(wert>=10)  itoa(wert,change,10);
   else { change[0]='0';
   itoa(wert,temp,10);
   strcat(change,temp);}

   for(i=0;i<=1;i++)
   {temp22=change[i];
   temp12=temp22;


bezwecken willst.

Ich werd das mal etwas umformatieren, damit man die Programm-
logik etwas besser sieht

void lcd_data_2xint(unsigned int wert)
{
   int i=0;
   unsigned char temp22=0,temp12=0;
   char change[2]={0};
   char temp[1]={0};

   if( wert >= 10 )
     itoa( wert,change,10 );
   else {
     change[0]='0';
     itoa( wert, temp, 10);
     strcat( change, temp );
   }

Ach, jetzt verstehe ich. Du willst führende Nullen haben.
Warum benutzt du eigentlich in dieser Funktion nicht die Funktion
lcd_string. Nach diesem ganzen Hack-Mack von da oben ist doch
das Endergebnis ein String. Und die Funktion um diesen auszugeben
heist nun mal lcd_string. Kein Grund das ganze Bit-gewackle
auch noch in diese Funktion hineinzupacken.

void lcd_data_2xint( unsigned int wert )
{
  char Ascii[10];    // langt dicke. Knausrig sein an dieser
                     // Stelle ist meist keine gute Idee

  itoa( wert, Ascii, 10 );

  if( wert < 10 )     // ist eine führende 0 notwendig, wenn ja
    lcd_data( '0' );  // -> '0' ausgeben

  lcd_string( Ascii );  // und dann noch die Stringrepräsentierung
                        // der Zahl hinten nachschicken.
}

Mal schauen was sich sonst noch so alles findet :-)

[Karl H. (kbuchegg)]

Der Rest sieht soweit beim schnellen drüberschauen eigentlich
ganz ok aus. Zumindest sollten keine wilden optimize-only
Effekte mehr drinnen sein.

Ein Wort noch:
Versuch möglichst bald zu einer vernünftigen Formatierung
zu kommen. Das was du zur Zeit hast ist noch sehr daneben.

Vergleich mal das hier

#include <stdlib.h>
#include <util/delay.h>
#include <stdio.h>
#include <string.h>

#include "lcdroutines.h"
#include "timefunktion.h"
#include "schrift_display.h"




int main(void)
{
   
  
   
   int minuten=0,sekunden=-1,hundertstel=-1,lauf=5,warte=0,z=0;


   lcd_init();

   
   hallo();         

   for(z=0;z<=180;z++)
   { _delay_ms(10); }
   lcd_clear();
   erste_zeile();   


   for(lauf=0;lauf<=120;lauf++)
       { 
       rise_time(&hundertstel);
       rise_time(&sekunden);    
         if(sekunden>=60) {rise_time(&minuten);sekunden=0;}
         
         zeitausgabe1(minuten,sekunden,hundertstel);
       for(warte=0;warte <=30;warte++)
         {_delay_ms(10);}
        } 

   while(1)
   {
       
   }
 
   return 0;
   
}

mit dem hier

#include <stdlib.h>
#include <util/delay.h>
#include <stdio.h>
#include <string.h>

#include "lcdroutines.h"
#include "timefunktion.h"
#include "schrift_display.h"

int main(void)
{
   int minuten     =  0
   int sekunden    = -1;
   int hundertstel = -1;
   int lauf        =  5;
   int warte       =  0;
   int z           =  0;

   lcd_init();

   hallo();         

   for( z = 0; z <= 180; z++ )
   {
     _delay_ms(10);
   }

   lcd_clear();
   erste_zeile();   

   for( lauf = 0; lauf <= 120; lauf++ )
   { 
     rise_time( &hundertstel );
     rise_time( &sekunden );    
     if( sekunden >= 60 )
     {
       rise_time( &minuten );
       sekunden = 0;
     }
         
     zeitausgabe1( minuten, sekunden, hundertstel );

     for( warte = 0; warte <= 30; warte++ )
     {
       _delay_ms(10);
     }
   } 

   while( 1 )
   {
   }
 
   return 0;
}

Es ist immer wieder interessant, dass Neulinge zwar möglichst
viel in einer Wurscht (also ohne Leerzeichen) dahinschreiben,
dafür aber eine Unmenge an aufeinanderfolgenden Leerzeilen
setzen.

* Aufeinanderfolgende Leerzeilen tragen selten zur Übersicht
  im Code bei. Wenn dir etwas wichtig genug ist, dass du
  es absetzen musst um es hervorzuheben, dann mach einfach nur
  eine Leerzeile und setzte einen Kommentar hinein, der den
  geneigten Leser darüber aufklärt was als nächstes passiert.
  Aber spar dir die vielen Leerzeilen. Sie ziehen den Code
  nur unnötig in die Länge.

* Für den Anfang:
  Eine Anweisung - Eine Zeile
  Bei einem if-Block ist das if und die davon abhängende
  Anweisung jeweils eine eigene Anweisung. Das if kriegt
  seine eigene Zeile. Die davon abhängenden Anweisung
  kriegt seine eigene Zeile (und ist eingerückt).

  Auf die Art vermeidest du Zeilen, die über den ganzen
  Bildschirm von links nach rechts reichen und das ganze
  wird auch optisch gegliedert. Auf der einen Seite (Zeile)
  die Bedingung, auf der anderen Seite (Zeile) das was von der
  Bedingung abhängt. Jede Information für sich kann so besser
  und schneller erfasst werden, als wie wenn du erst mal in dem
  Buchstabenwust die Teile auseinanderpfriemeln musst.

* Benutze Leerzeichen um den Code zu gliedern. Mach es dem
  Leser leicht, im Code einzelne 'Wörter' zu erkennen.

  Vergleich mal das hier

     {rise_time(&minuten);sekunden=0;}

  mit dem hier

    {
      rise_time( &minuten );
      sekunden = 0;
    }

  Im ersten Abschnitt musst du schon mal 10 Sekunden den Code
  zu studieren um den ';' zu bemerken und festzustellen, dass
  hier eigentlich 2 Anweisungen zu finden sind.

* Mach Einrückungen konsistent.
  Es ist nicht wirklich wichtig, wo du die { - } Klammern
  setzt. Es gibt da mehrere verbreitete Stile

  if( ... ) {
    Anweisung;
  }

  if( ... )
  {
    Anweisung;
  }

  if( ... )
    {
    Anweisung;
    }

  Es ist auch nicht wirklich wichtig, um wieviele Stellen du einrückst.
  Manche rücken um 2 Zeichen ein, manche um 4.

  Es ist aber wichtig, dass du den Stil konsequent durchziehst.
  Und nein: Kraut und Rüben ist kein anerkannter Stil :-)

[Robert P. (Firma: ---) (ausmilkel)]

@  Karl heinz Buchegger (kbuchegg):

danke für die erste Korrektur bei der Unterfunktion in der 
lcdrotuines.c. Es funktioniert jetzt auch  mit der Optimierung. Schon 
schwer zu finden so ne sache. Vor allem wenn es an sich ohne der 
Optimierung funktioniert hat.

mfg
Robert