Automatisches LED-Licht mit Funkmodulen

[Franz Josef M. (arras)]

Guten Tag,

ich habe einen Beitrag beigesteuert (main.c),von dem ich glaube,daß er 
interessant für einige ist. Es handelt hierbei um ein kleines 
Kunststoffgehäuse mit einer Elektronik,die unter anderem einen Attiny 
84A, eine Funkstrecke mit Sender und Empfänger (433 MHz), 5 Tasten,zwei 
Fotozellen und Keingram beheimatet. Das alles ist verbunden mit einer 
Software, die einen LED-Sreifen ansteuert, der u.a. einen automatischen 
Betrieb zuläßt.Per Funk kann es diese LED-Ansteuerung zu einem 
Slave-Modul übertragen, das eigene LED-Streifen steuert.
Wenn Interesse daran besteht, dann lesen bitte diese "main.c".
Mit freundlichen Grüßen
     -  arras -

[Tippgeber (Gast)]

Funktioniert das gezeigte Programm wirklich?

Erster Blick:

Deine main-Schleife schließt die Definition von

void Init_Anlauf ()   // Anlaufprogramm

mit ein. Diese ruft sich selbst rekursiv auf:

      // **** Init_Timers,Init_I2C und Init_ADC ****
     //
     Init_Timers ();
        Init_ADC ();
     Init_Anlauf();                    // mehr z.Z. nicht..

Die Formatierung des Quelltextes ist katastrophal, die Strukturierung 
ebenso.

[Tippgeber (Gast)]

Tippgeber schrieb:
> Diese ruft sich selbst rekursiv auf:
>       // **** Init_Timers,Init_I2C und Init_ADC ****
>      //
>      Init_Timers ();
>         Init_ADC ();
>      Init_Anlauf();                    // mehr z.Z. nicht..

Ok, diesen Teil nehme ich zurück, nachdem ich das Programm mal in einen 
Codeeditor geladen habe.

Es bleibt aber dabei, dass das Ganze sehr unübersichtlich ist und 
schlecht formatiert.

[fjm (Gast)]

Hallo,

bei meinem Entwicklungssystem (AVR,Atmel Studio 7) sieht aber alles sehr 
aufgeräumt aus. Ich kann Deine Einwende nicht nachvollziehen.
Es liegt am Editor, der macht einem TAB (09) irgentetwas..nur keine TAB.
Er muß bei einem Tabulator-Zeichen genau 4 Leerstellen machen.

Gruß arras

[Uhu U. (uhu)]

fjm schrieb:
> Er muß bei einem Tabulator-Zeichen genau 4 Leerstellen machen.

Schreib es doch einfach in den Quelltext hinein…

[AllesKönner (Gast)]

Tippgeber schrieb:
> Es bleibt aber dabei, dass das Ganze sehr unübersichtlich ist und
> schlecht formatiert.

und von mäßiger Code-Quality = 1.Lehrjahr!, weil:

- massenweise sinnlose Initialisierung von global Variables mit 0x00
- tonnenweise global Variables ist strukturlos
- händisches Adressenhandling im EEPROM, das macht struct von selbst
- if () if () ..., wenn ein if () mit der gesamten Bedingung reicht
- massenweise redundante Codebereiche
- massenweise Variable-Bezeichner ohne Sinn
- ...

Ich könnte hier x-weitere Punkte listen. Dieser Code ist unübersichtlich 
bzw. unlesbar und hat Klippschulniveau!

Ich würde mal ein C-Buch lesen UND mir guten Code anschauen UND dazu 
lernen
oder doch lieber Kuchenbacken.

[fjm (Gast)]

Hallo AllesKönner !

Ich wollte kein Lehrbuch in C schreiben, noch ein kurzes 
Quellcode-Release machen. Es kem mir darauf an, eine - ggf. umstängiges 
und weitreichendes Werk zu generieren, in dem auch Sequenzen enthalten 
sind, die nicht im HEX-file wiederverwenden sind. Tatsache ist aber, der 
Code passt in in 8 kb Speicher.
Gerade die Programme, die etwas mit "Funk" zu tun haben, machen dies 
notwengig.
Selbstverständlich sind alle Module getestet und laufen seit Wochen.

Gruß fjm

[AllesKönner (Gast)]

fjm schrieb:
> Ich wollte kein Lehrbuch in C schreiben,

Dann hast du jetzt wohl doch ein Buch mit einem negativ Beispiel 
abgegeben, wie man nicht programmiert! Beschäftige dich auch mal mit 
Refactoring.
Vor dem Coding kommt besser auch das Denken bzgl. 
Programm-/Datenstrukturen, was du hier wohl komplett ausgelassen hast. 
Deine zahllosen global Variables sind eine Vergewaltigung einfachster 
Grundsätze der Programmierung.

