Weihnachtsstern stürzt ab

[Christian S. (schoker)]

Hallo,

bisher konnte ich alle auftretenden Fehler und Probleme selber lösen, 
indem ich einfach mal die Weiten des Internets genutzt habe um zu 
gewissen Problematiken zu recherchieren. Doch jetzt habe ich schon eine 
Woche vergeblich versucht dieses Problem zu lösen.

Zunächst einmal eine kleine Projektbeschreibung:

Ich habe einen Weihnachtsstern gebaut, welcher aus 5*8 LEDs besteht. 
Diese LEDs hängen an 5 in Reihe geschalteten Schieberegistern (74HC595, 
in welche ich das aktuelle Leuchtbild seriell reinschiebe und dann auf 
dessen Ausgänge übertrage. Das ganze wird mit einem ATTiny45 gesteuert. 
Dazu gibt es noch einen Eingang als Interrupt, um zwischen verschiedenen 
Blinkmustern zu wechseln. Deweiteren gibt es noch einen Spannungsteiler 
mit einem lichtabhängigen Widerstand, der erkennt, ob es Hell oder 
Dunkel ist. Um somit die LEDs ausschalten zu können.

Jetzt tritt folgendes Phänomen auf: Immer wenn alle LEDs ausgeschaltet 
werden sollen, dann stürzt der ATTiny45 ab und das Programm startet sich 
von vorne.
Soweit konnte ich es jedenfalls schon einschränken, denn wenn ich das 
Programm so umschreibe, dass niemals alle LEDs ausgeschaltet sind, dann 
funktioniert es einwandfrei.

Den Code der einzelnen Modi (Blinkmuster) habe ich mal bewusst entfernt, 
da dieser meiner Einschätzung und Tests nichts mit dem Problem zu tun 
hat.
Meiner Meinung nach arbeit die Funktion LEDout() nicht richtig, wenn 
diese nur Nullen an die Schieberegister schicken soll, doch ich kann mir 
überhaupt nicht erklären warum.#

Ich habe zwar noch bis Weihnachten Zeit, aber ich hoffe echt, dass mir 
hier jemand bei meinem Problem helfen kann.

Hier nun der dazugehörige Code:

/*
 * ChristmasStar.c
 *
 * Created:        19.07.2012
 * Latest Modification:  17.10.2012
 *
 * Author: Christian Schoknecht
 */ 

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

#define F_CPU    8000000UL
#define modicounter  8
#define SI      0
#define RCK      3
#define SCK      4

volatile char interrupt_counter = 0;
volatile char flag        = 0;
volatile uint8_t LED0      = 0x00;
volatile uint8_t LED1      = 0x00;
volatile uint8_t LED2      = 0x00;
volatile uint8_t LED3      = 0x00;
volatile uint8_t LED4      = 0x00;
volatile uint8_t counter    = 9;
volatile uint8_t day      = 0;
volatile uint8_t cache      = 0x00;
volatile uint8_t steps      = 0;
volatile uint8_t random_mode  = 0;

/* function prototypes */
void setBit(uint8_t bit);
void resetBit(uint8_t bit);
void LEDout()__attribute__((optimize(0)));
void resetLEDs();
void random_();
void running();
void circle();
void circle2();
void snail();
void rider();
void blinking();
void filling();

/* function to set a single bit */
void setBit(uint8_t bit)
{
  PORTB |= (1<<bit);
}

/* function to reset a single bit */
void resetBit(uint8_t bit)
{
  PORTB &= ~(1<<bit);
}

/* function for serial output */
/* output of the current content of the LED-Array */
void LEDout()
{
  uint8_t i;    // i for 8 LEDs
  uint8_t LEDcache0, LEDcache, LEDcache2, LEDcache3, LEDcache4;
  
  LEDcache0 = LED0;
  LEDcache1 = LED1;
  LEDcache2 = LED2;
  LEDcache3 = LED3;
  LEDcache4 = LED4;
  
    //loops for the 8 LEDs in each shift register
    for (i=0; i<8; i++)
    {
      // write Date to SI-PIN
      if(LEDcache4 >= 0x80)
      {
        setBit(SI);
      }
      else
      {
        resetBit(SI);
      }
      LEDcache4 = LEDcache4 << 1;
      resetBit(SCK);
      setBit(SCK);
    }  
    for (i=0; i<8; i++)
    {
      // write Date to SI-PIN
      if(LEDcache3 >= 0x80)
      {
        setBit(SI);
      }
      else
      {
        resetBit(SI);
      }
      LEDcache3 = LEDcache3 << 1;
      resetBit(SCK);
      setBit(SCK);
    }  
    for (i=0; i<8; i++)
    {
      // write Date to SI-PIN
      if(LEDcache2 >= 0x80)
      {
        setBit(SI);
      }
      else
      {
        resetBit(SI);
      }
      LEDcache2 = LEDcache2 << 1;
      resetBit(SCK);
      setBit(SCK);
    }
    for (i=0; i<8; i++)
    {
      // write Date to SI-PIN
      if(LEDcache1 >= 0x80)
      {
        setBit(SI);
      }
      else
      {
        resetBit(SI);
      }
      LEDcache1 = LEDcache1 << 1;
      resetBit(SCK);
      setBit(SCK);
    }
    for (i=0; i<8; i++)
    {
      // write Date to SI-PIN
      if(LEDcache0 >= 0x80)
      {
        setBit(SI);
      }
      else
      {
        resetBit(SI);
      }
      LEDcache0 = LEDcache0 << 1;
      resetBit(SCK);
      setBit(SCK);
    }
  resetBit(RCK);
  setBit(RCK);
}

// switch off all LEDs
void resetLEDs()
{
  LED0 = 0x00;
  LED1 = 0x00;
  LED2 = 0x00;
  LED3 = 0x00;
  LED4 = 0x00;
  LEDout();
}

// Mode 0
void random_()
{
...
}

// Mode 1
void running()
{
...
}

// Mode 2
void circle()
{
...
}

// Mode 3
void rider()
{
...
}

// Mode 4
void snail()
{
...
}

// Mode 5
void blinking()
{
...
}

// Mode 6
void filling()
{
...
}

// Mode 7
void circle2()
{  
...
}


int main()
 {
   DDRB = 0x19;            // 7 6 5 4   3   2    1    0
                    // 0 0 0 1   1   0    0    1
                    // - - - SCK RCK ADC1 PCINT1 SI
   
   PCMSK |= (1<<PCINT1);        // Set Pin 1 (PB1) as the pin for the interrupt
   GIMSK |= (1<<PCIE);        // Enable Pin Change Interrupt
   
   // Timer1 Configuration
   TCCR1 |= (1<<CS13) | (1<<CS12) | (1<<CS11) | (1<<CS10);  // prescale of 16384   
   TIMSK |= (1<<TOIE1);        // enable overflow interrupt
   
   // ADC1 Configuration
   ADMUX |= (1<<ADLAR);        // adjust result left: value is in ADCH
   ADMUX |= (1<<MUX0);        // select ADC1
   
   ADCSRA |= (1<<ADEN);        // ADC enable
   ADCSRA |= (1<<ADIE);        // ADC interrupt enable   
   
   sei();                // enable global interrupt
  
  while(1)
  {
    if (day == 0)
    {    
      switch(interrupt_counter)
      {
        case 0:
        {
          random_();
          break;
        }
        case 1:
        {
          running();
          break;
        }
        case 2:
        {
          circle();
          break;
        }        
        case 3:
        {
          rider();
          break;
        }
        case 4:
        {
          snail();
          break;
        }
        case 5:
        {
          blinking();
          break;
        }
        case 6:
        {
          filling();
          break;
        }
        case 7:
        {
          circle2();
          break;
        }
      }
    }
    else
    {
      resetLEDs();
      _delay_ms(250);
    }    
  }
  return 1;
}


// Interrupt service routines
// Interrupt external switch
ISR(PCINT0_vect)
{
  if (PINB & (1<<PB1))
  {
    if(interrupt_counter == modicounter-1)
    {
      interrupt_counter = 0;
    }
    else
    {
      interrupt_counter += 1;
    }
    
    switch(interrupt_counter)
    {
      case 0:    LED0 = 0x01;
      break;
      case 1:    LED0 = 0x02;
      break;
      case 2:    LED0 = 0x04;
      break;
      case 3:    LED0 = 0x08;
      break;
      case 4:    LED0 = 0x10;
      break;
      case 5:    LED0 = 0x20;
      break;
      case 6:    LED0 = 0x40;
      break;
      case 7:    LED0 = 0x80;
      break;
    }
    LED1 = LED0;
    LED2 = LED0;
    LED3 = LED0;
    LED4 = LED0;
    LEDout();
    _delay_ms(1000);
    resetLEDs();
    _delay_ms(500);
  }
}

// Timer interrupt
ISR(TIM1_OVF_vect)
{
  ADCSRA |= (1<<ADSC);   // starting ADC conversion
}

// ADC conversion complete interrupt
ISR(ADC_vect)
{
  // switching between day and night with a
  if (ADCH <= 0x80)
  {
    day = 1;
  }
  if (ADCH >= 0xF0)
  {
    day = 0;
  }
  
}

[rggre (Gast)]

1,5s delay in der ISR? Nix gut. Flag setzen und ab in die main damit.

[holger (Gast)]

Du benutzt

    resetLEDs();

in einer ISR und in der main Loop.
Das macht man nicht!

[bko (Gast)]

Könnte auch ein Hardware Problem sein?

>Jetzt tritt folgendes Phänomen auf: Immer wenn alle LEDs ausgeschaltet
>werden sollen, dann stürzt der ATTiny45 ab und das Programm startet sich
>von vorne.
>Soweit konnte ich es jedenfalls schon einschränken, denn wenn ich das
>Programm so umschreibe, dass niemals alle LEDs ausgeschaltet sind, dann
>funktioniert es einwandfrei.

Wie sieht der Schaltplan aus, sind genug Blockkondensatoren
an den IC's speziell an den 74HC595 ?

--
[offtopic]
Irgentwie klingt der Titel nach "Ende der Welt" ;-)

