Naked ISR hier ok?

Ingo L. #5591023
Lesenswert?
• ▲
▼ 
Hallo,

ich habe ein paar Zeilen Code, es spielt sich absolut alles in der ISR 
ab, von daher ist die Frage, ob die ISR hier "naked" sein darf. Der 
Pro-/Epilog einer normalen ISR macht schon ganz ordentlich Laufzeit bei 
einem Systemtakt von 128kHz.
1
int main ( void )

2
{

3
  _delay_ms( 100 );

4


5
  // Pulse Output

6
  PULSE_HIGH();

7
  PULSE_DDR |= (1<<PULSE_PIN);

8
  

9
  // Disable AD-Comp

10
  ACSR |= (1<<ACD);

11
  

12
  // Unused Pin low

13
  DDRB |= (1<<PB2) | (1<<PB3) | (1<<PB4);

14
  PORTB &= ~( (1<<PB2) | (1<<PB3) | (1<<PB4) );

15
  

16
  //Setup AVR

17
  OCR0A = 111;

18
  TCCR0A |= (1<<WGM01);      // CTC

19
  TCCR0B |= (1<<CS00);      // Prescaler = 1

20
  TIMSK0 |= (1<<OCIE0A);      // Interrupt enabler

21
  

22
  //External Interrupt

23
  PORTB |= (1<<PB1);          // internal Pullup enable

24
  MCUCR |= (1<<ISC01);        // Trigger on falling edge

25
  CLEAR_SENSOR_FLAG();

26
  

27
  // Global Interrupt enable

28
  sei();

29
  

30
    /* Nice weather... */

31
    while ( "Cycling" );

32
}

33


34


35
ISR ( TIMEBASE_1MS, ISR_NAKED )

36
{

37
  static uint16_t TimeCounter = 0;

38
  static uint16_t RoundTrip[2] = {0,0};

39
  static uint16_t TimeDifference = PULSE_PERIOD_MAX_MS;

40
  static uint16_t Pulsegenerator = UINT16_MAX;

41
  static uint16_t TunedOutputPeriod = PULSE_PERIOD_MAX_MS;

42
  static uint16_t Timeout = ZERO_TIME_DETECT;

43


44
  // Nullerkennung

45
  if ( Timeout ) Timeout--;

46


47
  // Impuls Erfassung  

48
  TimeCounter++;

49
  

50
  if ( SENSOR_TRIGGERD ){

51
    Timeout = ZERO_TIME_DETECT;

52
    CLEAR_SENSOR_FLAG();

53
    RoundTrip[NEW] = TimeCounter;

54
    TimeDifference = RoundTrip[NEW] - RoundTrip[OLD];

55
    RoundTrip[OLD] = RoundTrip[NEW];

56
  }

57


58
  // Impuls Ausgabe

59
  if ( TimeDifference < PULSE_PERIOD_MIN_MS ){ // abgeregelt

60
    TunedOutputPeriod = PULSE_PERIOD_MIN_MS;

61
  }else{// unmanipuliert

62
    TunedOutputPeriod = TimeDifference;

63
  }

64
  

65
  if ( TunedOutputPeriod > PULSE_PERIOD_MAX_MS ) TunedOutputPeriod = PULSE_PERIOD_MAX_MS; 

66
  

67
  if ( Pulsegenerator >= TunedOutputPeriod ) Pulsegenerator = 0;

68
  else Pulsegenerator++;

69
  

70
  if ( Pulsegenerator < PULSE_DURATION_MS && Timeout ) PULSE_LOW();

71
  else PULSE_HIGH();

72


73
  reti();

74
}


Funktionieren tut es, aber gibts hier irgendwelche Fallstricke?
Antwort Zitieren Bearbeiten Thread verschieben Thread sperren Anmeldepflicht aktivieren Anpinnen Thread löschen Thread mit anderem zusammenführen Beitrag melden
Oliver S. #5591078
Lesenswert?
• ▲
▼ 
Rein theoretisch sollte die Endlosschleife im Hauptprogramm zu einem 
einfachen jmp zusammenschrumpfen, dem es egal ist, ob die ISR 
zwischenzeitlich Register oder Flags ändert.

