Code Optimierung Stepper mit ADC und Taster Debounce

[Tom C. (palmodos)]

Hallo erstmal mein Name ist Tom, bin schon länger passiver Leser dieses 
Forums und bin gerade bei meinem ersten Projekt.
Ich steuere mit meinem Atmega8 die L297/298 Kombination an, an der ein 
kleiner Schrittmotor hängt. Das Signal erzeuge ich indem ich mit dem 
Timer Overflowflag ein Bit Toggle. Der Motor wird über ein Poti, welches 
an PC0 hängt über einen ADC in der Drehzahl verändert.Hierzu übergebe 
ich vom ADC einen Wert Für den Timer Reload. Jetzt brauche ich einen 
entprellten Taster mit dem ich die ganze Sache ein und ausschalten kann. 
Die Schalter Entprellung funzt separat, aber nicht in  Kombination mit 
meiner ISR. Es wär echt cool, wenn mir jemand ein paar Tips zur 
Entwurschtellung meines Codes geben könnte. Der Ablauf meines Motor 
Signals in der ISR scheint mir nicht optimal, aber ich hab kein Plan, 
wie ich das anders realisieren kann.

Grüße

Tom

/*myheader.h*/
#include<avr/io.h>
#ifndef F_CPU
#define F_CPU 3686400UL     
#endif 
#include <util/delay.h>


/****************************Define**************************************************/
#define BUTTON_PORT PORTD       /* PORTx - Register für Taster Output */
#define BUTTON_PIN PIND         /* PINx - Register für Taster Input */
#define BUTTON_BIT PD2          /* bit für Taster input/output */


#define DEBOUNCE_TIME 25        /* Debounce Zeit */
#define LOCK_INPUT_TIME 250     /* Wartezeit nachdem Taster gedrückt wurde*/


#define LOW 0
#define HIGH 1
#define INPUT(port,pin)   DDR ## port &= ~(1<<pin)
#define OUTPUT(port,pin)  DDR ## port |= (1<<pin)
#define CLEAR(port,pin)   PORT ## port &= ~(1<<pin)
#define SET(port,pin)     PORT ## port |= (1<<pin)
#define TOGGLE(port,pin)  PORT ## port ^= (1<<pin)
#define READ(port,pin)   (PIN ## port & (1<<pin)) 




/*********************Funktions Prototypen*****************************************/


int button_is_pressed();
void delay_ms(uint16_t ms);
int button_is_pressed();
int read_adc(void);






/********************Stepper mit ADC.c*****************************/

#include <avr/interrupt.h>
#include <stdint.h>
#include "myheader.h"






volatile uint8_t reload=0;






int main (void)
{
     

    while (button_is_pressed(1))
        {
                
                  
              read_adc();
                 
                
        }
  
   
   
   
   
}


ISR(ADC_vect)
{

  OUTPUT(B,0);       //PORT B Pin 0 als Ausgang schalten
      
  TCCR0=(0<<CS02|1<<CS01|0<<CS00);  //Prescaler einstellen 1/8
  
  SET(B,0);   // PORT B Pin 0 auf 1 setzen
  
for (;;)
  {    
  
    if ( (bit_is_set (TIFR,0)))  //Abfrage des Timer Flags
    {   
      
      
       reload=ADCH;   
      reload=reload/2;       // Reloadwert zu groß
      TCNT0= reload; 
      TOGGLE(B,0);         // Ausgang Togglen
      TIFR=0x01;        //Timer Overflow Flag löschen mit 1
    

    }
    
    
  
  
  }
}

/**************buttondebounce.c***********************/

#include "myheader.h"


/*delay_ms - Verzögerungsschleife.*/ 
  void delay_ms(uint16_t ms) 
  {
        while ( ms ) 
        {
                _delay_ms(1);
                ms--;
        }
  }

/* Funktion um zu checken ob der Taster gedrückt wurde
   Gibt eine 1 zurück wenn der Taster gedrückt wurde ansonsten 0*/ 
  int button_is_pressed()
  {
        /* Der Taster ist gedrückt wenn er gegen Masse gezogen wird */
        if (bit_is_clear(BUTTON_PIN, BUTTON_BIT))
        {
                delay_ms(DEBOUNCE_TIME);
                
          if (bit_is_clear(BUTTON_PIN, BUTTON_BIT)) 
          return 1;
        }
        
        return 0;
  }

/*********************read_adc.c******************************/
#include <avr/interrupt.h>
#include <stdint.h>


int read_adc(void)
{

ADCSRA |= (1 << ADPS2) | (1 << ADPS1) | (1 << ADPS0); // Setze ADC prescaler von 128 - 125KHz sample rate @ 3,6864 Mhz

   ADMUX |= (1 << REFS0); // Setze ADC Vref auf externen Pin AVCC(4,6V)
   ADMUX |= (1 << ADLAR); // ADCH wird als erstes Nibble gestellt
   // Mux muss nicht eingestellt werden, da Pin 1 default  ist

   ADCSRA |= (1 << ADFR);  // Setze Free-Running Mode
   ADCSRA |= (1 << ADEN);  // Aktiviere ADC

   ADCSRA |= (1 << ADIE);  // Aktiviere ADC Interrupt
   sei();   //  Globalen Interrupt aktivieren

   ADCSRA |= (1 << ADSC);  // Starte A2D Conversion
   
  

   for(;;)  // Loop Forever
   {
     
   }

return(0);
}

