Funktionsaufruf Funktioniert nicht richtig..warum?

[Kay U. (Gast)]

Hallo Gemiende,

arbeite schon seit einer Weile an diesem Cod und haben für eine paar 
Tagen angefangen ihn zu optimieren. Doch leider Funktioniert das nicht 
ganz so wie ich mir das gedachte habe.
Mein Gedanke dabei war mehrere Kleine Funktionen zuschreiben und nur die 
Variablen zu ändern ohne jedes mal alles neu zuschreiben.. aber Seht 
selbst.

Wenn ich nun die Funktionen aufrufe Passiert nichts bzw. ändert sich 
nix.
Verstehen nur noch nicht ganz warum. Vielleicht könnt ihr mir ja helfen.

Grüße Kay

[Karl H. (kbuchegg)]

Nochmal zurück zur letzten funktionierenden Version und dann nicht alles 
auf einmal umbauen, sondern in kleinen Schritten. Teil für Teil.

Sowas kommt vor, dass man sich bei zuvielen Änderungen auf einmal 
irgendwo ein Eigentor geschossen hat. Wohl dem, der dann ein 
Versionskontrollsystem hat, oder Sicherungen von funktionierenden 
Versionen, auf die er zurück gehen kann und von der ausgehend die 
Transformation, diesmal in kleineren Schritten nochmal gemacht werden 
kann.


Unabhängig davon: sag mal. Ist dir das nicht zu blöd, da 27 uint8_t 
Variablen zu haben, die alle bis auf eine angehängte Ziffer gleich 
heißen? Also mir wär das zu blöd, weil es den Code unsinnigerweise 
aufbläht. Das Array ist schon lange erfunden und deine init Funktion 
würde isch dann so lesen.

// im Header File ...
#define NR_LEDS 27

...

uint8_t LED[NR_LEDS ];
uint8_t LED_OUTPUT[ 1 + NR_LEDS / 8];

void Variablen_Init(void)
{
  uint8_t i;

  //Reg 0x01 = MODE2 - set to 0xFF - noninverted logic state, open-drain
  MODE2 = 0x02;  
  
  // Set all Output of 0x00(0)
  for( i = 0; i < NR_LEDS; i++ )
    LED[i] = 0x00;
  
  //LED_OFF:
  LED_OFF = 0x00;

  //Reg 0x12 - Group PWM - Variablen
  GROUP_PWM = 0x00;

  //Reg 0x13 - Group Freq - Variablen
  GROUP_FREQ = 0x00;

  //Reg 0x14-0x17 - LED Output State Control - Variablen
  for( i = 0; i < 1 + NR_LEDS / 8; i+
    LED_OUTPUT[i] = 0xAA;
}

wesentlich kürzer. Und vor allen Dingen viel leichter auf eine andere 
Anzahl an Leds umzustellen. Und das betrifft nicht nur die Init 
Funktion. Von deinem jetzigen Code würden über den Daumen gepeilt rund 
2/3 aller Code Zeilen wegfallen.

[Kay U. (Gast)]

Hehe.. da hast du recht. Sicherungen hab ich auch noch aber wollte quasi 
so hin bekommen. Ach das gute alte array. das sollte der nächste schritt 
sein.. Irgendwie vermehre ich mich da immer und aus im Studium hab ich 
noch nicht viel programmiert. Also mühsam ernährt sich das Erdferkel ;).

Danke schon mal für die schnelle Antwort und den Tip. Sind sonst noch 
sachen aufgefallen oder meinst du ich sollte einfach schritt für schritt 
nochmal rann gehen?

[Karl H. (kbuchegg)]

Kay Ulbricht schrieb:
> Hehe.. da hast du recht. Sicherungen hab ich auch noch aber wollte quasi
> so hin bekommen.

Das einzige was mir auf die Schnelle aufgefallen ist ist, dass es 
zwischen der Adressierung des ersten IC und dem Start des 2.ten IC 
keinen TWI_Stop gibt.

> Danke schon mal für die schnelle Antwort und den Tip. Sind sonst noch
> sachen aufgefallen oder meinst du ich sollte einfach schritt für schritt
> nochmal rann gehen?

Wenn die fehlende Stop-Bedingung nicht das Problem ist - ja, würde ich 
tun. Auf die Schnelle ist nichts offensichtliches zu sehen (bis auf ein 
paar technische Feinheiten im Header File. Bei den Deklarationen der 
Variablen fehlt ein extern. Aber das bügelt der GCC-Linker für dich aus 
und sollte momentan nicht das Problem sein)
Auch hast du viele Dinge im Header File, die da gar nicht rein gehören, 
weil sie nichts damit zu tun haben, was main() von deinen Funktionen 
wissen muss um sie bedienen zu können. Aber auch die sind erst mal nicht 
das Problem