Rein praktisch solltest du dir den generierten Assemblercode anschauen, 
ob das auch so tatsächlich ist.

Oliver
Antwort Zitieren Bearbeiten Löschen Beitrag melden
Ingo L. #5591195
Lesenswert?
• ▲
▼ 
Bernd K. schrieb:
> Ich würde mir überlegen ob es in diesem konkreten Fall nicht sogar
> sinnvoller wäre komplett auf den Interrupt zu verzichten und
> stattdessen in der main-loop das entsprechende Timer-Bit zu pollen und
> alles in der main zu machen.
Wo wäre da der Vorteil gegenüber der jetzigen Lösung? Ich würde sie in 
diesem Fall sogar als absolut gleichwertig sehen
Antwort Zitieren Bearbeiten Löschen Beitrag melden
Peter D. #5591245
Lesenswert?
• ▲
▼ 
Ingo L. schrieb:
> Wo wäre da der Vorteil gegenüber der jetzigen Lösung? Ich würde sie in
> diesem Fall sogar als absolut gleichwertig sehen

Nö, die Loop hat mehrere Vorteile:
Bit testen, löschen und zurück springen geht schneller als 
Interrupteinsprung und Return.
Der Compiler merkt, daß die Loop nie verlassen wird und
wird viele Variablen in Registern halten.

Ingo L. schrieb:
>     while ( "Cycling" );

Ist jetzt nicht so die feine Art, einen Stringpointer als Bool zu 
vergewaltigen.
C erlaubt viele Schweinereien, man sollte sie aber nicht grundlos 
verwenden.
Heutzutage ist es hip, der Lesbarkeit den Vorzug zu geben.
Antwort Zitieren Bearbeiten Löschen Beitrag melden
Bernd K. #5591290
Lesenswert?
• ▲
▼ 
Ingo L. schrieb:
>> stattdessen in der main-loop das entsprechende Timer-Bit zu pollen und
>> alles in der main zu machen.
> Wo wäre da der Vorteil gegenüber der jetzigen Lösung? Ich würde sie in
> diesem Fall sogar als absolut gleichwertig sehen

Konkretes Beispiel:

Ich hab hier einen aktuellen Fall wo ich eine 80kHz PLL auf einem Silabs 
8051 (EFM8BB10) implementierte. Der erste Versuch verwendete den 
ADC-Interrupt (der dann mit 320kHz kam) und fast die ganze CPU-Zeit ging 
dafür drauf in den Interrupt hinein und wieder hinaus zu springen, 
zusätzlich noch der Overhead bis 4 zu zählen und jedesmal zu schauen 
welche der 4 Werte (I, Q, -I, -Q) jetzt gerade gesampelt wurde. Ich war 
schon bei 80% CPU allein vom Grundgerüst ohne schon irgendwas anderes zu 
machen als nur Samples einzulesen und zuzuordnen. Die Zeit hat nicht 
mehr gereicht noch irgendwas sinnvolles damit zu machen, es ging so 
nicht.

Im zweiten Anlauf machte ich eine main-loop die pro Durchlauf 4 mal auf 
das conversion complete bit wartet, danach jeweils ein bisschen was tut 
(erst dies, dann das, eben alles was einmal pro Periode zu tun ist schön 
verteilt), die Zeit reicht jetzt gut aus für alles und könnte jetzt 
sogar bis 100kHz gehen wenn ich wollte.
Antwort Zitieren Bearbeiten Löschen Beitrag melden
Jörg W. Moderator Persönliche Seite #5591380
Lesenswert?
• ▲
▼ 
Niklas G. schrieb:
> Beim AVR braucht das aber ggf. Register-Zugriffe...

Wenn man sleep_mode() nimmt, ja, weil das ja das Äquivalent zu

1
  sleep_enable();

2
  sleep_cpu();

3
  sleep_disable();


ist. Wenn man jedoch auf die (eigentlich empfohlene) Abfolge verzichtet 
und das SE-Bit immer gesetzt lässt:

1
  sleep_enable();

2


3
  while("awake")

4
    sleep_cpu();