Die Lesbarbeit deines Codes ist gleich null, zwei Beispiele

Refactored, 3 Instructions

// wenn Gleichstand besteht (A==B), Fast-PWM-Art (0101 = 5)
TCCR1A |= (1<<COM1B1) | (1<<WGM10)
// Pres.64 (250 msec), Fast-PWM-Art 2.Teil                  
TCCR1B |= (1<<WGM12) | (1<<CS11) | (1<<CS10); 
TIFR1  |= (1<<OCF1A) | (1<<OCF1B);

Original, 6 Instruction und sinnloses !(...)

TCCR1B |= ((!(1<<CS12)) | ((1<<CS11)) | ((1<<CS10)));// Pres.64 (250 msec)
TCCR1A |=((!(1<<WGM11)) |((1<<WGM10)));  // Fast-PWM-Art (0101 = 5)
TCCR1B |=((1<<WGM12) | (!(1<<WGM13))); // Fast-PWM-Art 2.Teil    
TCCR1A |= (((1<<COM1B1)) | (!(1<<COM1B0))); // wenn Gleichstand besteht (A==B)
TIFR1  |= (1<<OCF1A);
TIFR1  |= (1<<OCF1B);

Refactored

ISR (TIM0_COMPA_vect) {
   PORTA  |= (1<<PWM_0);
   //PORTA  ^= (1<<Ser_out); // 16 msek Basiszeit
   
   if (!impuls_1) impuls_1++;
   else impuls_1 = 0;

   zeit++;  // Basiszeit
   zeit0++;
   zeit1++;
   zeit2++;
   zeit3++;
   zeit_0++;
   zeit_01++;
   Basis_T2++;
   zeit_seriell++;
   
   if (sv_ausg ) sv_ausg--; // Ein/Aus Reset 
   if (zeit_ausg) zeit_ausg--; // Service Ausgabe (Byte)
   if (zeit_bit) zeit_bit--; // Bitzeit
   if (zeit_aus) zeit_aus--; // ein/aus Progr.              
   if (zeit_man) zeit_man--; // Manchestercodezeit    
   if (zeit_empf) zeit_empf--; // Empfangszeit  
   if (zeit_send) zeit_send--;  // Sende_zeit
   if (zeit_pre && Preambel==1) zeit_pre--; // Vorlaufzeit
   if (zeit_vor) zeit_vor--; // Überwachungszeit
}

Original, sinnlose Comments, redundante Schreibweise:

ISR (TIM0_COMPA_vect)
{
  PORTA  |= (1<<PWM_0);
  //PORTA  ^= (1<<Ser_out);            // 16 msek Basiszeit
  
  if (impuls_1 == 0)
  {
    impuls_1 = impuls_1 + 1;
  }
  else
  {
    impuls_1 = 0;
  }
  zeit = zeit + 1;      // Basiszeit

  zeit0   = zeit0 + 1;       // zeit0
  zeit1   = zeit1 + 1;      // zeit1
  zeit2   = zeit2 + 1;    // zeit2
  zeit3   = zeit3 + 1;    // teit3
  zeit_0  = zeit_0 + 1;    // zeit_0
  zeit_01 = zeit_01 + 1;    // zeit_01

  Basis_T2 = Basis_T2 +1;    // Basis_T2 inv
  zeit_seriell = zeit_seriell + 1;
  
  if (sv_ausg > 0)                // Ein/Aus Reset
  {
    sv_ausg = sv_ausg - 1;
  }
    
  if (zeit_ausg > 0)                // Service Ausgabe (Byte)
  {
    zeit_ausg = zeit_ausg - 1;
  }
    
  if (zeit_bit > 0)                // Bitzeit
  {
    zeit_bit = zeit_bit - 1;
  }
  
  if (zeit_aus > 0)                // ein/aus Progr.
  {
    zeit_aus = zeit_aus - 1;
  }  
              
  if (zeit_man > 0)                // Manchestercodezeit
  {
    zeit_man = zeit_man - 1;
  }
      
  if (zeit_empf > 0)                // Empfangszeit
  {
    zeit_empf = zeit_empf - 1;
  }
  
  if (zeit_send > 0)                // Sende_zeit
  {
    zeit_send = zeit_send - 1;
  }
        
  //if (zeit_pre > 0)
  if ((zeit_pre > 0) && (Preambel == 1))      // Vorlaufzeit
  {
    zeit_pre = zeit_pre - 1;
  }
    
  if (zeit_vor > 0)                // Überwachungszeit
  {
    zeit_vor = zeit_vor - 1;
  }

}

--------------
Mod: Formatierung korrigiert, es muss lauten:

[c]...[/c]

 und nicht [\c]