[Da D. (dieter)]

bko schrieb:
> --
> [offtopic]
> Irgentwie klingt der Titel nach "Ende der Welt" ;-)

Ich dachte bei dem Titel eher an eine zu schmale Fensterbank ;-)

[Christian S. (schoker)]

rggre schrieb:
> 1,5s delay in der ISR? Nix gut. Flag setzen und ab in die main damit.

Das dient derzeit nur zum Testen und direktem erkennen, ob übe

bko schrieb:
> Könnte auch ein Hardware Problem sein?

> Wie sieht der Schaltplan aus, sind genug Blockkondensatoren
> an den IC's speziell an den 74HC595 ?

Das würde ich erstmal bezweifeln, denn wenn ich in der 
ResetLED()-Funktion zwei der LED-Warte auf 0x01 setze, egal welche 
davon, dann stürzt das Programm nicht ab, dann erst wieder wenn in einem 
Leuchtmuster mal alles aus muss.
Mit 16 LEDs und einem ATMega8 sowie einem 89C51 hatte es bereits 
funktioniert und die Steuerung habe ich jetzt halt nur auf 5 
Schieberegister erweitert und dann einen µController gewählt, der 
günstig ist und mir alle Funktionen liefert, die ich brauche (Also auch 
einen ADC).

Schaltplan müsste ich mal zeichnen, aber bei den 16 LEDs hat es wie 
gesagt auch ohne Blockkondensatoren wunderbar funktioniert.

[R. M. (rmax)]

bko schrieb:
> Wie sieht der Schaltplan aus, sind genug Blockkondensatoren
> an den IC's speziell an den 74HC595 ?

