sei() innerhalb eines ISR()

@ D a v i d K. (oekel)

>ich verwende gerade jede Menge I2C Aufrufe. Eine davon sogar im ISR

Naja, hoffentlich ist die ISR-Frequenz nicht zu hoch.

>Nun habe ich pauschal alle I2c_* Funktionen mit einem cli()/sei();
>umschlossen.

Warum? Das ist im Normalfall nicht nötig. I2C ist synchron und du nutzt 
ja das I2C Modul deines AVRs. Da darf so ein I2C Zugriff auch mal durch 
einen ISR unterbrochen werden.

>Früher gab es ja noch SIGNAL und INTERRUPT (<-- heutige ISR)
>SIGNAL hat dabei ja Interrupts während der IR-Routine erlaubt.

Alter Kram. Vergiss das.

>Meine Frage wäre daher nun, ob ich mir künstlich wieder ein SIGNAL
>gebaut habe, dadurch dass ich in der ISR einen Funktionsaufruf verwende
>der sei() enthält?

Nun ja, selbst wenn man eine Funktion atomar ausführen will, sollte man 
im Normalfall die Funktionen aus util/atomic.h nutzen. Denn die können 
am Ende des atomaren Blocks den vorherigen Zustand des I-Bit wieder 
herstellen. Sprioch, wenn es vorher gelöscht war, bleibt es gelöscht!

1

#include <util/atomic.h>

2

3

    ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {

4

        // atomarer Funktionsblock, Interrupts sind gesperrt

5

    }

Siehe Doku der libc.

>Oder wird das I-Bit unter keinen umständen verändert, solange ich mich
>in der ISR befinde und bin somit safe?

Nö. Ein sei() setzt das I-Bit. Immer.

Falk B. schrieb:
> @ D a v i d K. (oekel)
>
>>ich verwende gerade jede Menge I2C Aufrufe. Eine davon sogar im ISR
>
> Naja, hoffentlich ist die ISR-Frequenz nicht zu hoch.
Teiler: 1024

> Nun ja, selbst wenn man eine Funktion atomar ausführen will, sollte man
> im Normalfall die Funktionen aus util/atomic.h nutzen. Denn die können
> am Ende des atomaren Blocks den vorherigen Zustand des I-Bit wieder
> herstellen. Sprioch, wenn es vorher gelöscht war, bleibt es gelöscht!
>
>

1

> #include <util/atomic.h>

2

> 

3

>     ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {

4

>         // atomarer Funktionsblock, Interrupts sind gesperrt

5

>     }

6

>

Mein theoretisches Szenario wäre das sich das ganze wie folgt mal michen 
könnte:

1

void I2C_WriteRegister(uint8_t busAddr, uint8_t deviceRegister, uint8_t data) {

2

  cli();

3

  i2c_start(busAddr);

4

  i2c_write(deviceRegister);

5

        //...nun kommt der ISR..

6

7

        //uint8_t I2C_ReadRegister(uint8_t busAddr, uint8_t deviceRegister) {

8

  cli();

9

  uint8_t data = 0;

10

  i2c_start(busAddr); // send device address

11

  i2c_write(deviceRegister); // set register pointer

12

  i2c_start(busAddr + I2C_READ); // restart as a read operation

13

  data = i2c_readNak(); // read the register data

14

  i2c_stop(); // stop

15

  //sei();

16

        //}

17

18

        //...weiter gehts

19

  i2c_write(data);

20

  i2c_stop();

21

  sei();

22

}