dann ist das ja nur ein inline asm für den SLEEP-Befehl, der sich 
natürlich nicht weiter um Register kümmert.
Antwort Zitieren Bearbeiten Löschen Beitrag melden
Ingo L. #5591467
Lesenswert?
• ▲
▼ 
egberto schrieb:
> Wenn das mal richtig läuft, kannst du den kompletten Code ja unter
> "Fahrzeugelektronik" Thema E-Bike Tuning posten ;-)
Funktioniert doch, ich habe die CPU jetzt auf 300kHz laufen, damit 
schaffe ich 2kHz Timerinterrupt.
1
int main ( void )

2
{

3
  _delay_ms( 100 );

4
  

5
  CLKPR = (1<<CLKPCE);

6
  CLKPR = (1<<CLKPS2);    // /16

7
  

8
  OSCCAL = 0x63;

9
  // Pulse Output

10
  PULSE_HIGH();

11
  PULSE_DDR |= (1<<PULSE_PIN);

12
  

13
  // Disable AD-Comp

14
  ACSR |= (1<<ACD);

15
  

16
  // Unused Pin low

17
  DDRB |= (1<<PB2) | (1<<PB3) | (1<<PB4);

18
  PORTB &= ~( (1<<PB2) | (1<<PB3) | (1<<PB4) );

19
  

20
  // Setup AVR

21
  OCR0A = 17;

22
  TCCR0A |= (1<<WGM01);      // CTC

23
  TCCR0B |= (1<<CS01);      // Prescaler = 1

24
  

25
  // External Interrupt

26
  PORTB |= (1<<PB1);          // internal Pullup enable

27
  MCUCR |= (1<<ISC01);        // Trigger on falling edge

28
  CLEAR_SENSOR_FLAG();

29


30
  

31
    /* Nice weather... */

32
    while ( "Cycling" ){

33
    if ( TIFR0 & (1<<OCF0A) ){

34
      #if DEBUG_MODE == 1

35
        PULSE_HIGH();

36
      #endif

37
      

38
      // Clear TimerFlag    

39
      TIFR0 |= (1<<OCF0A);

40
  

41
      // Nullerkennung

42
      if ( Timeout ) Timeout--;

43


44
      // Impuls Erfassung

45
      TimeCounter++;

46
  

47
      if ( SENSOR_TRIGGERD ){

48
        Timeout = ZERO_TIME_DETECT;

49
        CLEAR_SENSOR_FLAG();

50
        RoundTrip[NEW] = TimeCounter;

51
        TimeDifference = RoundTrip[NEW] - RoundTrip[OLD];

52
        RoundTrip[OLD] = RoundTrip[NEW];

53
      }

54


55
      // Impuls Ausgabe

56
      if ( TimeDifference < PULSE_PERIOD_MIN_MS ){ // abgeregelt

57
        TunedOutputPeriod = PULSE_PERIOD_MIN_MS;

58
      }else{// unmanipuliert

59
        TunedOutputPeriod = TimeDifference;

60
      }

61
  

62
      if ( TunedOutputPeriod > PULSE_PERIOD_MAX_MS ) TunedOutputPeriod = PULSE_PERIOD_MAX_MS;

63
  

64
      if ( Pulsegenerator >= TunedOutputPeriod ) Pulsegenerator = 0;

65
      else Pulsegenerator++;

66
      

67
      #if DEBUG_MODE == OFF

68
        if ( Pulsegenerator < PULSE_DURATION_MS && Timeout ) PULSE_LOW();

69
        else PULSE_HIGH();

70
      #elif DEBUG_MODE == ON

71
        if ( Pulsegenerator < PULSE_DURATION_MS && Timeout )PORTB &= ~(1<<PB1);

72
        else PORTB |= (1<<PB1); 

73
        PULSE_LOW();

74
      #endif

75
    }

76
  }

77
}
Antwort Zitieren Bearbeiten Löschen Beitrag melden
Ingo L. #5592087
Lesenswert?
• ▲
▼ 
Egberto schrieb:
> Na so richtig komplett ist es ja nun nicht.....trotzdem vielen
> Dank.
Richtig, das hier fehlt noch:
1
#include <avr/io.h>