[Stefan E. (sternst)]

Ein weiteres Problem ist das hier:

  while(1)
    {
      Variablen_Init();
      PCA9635();
...
    {
      ...
      LED0 = 0x20;
      LED1 = 0x33;
      LED2 = 0x20;
      LED11 = 0x42;
      PCA9635();
    }
...                
  }

[Kay U. (Gast)]

In der tat ist das fehlende TWI_Stop nicht ganz so schlimm. Ja ist mir 
auch schon aufgefallen das ich recht viel im Header weglassen bzw anders 
schreiben kann. Das ist noch vom C Programmieren drin... Okay dann 
arbeite ich das ganze nochmal durch und schau mal obs dann will.

[Kay U. (Gast)]

Ich glaube ich weiß was du meinst.. aber eine passende lösung hab ich 
grade nicht auf lagen.. :D

[Karl H. (kbuchegg)]

Stefan Ernst schrieb:
> Ein weiteres Problem ist das hier:


Umpf.
Da hast du recht. Das hab ich nicht gesehen bzw. bedacht.
Er verwendet ja get_keyshort (wahrscheinlich aus den PeDa Routinen)

D.h. die Outputs werden ganz kurz und einmalig bei Tastendruck gesetzt 
und im nächsten Schleifendurchlauf wieder gelöscht. Jep. da wird man 
nichts sehen. Das geht viel zu schnell, dass die LED kurz aufleuchten 
und wieder verlöschen.

[Karl H. (kbuchegg)]

Kay Ulbricht schrieb:
> Ich glaube ich weiß was du meinst.. aber eine passende lösung hab ich
> grade nicht auf lagen.. :D

Na komm.
Das liegt doch auf der Hand. Die Initialisierung der Variablen aus der 
Hauptschleife raus.
Das solltest du dir sowieso angewöhnen. Eine normale Programmstruktur 
sieht so aus

void main
{

   Alle Initialisierungen
   dazu gehört die Initialisierung aller Port Register
   aber auch das einmalige Setzen von globalen Variablen
   auf ihre Erst-Werte

   sei();  // falls Interrupts benötigt werden

   while( 1 ) {

     Logik der Hauptschleife, wie zb Feststellung von Tastendrücken
     und entsprechende Reaktion darauf.

   }
}

alles was nach Initialisierung stinkt, findet VOR der Hauptschleife 
statt und nicht in ihr.

[Kay U. (Gast)]

Ahh.. da muss ich zugeben das das mit den Tasten wir so oder so nach 
nicht wirklich geheuer ist.. Also da muss ich eh noch mal rann.. Die 
machen eher was sie wollen. Bin für Tipps dankbar :)

[Karl H. (kbuchegg)]

Kay Ulbricht schrieb:
> Ahh.. da muss ich zugeben das das mit den Tasten wir so oder so nach
> nicht wirklich geheuer ist.. Also da muss ich eh noch mal rann.. Die
> machen eher was sie wollen.

Dann hast du zu viele Baustellen auf einmal.
Eins nach dem anderen.
Das ist einer der meist gemachten Fehler aller Anfänger: zu viele offene 
Baustellen. NIchts funktioniert und wenn die Baustellen dann immer mehr 
werden, dann verliert man den Überblick, welches Problem auf wessen 
Konto geht.

[Kay U. (Gast)]

Quasi so:

...
/** Initzialisiere PCA9635 **/

    Variablen_Init();
    PCA9635();
    
    
/** Endlosschleife **/

  while(1)
  {
      
        
/** Write byte(s) to the slave **/

    if( get_key_short( 1<<KEY0 ))
    {
      PORTD = PORTD | ( 1<<PD5 );
      PCA9635();
    }
    if ( get_key_long( 1<<KEY0 ))
    {
      Variablen_Init();
      PCA9635();
      PORTD &= ~( 1<<PD5 );
    }
                
  }

?

[Karl H. (kbuchegg)]

Kay Ulbricht schrieb:

> ?

genau


Wobei ich persönlich kein Freund von Short und Long-Tastendrücken bin. 
Ich hab lieber 2 getrennte Tasten. Da gibt es dann auch keine 
Diskrepanzen mit dem Thema "Wie lange muss ich eigentlich drücken, damit 
aus einem Short ein Long wird"