Sprich es kommt im besten Fall Müll auf dem Display an (dafür verwende 
ich das "I2C_WriteRegister") im Schlimmsten Fall hängt er sich auf? 
(darartigen Fall habe ich gerade sporadisch in der Praxis und schiebe es 
auf den ISR.
(konkret läuft eine Endlosschleife mit Testausgaben: 40-100x ok. dann 
steht alles...)

PPS:

folgenden minimal-Wert für "multi" habe ich gerade per TryAndError für 
mich ermittelt:

1

void timer0_init() {

2

3

  TCCR0 = (CS00 || CS02); /* Takt 1024 */

4

5

  //"Timer/Counter Interrupt Mask" (TIMSK)

6

  //"Timer/Counter Interrupt Enable"(TOIE0)

7

  TIMSK |= (1 << TOIE0);

8

9

}

10

uint16_t multi = 0;

11

ISR( TIMER0_OVF_vect) {

12

13

  if (multi == 0) {

14

    multi = 350;

15

16

    if(SW2){

17

      PORTC ^= ( 1 << PC6 );

18

      SW2 = false;

19

    }

20

    updateSwitches();

21

  }

22

  multi--;

23

}

24

25

void updateSwitches() {

26

27

  if (SW1 || SW2 || SW3)

28

    return;

29

  //uint16_t ks = Key_ReadCom(uint8_t ks);

30

  uint8_t ks = I2C_ReadRegister(HT16K33, 0x41);

31

  SW1 = (ks & 0b00010000);

32

  SW2 = (ks & 0b00001000);

33

  SW3 = (ks & 0b00000100);

34

}

Wieso ich allerdings hier schon 1024*350 Takte für brauche erschließt 
sich mir nicht.

Achim S. schrieb:
> cli und sei in Funktion vermeiden. Maximal im Hauptstrang.
Guter Einwand...

@D a v i d K. (oekel)

>Mein theoretisches Szenario wäre das sich das ganze wie folgt mal michen
>könnte:

Stimmt. Dann braucht man eine atomare Sicherung. Aber in den meisten 
Fällen ist es besser, keinerlei I2C Zugriffe in der ISR zu machen und 
statt dessen alle Zugriffe nur als normale Funktion in der 
Hauptschleife.

https://www.mikrocontroller.net/articles/Interrupt#Steuersignale_zwischen_ISR_und_Hauptprogramm

Das hat auch den Vorteil, daß man sehr langsame Funktionen nutzen kann, 
die in einer ISR nichts zu suchen haben.

Hallo,

das scheit so nicht zu passen
 TCCR0 = (CS00 || CS02); /* Takt 1024 */

CS00 = 0
CS02 = 2

ergibt TCCR0=2   => CLK/8

Sascha

Sascha W. schrieb:
> Hallo,
>
> das scheit so nicht zu passen
>  TCCR0 = (CS00 || CS02); /* Takt 1024 */

Autsch!
TCCR0 = (1 << CS00) | (1 << CS02); /* Takt 1024 */

Falk B. schrieb:

> Aber in den meisten
> Fällen ist es besser, keinerlei I2C Zugriffe in der ISR zu machen und
> statt dessen alle Zugriffe nur als normale Funktion in der
> Hauptschleife.

Hatte ich auch ursprünglich nicht vor. Doch wie die Flanken eines 
Tasters am HT16K33 rechtzeitig erkennen, wenn ich nicht ständig auf dem 
I2C-Bus pulle?

@ D a v i d K. (oekel)

>Hatte ich auch ursprünglich nicht vor. Doch wie die Flanken eines
>Tasters am HT16K33 rechtzeitig erkennen, wenn ich nicht ständig auf dem
>I2C-Bus pulle?

Man kann die periodische Abfrage (vulgo "pulle") innerhalb einer 
Funktion im Hauptprogramm machen. Wie das geht, steht in meinem Link.

Falk B. schrieb:
> @ D a v i d K. (oekel)

> Man kann die periodische Abfrage (vulgo "pulle") innerhalb einer
> Funktion im Hauptprogramm machen. Wie das geht, steht in meinem Link.

Danke, hab es jetzt genau so gelöst.
Die Hauptroutine wirkte anfänglich so riesig, dass ich vermutet habe 
"dort muss unbedingt zwischengesprungen werden". Aber tatsächlich wird 
jeder REST-Zweig mit 5-6 if-Abfragen erreicht und die vorgelagerte

1

if(flag)

2

  alter_ISR_Teil();

3

else

4

  REST();

stört den "REST-Ablauf" nicht wirklich.
Endlich wieder Determinismus. Danke!

PS: In der Theorie hatte ich das alles schon gelesen (lese ja viel der 
hiesigen Wiki-Artikel). In der Praxis muss man aber immer wieder drauf 
gestoßen werden, um seinen Code zu strukturieren.

D a v i d K. schrieb:
> Nun habe ich pauschal alle I2c_* Funktionen mit einem cli()/sei();
> umschlossen.

"Pauschal" ist immer ein schlechter Programmierstil. Man muß sich bei 
jeder Anweisung auch was gedacht haben. Insbesondere Anweisungen 
bezüglich der Interruptlogik sollte man nur mit Bedacht einfügen.

Letzte Nachricht von mir diesbezüglich:

Gibt es einen Grund wieso man einen Eröffnungspost negativ bewertet?
Antworten zu markieren finde ich ja logisch, aber "dumme Fragen" gibt es 
nicht und die Höflichkeit gebietet es doch nachzuhaken oder des Weges zu 
gehen!?