Interrupts und Tasks - Codeoptimierung
-
Hallo zusammen
Ich habe da einen Code geschrieben und wollte sicher gehen, dass dieser auch effizient ist. (Ich bin C-Anfänger)
Es geht darum, dass ich zuerst einen einzigen längeren Puls und darauffolgend kürzere Pulse erzeugen möchte.
Anfangs habe ich mir gedacht lege ich Startbedingungen fest.
Der Interrupt DATARDY kommt bevor das ganze los geht nur einmal.
Ich habe insgesamt immer 4 Durchgänge, sprich 1 Longpuls, 10 Shortpulses und das Ganze in Serie 4x)
Der Interrupt ARR (AutoReloadRegister) kommt sowohl beim ersten Puls bei einer fallenden Flanke als auch später bei den fallenden Flanken der kürzeren Pulse.
Am Schluss, wenn die kurzen Pulse fertig sind, wird ebenfalls in den Interrupt gesprungen. Das Ende ist mit dem Setzen des UG Bit erkennbar.
Wenn Timer1--> Disable_Timerx_Counter(TIM1), ist alles ausgeschaltet und es kommen keine Interrupts mehr. Das sollte aber immer beim Setzen des UG Bits automatisch passieren, das habe ich aber trotzdem im 4. Schritt im Task2 drinnen.
Die Zeile "warte bis TCDS pulse fertig" ist deshalb drin, weil der längere Puls früher fertig wird als die Zeit die zum Warten nötig ist. Da habe ich mir überlegt, dass ich dies mit einer for-Schleife erledige.
Könnte das so funktionieren oder ist der Code so absolut unbrauchbar?Vielen Dank schon mal für eure Hilfe
buell/* Interrupt DATARDY: kommt nur einmal am Anfang*/ // Anfagnsbedingungen uint8_t u8count1=0; bool LongPulse=false; bool TASK1=false; bool TASK2=false; bool StartReadClocks=false; // Anfagnsbedingungen Disable_Shutter_TIM15(); u8count1++; LongPulse = true; --> startet TASK1 LongPulse /* Ende Interrupt DATARDY*/ /* Interrupt ARR*/ Interrupt_ARR_erreicht_LongPulse: //Interrupt kommt erst nach dem 1. LongPulse, deshalb nur TASK1== false Abfrage. if (TASK1==true) { while(TASK1==false); dann StartReadClocks=true // startet TASK2 short pulse LongPulse = false; } if (TASK1==false) { StartReadClocks==true // startet TASK2 short pulse LongPulse = false; } if (TIM1->EGR & TIM_EGR_UG & TASK2 == TRUE) { while(TASK2==false); // warte bis TASK2 fertig Lösche UG bit LongPulse = true; // startet TASK1 long pulse am Ende Repcount StartReadClocks=false; // TASK2 is stopped u8count1++; } if (TIM1->EGR & TIM_EGR_UG & TASK2 == FAlse) { Lösche UG bit LongPulse = true; // startet TASK1 long pulse am Ende Repcount StartReadClocks=false; // TASK2 is stopped u8count1++; } /* Ende Interrupt ARR*/ /* TASK1(LongPulseTIM1)*/ if (LongpulseTIm1 == true & TASK2 == false & StartReadClocks==false) { TASK1 = true; if (u8count1 == 1) { SetTIM1Param(0, TRD_PULSE, LONG_TIM1_PULSE_REPETITION_COUNT_VALUE); Enable_EPCReadCLK_CH(EN_EPC1_CH); Enable_Timerx_Counter(TIM1); warte bis TCDS pulse fertig } if ((HAL_GPIO_ReadPin(DataRdy2.port, DataRdy2.pin) == 1) && (u8count1 == 2)) { SetTIM1Param(0, TRD_PULSE, LONG_TIM1_PULSE_REPETITION_COUNT_VALUE); Enable_EPCReadCLK_CH(EN_EPC2_CH); Enable_Timerx_Counter(TIM1); warte bis TCDS pulse fertig } if ((HAL_GPIO_ReadPin(DataRdy3.port, DataRdy3.pin) == 1) && (u8count1 == 3)) { SetTIM1Param(0, TRD_PULSE, LONG_TIM1_PULSE_REPETITION_COUNT_VALUE); Enable_EPCReadCLK_CH(EN_EPC3_CH); Enable_Timerx_Counter(TIM1); warte bis TCDS pulse fertig } if ((HAL_GPIO_ReadPin(DataRdy4.port, DataRdy4.pin) == 1) && (u8count1 == 4)) { SetTIM1Param(0, TRD_PULSE, LONG_TIM1_PULSE_REPETITION_COUNT_VALUE); Enable_EPCReadCLK_CH(EN_EPC4_CH); Enable_Timerx_Counter(TIM1); warte bis TCDS pulse fertig } TASK1 = false; // If im Interrupt_ARR_erreicht_LOngPulse wird aktiv /* Ende TASK1(LongPulseTIM1)*/ /* TASK2(ShortPulseTim1)*/ if (LongPulseTim1 == false & TASK1 == false & StartReadClocks==true) { TASK2 = true; if (count1 == 1) { select_ADC_Channel_to_Convert(1); SetTIM1Param(0, TRD_CLOCK, 1027); Enable_EPCReadCLK_CH(EN_EPC1_CH | EN_ADC_TRG_CH); Enable_Timerx_Counter(TIM1); } if (count1 == 2) { SetTIM1Param(0, TRD_CLOCK, 1027); Enable_EPCReadCLK_CH(EN_EPC2_CH | EN_ADC_TRG_CH); Enable_Timerx_Counter(TIM1); StartReadClocks=false; } if (count1 == 3) { SetTIM1Param(0, TRD_CLOCK, 1027); Enable_EPCReadCLK_CH(EN_EPC3_CH | EN_ADC_TRG_CH); Enable_Timerx_Counter(TIM1); StartReadClocks=false; } if (count1 == 4) { SetTIM1Param(0, TRD_CLOCK, 1027); Enable_EPCReadCLK_CH(EN_EPC3_CH | EN_ADC_TRG_CH); Enable_Timerx_Counter(TIM1); StartReadClocks=false; Enable_Shutter_TIM15(); Disable_Timerx_Counter(TIM1); // Anfangszustände herstellen count1 = 0; LongPUlse=false; TASK1=false; TASK2=false; StartReadClocks=false; } TASK2 = false; // make sure that TASK1 gets active /* Ende TASK2(ShortPulseTim1)*/
-
Hallo nochmals
Ich hätte so denke ich den Code gleich direkt in der IDE schreiben sollen. Ich habe noch einige Fehler gefunden und diese behoben.
Prinzipiell würde es mir darum gehen, zu erfahren, ob der Code mit diesen if-Anweisungen so passt oder man es besser machen könnte.
Task1 und Task2 sind eigentlich keine Funktionen, darum muss ich mich später kümmern. Diese dienen nur zum Anzeigen, dass es später Tasks sein sollten.
Ich wäre jemandem sehr dankbar, wenn ich diesbezüglich ein paar Inputs bekommen könnte. Hier nochmals der korrigierte Code.
Mir ist vor allem nicht klar, wie ich das mit der Abfrage besser machen kann, da ich zweimal das Selbe abfrage, da ich ja nicht weiss, ob der Task2 bereits fertig ist, wenn das UG bit gesetzt wurde. Ansonsten springt er mir aus dem Interrupt raus und führt den Befehl nicht aus.Aussderdem weiss ich nicht, warum use of undeclared identifier "DataRdy1" bis DataRdy4 kommt, obwohl diese im config.h definiert sind und im main.c eingebunden ist.
Weiters habe ich den Fehler implicite declaration of function Set_Destination_DMA1_ADC is invalid in C99. Bei den weiteren gleichen Funktionen meckert der Compiler aber nicht.
Ich wäre euch sehr dankbar für kurzen Input und Hilfe
Hier ist der Code:
// globale Variablen im Main uint8_t u8count1=0; bool LongPulse=false; bool TASK1=false; bool TASK2=false; bool StartReadClocks=false; // Funktionen der Tasks nur zur Visualisierung void TASK1_long_Pulse() { if (LongPulse == true & TASK2 == false & StartReadClocks==false) { TASK1 = true; uint32_t u32i; if (u8count1 == 1) { SetTIM1Param(0, TRD_PULSE, LONG_TIM1_PULSE_REPETITION_COUNT_VALUE); Enable_EPCReadCLK_CH(EN_EPC1_CH); Enable_Timerx_Counter(TIM1); for(u32i = 0; u32i < 500000; u32i++); } if ((HAL_GPIO_ReadPin(DataRdy2.port, DataRdy2.pin) == 1) && (u8count1 == 2)) { SetTIM1Param(0, TRD_PULSE, LONG_TIM1_PULSE_REPETITION_COUNT_VALUE); Enable_EPCReadCLK_CH(EN_EPC1_CH); Enable_Timerx_Counter(TIM1); for(u32i = 0; u32i < 500000; u32i++); } if ((HAL_GPIO_ReadPin(DataRdy3.port, DataRdy3.pin) == 1) && (u8count1 == 3)) { SetTIM1Param(0, TRD_PULSE, LONG_TIM1_PULSE_REPETITION_COUNT_VALUE); Enable_EPCReadCLK_CH(EN_EPC1_CH); Enable_Timerx_Counter(TIM1); for(u32i = 0; u32i < 500000; u32i++); } if ((HAL_GPIO_ReadPin(DataRdy4.port, DataRdy4.pin) == 1) && (u8count1 == 4)) { SetTIM1Param(0, TRD_PULSE, LONG_TIM1_PULSE_REPETITION_COUNT_VALUE); Enable_EPCReadCLK_CH(EN_EPC1_CH); Enable_Timerx_Counter(TIM1); for(u32i = 0; u32i < 500000; u32i++); } TASK1 = false; // if in ARR Interrupt reacts } } void TASK2_short_Pulse() { if (LongPulse == false & TASK1 == false & StartReadClocks==true) { TASK2 = true; if (u8count1 == 1) { 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); } if (u8count1 == 2) { select_ADC_Channel_to_Convert(2); // set the ADC channel Set_Destination_DMA1_ADC(dma1_buffer); // set the DMA destination SetTIM1Param(0, TRD_CLOCK, 1027); Enable_EPCReadCLK_CH(EN_EPC2_CH | EN_ADC_TRG_CH); Enable_Timerx_Counter(TIM1); } if (u8count1 == 3) { select_ADC_Channel_to_Convert(3); // set the ADC channel Set_Destination_DMA1_ADC(dma2_buffer); // set the DMA destination SetTIM1Param(0, TRD_CLOCK, 1027); Enable_EPCReadCLK_CH(EN_EPC3_CH | EN_ADC_TRG_CH); Enable_Timerx_Counter(TIM1); } if (u8count1 == 4) { select_ADC_Channel_to_Convert(4); // set the ADC channel Set_Destination_DMA1_ADC(dma3_buffer); // set the DMA destination SetTIM1Param(0, TRD_CLOCK, 1027); Enable_EPCReadCLK_CH(EN_EPC4_CH | EN_ADC_TRG_CH); Enable_Timerx_Counter(TIM1); Enable_Shutter_TIM15(); Disable_Timerx_Counter(TIM1); // make sure the timer1 is disabled // set the start conditions u8count1 = 0; LongPulse=false; TASK1=false; TASK2=false; } StartReadClocks=false; TASK2 = false; // make sure that TASK1 gets active } } // Interrupt DataRdy 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 } } // Interrupt ARR erreicht - fallende Flanken 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 LongPulse = true; // starts TASK1 long pulse at the end of Repetition counter StartReadClocks=false; // TASK2 is stopped u8count1++; // increase counter } }
-
- globale Variablen sind Schrott
- statt deiner if == 1,2,3 Kaskaden 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.Interruptprogrammierung ist kein Grund dafür, schlechtes C zu schreiben.
-
[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
-
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) == 1mit 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) == 1mit 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
-
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 switchvoid 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 } }
-
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 } }
-
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 } }
-
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.
-
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 3dü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 TCDSDiese 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 doneDas 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 demTASK1 = true;auszuführen?
P.S. Vielleicht soll es ja auch
if (TASK1==true) { while(TASK1); // wait until Task1 is doneheissen? Dann würde der Code auch zum Kommentar passen. Weiter unten analog?
Dann würde das Ganze zuvoid 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 3Die 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
Sollteif (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 doneStartReadClocks = true; // start Task2
LongPulse = false; // both commands are needed to start TASK2if (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.
-
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 3Die 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.
-
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?