Da würde ich die Probleme aber beim Einschalten aller LEDs erwarten, 
nicht beim Ausschalten.

[bko (Gast)]

@ Christian Schoknecht
Es werden also 40-LEDs auf einmal abgeschaltet?
Dies könnte auch einen kurzen Überschwinger in der
Versorgung verursachen welche den UC stören...

Ohne Bild von Layout oder Schaltplan ist das Rätselraten...

Aber 100nF an jedem HC-MOS IC sind eigtl. üblich:
Beitrag "74hct595 100nF Kondensator"

Teste dies: Schalte nicht alle auf einmal ab,
            sondern kurz (alle 100 usec oder so)
            nacheinander jeweils z.B. 8 Stück
            und siehe was passiert.

Wenn dies geht, ists doch recht Wahrscheinlich die Versorgung

[Konrad S. (maybee)]

Da Dieter schrieb:
>> [offtopic]
>> Irgentwie klingt der Titel nach "Ende der Welt" ;-)
>
> Ich dachte bei dem Titel eher an eine zu schmale Fensterbank ;-)

Ein Fall für das Astronomie-Forum von http://www.raumfahrer.net ?  ;-)

[Christian S. (schoker)]

bko schrieb:
> Es werden also 40-LEDs auf einmal abgeschaltet?
> Dies könnte auch einen kurzen Überschwinger in der
> Versorgung verursachen welche den UC stören...

Nicht in jedem Fall, manchmal werden auch nur zwei oder drei einzeln 
ausgeschaltet, das hängt ja vom Blink-Modus ab. Oder momentan nach 
Verlassen der ISR werden immer 5 abgeschaltet und da hängt es sich auf.

Schaltplan liefere ich aber gleich nach.

[Christian S. (schoker)]

bko schrieb:
> Teste dies: Schalte nicht alle auf einmal ab,
>             sondern kurz (alle 100 usec oder so)
>             nacheinander jeweils z.B. 8 Stück
>             und siehe was passiert.
>
> Wenn dies geht, ists doch recht Wahrscheinlich die Versorgung

Das bringt zumindest nach dem ersten Test Besserung, muss das Programm 
jetzt nur mal umschreiben, um zu sehen, ob dies überall der Fall ist.

Edit: Jap, damit funktioniert es. Okay, vielen Dank, dann kann komme ich 
jetzt weiter.

[Christian S. (schoker)]

So, hier jetzt der Schaltplan

[holger (Gast)]

>So, hier jetzt der Schaltplan

Null Abblockkondensatoren an den ICs?
Dann ist es kein Wunder das deine Schaltung spinnt.

[Christian S. (schoker)]

Okay, dann muss ich jetzt mal auf meiner Platine noch Platz suchen, bei 
den weiteren Exemplaren kann ich das Layout dann ja anpassen, so dass es 
auch gleich funktioniert.

Ich sehe gerade, dass es ja auch Sockel mit Abbklockkondensator gleich 
gibt. Okay, ich werde dann berichten, ob das ganze das Problem löst.

[Route_66 H. (route_66)]

Hallo!
>  uint8_t LEDcache0, LEDcache, LEDcache2, LEDcache3, LEDcache4;

>  LEDcache0 = LED0;
>  LEDcache1 = LED1;
>  LEDcache2 = LED2;
>  LEDcache3 = LED3;
>  LEDcache4 = LED4;

Wo wird eigentlich "LEDcache1" definiert?

[Christian S. (schoker)]

Das ist mir beim Übertragen wohl ein Fehler passiert.

Ich hatte nämlich noch paar unnötige Zeilen und Kommentare rausgelöscht 
und dabei alles in eine Zeile geschrieben. LEDcache soll natürlich 
LEDcache1 sein.

So Kondensatoren sind bestellt, in der größe, die noch auf meine Platine 
passt waren die nämlich bei Conrad nicht vorrätig. Ergebnisse folgen.

[Karl H. (kbuchegg)]

Christian Schoknecht schrieb:
> Das ist mir beim Übertragen wohl ein Fehler passiert.
>
> Ich hatte nämlich noch paar unnötige Zeilen und Kommentare rausgelöscht

Da geht noch einiges mehr. Hast du den Kopf frei für 
Programmvereinfachungen?

[Christian S. (schoker)]

Karl Heinz Buchegger schrieb:
> Da geht noch einiges mehr. Hast du den Kopf frei für
> Programmvereinfachungen?

Prinzipiell ja, wollte bisher nur erstmal ein Programm erzeugen, dass 
auch funktioniert und das auch noch übersichtlich ist, bevor ich da 
irgendwelche Vereinfachungen reinbaue.

[Karl H. (kbuchegg)]

Christian Schoknecht schrieb:
> Karl Heinz Buchegger schrieb:
>> Da geht noch einiges mehr. Hast du den Kopf frei für
>> Programmvereinfachungen?
>
> Prinzipiell ja, wollte bisher nur erstmal ein Programm erzeugen, dass
> auch funktioniert und das auch noch übersichtlich ist, bevor ich da
> irgendwelche Vereinfachungen reinbaue.


Genau darum gehts :-)
Die Vereinfachungen sind nicht von der Kategorie 'möglichst viel 
tricksen', sondern von der Kategorie:
Eine 5-zeilige Funktion ist leichter zu überblicken, zu verstehen und im 
Fehlerfall zu analysieren, als derselbe Sachverhalt auf 2 
Bildschirmseiten :-)

[Karl H. (kbuchegg)]

zb.

ISR(PCINT0_vect)
{
  uint8_t LedCodes[] = { 0x01, 0x02, 0x04, 0x08, 0x10, 0x20, 0x40, 0x80 };

  if (PINB & (1<<PB1))
  {
    interrupt_counter++;
    if(interrupt_counter == modicounter)
    {
      interrupt_counter = 0;
    }

    LED0 = LedCodes[modicounter];

    LED1 = LED0;
    LED2 = LED0;
    LED3 = LED0;
    LED4 = LED0;
    LEDout();
    _delay_ms(1000);
    resetLEDs();
    _delay_ms(500);
  }
}