2 tasten, normales get_key_press und die Sache ist erst mal vom Tisch. 
Auf 1-Knopf Bedienung kann man immer noch umstellen, wenn alles fertig 
ist.


(und iirgendwo in deinen Init Funktionen muss sich noch ein sei() 
verstecken. Denn so wie es jetzt ist, ist die letzte Aktion in main() 
diesbezüglich ein cli(). Damit sind die Interrupts aber gesperrt.
Persönlich halte ich ebenfalls nicht viel davon, in den diversen xxxInit 
Funktionen einen sei() zu verstecken. Der darf ruhig und prominent die 
letzte Aktion in main() vor der Hauptschleife sein, wo ihn jeder sieht

void main()
{
   DDR.....
   init_xyz():
   init_abc();


   // jetzt gilts.
   // Alle Interrupts: Feuer frei
   sei();

   while( 1 ) {

     und arbeiten, arbeiten, arbeiten
     was so anliegt und was uns die Interrupts so zutragen
   }
}

Den sei() kann ich nicht übersehen und er steht an genau der einzig 
sinnvollen Stelle. Nämlich als letzte Aktion der Initialisierung, bevor 
die Hauptschleife beginnt.

[Kay U. (Gast)]

Okay.. hab jetzt alles nochmal durch geschaut und umgebaut:

...
int main (void)
{
  cli();
  
/** Initzialisiere Ports **/
  
  port_init();
    
/** Configure debouncing routines **/

  KEY_DDR &= ~ALL_KEYS;                    // configure key port for input
  KEY_PORT |= ALL_KEYS;                    // and turn on pull up resistors
    
  TCCR0 = (1<<CS02)|(1<<CS00);                // divide by 1024
  TCNT0 = (uint8_t)(int16_t)-(F_CPU / 1024 * 10e-3 + 0.5);  // preload for 10ms
  TIMSK |= 1<<TOIE0;                      // enable timer interrupt
    
/** Wait 1 second for POR **/

  _delay_ms(1000);

/** Initzialisiere RS232 **/
  
  uart_init();

/** Initzialisiere TWI Master Interface mit bitrate von 1000 Hz **/

  if (!TWIM_Init (1000))
    {
    printf ("Error in initiating TWI interface\n");
    while (1);
    }
    
/** Initzialisiere PCA9635 **/

    Variablen_Init();
    PCA9635();
    
    sei();
    
/** Endlosschleife **/

  while(1)
  {  
        
/** Write byte(s) to the slave **/

    if( get_key_press( 1<<KEY0 ))
    {
      PORTD = PORTD | ( 1<<PD5 );
      Full_Output();
      PCA9635();
    }
    if ( get_key_press( 1<<KEY1 ))
    {
      Variablen_Init();
      PCA9635();
      PORTD &= ~( 1<<PD5 );
    }
                
  }
return 0;
}

Die tasten sache Funktioniert jetzt nur leider nicht das die LEDs alle 
angehen.. :/

[Karl H. (kbuchegg)]

Kay Ulbricht schrieb:

> Die tasten sache Funktioniert jetzt nur leider nicht das die LEDs alle
> angehen.. :/

Gegenfrage: hat das schon mal funktioniert?

Hintergrund: Ich kenn den PCA_irgendwas nicht. Von daher kann ich nicht 
sagen, ob dein rausschreiben von Bytes Sinn macht oder nicht.

[Karl H. (kbuchegg)]

Moment

  //Erster TLC59116 Chip:
  TWIM_Start (PCA9635_ADD1, TWIM_WRITE);

  //Autoincrement ALL registers, start at reg 0 - we're initting
  TWIM_Write(AUTO_INCREMENT_ALL_REGISTERS);    //Write 0x80

  //Reg 0x00 = MODE1 - set to 0x80 - autoincrement enabled (this is readonly?), do not respond to subaddresses or allcall
  TWIM_Write(AUTO_INCREMENT_ALL_REGISTERS);    //Write 0x80

2 mal AUTO_INCREMENT setzen?
Das ist nicht logisch. Sicher, dass das stimmt?

[Kay U. (Gast)]

Verstehe was du meinst.. aber vor dem Umbau hat das alles Funktioniert. 
Also mit Tasten lang und kurz drücken usw... Das sollte also stimmten 
aber ja der gedanke kam mir auch und ist auch mehr eine Lösung durchs 
testen entstanden...