2
#include <util/delay.h>

3
#include <avr/interrupt.h>

4


5
#define TIMEBASE_1MS      TIM0_COMPA_vect

6
#define PULSE_DDR        DDRB

7
#define PULSE_PORT        PORTB

8
#define PULSE_PIN        PB0

9
#define PULSE_LOW()        (PULSE_PORT &= ~(1<<PULSE_PIN))

10
#define PULSE_HIGH()      (PULSE_PORT |= (1<<PULSE_PIN))

11


12
#define INTERRUPT_FREQ_KHZ    2

13


14
#define PULSE_PERIOD_MIN_MS    359 * INTERRUPT_FREQ_KHZ    // 22km/h

15
#define PULSE_PERIOD_MAX_MS    7898 * INTERRUPT_FREQ_KHZ    // 1km/h

16
#define PULSE_DURATION_MS    2 * INTERRUPT_FREQ_KHZ

17


18
#define ZERO_TIME_DETECT    8000*INTERRUPT_FREQ_KHZ

19


20
#define SENSOR_TRIGGERD      (GIFR & (1<<INTF0))

21
#define CLEAR_SENSOR_FLAG()    (GIFR |= (1<<INTF0))

22


23
#define NEW            1

24
#define OLD            0

25


26
#define ON            1

27
#define OFF            0

28


29
#define DEBUG_MODE        OFF

30


31
static uint16_t TimeCounter = 0;

32
static uint16_t RoundTrip[2] = {0,0};

33
static uint16_t TimeDifference = PULSE_PERIOD_MAX_MS;

34
static uint16_t Pulsegenerator = UINT16_MAX;

35
static uint16_t TunedOutputPeriod = PULSE_PERIOD_MAX_MS;

36
static uint16_t Timeout = ZERO_TIME_DETECT;
Antwort Zitieren Bearbeiten Löschen Beitrag melden
Ingo L. #5592251
Lesenswert?
• ▲
▼ 
Dennis Restle schrieb:
> Der Code in der ISR hat den Vorteil, dass er immer exakt x Mikrosekunden
> nach dem Bit stattfindet. Bei einem Polling hast du einen Jitter von
> mehreren Taktzyklen.
Der aber bei einer Zeitauflösung von 500µs und einer 
Ausführungsgeschwindigkeit von 3 1/3µs pro Takt nicht auffällt...
Hier die Entprellte Version:
1
#include <avr/io.h>

2
#include <util/delay.h>

3
#include <avr/interrupt.h>

4


5
#define TIMEBASE_1MS      TIM0_COMPA_vect

6
#define PULSE_DDR        DDRB

7
#define PULSE_PORT        PORTB

8
#define PULSE_PIN        PB0

9
#define PULSE_LOW()        (PULSE_PORT &= ~(1<<PULSE_PIN))

10
#define PULSE_HIGH()      (PULSE_PORT |= (1<<PULSE_PIN))

11


12
#define INTERRUPT_FREQ_KHZ    2

13


14
#define PULSE_PERIOD_MIN_MS    359 * INTERRUPT_FREQ_KHZ    // 22km/h

15
#define PULSE_PERIOD_MAX_MS    7898 * INTERRUPT_FREQ_KHZ    // 1km/h

16
#define PULSE_DURATION_MS    2 * INTERRUPT_FREQ_KHZ

17


18
#define ZERO_TIME_DETECT    8000*INTERRUPT_FREQ_KHZ

19


20
#define SENSOR_TRIGGERD      (GIFR & (1<<INTF0))

21
#define CLEAR_SENSOR_FLAG()    (GIFR |= (1<<INTF0))

22


23
#define NEW            1

24
#define OLD            0

25


26
#define ON            1

27
#define OFF            0

28


29
#define DEBOUNCE_TIME_MS    100 * INTERRUPT_FREQ_KHZ  

30


31
#define DEBUG_MODE        OFF

32


33
static uint16_t TimeCounter = 0;

34
static uint16_t RoundTrip[2] = {0,0};

35
static uint16_t TimeDifference = PULSE_PERIOD_MAX_MS;

