Interrupts und Tasks - Codeoptimierung

[quote="Wutz"]- globale Variablen sind Schrott
- statt deiner if == 1,2,3 Kaskasden nimmt man switch+case
- volatile Qualifizierer fehlt für deine globalen Variablen
- void funktion(void) ist Schrottdesign (weil dies globale Variablen bedingt)
- Mischung von Bitlogik und Boollogik ist Schrott z.B. (LongPulse == true & TASK2 == false & StartReadClocks==false)
- Klammern zur besseren Lesbarkeit fehlen z.B. (TIM1->EGR & TIM_EGR_UG & TASK2 == false)
- Z. 158/159 sind sinnfrei
- u.v.a.m.

quote]

Guten Morgen

Ich werde mich gleich mal an die Arbeit machen. Danke für deine Inputs.

Wutz schrieb:

Interruptprogrammierung ist kein Grund dafür, schlechtes C zu schreiben.

Ja stimmt, das ist vielleicht schlechtes C. Deshalb frage ich euch Experten, um zu lernen. Was kann ich denn sonst tun? Ich hab ja sonst niemanden
Danke

camper

Wutz schrieb:

- statt deiner if == 1,2,3 Kaskasden nimmt man switch+case

Genaugenommen sind in diesem Fall die Verschiedenen if-Blöcke praktisch identisch, so dass im Grunde nur die Bedingungen oder Blöcke entsprechend parametrisiert werden müssen.

Etwa

(HAL_GPIO_ReadPin(DataRdy2.port, DataRdy2.pin) == 1)  && (u8count1 == 2)

-->

