Die IF Abfrage soll also schauen wenn der ARRAY ende erreicht ist
und den Zeiger zurücksetzen. Das klappt so bei beiden TX Zeigern
und dem RX schreib Zeiger auch wunderbar
Nur hier wird das IF niemals ausgeführt und ich bekomme einen Buffer
überlauf
Geht:
1
uint8_tUSART_RX_Byte(void){
2
uint8_ttemp;
3
temp=*usart_rx_read;
4
usart_rx_read++;
5
6
uint8_tx=(usart_rx_read-usart_rx_buffer)
7
if(x==USART_RX_BUFFER_MAX){//check if Buffer full now
8
usart_rx_read=usart_rx_buffer;
9
}
10
returntemp;
11
}//end RX_BYTE
Führe ich den Pointervergleich einzeln aus und vergleiche dann gegen
meine Konstante ist wieder alles schön.
das IF macht was es soll und mein Zeiger rennt nicht wild im Speicher
rum.
Was läuft hier verkehrt ? Das hatte mit der ersten Version geklappt,
und diese lauft auch für die anderen drei Zeiger ohne dieses
Fehlerverhalten. Einzige Änderung, mein Projekt ist an anderen stellen
größer geworden...
Vermutung:
der "usart_rx_read" liegt nun ungünstig im Speicher und
der Vergleich (ptr == ptr + Konstante) ist im Prinzip schon falsch
weil...
ja weil...falscher cast ?
Marc D. schrieb:> und diese lauft auch für die anderen drei Zeiger ohne dieses> Fehlerverhalten. Einzige Änderung, mein Projekt ist an anderen stellen> größer geworden...
Womit es auch die Möglichkeit gibt, dass du genau an diesen anderen
Stellen irgendwo den Speicher niedergebügelt hast und dabei zufällig
genau diesen einen Pointer erwischt hast.
Was du siehst sind die Symptome. Die Ursache dafür kann aber auch an
ganz andere Stelle zu finden sein. Überprüf mal deine letzten
Änderungen, ob du irgendwo einen Array-Overflow produziert hast, oder
sonst irgendwie über einen ungültigen Pointer dir den Speicher
zerschiesst.
Ein anderer Verdacht (globale Variablen sind ja immer supekt):
Evtl. änderst du usart_rx_read versehentlich noch an einer anderen
Stelle, und der Test auf Gleichheit klappt z.B. deshalb nicht, weil der
Wert durch ein zusätzliches Erhöhen schon über dem Vergleichswert liegt?
Karl Heinz Buchegger schrieb:> Überprüf mal deine letzten> Änderungen, ob du irgendwo einen Array-Overflow produziert hast, oder> sonst irgendwie über einen ungültigen Pointer dir den Speicher
Ich werde mal suchen, bzw. Rauschmeißen / reduzieren.
Wobei dann aber im Prinzip mein "FIX" auch nicht
gehen dürfte.
Also Speicher Adresse = Puffer Anfang + Max Buffer.
Geht nicht (ging aber und tut bei den anderen)
Meine Korrektur dazu.
1
uint8_tx=(usart_rx_read-usart_rx_buffer)
2
if(x==USART_RX_BUFFER_MAX){
3
usart_rx_read=usart_rx_buffer;}
Also Speicher Adresse - Pufferadresse und dann vergleichen mit MAX
Puffer.
Und schon klappt es wider wie gewünscht.
Ich verstehe nur nicht was da anders läuft , weil das Ergebnis
(also die rechen Operation) das gleiche ist.
Karl Heinz Buchegger schrieb:> Womit es auch die Möglichkeit gibt, dass du genau an diesen anderen> Stellen irgendwo den Speicher niedergebügelt hast und dabei zufällig> genau diesen einen Pointer erwischt hast.
Zumal die Variable evtl. genau hinter dem Feld liegt; ein Zugriff hinter
das Feld würde genau usart_rx_read treffen.
Womit die unsichtbare Funktion interessant wäre, die das Feld füllt...
Klaus Wachtler schrieb:> Ein anderer Verdacht (globale Variablen sind ja immer supekt):> Evtl. änderst du usart_rx_read versehentlich noch an einer anderen> Stelle.
So global sind die auch nicht. Static gesetzt in globalen teil der C
Datei.
damit so erstmals nur in der Datei zu erreichen. Und es ist ein Pointer
auf ein Array. also auch keine 08/15 Variabel.
Marc D. schrieb:> So global sind die auch nicht. Static gesetzt in globalen teil der C> Datei.> damit so erstmals nur in der Datei zu erreichen. Und es ist ein Pointer> auf ein Array. also auch keine 08/15 Variabel.
Und das soll gegen versehentliches Überschreiben schützen?
Wenn du nur ein Fragment zeigst und behauptest, der Rest sei natürlich
in Ordnung, wird man dir nicht viel helfen können.
Marc D. schrieb:> So global sind die auch nicht. Static gesetzt in globalen teil der C> Datei.
Ist wurscht.
Sie existieren im RAM und sind damit über einen wilgewordenen Pointer
bzw. Arrayzugriff zu erreichen.
Im Fehlerfall kannst du dich auf gar nichts mehr verlassen. Wenn
Speicher niedergebügelt wird, kann in weiterer Folge alles mögliche
passieren.
Variablen die unvorhersehbar ihren Wert ändern, sind meistens ein Indiz
für sowas.
Karl Heinz Buchegger schrieb:> Sie existieren im RAM und sind damit über einen wilgewordenen Pointer> bzw. Arrayzugriff zu erreichen.
Ja richtig soweit auch klar, da habe ich mich nicht richtig ausgedrückt.
Geht nicht
Ich arbeite also an der gleichen stelle nur mit einer
anderen Berechnung meines Pointers auf das Daten Array.
Wenn irgendwas den Pointer verbiegen würde, das würde sich
das doch auch bei dem "geht" durchschlagen.
Klaus Wachtler schrieb:> Wenn du nur ein Fragment zeigst und behauptest, der Rest sei natürlich> in Ordnung, wird man dir nicht viel helfen können.
Behaupte ich nicht. sage nur das der Fehler an der von mir
angesprochenen stelle ist und durch umstellen wieder funktionsfähig ist.
aber snbei die komplette usrat.c, allerdings fehlt da die Änderung.
>> Ich arbeite also an der gleichen stelle nur mit einer> anderen Berechnung meines Pointers auf das Daten Array.>> Wenn irgendwas den Pointer verbiegen würde, das würde sich> das doch auch bei dem "geht" durchschlagen.
Nein, würde es nicht notwendigerweise.
Die beiden Codesequenzen sind nicht identisch.
im ersten Fall müssen alle 16 Bit der Pointer übereinstimmen.
Im zweiten Fall aber, wegen uint8_t x, nur die Low-Bytes.
D.h. im ersten Fall hast du eine 1:65536 Chance, dass der Vergleich
trotz verbogenem Pointer auf Gleich plädiert. Im zweiten Fall hast du
eine 1:256 Chance. Mit einem bischen Glück (oder Pech, je nachdem wie
man es sieht), kann es sein, dass dir der überschriebene Pointer nicht
weit genug davonläuft, du also nach wie vor einen Fehler im Programm
hast, ihn aber nicht bemerkst weil es sich momentan (noch) nicht
auswirkt.
Danke für den Quelltext.
Ich wiederhole mich zwar, aber:
Wenn du Variablen in einer ISR und im Hauptthread verwendst, müssen sie
als volatile deklariert sein.
Das sind sie bei dir nicht.
Klaus Wachtler schrieb:> Wenn du Variablen in einer ISR und im Hauptthread verwendst, müssen sie> als volatile deklariert sein.
Außerdem müssen die Zugriffe auf Variablen größer 8 Bit (das schließt
alle Zeiger mit ein) außerhalb der ISR atomar ausgeführt werden. Auch
das ist nicht der Fall.
Klaus Wachtler schrieb:> Danke für den Quelltext.>> Wenn du Variablen in einer ISR und im Hauptthread verwendst, müssen sie> als volatile deklariert sein.> Das sind sie bei dir nicht.
Ja stimmt, wird hier aber so immer noch nicht verwendet.
Bleiben wir bei "RX_BUFFER", das ist ein ARRAY das über
zwei Pointer angesprochen wird.
Pointer in der ISR ist also ein anderer als im Hauptthread.
der Pointer und das Array sind statisch im gleichen File wie
auch die ISR Funktion. Da bedarf es kein volatile.
Macht aber auch keinen unterschied.
Damit haben wir:
usart_rx_read = ein Pointer ausschließlich in der USART_RX_Byte(void)
verwendet wird
usart_rx_buffer = das array, das in der Funktion und in der ISR
zwar ÜBER den Pointer gefüllt wird, sonst einfach nur seine Adresse
liefern soll zu verglich wo ich mich im array bin.
und ein umstellen der Berechnung die das Problem löst, mir aber nicht
klar ist was daran anders sein soll.
Marc D. schrieb:> und ein umstellen der Berechnung die das Problem löst, mir aber nicht> klar ist was daran anders sein soll.
Dann wirst du wohl oder übel mal den generierten Code analysieren
müssen.
Klaus Wachtler schrieb:> Nur nebenbei: dein uint8_t USART_RX_Byte() liest auch, wenn nie etwas in> den Puffer geschrieben wurde.
Im Code ist dafür oben in der Funktion eine Abfrage (Grade
auskommentiert)
USART_RX_Byte() wird aber ausschließlich in der USRAT_Menu() aufgerufen.
Diese wiederum wird nur getriggert wenn die USART RX ISR auch ein
Byte geschrieben hat.(da wird ein Flag gesetzt) Damit ist das erst
einmal ausgeschlossen.
Aber auch wenn, sollte hier der Pointer wieder auf array Start
gesetzt werden wenn USART_RX_BUFFER_MAX erreicht wird.
Jörg Wunsch schrieb:> Dann wirst du wohl oder übel mal den generierten Code analysieren> müssen.
Ich denke auch. Werde ich mich mich heute Abend mal daransetzen.
Marc D. schrieb:> und ein umstellen der Berechnung die das Problem löst, mir aber nicht> klar ist was daran anders sein soll.
Sie 'löst' sehr wahrscheinlich das Problem nicht. Sie kaschiert es nur.
Denn deine beiden Codestücke sind eben NICHT funktional identisch! Das
eine ist ein 16-Bit Vergleich, das andere ein 8-Bit Vergleich.
Hier
1
staticuint8_tUSART_RX_Byte(void){
2
uint8_ttemp;
3
// if(usart_rx_read == usart_rx_write){ //security checks if there is a byte on read buffer DISABLE save 20Byte flash
4
// USART_TX_Byte('F');
5
// return 0;
6
// }
7
temp=*usart_rx_read;//read from ring buffer
8
usart_rx_read++;//point to next byte in buffer
9
if(usart_rx_read==usart_rx_buffer+USART_RX_BUFFER_MAX){//check if Buffer full now
10
usart_rx_read=usart_rx_buffer;//reset to buffer start address (load Pointer with buffer start address)
11
}
12
returntemp;
13
}//end RX_BYTE
sollte die Manipulation von usart_rx_read unter Interrupt Sperre
passieren, da in der ISR ebenfalls darauf zugegriffen wird.
Ich denke allerdings immer noch, dass du den falschen Baum anbellst.
Dein eigentliches Problem ist an einer ganz anderen Stelle im Code und
hat mit der UART nichts zu tun. Der UART Code ist nur das Symptom, an
dem du siehst dass du einen Fehler im Programm hast.
@Marc
Hättest Du Dich nicht besser gestanden, wenn Du das ganze Indiziert
aufgebaut hättest?
Dann hättest Du einen konstanten, unveränderlichen Pufferanfang und zwei
8-Bit Laufvariablen.
Auch würde der Zugriff wahrscheinlich schneller sein, da die meisten
Vergleiche zwischen Bytes stattfinden und nur die "echten" Zugriffe
Pointeroperationen wären.
Vor allem aber wäre die Arithmetik "offensichtlicher". So etwas hat 'ne
Menge Vorteile, wenn du nächstes Jahr mal was ändern musst.
Du aber arbeitest ständig mit Pointern - wohl 16 Bit breit - und dem
entsprechenden arithmetischen Aufwand, bei Deinem 8-Bit Prozessor.
Bei einem "echten" 16-Bit Prozessor wäre Dein Ansatz wohl schneller.
Karl Heinz Buchegger schrieb:> sollte die Manipulation von usart_rx_read unter Interrupt Sperre> passieren, da in der ISR ebenfalls darauf zugegriffen wird.
und das ist meines Erachtens falsch. usart_rx_read ist nur in der
Funktion in Verwendung nicht in der ISR.
Gemeinsamer Nenner wäre einzig das Array ,das über den Pointer
angesprochen wird. Wobei einer nur schreibt und einer nur liest.
Aber weiter oben sagst du:
> Sie 'löst' sehr wahrscheinlich das Problem nicht. Sie kaschiert es nur.> Denn deine beiden Codestücke sind eben NICHT funktional identisch! Das> eine ist ein 16-Bit Vergleich, das andere ein 8-Bit Vergleich.
Mhhh, das stimmt allerdings.
Bzw:
>dass du den falschen Baum anbellst.
ich mich durchaus mal auf die suche nach einen anderen Baum mache.
Marc D. schrieb:>> sollte die Manipulation von usart_rx_read unter Interrupt Sperre>> passieren, da in der ISR ebenfalls darauf zugegriffen wird.>> und das ist meines Erachtens falsch. usart_rx_read ist nur in der> Funktion in Verwendung nicht in der ISR.
Ist egal.
Es ist eine 16 Bit Einheit und kann daher als solches nicht zb in einem
Zug inkrementiert werden.
Wechselt der Pointer von 0x00FF beim Inkrement auf 0x0100
UND
kommt der Interrupt Aufruf genau zwischen dem Inkrement des Low_bytes
und dem Inkrement des High_bytes, dann sieht die ISR unter Umständen
bereits das neue Low-Byte 0x00 aber immer noch das alte High-Byte 0x00.
Die ISR benutzt also den teilinkrementierten "Zwischenstand" 0x0000 als
Wert vom Pointer. Und der hat weder mit 0x00FF, noch mit 0x0100 etwas zu
tun.
(Ist ein mögliches Szenario. Ich sag nicht das es so ist - aber es ist
möglich)
Spiele nach den Regeln und du hast weniger Probleme.
Hast du Mehrbyte-Variablen UND werden die in ISR Funktionen ALS AUCH in
nicht ISR Funktionen benutzt, dann müssen die volatile sein und der
Zugriff muss atomar abgesichert werden.
Alles andere kann funktionieren, kann aber auch in die Hose gehen, je
nach Umständen, die wir als C-Programmierer nicht kontrollieren können.
Ein cli() vor den 3 Zeilen Code und ein sei() danach behebt das Problem
und kostet nicht viel, ausser dass ein möglicher Interrupt ein paar
Taktzyklen verzögert zur Ausführung kommt, was bei einem UART Interrupt
nun wirklich ziemlich egal ist.
Karl Heinz Buchegger schrieb:> Wechselt der Pointer von 0x00FF beim Inkrement auf 0x0100> UND> kommt der Interrupt Aufruf genau zwischen dem Inkrement des Low_bytes> und dem Inkrement des High_bytes, dann sieht die ISR unter Umständen> bereits das neue Low-Byte 0x00
Die ISR sieht diesen Pointer nicht, hier ist es wirklich schnuppe, ob
die ISR in den Inkrement hineinpfuscht.
Da hat Marc recht, er verwendet diesen Pointer schließlich
ausschließlich in seiner Fifo-Lese-Routine.
Karl Heinz Buchegger schrieb:>> und das ist meines Erachtens falsch. usart_rx_read ist nur in der>> Funktion in Verwendung nicht in der ISR.>> Ist egal.
Nein ist es nicht
> Es ist eine 16 Bit Einheit und kann daher als solches nicht zb in einem> Zug inkrementiert werden.
Soweit klar.
> Wechselt der Pointer von 0x00FF beim Inkrement auf 0x0100> UND> kommt der Interrupt Aufruf genau dazwischen> dann sieht die ISR unter Umständen bereits das neue Low-Byte 0x00 aber> immer noch das alte High-Byte 0x00. Die ISR benutzt also den> Zwischenstand 0x0000 als Wert vom Pointer. Und der hat weder mit 0x00FF,> noch mit 0x0100 etwas zu tun.
Zauber ISR ? Wie sieht denn die ISR das neue LOW Byte ?
Gar nicht, den das Ding wird in der ISR nicht angefasst.
nicht berührt, nicht verändert und nicht einmal gelesen.
in der ISR wird ein Anderer Pointer benutzt der nur aufs gleiche
DATEN Array im RAM zeigt.
> Spiele nach den Regeln und du hast weniger Probleme.
Mach ich ja (meines Erachtens).
Also Warum einen zugriff Atomar gestalten wenn der
zugriff zwar unterbrochen aber nicht geänderter werden kann.
Nicht desto Trotz:
> Ein cli() vor den 3 Zeilen Code und ein sei() danach behebt das Problem
Hatte ich gestern schon gemacht (hatte auch die beteiligten Pointer
auf volatile), werde das nachher aber noch mal etwas gründlicher
angehen.
Mein Aktueller Verdacht: um so länger ich drüber nachdenke.
Das High Byte des usart_rx_read Pointers wird Irgendwo
überrannt (du sagtest schon das wird was wild laufendes).
Indizien:
usart_rx_read wird nur in der Funktion benutz als Pointer.
TINY2313 braucht mit seinen 128Byte Ram nur den LowPart von Pointer.
Alle andere gleichartigen Abfragen funktionieren
Beweise: folgen wenn ich welche habe
Rufus Τ. Firefly schrieb:> Karl Heinz Buchegger schrieb:>> Wechselt der Pointer von 0x00FF beim Inkrement auf 0x0100>> UND>> kommt der Interrupt Aufruf genau zwischen dem Inkrement des Low_bytes>> und dem Inkrement des High_bytes, dann sieht die ISR unter Umständen>> bereits das neue Low-Byte 0x00>> Die ISR sieht diesen Pointer nicht, hier ist es wirklich schnuppe, ob> die ISR in den Inkrement hineinpfuscht.
Moment, Hab ich mich verlesen
<studier>
Ooops.
Hab ich
1
ISR(USART_RX_vect){
2
*usart_rx_write=UDR;//store Data to write pointer address
3
usart_rx_write++;
4
if(usart_rx_write==usart_rx_buffer+USART_RX_BUFFER_MAX){//check if Buffer Overflow now
5
usart_rx_write=usart_rx_buffer;//reset to buffer start Address.
6
}
7
GPIOR0|=(1<<USART_NEW_BYTE);//Set FLAG for action in Mainloop
8
}//end WS2812_LEDCount_RX_Vect
Wieso ist da keine Prüfung auf Buffer Overflow drinnen?
(Vergleich des Read Pointers mit dem Write Pointer)
Ok. Ich nehms zurück. Da gibt es kein Problem mit atomarem Zugriff.
> Wieso ist da keine Prüfung auf Buffer Overflow drinnen?> (Vergleich des Read Pointers mit dem Write Pointer)
spart Platz im Flash :-), deswegen ist das auch in der Lese Funktion
ausgeklammert und hier gar nicht vorhanden.
zweites ich weiß wie viel ich in den Puffer packen kann.
und wie schnell der dann wieder entleert wird.
Der USART wird von PC über Hand befüllt....
Damit ist das also schon Optimierter Code für meine Anwendung
und keine Musterlösung für einen nach allen Seiten abgesicherter
Ringbuffer.
> Ok. Ich nehms zurück. Da gibt es kein Problem mit atomarem Zugriff.
das beruhigt mich dann auch. Nicht das ich noch anfange mein
quasi frisches wissen selber in frage zu stellen.
Marc D. schrieb:> Damit ist das also schon Optimierter Code für meine Anwendung
Naja, Optimierung sieht für mich anders aus.
> und keine Musterlösung für einen nach allen Seiten abgesicherter> Ringbuffer.
Der aber deutlich übersichtlicher, kürzer und schneller wäre.
Paul schrieb:> Naja, Optimierung sieht für mich anders aus.
Wie denn ?
>> und keine Musterlösung für einen nach allen Seiten abgesicherter>> Ringbuffer.>> Der aber deutlich übersichtlicher, kürzer und schneller wäre.
Übersichtlichkeit ? Ich finde das sehr übersichtlich, ich mag Pointer
Kürzer und Schneller ?
Mhhh Interessant.. wie denn ? erleuchte mich doch bitte.
Fall erledigt. Kann geschlossen und vergessen werden...
Karl Heinz allererste Annahme war richtig.
Im meinen Fall ein Pointer auf ein Array das STRUCT Referenzen enthält.
ein zugriff erfolgte ohne konkrete Zuweisung des Pointers.
danke für die Denkanstöße..
Thread beobachten
Seitenaufteilung abschalten