36
static uint16_t Pulsegenerator = UINT16_MAX;

37
static uint16_t TunedOutputPeriod = PULSE_PERIOD_MAX_MS;

38
static uint16_t Timeout = ZERO_TIME_DETECT;

39
static uint8_t Debounce = DEBOUNCE_TIME_MS;

40


41
int main ( void )

42
{

43
  _delay_ms( 100 );

44
  

45
  CLKPR = (1<<CLKPCE);

46
  CLKPR = (1<<CLKPS2);    // /16

47
  

48
  OSCCAL = 0x63;

49
  // Pulse Output

50
  PULSE_HIGH();

51
  PULSE_DDR |= (1<<PULSE_PIN);

52
  

53
  // Disable AD-Comp

54
  ACSR |= (1<<ACD);

55
  

56
  // Unused Pin low

57
  DDRB |= (1<<PB2) | (1<<PB3) | (1<<PB4);

58
  PORTB &= ~( (1<<PB2) | (1<<PB3) | (1<<PB4) );

59
  

60
  // Setup AVR

61
  OCR0A = 17;

62
  TCCR0A |= (1<<WGM01);      // CTC

63
  TCCR0B |= (1<<CS01);      // Prescaler = 8

64
  

65
  // External Interrupt

66
  PORTB |= (1<<PB1);          // internal Pullup enable

67
  MCUCR |= (1<<ISC01);        // Trigger on falling edge

68
  CLEAR_SENSOR_FLAG();

69


70
  

71
    /* Nice weather... */

72
    while ( "Cycling" ){

73
    if ( TIFR0 & (1<<OCF0A) ){

74
      #if DEBUG_MODE == 1

75
        PULSE_HIGH();

76
      #endif

77
      

78
      if ( Debounce ) Debounce--;

79
      if ( Debounce == 1 ) CLEAR_SENSOR_FLAG();

80
      

81
      // Clear TimerFlag    

82
      TIFR0 |= (1<<OCF0A);

83
  

84
      // Nullerkennung

85
      if ( Timeout ) Timeout--;

86


87
      // Impuls Erfassung

88
      TimeCounter++;

89
  

90
      if ( SENSOR_TRIGGERD && !Debounce ){

91
        Timeout = ZERO_TIME_DETECT;

92
        Debounce = DEBOUNCE_TIME_MS;

93
        RoundTrip[NEW] = TimeCounter;

94
        TimeDifference = RoundTrip[NEW] - RoundTrip[OLD];

95
        RoundTrip[OLD] = RoundTrip[NEW];

96
      }

97


98
      // Impuls Ausgabe

99
      if ( TimeDifference < PULSE_PERIOD_MIN_MS ){ // abgeregelt

100
        TunedOutputPeriod = PULSE_PERIOD_MIN_MS;

101
      }else{// unmanipuliert

102
        TunedOutputPeriod = TimeDifference;

103
      }

104
  

105
      if ( TunedOutputPeriod > PULSE_PERIOD_MAX_MS ) TunedOutputPeriod = PULSE_PERIOD_MAX_MS;

106
  

107
      if ( Pulsegenerator >= TunedOutputPeriod ) Pulsegenerator = 0;

108
      else Pulsegenerator++;

109
      

110
      #if DEBUG_MODE == OFF

111
        if ( Pulsegenerator < PULSE_DURATION_MS && Timeout ) PULSE_LOW();

112
        else PULSE_HIGH();

113
      #elif DEBUG_MODE == ON

114
        if ( Pulsegenerator < PULSE_DURATION_MS && Timeout )PORTB &= ~(1<<PB1);

115
        else PORTB |= (1<<PB1); 

116
        PULSE_LOW();

117
      #endif

118
    }

119
  }

120
}
Antwort Zitieren Bearbeiten Löschen Beitrag melden
Thread beobachten

Antwort schreiben

Bitte melde dich an, um einen Beitrag zu schreiben.

Bestehender Account

oder

Mit Google-Account einloggen

Noch kein Account?

Die Registrierung ist kostenlos und dauert nur eine Minute.

Jetzt registrieren