[Karl H. (kbuchegg)]

zb

void SingleLEDout( uint8_t value )
{
  uint8_t i;

  for (i=0; i<8; i++)
  {
    if(value & 0x80)
    {
      setBit(SI);
    }
    else
    {
      resetBit(SI);
    }
    value = value << 1;
    resetBit(SCK);
    setBit(SCK);
  }  
}

/* function for serial output */
/* output of the current content of the LED-Array */
void LEDout()
{
  cli();  // sicher stellen, dass auch wirklich ALLE LEDx Variablen
          // in sich konsistent sind!

  uint8_t LEDcache0 = LED0;
  uint8_t LEDcache1 = LED1;
  uint8_t LEDcache2 = LED2;
  uint8_t LEDcache3 = LED3;
  uint8_t LEDcache4 = LED4;

  sei();  
  
  SingleLEDout( LEDcache4 );
  SingleLEDout( LEDcache3 );
  SingleLEDout( LEDcache2 );
  SingleLEDout( LEDcache1 );
  SingleLEDout( LEDcache0 );

  resetBit(RCK);
  setBit(RCK);
}

[Christian S. (schoker)]

Das ist natürlich bei gleicher Funktion wirklich deutlich kürzer.

[Karl H. (kbuchegg)]

Mal ein Versuch

    if (day == 0)
    {    
      switch(interrupt_counter)
      {
        case 0:
        {
          random_();
          break;
        }
        case 1:
        {
          running();
          break;
        }
        case 2:
        {
          circle();
          break;
        }        
        case 3:
        {
          rider();
          break;
        }
        case 4:
        {
          snail();
          break;
        }
        case 5:
        {
          blinking();
          break;
        }
        case 6:
        {
          filling();
          break;
        }
        case 7:
        {
          circle2();
          break;
        }
      }

versus

    if (day == 0)
    {    
      if     ( interrupt_counter == 0 )     random_();
      else if( interrupt_counter == 1 )     running();
      else if( interrupt_counter == 2 )     circle();
      else if( interrupt_counter == 3 )     rider();
      else if( interrupt_counter == 4 )     snail();
      else if( interrupt_counter == 5 )     blinking();
      else if( interrupt_counter == 6 )     filling();
      else if( interrupt_counter == 7 )     circle2();
    }

welche Variante gefällt dir besser?
Was mir an deiner nicht gefällt: das sie extrem in die Länge gezogen 
ist, ohne das es dafür einen guten Grund gibt.

switch-case ist nicht immer in der Lesbarkeit besser.

(Und ja: manchmal darf man sich auch über Formatiervorschriften 
hinwegsetzen, WENN es dafür einen extrem guten Grund gibt. Hier ist es 
mir wichtig, dass die gedankliche Zuordnung von 0, 1, 2, 3, ... zur 
aufgerufenen Funktion schnell, leicht und einfach herzustellen ist. Denn 
im Prinzip hast du es ja hier mit einer 'Tabelle' zu tun. In der einen 
Tabellenspalte steht 'Indexnummer', in der anderen Spalte die 
aufzurufende Funktion. Genau das spiegelt aber auch diese auf den ersten 
Blick seltsame Struktur wieder. WEnn man sich die Formatierung aber mal 
genau ansieht, dann macht sie absolut Sinn.)

[Christian S. (schoker)]

Wobei das ja auch so funktionieren würde:

if (day == 0)
    {    
      switch(interrupt_counter)
      {
        case 0: random_;  break;
        case 1: running();  break;
        case 2: circle();  break;
        case 3: rider();  break;
        case 4: snail();  break;
        case 5:  blinking();  break;
        case 6: filling();  break;
        case 7: circle2();  break;
      }
    }

Was allerdings nicht funktioniert hatte: Ich hatte erst LED0-4 als Array 
deklariert und dann eine Schleife in LEDout(), die LED[0] bis LED[4] 
nacheinander abarbeiten soll, und das hat nicht funktioniert, nehme an, 
dass es falsch optimiert worden ist vom Compiler.

[R. M. (rmax)]

... oder die Funktionen in ein Array stecken und interrupt_counter als 
Index verwenden.

[Karl H. (kbuchegg)]

Christian Schoknecht schrieb:
> Wobei das ja auch so funktionieren würde:

Ja - Genau.


Nicht falsch verstehen.
Ich bin ein absoluter Verfechter von Formatierungsvorschriften.
Aber - es gibt auch Fälle, in denen man sie 'etwas zurecht biegt' und 
dafür einen Zusatznutzen bekommt.

[Christian S. (schoker)]

Ich verstehe was du meinst.

Wenn man mit einem großen Monitor arbeitet, dann stört es einen weniger, 
wenn man da ein paar Zeilen scrollen muss bzw. in Editoren die gerade 
nicht benötigten Funktionen einklappt.

Deshalb fällt es ja kaum auf. Hier ist es natürlich klar, wenn das 
jemand liest und das so lang ist, obwohl es kürzer sein könnte, dann ist 
das eventuell ärgerlich.

[Karl H. (kbuchegg)]

Christian Schoknecht schrieb:
> Ich verstehe was du meinst.
>
> Wenn man mit einem großen Monitor arbeitet, dann stört es einen weniger,
> wenn man da ein paar Zeilen scrollen muss bzw. in Editoren die gerade
> nicht benötigten Funktionen einklappt.
>
> Deshalb fällt es ja kaum auf. Hier ist es natürlich klar, wenn das
> jemand liest und das so lang ist, obwohl es kürzer sein könnte, dann ist
> das eventuell ärgerlich.

Nicht nur. Es wird auch dir irgendwann sauer aufstossen, warum deine 
LEDout Funktion so dermassen in die Länge gezogen ist. Und es wird der 
Tag kommen, an dem du dich fragst, ob eigentlich alle 5 'Ausgabestufen' 
in dieser Funktion tatsächlich Buchstabe für Buchstabe identisch sind 
(abgesehen von dem einen Variablennamen) oder nicht. (Spätestens dann, 
wenn du beobachtest, dass sich SR Nummero 3 aus irgendeinem Grund anders 
verhält)

Dabei hättest du dir mit einer Funktion, die die Ausgabe für 1 SR macht, 
die dann 5 mal aufgerufen wird, von vorne herein gleich mal Zeit 
gespart. Und diese Frage wird auch nie aufgekommen.

Es ist ein Trugschluss zu glauben: "Och, das räum ich später dann schon 
irgendwann mal auf." Denn, Hand aufs Herz, ... es passiert nicht.

[R. M. (rmax)]

Hier noch der Code zu meinem letzten Vorschlag. Er wirft beim 
Compilieren keine Warnungen, weiter getestet habe ich ihn aber nicht.

typedef void (bfunc)(void);
bfunc *bfuncs[] = {
    resetLEDs, random_, running, circle,
    snail, rider, blinking, filling
};

// ...

    if (day == 0)
    {
        bfuncs[interrupt_counter]();
    }

[Karl H. (kbuchegg)]

Wobei man ja auch noch die Frage aufwerfen kann:
Müssen das eigentlich eigene Funktionen sein?

Ist es nicht möglich die einzelnen Muster (mit Ausnahme des 
Zufallsmusters in random) nicht in einer gemeinsamen Basis darzustellen?

(Ich will wieder mal auf ein Array raus, in dem die Abfolge der zu 
leuchtenden LEDs festgelegt ist)

Denn: dann wird auch das Hinzufügen eines neuen Musters trivial und 
beschränkt sich einfach nur auf das Hinzufügen von ein paar Einträgen in 
einem Array und dem Anpassen von ein paar #define.



:-)
Und wenn man R. Max-es Vorschlag und meine zusammennimmt, dann könnte 
man daraus auch einen gemeinsamen Nenner ziehen: Es ist oft gar nicht so 
verkehrt, anstelle von immer gleichen Sequenzen Arrays einzusetzen, wenn 
man eine einfache Möglichkeit hat, die Unterschiede einfach nur in den 
Daten festzunageln.

