Dj Wandbeleuchtung

[Kardoffelbrai (Gast)]

Schönen guten Abend,

ich bin noch relativ unerfahren im Programmieren und habe jetzt mal mein 
erstes größeres Programm geschrieben!
Es handelt sich um eine Dj Wandbeleuchtung.
Es soll später noch erweitert werden, sodass noch mehr Darstellungen im 
Programm sind und RGB LEDs verwendet werden.

Wäre sehr nett wenn ihr mal einen kurzen Blick drüber werfen könntet und 
evtl. Tipps oder Verbesserungen geben sagen könntet.

/*
 *             PD PC
 *            PD   PC
 *           PD     PC
 *          PD       PC
 *         PD         PC
 *        PD           PC
 *       PD             PC
 *        PBPBPBPBPBPBPB
 *
 * Projekt; Wand Dj Beleuchtung
 *
 */

#include<avr/io.h>
#include<util/delay.h>

void main (void){

  DDRB=0xff;
  PORTB=0x00;
  DDRC=0xff;
  PORTC=0x00;
  DDRD=0xff;
  PORTD=0x00;

  while(1){

    //jede seite leuchtet kurz auf
    DDRB=0xff;
      _delay_ms(100);
    DDRB&=~0xff;
    DDRC=0xff;
      _dalay_ms(100);
    DDRC&=~0xff;
    DDRD=0xff;
      _delay_ma(100);
    DDRD&=~0xff;
      _delay_ms(250);

    // alle LEDS Blinken
    DDRB=0xff, DDRC=0xff, DDRD=0xff;
      _delay_ms(50);
    DDRB&=~0xff, DDRC&=~0xff, DDRD&=~0xff;
      _delay_ms(50);
    DDRB=0xff, DDRC=0xff, DDRD=0xff;
      _delay_ms(50);
    DDRB&=~0xff, DDRC&=~0xff, DDRD&=~0xff;
      _delay_ms(250);

    // jeder Port einzeln gegen Uhrzeiger
    DDRD=0xff;
      _delay_ms(100);
    DDRD&=~0xff;
    DDRB=0xff;
      _dalay_ms(100);
    DDRB&=~0xff;
    DDRC=0xff;
      _delay_ma(100);
    DDRC&=~0xff;
      _delay_ms(250);

    // jeder Pin einzeln
    for(int i=0; i<=7; i++)
    {
      PORTB=(1<<i);
        _delay_ms(50);
      PORTB=0x00;
    }
    if(i==7)
    {
      for(int o=0; o<=7; o++(
      {
        PORTB=0x00;
        PORTC=(1<<o);
          _delay_ms(50);
        PORTC=0x00;
      }
    }
    if(o==7)
    {
      for(int p=0; p<=7; p++)
      {
        PORTC=0x00;
        PORTD=(1<<p);
          _delay_ms(50);
        PORTD=0x00;
      }
    }

    // bei jedem Port gleichzeitig alle leds aufsteigend an
    DDRB&=~0xFF, DRC&=~0xFF, DDRD&=~0xFF;
      _delay_ms(50);
    DDRB=0x01, DDRC=0x01, DDRD=0x01;
      _delay_ms(50);
    DDRB=0x03, DDRC=0x03, DDRD=0x03;
      _delayl_ms(50);
    DDRB=0x07, DDRC=0x07, DDRD=0x07;
      _delay_ms(50);
    DDRB=0x0F, DDRC=0x0F, DDRD=0x0F;
      _delay_ms(50);
    DDRB=0x1F, DDRC=0x1F, DDRD=0x1f;
      _delay_ms(50);
    DDRB=0x3F, DDRC=0x3F, DDRD=0x3F;
      _delay_ms(50);
    DDRB=0x7F, DDRC=0x7F, DDRD=0x7F;
      _delay_ms(50);
    DDRB=0xFF, DDRC=0xFF, DDRD=0xFF;
      _delay_ms(100);

    // bei jedem Port gleichzeitig alle leds absteigend aus
    DDRB=0xFF, DDRC=0xFF, DDRD=0xFF;
      _delay_ms(50);
    DDRB=0x7F, DDRC=0x7F, DDRD=0x7F;
      _delayl_ms(50);
    DDRB=0x3F, DDRC=0x3F, DDRD=0x3F;
      _delay_ms(50);
    DDRB=0x1F, DDRC=0x1F, DDRD=0x1F;
      _delay_ms(50);
    DDRB=0x0F, DDRC=0x0F, DDRD=0x0F;
      _delay_ms(50);
    DDRB=0x07, DDRC=0x07 DDRD=0x07;
      _delay_ms(50);
    DDRB=0x03, DDRC=0x03, DDRD=0x03;
      _delay_ms(50);
    DDRB=0x01, DDRC=0x01, DDRD=0x01;
      _delay_ms(50);
    DDRB&=~0xFF, DRC&=~0xFF, DDRD&=~0xFF;
      _delay_ms(100);
  }
  return 1;
}


Getestet ist das Programm leider noch nicht, da ich mein Eclipse für Mac 
nicht richtig eingerichtet bekomme...was ich aber die nächsten Tage 
weiter in Angriff nehme um sobald wie möglich die Beleuchtung fertig 
stellen zu können!
Fotos kommen anschließend rein wenn es fertig ist :).

Vielen Dank und einen schönen Abend noch

[Kardoffelbrai (Gast)]

Mh..

ist es ein gutes Zeichen das sich hier keiner meldet und evtl. 
Veränderungsmöglichkeiten oder einfach nur Tipps gibt bzw. sagt?

[Master S. (snowman)]

weisst du, den meisten hier ist die repeatur der ständig kaputtgehenden 
glaskugel zu teuer geworden und raten ist einfach nicht so toll ...oder 
in anderen worten: woher sollen wir wissen, wie die dazugehörende HW 
aussieht? oder wo liegt dein problem bei dem wir helfen söll(t)en? oder 
was möchtest du ändern? oder glaubst du, jemand baut so schtell das 
nach, um dir dann zu sagen, was nicht geht. hey, überleg doch mal was du 
an unserer stelle mit diesem post machen würdest ^^
...viel glück!

[Kardoffelbrai (Gast)]

Naja allgemein ob der Code wie er dort ist richtig ist oder ob ich iwo 
nen Fehler rein gemacht habe und dann ob man Teile des Programms vlt. 
vereinfacht oder besser schreiben kann/sollte?

[dunno.. (Gast)]

naja vom ästhetischen her würde sich anbieten, sämtliche variablen 
(i,o,p) direkt am anfang des programmes zu deklarieren, und nicht mitten 
im code..

außerdem hat der mikrocontroller nunmal höchst wahrscheinlich 8bit, also 
würde es sich anbiete, unsigned char für die variablen zu benutzen... 
wozu riesige ints verballern, du arbeitest ja nicht auf dem pc.

des weiteren kannst du deine variablen i,o,p einfach wiederverwenden.. 
brauchst also eigentlich nur eine.

 for(int i=0; i<=7; i++)
    {
      PORTB=(1<<i);
        _delay_ms(50);
      PORTB=0x00;
    }
    if(i==7)
    {
      for(int o=0; o<=7; o++(
      {
        PORTB=0x00;
        PORTC=(1<<o);
          _delay_ms(50);
        PORTC=0x00;
      }
    }
    if(o==7)
    {
      for(int p=0; p<=7; p++)
      {
        PORTC=0x00;
        PORTD=(1<<p);
          _delay_ms(50);
        PORTD=0x00;
      }
    }

wozu die if abfragen? das programm verlässt die schleife eh erst, wenn 
der zähler = 7 ist, also ist das if ziemlich überflüssig..

mfg

[dunno.. (Gast)]

ach, und ganz übersehen:

DDRB&=~0xFF, DRC&=~0xFF, DDRD&=~0xFF;

das hier sind alles eigenständige anweisungen, durch ; zu trennen. so 
macht das der compiler sicher nicht mit.

tippfehler sind auch drin, (siehe die zitierte zeile) aber das wird der 
compiler auch ankreiden..

[... (Gast)]

dunno.. schrieb:
> DDRB&=~0xFF, DRC&=~0xFF, DDRD&=~0xFF;
>
> das hier sind alles eigenständige anweisungen, durch ; zu trennen. so
> macht das der compiler sicher nicht mit.

Mal abgesehen vom Tippfehler bei DDRC macht der Kompiler das sehr wohl 
mit. Auch wenn der Komma-Operator hier eher fehl am Platz ist und ein ; 
sinvoller, funktionieren tuts auch so.

Aber spätesten bei der Zeile "if(o==7)" wird der Kompiler tatsächlich 
weinen (im Originalcode schon bei "if(i==7)"), weil er die Variablen an 
der Stelle nicht kennt.

[Peter D. (peda)]

Ich mag keinen Spaghetticode.

Für Ausgabe von Mustern nehme ich gerne eine Tabelle:

Beitrag "AVR Sleep Mode / Knight Rider"

Man ist damit sehr flexibel und kann schnell Änderungen vornehmen.
Bei längeren Mustern muß man allerdings beim AVR-GCC etwas umständlich 
die Tabelle in den Flash auslagern.
Vermutlich benutzt Du einen ATmega32, da kann man schon sehr große 
Muster definieren, ehe der RAM ausgeht.


Peter

[Kardoffelbrai (Gast)]

Schönen guten Morgen,

danke für die Zahlreichen Kommentare.
Okey dann war das ein Denkfehler mit der if Anweisung. Ich werde die 
Fehler gleich einmal ausbessern, genauso wie diese DDRx Geschichte werde 
ich direkt überarbeiten und im Anschluss schau ich mal was es mit dem 
"unsigned char" aufsich hat und verusch es im Programm einzubinden!

Vielen Dank

[Rolf Magnus (Gast)]

dunno.. schrieb:
> naja vom ästhetischen her würde sich anbieten, sämtliche variablen
> (i,o,p) direkt am anfang des programmes zu deklarieren, und nicht mitten
> im code..

Das ist Ansichtssache. Ich finde es besser, wenn sie da definiert 
werden, wo sie benutzt werden und nicht irgendwo anders. So hat man 
alles, was zusammen gehört, auch zusammen. In C++ ist das üblich so, 
weil es dort (im Gegensatz zu C) schon von Anfang an möglich war.

> des weiteren kannst du deine variablen i,o,p einfach wiederverwenden..
> brauchst also eigentlich nur eine.

Welchen Vorteil hat das?

Ich hätte noch was:

PORTB=(1<<i);

das ist eine recht teure Operation, da der AVR keinen Barrel Shifter 
hat. Das heißt, daß er nicht um i schieben kann, sondern nur um 1. Der 
Compiler muß  aus diesem Code also eine Schleife machen, die i mal um 1 
schiebt. Da du danach sowieso 50 Millsekunden wartest, ist das in diesem 
Fall nicht weiter tragisch, aber prinzipiell würde ich das eher so 
machen:

    uint_fast8_t wert = 1;
    for(int i=0; i<=7; i++)
    {
        PORTB=wert;
        wert <<= 1;
      _delay_ms(50);
       PORTB=0x00;
    }

oder gleich Wert für Port als quasi-Schleifenzähler verwenden:

    for(uint8_t i = 1; i != 0; i <<= 1)
    {
        PORTB=i;
      _delay_ms(50);
       PORTB=0x00;
    }

Für den Schleifenabbruch ist es notwendig, daß die Schleifenvariable 
genau 8 Bit breit ist. Dadurch wird der Wert 0, sobald die 1 einmal 
komplett durchgeschoben wurde und sozusagen "oben rausfällt".

[Kardoffelbrai (Gast)]

Ah okey danke!
Ich habe das so gemacht
PORTB=(1<<i);
weil es so in C Programmier Buch stand, darum dachte ich es ist so in 
Ordnung. Aber gut dann werde ich es mal so versuchen, da dies was du 
geschrieben hast mir sehr plausiebel ercheint. Ich werde mich in der 
Richtung noch ein wenig weiter Infromieren!

Gruß