HAL_GPIO_ReadPin(DataRdys[u8count1].port, DataRdys[[u8count1].pin) == 1

mit einem passenden Array DataRdys

select_ADC_Channel_to_Convert(1);                                                  // set the ADC channel
            Set_Destination_DMA1_ADC(dma0_buffer);                                             // set the DMA destination
            SetTIM1Param(0, TRD_CLOCK, 1027);
            Enable_EPCReadCLK_CH(EN_EPC1_CH | EN_ADC_TRG_CH);
            Enable_Timerx_Counter(TIM1);

-->

select_ADC_Channel_to_Convert(u8count1);                                                     // set the ADC channel
            Set_Destination_DMA1_ADC(dma_buffers[u8count1]);                                             // set the DMA destination
            SetTIM1Param(0, TRD_CLOCK, 1027);
            Enable_EPCReadCLK_CH(EN_EPC1_CH | EN_ADC_TRG_CH);
            Enable_Timerx_Counter(TIM1);

mit einem passenden Array dma_buffers

camper schrieb:

Etwa

(HAL_GPIO_ReadPin(DataRdy2.port, DataRdy2.pin) == 1)  && (u8count1 == 2)

-->

HAL_GPIO_ReadPin(DataRdys[u8count1].port, DataRdys[[u8count1].pin) == 1

mit einem passenden Array DataRdys

[code="c"] (

Ich denke, dass dies nicht so einfach geht, da meine Signale DataRdy1-4 alle zur gleichen Zeit aktiv sind. Ich benötige irgend eine Unterscheidung und das erreiche ich mit diesem counter.
Oder sehe ich etwas falsch?

gruss buell

Th69

Was soll man da noch drauf antworten? Weißt du nicht, wie Arrays funktionieren? Camper hat nicht geschrieben, daß du den Counter weglassen sollst, sondern statt der Einzelvariablen eben ein Array benutzen.
Das gleiche gilt dann auch für deine Funktion 'TASK2_short_Pulse'...

danke für die Ideen

Th69 schrieb:

Was soll man da noch drauf antworten? Weißt du nicht, wie Arrays funktionieren? Camper hat nicht geschrieben, daß du den Counter weglassen sollst, sondern statt der Einzelvariablen eben ein Array benutzen.
Das gleiche gilt dann auch für deine Funktion 'TASK2_short_Pulse'...

hab ich jetzt auch verstanden

kurze Frage noch:
Ich habe den Code abgeändert und ein Teil sieht so aus.
Gibt es eigentlich eine Möglichkeit wie ich EN_EPC1_CH mittels
#define EN_EPC(m) EN_EPC##m##_CH eliminieren kann?
Dann bräuchte ich nicht mal ein switch

void TASK2_short_Pulse()
{

     if (LongPulse == false & TASK1 == false & StartReadClocks==true) 
     { 

         TASK2 = true;

         select_ADC_Channel_to_Convert(u8count1);                                                  // set the ADC channel
         Set_Destination_DMA1_ADC(dmaArray[u8count1-1].buffer);                                    // set the DMA destination
         SetTIM1Param(0, TRD_CLOCK, READ_CLK_TIM1_REPETITION_COUNT_VALUE);                         // configure TIM1 settings

         switch(u8count1)
         {
            case 1:
                Enable_EPCReadCLK_CH(EN_EPC1_CH | EN_ADC_TRG_CH);
                Enable_Timerx_Counter(TIM1); 
            break;

            case 2:
                Enable_EPCReadCLK_CH(EN_EPC2_CH | EN_ADC_TRG_CH);
                Enable_Timerx_Counter(TIM1);    
            break;

            case 3:
                Enable_EPCReadCLK_CH(EN_EPC3_CH | EN_ADC_TRG_CH);
                Enable_Timerx_Counter(TIM1);    
            break;

            case 4:
                Enable_EPCReadCLK_CH(EN_EPC4_CH | EN_ADC_TRG_CH);
                Enable_Timerx_Counter(TIM1);

//              while(TIM1->CR1 & TIM_CR1_CEN);
//              Disable_Timerx_Counter(TIM1);                       // make sure the timer1 is disabled 
                Enable_Shutter_TIM15(); 

                // set the start conditions
                u8count1 = 0; 
                LongPulse=false; 
                TASK1=false; 
                TASK2=false; 
            break; 

         }

         StartReadClocks=false;  
         TASK2 = false;                                      // make sure that TASK1 gets active 
     }
}

camper

Mit den EN_EPCX_CH Konstanten kannst du das Gleiche anstellen. Oder diese evtl. direkt berechnen (kenne ja die Werte nicht). Dann bleibt von dem switch nur ein if(u8count1==4) übrig.
Wenn sonst nichst geht:

int EN_EPCx_CHs[] = { EN_EPC1_CH, EN_EPC2_CH, EN_EPC3_CH, EN_EPC4_CH };

camper schrieb:

Mit den EN_EPCX_CH Konstanten kannst du das Gleiche anstellen. Oder diese evtl. direkt berechnen (kenne ja die Werte nicht). Dann bleibt von dem switch nur ein if(u8count1==4) übrig.
Wenn sonst nichst geht:
int EN_EPCx_CHs[] = { EN_EPC1_CH, EN_EPC2_CH, EN_EPC3_CH, EN_EPC4_CH };

Jetzt sieht es langsam richtig gut aus.
Danke euch allen, das ist super top
.. und macht noch Spass

// TypeDef DMA
typedef struct _dma_t {
    uint16_t buffer[DmaArrayLength];
} dma_t;

static dma_t dmaArray[DMA_CHANNELS]; 

// TypeDef TIMER
typedef struct TIMxPIN{
	GPIO_TypeDef* port;
	uint16_t pin;
} TIMxPIN_t;

// TypeDef DataRdy
typedef struct _DataRdy_ {
    GPIO_TypeDef* port;
	 uint16_t pin;
} DataRdy_t;

static DataRdy_t DataRdys[DataRdyAmount]; 

void TASK1_long_Pulse()
{
   if (LongPulse == true && TASK2 == false && StartReadClocks==false) 
   { 

         TASK1 = true;
         SetTIM1Param(0, TRD_PULSE, LONG_TIM1_PULSE_REPETITION_COUNT_VALUE);      

         if (u8count1 == 1) 
         { 
            Enable_EPCReadCLK_CH(EN_EPC1_CH); 
         } 

         if ((HAL_GPIO_ReadPin(DataRdys[u8count1].port, DataRdys[u8count1].pin) == 1)  && (u8count1 == 2)) 
         { 
            Enable_EPCReadCLK_CH(EN_EPC2_CH); 
         } 

          if ((HAL_GPIO_ReadPin(DataRdys[u8count1].port, DataRdys[u8count1].pin) == 1)  && (u8count1 == 3)) 
         { 
            Enable_EPCReadCLK_CH(EN_EPC3_CH); 
         } 

         if ((HAL_GPIO_ReadPin(DataRdys[u8count1].port, DataRdys[u8count1].pin) == 1)  && (u8count1 == 4)) 
         { 
            Enable_EPCReadCLK_CH(EN_EPC4_CH); 

         } 

         Enable_Timerx_Counter(TIM1); 
         for(uint32_t i = 0; i < 500000; i++);                                               // wait until TCDS
         TASK1 = false;                                                                      // if in ARR Interrupt reacts  
   }      
}

void TASK2_short_Pulse()
{

     if (LongPulse == false && TASK1 == false && StartReadClocks==true) 
     { 

         TASK2 = true;

         select_ADC_Channel_to_Convert(u8count1);                                                  // set the ADC channel
         Set_Destination_DMA1_ADC(dmaArray[u8count1-1].buffer);                                    // set the DMA destination
         SetTIM1Param(0, TRD_CLOCK, READ_CLK_TIM1_REPETITION_COUNT_VALUE);                         // configure TIM1 settings
         Enable_EPCReadCLK_CH(EN_EPCx_CHs[u8count1] | EN_ADC_TRG_CH);
         Enable_Timerx_Counter(TIM1); 
         if(u8count1==4)
         {
             Enable_EPCReadCLK_CH(EN_EPC4_CH | EN_ADC_TRG_CH);
             Enable_Timerx_Counter(TIM1);

//              while(TIM1->CR1 & TIM_CR1_CEN);
//              Disable_Timerx_Counter(TIM1);                                                      // make sure the timer1 is disabled 
             Enable_Shutter_TIM15(); 

             // set the start conditions
             u8count1 = 0; 
             LongPulse=false; 
             TASK1=false; 
             TASK2=false;                 
         }

         StartReadClocks=false;  
         TASK2 = false;                                                                            // make sure that TASK1 gets active 
     }
}

Th69

In der Kürze liegt die Würze.

wow, ich hab noch was gesehen
die ganzen if beim anderen Task brauch ich auch nicht mehr

void TASK1_long_Pulse()
{
if (LongPulse == true && TASK2 == false && StartReadClocks==false)
{

TASK1 = true;
SetTIM1Param(0, TRD_PULSE, LONG_TIM1_PULSE_REPETITION_COUNT_VALUE);
Enable_EPCReadCLK_CH(EN_EPCx_CHs[u8count1]);
Enable_Timerx_Counter(TIM1);
for(uint32_t i = 0; i < 500000; i++); // wait until TCDS
TASK1 = false; // if in ARR Interrupt reacts
}
}

void TASK2_short_Pulse()
{

if (LongPulse == false && TASK1 == false && StartReadClocks==true)
{

TASK2 = true;

select_ADC_Channel_to_Convert(u8count1); // set the ADC channel
Set_Destination_DMA1_ADC(dmaArray[u8count1-1].buffer); // set the DMA destination
SetTIM1Param(0, TRD_CLOCK, READ_CLK_TIM1_REPETITION_COUNT_VALUE); // configure TIM1 settings
Enable_EPCReadCLK_CH(EN_EPCx_CHs[u8count1] | EN_ADC_TRG_CH);
Enable_Timerx_Counter(TIM1);
if(u8count1==4)
{
Enable_EPCReadCLK_CH(EN_EPC4_CH | EN_ADC_TRG_CH);
Enable_Timerx_Counter(TIM1);

// while(TIM1->CR1 & TIM_CR1_CEN);
// Disable_Timerx_Counter(TIM1); // make sure the timer1 is disabled
Enable_Shutter_TIM15();

// set the start conditions
u8count1 = 0;
LongPulse=false;
TASK1=false;
TASK2=false;
}

StartReadClocks=false;
TASK2 = false; // make sure that TASK1 gets active
}
}

Hier noch der abgeänderte Code für Interessierte:
Mit dem void TIM1_UP_TIM16_IRQHandler (void) bin ich nur noch nicht so ganz zufrieden, aber ich denke, dass ich schon auf dem guten Weg bin.
Danke liebe Experten

uint16_t EN_EPCx_CHs[] = {EN_EPC1_CH, EN_EPC2_CH, EN_EPC3_CH, EN_EPC4_CH};

#define DMA_CHANNELS             3
#define DataRdyAmount            3

// Enable Channel EPC
#define EN_EPC1_CH                              TIM_CCER_CC2E
#define EN_EPC2_CH                              TIM_CCER_CC1E
#define EN_EPC3_CH                              TIM_CCER_CC3E
#define EN_EPC4_CH                              TIM_CCER_CC4E
#define EN_ADC_TRG_CH		                TIM_CCER_CC5E 

// TypeDef DMA
typedef struct _dma_t {
    uint16_t buffer[DmaArrayLength];
} dma_t;

static dma_t dmaArray[DMA_CHANNELS]; 

// TypeDef TIMER
typedef struct TIMxPIN{
	GPIO_TypeDef* port;
	uint16_t pin;
} TIMxPIN_t;

// TypeDef DataRdy
typedef struct _DataRdy_ {
    GPIO_TypeDef* port;
	 uint16_t pin;
} DataRdy_t;

static DataRdy_t DataRdys[DataRdyAmount]; 

// globale Variablen für das Auslesen TIM1
volatile uint8_t u8count1=0; 
volatile bool LongPulse=false; 
volatile bool TASK1=false; 
volatile bool TASK2=false; 
volatile bool StartReadClocks=false; 

void TASK1_long_Pulse()
{
   if (LongPulse == true && TASK2 == false && StartReadClocks==false) 
   { 

         TASK1 = true;
         SetTIM1Param(0, TRD_PULSE, LONG_TIM1_PULSE_REPETITION_COUNT_VALUE);
         Enable_EPCReadCLK_CH(EN_EPCx_CHs[u8count1]);       
         Enable_Timerx_Counter(TIM1); 
         for(uint32_t i = 0; i < 500000; i++);                                               // wait until TCDS
         TASK1 = false;                                                                      // if in ARR Interrupt reacts  
   }      
}

void TASK2_short_Pulse()
{

     if (LongPulse == false && TASK1 == false && StartReadClocks==true) 
     { 

         TASK2 = true;

         select_ADC_Channel_to_Convert(u8count1);                                            // set the ADC channel
         Set_Destination_DMA1_ADC(dmaArray[u8count1-1].buffer);                              // set the DMA destination
         SetTIM1Param(0, TRD_CLOCK, READ_CLK_TIM1_REPETITION_COUNT_VALUE);                   // configure TIM1 settings
         Enable_EPCReadCLK_CH(EN_EPCx_CHs[u8count1] | EN_ADC_TRG_CH);
         Enable_Timerx_Counter(TIM1); 
         if(u8count1==4)
         {            
//         while(TIM1->CR1 & TIM_CR1_CEN);
//         Disable_Timerx_Counter(TIM1);                                                     // make sure the timer1 is disabled 
           Enable_Shutter_TIM15(); 

           // set the start conditions
           u8count1 = 0; 
           LongPulse=false; 
           TASK1=false; 
           TASK2=false;                 
         }

         StartReadClocks=false;  
         TASK2 = false;                                                                            // make sure that TASK1 gets active 
     }
}

void EXTI15_10_IRQHandler (void) {                                                     // Interrupt on PC12 DATARDY 1 --> is generated once before reading out process

  if ((EXTI->PR1 & EXTI_PR1_PIF12))
  {
    EXTI->PR1 = EXTI_PR1_PIF12;

    // schalte Led ein
    uint32_t ODR = GPIOA->ODR;
    GPIOA->BSRR = ((ODR & (1 << 5)) << 16) | ((~ODR) & (1 << 5));

    Disable_Shutter_TIM15();                                                           // for avoiding TCDS and Shutter time overlaps according to epc datasheet
    u8count1++;                                                                        // increade the counter
    LongPulse = true;                                                                  // starts Task1
  }
}

void TIM1_UP_TIM16_IRQHandler (void)                                                    // LONG_PULS overflow interrupt to start Read Clocks of CH1
{ 
    if (TASK1==true) 
    {         
      while(TASK1==false);                                                              // wait until Task1 is done
      // both commands are needed to start TASK2
      StartReadClocks=true;                                                             
      LongPulse = false; 
    } 

    if (TASK1==false) 
    { 
      // both commands are needed to start TASK2       
      StartReadClocks = true;                                                           // start Task2
      LongPulse = false; 
    } 

    if (TIM1->EGR & TIM_EGR_UG & TASK2 == true) 
    { 
      while(TASK2==false);                                                              // wait until Task2 is done
      TIM15->SR &= ~TIM_EGR_UG;                                                         // clear UG Bit
      LongPulse = true;                                                                 // starts TASK1 long pulse at the end of Repetition counter
      StartReadClocks=false;                                                            // TASK2 is stopped 
      u8count1++;                                                                       // increase counter
    } 

    if (TIM1->EGR & TIM_EGR_UG & TASK2 == false) 
    { 
      TIM15->SR &= ~TIM_EGR_UG;                                                         // clear UG Bit
      LongPulse = true;                                                                 // starts TASK1 long pulse at the end of Repetition counter
      StartReadClocks=false;                                                            // TASK2 is stopped 
      u8count1++;                                                                       // increase counter
    } 
}

camper

Die Duplikate sind jetzt fast alle verschwunden - soll jetzt der Code auch noch korrigiert und verbessert werden? Bis jetzt ist im Grunde bloß grober Unfug entfernt worden

camper schrieb:

Die Duplikate sind jetzt fast alle verschwunden - soll jetzt der Code auch noch korrigiert und verbessert werden? Bis jetzt ist im Grunde bloß grober Unfug entfernt worden

Gerne. Mit dem Interrupt und den ifs bin ich zwar noch nicht zufrieden, aber geht es noch besser? Ich sehe nichts mehr. Jetzt bin ich mal gespannt.

camper

Zunächst Triviales:

uint16_t EN_EPCx_CHs[] = {EN_EPC1_CH, EN_EPC2_CH, EN_EPC3_CH, EN_EPC4_CH};

Das sollte man als const deklarieren. Ausserdem scheint mir, dass diese Zeile nicht vor den zugehörigen #defines stehen kann.

#define DMA_CHANNELS             3
#define DataRdyAmount            3

dürfte wohl falsch sein. Weil u8count bisher mindestens 4 verschiedene Werte annehmen konnte.

if (LongPulse == true && TASK2 == false && StartReadClocks==false)

Bitte keine bools mit true oder false vergleichen.

if (LongPulse && !TASK2 && !StartReadClocks)

ist erheblich besser lesbar.

for(uint32_t i = 0; i < 500000; i++);                                               // wait until TCDS

Diese Zeile überlebt keinen optimierenden Compiler. Wenn keine geeignete Systemfunktion existiert, könntest du z.B. volatile-Writes zu Hilfe nehmen.

volatile int dummy;
...
for(uint32_t i = 0; i < 500000; i++)
    dummy = 0;

o.ä.

if (LongPulse == false && TASK1 == false && StartReadClocks==true)

s.o.

if (TASK1==true)
    {        
      while(TASK1==false);                                                              // wait until Task1 is done

Das verstehe ich nicht. Es ist entweder sinnlos oder der Code enthält data races. Gefolgt von

// both commands are needed to start TASK2
      StartReadClocks=true;                                                            
      LongPulse = false;
    }

    if (TASK1==false)
    {
      // both commands are needed to start TASK2      
      StartReadClocks = true;                                                           // start Task2
      LongPulse = false;
    }

wird es noch sinnloser. Codeduplikation?

if (TIM1->EGR & TIM_EGR_UG & TASK2 == true)
    {
      while(TASK2==false);                                                              // wait until Task2 is done
      TIM15->SR &= ~TIM_EGR_UG;                                                         // clear UG Bit
      LongPulse = true;                                                                 // starts TASK1 long pulse at the end of Repetition counter
      StartReadClocks=false;                                                            // TASK2 is stopped
      u8count1++;                                                                       // increase counter
    }

    if (TIM1->EGR & TIM_EGR_UG & TASK2 == false)
    {
      TIM15->SR &= ~TIM_EGR_UG;                                                         // clear UG Bit
      LongPulse = true;                                                                 // starts TASK1 long pulse at the end of Repetition counter
      StartReadClocks=false;                                                            // TASK2 is stopped
      u8count1++;                                                                       // increase counter
    }

Unnötige Codeduplikation.

Komplizierter:

TASK1 = true;
         SetTIM1Param(0, TRD_PULSE, LONG_TIM1_PULSE_REPETITION_COUNT_VALUE);
         Enable_EPCReadCLK_CH(EN_EPCx_CHs[u8count1]);      
         Enable_Timerx_Counter(TIM1);
         for(uint32_t i = 0; i < 500000; i++);                                               // wait until TCDS
         TASK1 = false;                                                                      // if in ARR Interrupt reacts

(und analog in anderen Teilen).
Muss das funktionieren? Soweit ich sehe, sind die Zugriffe auf TASK1 keine richtigen Fences. Ist der Compiler daran gehindert, z.B. SetTIM1Param vor dem

TASK1 = true;

auszuführen?

P.S. Vielleicht soll es ja auch

if (TASK1==true)
    {        
      while(TASK1);                                                              // wait until Task1 is done

heissen? Dann würde der Code auch zum Kommentar passen. Weiter unten analog?
Dann würde das Ganze zu

void TIM1_UP_TIM16_IRQHandler (void)                                                    // LONG_PULS overflow interrupt to start Read Clocks of CH1
{
    while(TASK1)
      ;                                                                                 // wait until Task1 is done

    StartReadClocks = true;                                                             // start Task2
    LongPulse = false;                                                                  // both commands are needed to start TASK2      

    if (TIM1->EGR & TIM_EGR_UG)
    {
      while (TASK2)
        ;                                                                               // wait until Task2 is done
      TIM15->SR &= ~TIM_EGR_UG;                                                         // clear UG Bit
      LongPulse = true;                                                                 // starts TASK1 long pulse at the end of Repetition counter
      StartReadClocks=false;                                                            // TASK2 is stopped
      u8count1++;                                                                       // increase counter
    }
}

Hi

Vielen Dank für deine Mühe.

Warum eigentlich folgendes const.? Was könnte im schlimmsten Fall passieren?

uint16_t EN_EPCx_CHs[] = {EN_EPC1_CH, EN_EPC2_CH, EN_EPC3_CH, EN_EPC4_CH};

#define DMA_CHANNELS             3
#define DataRdyAmount            3

Die Arrays sind 3 lang, mit der 0 hat es eine Grösse von 4.
Deshalb hab ich in den Uebergaben auch immer u8count1 - 1 drin. u8count1 ist am Anfang 0 und wird zu eins nach dem einmaligen Interrupt EXTI15_10_IRQHandler.
Das sollte so passen.

if (LongPulse && !TASK2 && !StartReadClocks)

ist erledigt

for(uint32_t i = 0; i < 500000; i++);                                               // wait until TCDS	
Diese Zeile überlebt keinen optimierenden Compiler. Wenn keine geeignete Systemfunktion existiert, könntest du z.B. volatile-Writes zu Hilfe nehmen.

Das verstehe ich nicht, warum einen dummy einbauen? Sorry für die doofen Fragen?

Genau wegen dem Interrupt Teil, dem komplizierten Teil bin ich auch so unzufrieden. Der Ablauf ist folgender: (Mein Problem ist einiges weiter unten)
Nach dem 1. Interrupt DataRdy1 (EXTI15_10_IRQHandler), kann ausgelesen werden, das ist das Startsignal.
DataRdy 2 muss ich dann einfach mit abfragen, bevor ich den 2. Sensor auslese, sprich DataRdy 3 vor dem 3. und 4 vor dem 4. Darum muss ich das 4 mal machen.

Vor dem Auslesen muss ich einen etwas längeren Puls erzeugen, dann sehr kurze Pulse, so wie es die Beschreibung verlangt.

Die Startbedingungen sind mit den volatile Definitionen festgelegt.

volatile uint8_t u8count1=0; 
volatile bool LongPulse=false; 
volatile bool TASK1=false; 
volatile bool TASK2=false; 
volatile bool StartReadClocks=false;

Mein Problem fängt da schon an. Ich habe den Timer 1 und nur noch einen einzigen Interrupt TIM1_UP_TIM16_IRQHandler. Dieser kommt immer bei einer fallenden Flanke, egal welcher Puls und ganz am Ende wenn der Repetition Counter 0 wird. Dann wird das UG bit gesetzt und in den Interrupt gesprungen.
Das heisst, die Interrupts kommen schneller (am schnellsten mit Periode 20*12.5=250ns) als ich die Tasks mit den Umkonfigurationen etc. durchführen kann. Teilweise nach us abgeschlossen.

Deshalb denke ich, ich benötige eher den

if (TIM1->EGR & TIM_EGR_UG & TASK2 == true)

als den

if (TIM1->EGR & TIM_EGR_UG & TASK2 == true)

Aber sicher bin ich mir nicht, ich könnte das mit dem Oszi mal messen.
Selbiges Problem habe ich mit TASK1 true und false.

Ich weiss aber auch nicht wie man das am Besten lösen kann das Problem, denn im Interrupt zu warten bis der Task zu Ende ist, ist so als würde ich den Task direkt im Interrupt ausführen..

Ich wäre dir hier noch sehr dankbar, wenn dieser Knoten in meinem Kopf auch noch gelöst werden könnte.

Nachtrag:
Das UG Bit wird nur gesetzt, wenn der Rep. Counter 0 wird, damit wird gekennzeichnet, dass die schnellen Clocks abgeschlossen sind sind und der 2. Sensor ausgelesen werden kann.

Korrektur:

Deshalb denke ich, ich benötige eher den

if (TIM1->EGR & TIM_EGR_UG & TASK2 == true)

als den
Sollte

if (TIM1->EGR & TIM_EGR_UG & TASK2 == false)

heissen

camper schrieb:

void TIM1_UP_TIM16_IRQHandler (void) // LONG_PULS overflow interrupt to start Read Clocks of CH1
{
while(TASK1); // wait until Task1 is done

StartReadClocks = true; // start Task2
LongPulse = false; // both commands are needed to start TASK2

if (TIM1->EGR & TIM_EGR_UG)
{
while (TASK2); // wait until Task2 is done
TIM15->SR &= ~TIM_EGR_UG; // clear UG Bit
LongPulse = true; // starts TASK1 long pulse at the end of Repetition counter
StartReadClocks=false; // TASK2 is stopped
u8count1++; // increase counter
}
}

Ich glaub das ist es
Das löst glaucb ich mein Problem, aber dann würde er immer noch im Interrupt stecken auch wenn 1000 weitere kommen.

camper

buell schrieb:

HWarum eigentlich folgendes const.? Was könnte im schlimmsten Fall passieren?
uint16_t EN_EPCx_CHs[] = {EN_EPC1_CH, EN_EPC2_CH, EN_EPC3_CH, EN_EPC4_CH};

Wenn es nicht const ist, könnte jemand auf den Gedanken kommen, dass sich die Werte ändern können. U.U. hindert es auch den Compiler daran, bestimmte Optimierungen auszuführen, wenn er nicht sicher sein kann, dass die Werte unveränderlich sind.

buell schrieb:

#define DMA_CHANNELS             3
#define DataRdyAmount            3
Die Arrays sind 3 lang, mit der 0 hat es eine Grösse von 4.

static DataRdy_t DataRdys[DataRdyAmount];

Sagt dem Compiler, dass DataRdys ein Array mit DataRdyAmount Elementen sein soll (0...DataRdyAmount-1).

buell schrieb:

for(uint32_t i = 0; i < 500000; i++);                                               // wait until TCDS	
Diese Zeile überlebt keinen optimierenden Compiler. Wenn keine geeignete Systemfunktion existiert, könntest du z.B. volatile-Writes zu Hilfe nehmen.

Das verstehe ich nicht, warum einen dummy einbauen?

Grundsätzlich steht es dem Compiler frei, irgendwelchen Code zu erzeugen, sofern sich das beobachtbare Verhalten nicht ändert. Was als beobachtbar zu betrachten ist, wird ebenso vom Standard festgelegt (vereinfacht):
- Die Reihenfolge der Zugriffe auf volatile Objekte
- Reihenfolge von Input-/Output (die genaue Bedeutung hängt von der Implementation ab, aber im Allgemeinen ist das jeder Aufruf einer Systemfunktion, ggf. auch indirekt über Funktionen der Standardbibliothek).
Die Schleife verändert keine dieser beiden Aspekte, tut also im Sinne des so definierten beobachtbaren Verhaltens nichts. Der Compiler kann sie folglich einfach ignorieren. Der reine Zeitablauf als solcher ist im Allgemeinen kein Aspekt beobachtbaren Verhaltens.

P.S. Auch mit dummy-Writes ist das nat. keine Schleife, die geeignet ist, eine bestimmte Zeitdauer zu überbrücken. Im Allgemeinen wird es ja so sein, das bekannt ist (Spezifikation, RL-Zeitmass), wie lange mindestens gewartet werden muss. Es ist allerdings keineswegs per se gesichert, dass pro Schleifendurchgang (+Write) eine bestimmte konstante Zeit vergeht. Dafür gibt es dann normalerweise Systemfunktionen, die den Programablauf mit irgendwelchen Uhrwerken synchronisieren.
So eine volatile write-Schleife ist also die schlechteste aller Möglichkeiten, die wirklich nur in Betracht kommen sollte, wenn keine Alternative besteht.

Der echte Tim

Es ist ja schön, dass der Code verschönert wird, aber geht das ganze in HW nicht doch etwas eleganter?

- Was soll denn eigentlich gemacht werden?
- Welcher µC wird denn verwendet?

camper

buell schrieb:

Das löst glaucb ich mein Problem, aber dann würde er immer noch im Interrupt stecken auch wenn 1000 weitere kommen.

Zum abstrakten Ablauf habe ich mir bisher überhaupt keine Gedanken gemacht, weil mir nicht bekannt ist, welche Art Gerät überhaupt programmiert wird.

Im Code wird von TASKs gesprochen. Reden wir handelt es sich hierbei um separate Threads? Oder arbeitet das Programm evtl. nur mit einem Idle-Loop, so dass die Abläufe sequentiell geordnet sind? Sind Interrupts unterbrechbar (andere Interrupts/Scheduler) und müssen die entsprechenden Routinen ggf. reentrant sein?

buell

camper schrieb:

buell schrieb:
HWarum eigentlich folgendes const.? Was könnte im schlimmsten Fall passieren?
uint16_t EN_EPCx_CHs[] = {EN_EPC1_CH, EN_EPC2_CH, EN_EPC3_CH, EN_EPC4_CH};
Wenn es nicht const ist, könnte jemand auf den Gedanken kommen, dass sich die Werte ändern können. U.U. hindert es auch den Compiler daran, bestimmte Optimierungen auszuführen, wenn er nicht sicher sein kann, dass die Werte unveränderlich sind.

buell schrieb:
#define DMA_CHANNELS             3
#define DataRdyAmount            3
Die Arrays sind 3 lang, mit der 0 hat es eine Grösse von 4.
static DataRdy_t DataRdys[DataRdyAmount];
Sagt dem Compiler, dass DataRdys ein Array mit DataRdyAmount Elementen sein soll (0...DataRdyAmount-1).

buell schrieb:
for(uint32_t i = 0; i < 500000; i++);                                               // wait until TCDS	
Diese Zeile überlebt keinen optimierenden Compiler. Wenn keine geeignete Systemfunktion existiert, könntest du z.B. volatile-Writes zu Hilfe nehmen.
Das verstehe ich nicht, warum einen dummy einbauen?
Grundsätzlich steht es dem Compiler frei, irgendwelchen Code zu erzeugen, sofern sich das beobachtbare Verhalten nicht ändert. Was als beobachtbar zu betrachten ist, wird ebenso vom Standard festgelegt (vereinfacht):
- Die Reihenfolge der Zugriffe auf volatile Objekte
- Reihenfolge von Input-/Output (die genaue Bedeutung hängt von der Implementation ab, aber im Allgemeinen ist das jeder Aufruf einer Systemfunktion, ggf. auch indirekt über Funktionen der Standardbibliothek).
Die Schleife verändert keine dieser beiden Aspekte, tut also im Sinne des so definierten beobachtbaren Verhaltens nichts. Der Compiler kann sie folglich einfach ignorieren. Der reine Zeitablauf als solcher ist im Allgemeinen kein Aspekt beobachtbaren Verhaltens.

P.S. Auch mit dummy-Writes ist das nat. keine Schleife, die geeignet ist, eine bestimmte Zeitdauer zu überbrücken. Im Allgemeinen wird es ja so sein, das bekannt ist (Spezifikation, RL-Zeitmass), wie lange mindestens gewartet werden muss. Es ist allerdings keineswegs per se gesichert, dass pro Schleifendurchgang (+Write) eine bestimmte konstante Zeit vergeht. Dafür gibt es dann normalerweise Systemfunktionen, die den Programablauf mit irgendwelchen Uhrwerken synchronisieren.
So eine volatile write-Schleife ist also die schlechteste aller Möglichkeiten, die wirklich nur in Betracht kommen sollte, wenn keine Alternative besteht.

Du hast mir heute sehr viel beigebracht. Ich danke dir vielmals. Vielen Dank aber auch an alle anderen.
Das mit der for-Schleife schaue ich mir morgen an.

Ich habe noch eine letzte Frage:

Bei deinem Code:

void TIM1_UP_TIM16_IRQHandler (void)                                                    // LONG_PULS overflow interrupt to start Read Clocks of CH1 
{ 
    while(TASK1) 
      ;                                                                                 // wait until Task1 is done 

    StartReadClocks = true;                                                             // start Task2 
    LongPulse = false;                                                                  // both commands are needed to start TASK2       

    if (TIM1->EGR & TIM_EGR_UG) 
    { 
      while (TASK2) 
        ;                                                                               // wait until Task2 is done 
      TIM15->SR &= ~TIM_EGR_UG;                                                         // clear UG Bit 
      LongPulse = true;                                                                 // starts TASK1 long pulse at the end of Repetition counter 
      StartReadClocks=false;                                                            // TASK2 is stopped 
      u8count1++;                                                                       // increase counter 
    } 
}

habe ich folgendes Problem und zwar:
wenn der Task 1 durchgeführt wurde und Task1 false wird, sollte im Anschluss Task 2 aktiv werden. Wenn Task2 aktiv ist, ist Task1 sicher false.
Wenn aber der nächste Interrupt kommt, sollte er mir die folgenden Befehle nach der 1. while nicht ausführen.

Beim Task1 benötige ich also die if, das ist der Code unten.
Jetzt besteht aber das Problem, dass er mir die Befehle nicht mehr ausführt, wenn Task1 false ist und ich komme nie in den Task2

Entweder ist das so verzwickt oder ich stelle mich so blöd an

void TIM1_UP_TIM16_IRQHandler (void)                                                    // LONG_PULS overflow interrupt to start Read Clocks of CH1
{ 

    if (TASK1) 
    {  
       while(TASK1);                                                                    // wait until Task1 is done 
       StartReadClocks = true;                                                          // start Task2 
       LongPulse = false;                                                               // both commands are needed to start TASK2 
    }       

    if (TIM1->EGR & TIM_EGR_UG) 
    { 
      while (TASK2);                                                                    // wait until Task2 is done 
      TIM15->SR &= ~TIM_EGR_UG;                                                         // clear UG Bit 
      LongPulse = true;                                                                 // starts TASK1 long pulse at the end of Repetition counter 
      StartReadClocks=false;                                                            // TASK2 is stopped 
      u8count1++;                                                                       // increase counter 
    } 

}