[Christian S. (schoker)]

Da hast du natürlich recht und deshalb ist es so dann wieder 
übersichtlicher:

void BitImpulse(uint8_t bit)
{
  resetBit(bit);
  setBit(bit);
}

void SRout(uint8_t SR)
{
  uint8_t  LEDcache, i;
  LEDcache = LED[SR];
  
  for (i=0; i<8; i++)
  {
    // write Date to SI-PIN
    if(LEDcache >= 0x80)
      setBit(SI);
    else
      resetBit(SI);
    LEDcache = LEDcache << 1;
    BitImpulse(SCK);
  }
}

/* function for serial output */
/* output of the current content of the LED-Array */
void LEDout()
{
  SRout(4); SRout(3); SRout(2); SRout(1); SRout(0);
  
  BitImpulse(RCK);
}

[Karl H. (kbuchegg)]

Christian Schoknecht schrieb:

> void SRout(uint8_t SR)
> {
>   uint8_t  LEDcache, i;
>   LEDcache = LED[SR];

Ah,
Ich sehe, du bist auf ein Array übergegangen anstelle der LED0, LED1, 
etc.

gute Entscheidung :-)


PS: Gefällt mir, das du da mitmachst.

[Christian S. (schoker)]

Das hatte ich auch vorher überlegt, weil das mit einem zweidimensionalen 
Array gut machbar wäre, Zeilen für das Muster, Spalte für die einzelnen 
Schieberegister, aber allein der Modus snail() hätte dann schon 5*40 
Einträge. Von der Geschwindigkeit mag das ein enormer Vorteil sein. Der 
ist aber durch die vielen Verzögerungen in diesem Fall nicht so wichtig.

Aber bei Interesse kann ich auch gerne noch mal die Blink-Modi posten. 
Habe diese gerade auch schon der Übersichtlichkeit etwas umformatiert.
Denn mein Code ist inzwischen fast nur noch halb so lang.

[Christian S. (schoker)]

Karl Heinz Buchegger schrieb:
> Ah,
> Ich sehe, du bist auf ein Array übergegangen anstelle der LED0, LED1,
> etc.
>
> gute Entscheidung :-)


Hatte ich vorher auch schon mal. Bei der Fehlersuche, warum die 
verschachtelten Schleifen nicht funktionierten, habe ich dann einzelne 
Werte daraus gemacht. Was keine Besserung brachte und hatte es bisher 
nur noch nicht zurückgeändert.

In dem Fall mit dem Array ist das Programm natürlich leichter zu 
modifzieren, wenn man irgendwann einen sechszackigen oder 4 zackigen 
Stern bauen will.

R. Max schrieb:
> Hier noch der Code zu meinem letzten Vorschlag. Er wirft beim
> Compilieren keine Warnungen, weiter getestet habe ich ihn aber nicht.typedef 
void (bfunc)(void);
> bfunc *bfuncs[] = {
>     resetLEDs, random_, running, circle,
>     snail, rider, blinking, filling
> };
>
> // ...
>
>     if (day == 0)
>     {
>         bfuncs[interrupt_counter]();
>     }

Das ist auch eine nette Idee, aber das wird mir dann gänzlich 
unübersichtlich.

[Karl H. (kbuchegg)]

Christian Schoknecht schrieb:
> Das hatte ich auch vorher überlegt, weil das mit einem zweidimensionalen
> Array gut machbar wäre, Zeilen für das Muster, Spalte für die einzelnen
> Schieberegister, aber allein der Modus snail() hätte dann schon 5*40
> Einträge.