[... .. (docean)]

was sollen die zwei for(;;) Schleifen? Das kann nicht gehen...

EINE Endlos Schleife in der MAIN!!!

[Tom C. (palmodos)]

Ich hab die Module einzeln getestet und etwas den Überblick verloren.
Danke für den Tip, werd da mal etwas dran rumspielen.

[Karl H. (kbuchegg)]

Tom Cee schrieb:
> Ich hab die Module einzeln getestet und etwas den Überblick verloren.

Fang als erstes damit, die überzähligen Leerzeilen rauszuwerfen.
Wenn du mehr als eine Leerzeile hintereinander machst um etwas 
abzusetzen, dann hat es sich der nächste Code-Abschnitt verdient, dass
  * er nur mit 1 Leerzeile eingeleitet wird
  * dafür aber ein Blockkommentar vor dem Block beschreibt, was
    an dieser Stelle im Code, global gesehen, passieren wird.


So was

#define TOGGLE(port,pin)  PORT ## port ^= (1<<pin)
#define READ(port,pin)   (PIN ## port & (1<<pin)) 




/*********************Funktions Prototypen*****************************************/


int button_is_pressed();
void delay_ms(uint16_t ms);
int button_is_pressed();
int read_adc(void);






/********************Stepper mit ADC.c*****************************/

#include <avr/interrupt.h>
#include <stdint.h>
#include "myheader.h"






volatile uint8_t reload=0;






int main (void)
{
     

    while (button_is_pressed(1))
        {
                
                  
              read_adc();
                 
                
        }
  
   
   
   
   
}

erhöht keine Übersicht, sindern verringert sie, weil es den Code 
unnötigerweise einfach nur in die Länge zieht und man so auf dem 
Bildschirm weniger Nutzinformation sieht.

#define TOGGLE(port,pin)  PORT ## port ^= (1<<pin)
#define READ(port,pin)   (PIN ## port & (1<<pin)) 

/*********************Funktions Prototypen*****************************************/
int button_is_pressed();
void delay_ms(uint16_t ms);
int button_is_pressed();
int read_adc(void);

/********************Stepper mit ADC.c*****************************/
#include <avr/interrupt.h>
#include <stdint.h>
#include "myheader.h"

volatile uint8_t reload=0;

int main (void)
{
  while (button_is_pressed(1))
  {
    read_adc();
  }
}

Hmm. Die übliche logische Sequenz in main ist doch eher die

int main()
{
  ... die benutzte Hardware initialisieren
  ... das bedeutet zb die DDR richtig setzen
  ... Pullups einschalten, falls gebraucht
  ... Timer richtig einstellen, etc, etc
  ... kurz und gut: die Vorraussetzungen schaffen
  ... das der Prozessor richtig konfiguriert ist um
  ... überhaupt mit der eigentlichen Arbeit anfangen
  ... zu können

  sei();    // falls Interrupts benutzt werden

  while( 1 ) {

    .. Programmlogik
    .. zb. Wenn eine Taste gedrückt wird, mach etwas

  }
}

Jetzt vergleich einmal deinen Ansatz für main() mit dem Grundschema und 
versuche da die Entsprechungen zu finden. Du wirst keine finden :-)
Hinweis: Auch wenn deine read_adc() Funktion suggeriert, dass dort der 
ADC ausgelesen wird, so tut sie das eigentlich nicht. Deine 'read_adc' 
ist eigentlich mehr eine Initialisierungsfunktion, die den ADC in den 
Arbeitsmodus schaltet. Initialisierungsfunktion -> hat in der 
Hauptschleife nichts verloren, sondern wird vor die Hauptschleife 
gesetzt. Auch sehe ich nirgends, wo zb die Voraussetzungen für den 
Taster schaffst, oder braucht du keinen Pullup?

Und nimm auch den sei() aus der read_adc raus. Der hat dort drinnen auch 
nichts verloren. Eine derartige wichtige Entscheidung, 'ab jetzt können 
Interrupts auslösen', legt man nicht in eine Funktion die nur die 
Aufgabe hat ein Gerät zu initialisieren. Diese Entscheidung, Interrupts 
"ja bitte" macht man nachdem alles initialisiert ist und bevor die 
while-Hauptschleife loslegt. Das kann aber eine einzelne 
Initialisierungsfunktion nicht für sich entscheiden. Nach dem ADC könnte 
ja zb der Timer drann sein, initialisiert zu werden.

[Tom C. (palmodos)]

Danke für die wertvollen Infos, Karl Heinz.
Ich hab wirklich noch große Probleme einen logischen Ablauf bei 
komplexeren Programmen hinzubekommen. Bei mir läuft das alles über Trial 
and Error und das ist ziemlich mühselig.