OK. Ich gesth dir natürlich zu, dass wenn es eine einfache Möglichkeit 
gibt ein Muster algorithmisch zu generieren, dann ist das sicherlich 
einfacher, als so was.

#define ARRAY_SIZE(x)  ((sizeof(x)/sizeof(*x))
struct Frame
{
  uint8_t byte[5];
};

struct Muster
{
  uint8_t       nrFrames;
  struct frame* frames;
};

struct Frame snailPattern[] =
{
  { 0b00000000, 0b00000000, 0b00000000, 0b00000000, 0b00000000 },
  { 0b00000000, 0b00000000, 0b00000000, 0b00000000, 0b00000001 },
  { 0b00000000, 0b00000000, 0b00000000, 0b00000000, 0b00000011 },
  { 0b00000000, 0b00000000, 0b00000000, 0b00000000, 0b00000111 },
  { 0b00000000, 0b00000000, 0b00000000, 0b00000000, 0b00001111 },
  { 0b00000000, 0b00000000, 0b00000000, 0b00000000, 0b00011110 },
  { 0b00000000, 0b00000000, 0b00000000, 0b00000000, 0b00111100 },
  { 0b00000000, 0b00000000, 0b00000000, 0b00000000, 0b01111000 },
  { 0b00000000, 0b00000000, 0b00000000, 0b00000000, 0b11110000 },
  { 0b00000000, 0b00000000, 0b00000000, 0b00000001, 0b11100000 },
  { 0b00000000, 0b00000000, 0b00000000, 0b00000011, 0b11000000 },
  { 0b00000000, 0b00000000, 0b00000000, 0b00000111, 0b10000000 },
  { 0b00000000, 0b00000000, 0b00000000, 0b00001111, 0b00000000 },
  { 0b00000000, 0b00000000, 0b00000000, 0b00011110, 0b00000000 },
  { 0b00000000, 0b00000000, 0b00000000, 0b00111100, 0b00000001 },
  { 0b00000000, 0b00000000, 0b00000000, 0b01111000, 0b00000010 },
  { 0b00000000, 0b00000000, 0b00000000, 0b11110000, 0b00000100 },
  { 0b00000000, 0b00000000, 0b00000001, 0b11100000, 0b00001000 },
  { 0b00000000, 0b00000000, 0b00000011, 0b11000000, 0b00010000 },
  ...
};

struct Muster snail = { ARRAY_SIZE( snailPattern ), snailPattern };


...

void playIt( struct Muster* muster )
{
  uint8_t i, j;

  for( i = 0; i < muster->nrFrames; i++ )
  {
    for( j = 0; j < 5; j++ )
      LED[j] = muster->frames[i][j];
  }
}

> Von der Geschwindigkeit mag das ein enormer Vorteil sein.

Gar nichtg mal so sehr.
Wenn du dir das Beispiel ansiehst, dann sieht man auf einen Blick sehr 
schnell, wie das Muster eigentlich aussieht, weil man die 1-en in der 
Initialisierung gut verfolgen kann.
D.h. das Musterdesign ist viel einfacher, weil ich einfach in diesen 
(zugegebenen) Datenwust, die 1-en da hin setzte, wo ich sie haben will 
ohne lang rumwurschteln zu müssen. Im Beispiel läuft einer 4er Schlange 
eine einzelne LED nach. Algorithmisch ist das gar nicht mehr so einfach. 
Will ich dann vielleicht von der Gegenseite auch noch eine Schlange 
einlaufen lassen, dann ist das im Array einfach. Ich schreib einfach die 
1-en dort hin, wo ich sie brauche :-)

[R. M. (rmax)]

Karl Heinz Buchegger schrieb:

> Es ist oft gar nicht so verkehrt, anstelle von immer gleichen
> Sequenzen Arrays einzusetzen, wenn man eine einfache Möglichkeit
> hat, die Unterschiede einfach nur in den Daten festzunageln.

Tja, und der nächste Schritt wäre dann, die Arrays nicht mehr im Code 
unterzubringen, sondern im EEPROM, so daß man sie ändern kann, ohne den 
ganzen Controller neu zu flashen.

[Christian S. (schoker)]

Karl Heinz Buchegger schrieb:
> OK. Ich gesth dir natürlich zu, dass wenn es eine einfache Möglichkeit
> gibt ein Muster algorithmisch zu generieren, dann ist das sicherlich
> einfacher, als so was.

Für die snail()-Funktion in diesem Fall schon:

// Mode 4
void snail()
{
  if(LED[4] == 0xFF)
  {
    LED[0] = 0x01; LED[1] = 0x00; LED[2] = 0x00; LED[3] = 0x00; LED[4] = 0x00; 
  }
  if(LED[4] == LED[0])
  {
    LED[0] = (LED[0]*2)+1;
  }  
  else
  {
    LED[4] = LED[3]; LED[3] = LED[2]; LED[2] = LED[1]; LED[1] = LED[0];
  }
  LEDout();
  _delay_ms(100);
}

[Karl H. (kbuchegg)]

> Aber bei Interesse kann ich auch gerne noch mal die Blink-Modi posten.

Ein simples Wechselblinken würde dann so aussehen (neuer Modus)

struct Frame blink4Pattern[] =
{
  { 0b00000011, 0b00000011, 0b00000011, 0b00000011, 0b00000011 },
  { 0b00001100, 0b00001100, 0b00001100, 0b00001100, 0b00001100 },
  { 0b00110000, 0b00110000, 0b00110000, 0b00110000, 0b00110000 },
  { 0b11000000, 0b11000000, 0b11000000, 0b11000000, 0b11000000 },
};

struct Muster blink4 = { ARRAY_SIZE( blink4Pattern), blink4Pattern};

Noch der PlayIt Funktion zu gegebener Zeit einen Pointer auf blink4 
übergeben und fertig ist ein neues Muster.

Und da fängt jetzt die zugegebenermassen etwas kompliziertere 
Infrastruktur für dich zu arbeiten an - neue Muster hinzufügen ist 
plötzlich einfach geworden.

[Karl H. (kbuchegg)]

Christian Schoknecht schrieb:
> Karl Heinz Buchegger schrieb:
>> OK. Ich gesth dir natürlich zu, dass wenn es eine einfache Möglichkeit
>> gibt ein Muster algorithmisch zu generieren, dann ist das sicherlich
>> einfacher, als so was.
>
> Für die snail()-Funktion in diesem Fall schon:
Ah. du hast das anders aufgebaut.
Pro Aufruf ein Musterschritt - sehr vernünftig.

Dann eben

struct Muster
{
  uint8_t       nrFrames;
  struct frame* frames;
  uint8_t       actualFrame;
};


void playIt( struct Muster* muster )
{
  uint8_t j;

  muster->actualFrame++;
  if( muster->actualFrame >= muster->nrFrames )
    muster->actualFrame = 0;

  for( j = 0; j < 5; j++ )
    LED[j] = muster->frames[muster->actualFrame][j];

  LEDout();
  _delay_ms(100);
}

[Christian S. (schoker)]

Ich frage mich gerade nur, warum du nicht gleich ein zweidimensionales 
Array statt einer Struktur benutzt. Ich sehe darin jetzt keinen Vorteil.

Muster[4][5] = {
    {0x03,0x03,0x03,0x03,0x03},
    {0x0C,0x0C,0x0C,0x0C,0x0C},
    {0x30,0x30,0x30,0x30,0x30},
    {0xC0,0xC0,0xC0,0xC0,0xC0}};

Beziehungsweise dann auch in der Bitdarstellung.

Denn so kann man nun in Abhängigkeit von static Laufvariablen auch in 
der Funktion durchlaufen lassen.


Karl Heinz Buchegger schrieb:
>> Für die snail()-Funktion in diesem Fall schon:
> Ah. du hast das anders aufgebaut.
> Pro Aufruf ein Musterschritt - sehr vernünftig.

Ja klar, damit nach einem Interrupt sofort in den nächsten Modus 
gewechselt wird, ohne dieses Muster zu Ende laufen lassen zu müssen.

Allerdings funktioniert das mit festen Werten auch. Allerdings braucht 
man dann eventuell noch zwei Laufvariablen.

In dem Fall, dass alle 5 SR gleiche Inhalte bekommen, reicht natürlich 
auch ein 1-Dimensionales Array und die Werte werden nur kopiert.

[Karl H. (kbuchegg)]

Christian Schoknecht schrieb:
> Ich frage mich gerade nur, warum du nicht gleich ein zweidimensionales
> Array statt einer Struktur benutzt. Ich sehe darin jetzt keinen Vorteil.

Weil ich gerne das Array zusammen mit der Arraygröße in einer STruktur 
haben möchte. Dann hab ich alles beisammen, was für ein Muster relevant 
ist. Da könnte zb noch für jedes Muster eine spezifische 
Verzögerungszeit dazukommen oder wie in der letzten Modifikation die 
Schrittnummer des gerade aktuellen Schrittes.

Meine Sicht der Dinge ist eben die, dass zu der vollständigen 
Beschreibung eines Musters nicht nur die auszugebenden Bytewerte 
gehören, sondern noch mehr.

> Denn so kann man nun in Abhängigkeit von Laufvariablen auch in der
> Funktion durchlaufen lassen.
Überleg dir nochmal, was das bedeutet, wenn du mehr als ein Muster hast. 
Du hast ein

uint8_t Blink1[3][5] = { ... };
uint8_t Clockwise[8][5] = { ... };
uint8_t AntiClockWise[8][5] = { ... };
uint8_t Race[12][5] = { ... };
...

jedes Muster besteht aus einer unterschiedlichen Anzahl von Schritten. 
Diese Information muss irgendwo herkommen, damit die 'Schleifen' richtig 
arbeiten können.
So, mit der Struktur, übergebe ich einen Pointer auf so ein Muster an 
die Funktion, und die Funktion hat alles was sie braucht.


Geht ja dann noch weiter.
Denn ich will ja auch in weiterer Folge, deinen 'interrupt_counter' mit 
einem Muster verheiraten. D.h. da wird es noch ein Array geben, welches 
diesen interrupt_counter in ein Muster übersetzt.

struct Muster* alleMuster[] =
{
  &snail,
  &blink4,
  ...
};

und der Aufruf lautet dann

int main()
{
  ...
  sei();                // enable global interrupt
  
  while(1)
  {
    if (day == 0)
      // spiele einen Schritt vom momentan eingestellten aktiven Muster ab.
      playIt( alleMuster[ interrupt_counter ] );

    else
    {
      resetLEDs();
      _delay_ms(250);
    }    
  }

  return 1;
}

Wir nähern uns immer mehr einer allgmeinen 'Maschine', deren Fähigkeiten 
einzig und alleine durch ein paar Einträge in Arrays geregelt werden :-)

[R. M. (rmax)]

Karl Heinz Buchegger schrieb:
> Da könnte zb noch für jedes Muster eine spezifische
> Verzögerungszeit dazukommen

... oder sogar verschiedene Verzögerungszeiten für die einzelnen 
Schritte eines Musters (wie bei animierten GIFs für die einzelnen 
Frames).

> oder wie in der letzten Modifikation die
> Schrittnummer des gerade aktuellen Schrittes.

Das leutchtet mir jetzt nicht ein, denn es ist ja immer nur ein Muster 
aktiv. Warum also den aktuellen Schritt in den Mustern speichern und 
nicht in einer globalen oder statischen Variablen? Nur damit die Muster 
beim Umschalten dort weiter machen, wo man sie zuletzt weggeschaltet 
hat?

[Christian S. (schoker)]

Karl Heinz Buchegger schrieb:
> Da könnte zb noch für jedes Muster eine spezifische
> Verzögerungszeit dazukommen.

Welche bei mir nicht immer konstant ist. Die ist teilweise abhängig von 
der Anzahl der Funktionsaufrufe.

Aber über deinen Vorschlag müsste ich noch mal etwas genauer nachdenken.
Bei dir wird das Muster (mit Schrittzahl, Blinkmuster und 
Verzögerungszeit) ja quasi zu einem Objekt. Auch wenn man diese in C 
nicht hat, aber du würdest alle Eigenschaften in der Struktur abbilden, 
wenn ich das richtig verstehe.

[Karl H. (kbuchegg)]

R. Max schrieb:

>> oder wie in der letzten Modifikation die
>> Schrittnummer des gerade aktuellen Schrittes.
>
> Das leutchtet mir jetzt nicht ein, denn es ist ja immer nur ein Muster
> aktiv. Warum also den aktuellen Schritt in den Mustern speichern und
> nicht in einer globalen oder statischen Variablen? Nur damit die Muster
> beim Umschalten dort weiter machen, wo man sie zuletzt weggeschaltet
> hat?

Ist nur ein Vorschlag.
Kann jeder machen wie er will :-)

[Karl H. (kbuchegg)]

Christian Schoknecht schrieb:

> Aber über deinen Vorschlag müsste ich noch mal etwas genauer nachdenken.
> Bei dir wird das Muster (mit Schrittzahl, Blinkmuster und
> Verzögerungszeit) ja quasi zu einem Objekt.

Bingo!
(Na ja. Fast)

> Auch wenn man diese in C
> nicht hat, aber du würdest alle Eigenschaften in der Struktur abbilden,
> wenn ich das richtig verstehe.

Jep. Das brauch ich, damit ich in der Hauptschleife dann schreiben kann

  while(1)
  {
    if (day == 0)
      // spiele einen Schritt vom momentan eingestellten aktiven Muster ab.
      playIt( alleMuster[interrupt_counter] );

    else
    {
      resetLEDs();
      _delay_ms(250);
    }    
  }

und es passiert immer das richtige. Warum? Weil das Muster-Objekt selbst 
alles mithat, was die Funktion benötigt um es abspielen zu können. Ich 
hab quasi eine 2-teilung. Die Funktion stellt den Mechanismus - das 
Muster Objekt die Details.

[Christian S. (schoker)]

Und ich hatte mir überlegt, dass die Blinkmuster-Funktion immer den 
nächsten "Blink-Schritt" aufgrund des vorherigen berechnet/zuweist.

Wobei ich deine Überlegungen gut nachvollziehen kann. Ich es aber 
durchaus herausfordernd fand einen Algorithmus für die Leuchtmuster zu 
suchen. Es gibt sicher auch bei mir Fälle, in denen es einfacher ist, 
wenn es fest codiert ist. Nur sehe ich in der algorithmischen Berechnung 
den Vorteil, wenn der Stern auf weitere Register erweitert werden 
sollte. Denn der Speicheraufwand für die festcodierten Werte steigt dann 
ja enorm an.
Allein für meine jetzigen Blinkmuster bräuchte ich schon 5*137 Byte an 
Speicher, was in etwa einem Drittel des jetzigen Programms entspricht.

Bei Erweiterung um weitere SR wächst der Bedarf allein an Speicher für 
jedes SR dann um 137 Byte und das nur bei der jetzigen Anzahl an 
Mustern. Algorithmisch sind das ein paar Speicherplätze in dem 
LED[]-Array und ein paar mehr Codezeilen, weil es ein paar mehr 
Zuweisungen geben muss.

Aber schon einmal vielen Dank für das Interesse an dem Projekt und den 
zahlreichen Hinweisen zur Optimierung.

Sobald die Kondensatoren eingetroffen sind, ich diese angelötet habe und 
der Stern tut, was er soll, werde ich dann mal ein Video erstellen und 
das hierher verlinken. Vielleicht regt das dann ja den einen oder 
anderen zum Nachbau an und dem Ausprobieren deiner Idee mit der 
konsequent umgesetzten Zustandsmaschine. Immerhin ist bald Weihnachten 
und mein Stern soll ein Geschenk werden.

[R. M. (rmax)]

Christian Schoknecht schrieb:

> Nur sehe ich in der algorithmischen Berechnung den Vorteil,
> wenn der Stern auf weitere Register erweitert werden
> sollte. Denn der Speicheraufwand für die festcodierten
> Werte steigt dann ja enorm an.

Nur dann, wenn jede LED einzeln ansteuerbar sein soll (z.B. bei einem 
umlaufenden Lauflicht). Falls es aber innerhalb des Sterns bei jedem 
Schritt eines Musters periodische Wiederholungen gibt (z.B. alle Zacken 
machen das gleiche), muß nur jeweils eine Periode gespeichert und dann 
entsprechend oft in die Register geschoben werden.

[Christian S. (schoker)]

Okay, es sind definitiv die Abblockkondensatoren gewesen, denn jetzt 
läuft es wie am Schnürchen. Vielen Dank für den Hinweis.

Deshalb hier mal ein Video, wie das ganze dann im Dunkeln aussieht.

http://www.youtube.com/watch?v=wExSOsv7RWU

[R. M. (rmax)]

Sieht nett aus! :)

Hier noch ein Mustervorschlag: Alle LEDs sind an und es wird zufällig 
immer mal wieder eine kurz (<1s) ausgeschaltet. Mit dem richtigen Timing 
wirkt das wie ein glitzern oder funkeln.

Am besten sieht es aus, wenn die jeweilige LED nicht schlagartig 
ausgeht, sondern aus- und wieder eingeblendet wird, aber ich weiß nicht, 
ob Deine Schieberegister-Kette schnell genug ist, um auf einzelnen LEDs 
auch noch PWM zu machen.