UART lib von Peter Fleury --> Bug

[chillfactor (Gast)]

Hallo liebe Freaks hier,

ich benutze seit kurzem die Libs von Peter Fleury (Danke erstmal 
dafür^^).
Im uart hab ich nen kleinen Bug gefunden:

Und zwar gehts um die RX Puffergröße.

In der Interruptroutine für ein empfangenes byte erhöht er den Head 
pointer und fügst das byte im Puffer an dieser Stelle hinzu.

Soweit alles ok, da der Tail und Head pointer stets auf der zuletzt 
gelesene/geschrieben stelle im Puffer stehen und erst beim Zugriff 
inkrementiert werden.

Wenn man sich nun einen leeren Puffer mit Initialwerten Tail=0 und 
Head=0 ansieht, wird das erste byte an die Stelle 1 geschrieben. Der 
Tail ist aber noch 0.

Schreibt man nun 32 bytes in einen 32byte großen Puffer, so ist beim 
Aufruf der Interruptroutine bei Empfang des 32. byte der head bereits 31 
(letzte geschriebene Stelle), der tail jedoch immernoch 0 (noch nie was 
gelesen).
Da der Head nun vor (!) der Überprüfung (head==tail) incrementiert wird, 
schlät der Overflowvergleich bereits jetzt zu, obwohl die Stelle 0 ja 
noch frei wäre (ist ja das 32. byte, da auch erst ab Stelle 1 gelesen 
wird!).

Fazit: Der Puffer hat ein byte weniger als im #define angegeben.

Fehlerbehebung: gibt es mehrere, die einfachste wäre den Vergleich auf 
Pufferüberlauf noch VOR dem Inkrementieren des Heads zu machen.

P.S.: Ich hoffe Peter liest hier regelmäßig mit und kann den Bug in 
seiner Lib beheben...

[Stefan E. (sternst)]

chillfactor wrote:

> Fazit: Der Puffer hat ein byte weniger als im #define angegeben.

Das ist kein Bug. Bei einem Ringpuffer, der über zwei Zeiger (oder zwei 
Indizes) verwaltet wird, hast du immer dieses eine "verschwendete" Byte. 
Du könntest nämlich sonst einen leeren nicht von einem vollen Puffer 
unterscheiden.

[chillfactor (Gast)]

Ja so implementiert schon, aber man könnte doch einen counter 
"buffer_bytes_free" definieren, der den Füllstand des Ringpuffers 
mitzählt und somit den kompletten Speicher nutzt.

Gerade die Bedingung, dass die definierte Size 2^n haben muss ist 
problematisch, da man meist genau einen solch großen Puffer benötigt 
(blocktransfer), und nicht einen mit einem byte weniger.

Außerdem wäre diese Einschränkung zumindest beim define zu erwähnen.

[Karl H. (kbuchegg)]

chillfactor wrote:
> Ja so implementiert schon, aber man könnte doch einen counter
> "buffer_bytes_free" definieren, der den Füllstand des Ringpuffers
> mitzählt und somit den kompletten Speicher nutzt.

Du hast einen Buffer von 32 Bytes, von dem 1 Byte ungenutzt bleibt.
Speicherverbrauch = 32 Bytes

Du hast einen Buffer von 31 Bytes, bei dem jedes Byte genutzt wird.
Zusätzlich noch einen Counter.
Speicherverbrauch = 32 Bytes

Beide Buffer sind effektiv gleich gross und brauchen auch den gleichen 
Speicher. Der zusätzliche Counter will aber auch behandelt werden.
Unterm Strich hast du nichts gewonnen.

> Gerade die Bedingung, dass die definierte Size 2^n haben muss ist
> problematisch, da man meist genau einen solch großen Puffer benötigt
> (blocktransfer), und nicht einen mit einem byte weniger.

Diese Einschränkung resultiert aber lediglich aus der 'Indexarithmetik' 
und könnte auch anders gelöst werden.

> Außerdem wäre diese Einschränkung zumindest beim define zu erwähnen.

Da geb ich dir allerdings recht.

[MarkusB (Gast)]

Lösungen gibt es dafür sicher andere. Aber diese ist wohl darauf 
ausgerichtet möglichst wenig Code zu erzeugen und dabei auch möglichst 
kurze Ausführungszeiten zu haben. Ich denke das ich wichtiger als ein 
einzelnes Byte

[Karl H. (kbuchegg)]

MarkusB wrote:
> Ich denke das ich wichtiger als ein
> einzelnes Byte

Im Prinzip gebe ich chillfaktor in einem Punkt recht:
Wenn ich beim Empfangsbuffer einstelle, dass ich eine Größe von 32 Bytes 
habe, dann wird jeder normal denkende Programmierer zuerst mal davon 
ausgehen, dass da auch wirklich 32 Bytes hineinpassen. Empfange ich 
einen Block (im Blocktransfer) von 32 Bytes, dann kann der 
zwischengespeichert werden, sollte man meinen.

Beim Senden ist die Buffergröße hingegen nicht so das Problem, weil ja 
das erste Zeichen sowieso sofort an die UART geht und damit zusammen mit 
den 31 Bytes im Buffer wieder die Blockgröße von 32 erreicht wird. Und 
ausserdem wird der Buffer ja auch von der UART geleert, während ich ihn 
befülle.

Allerdings: Meine persönliche Meinung. Wer einen Buffer so dermassen 
knapp dimensioniert, dass ein Übertragungsblock da nur auf knirsch 
reinpasst, der hat es eh nicht besser verdient :-)
Und bei 'normaler' UART Benutzung, also mit einem menschlichen Bediener 
am anderen Ende, ist das eh alles hinfällig.

Viel wichtiger als ein unbenutzes Byte im Buffer wäre die 
Implementierung eines Handshake-Protokolls :-)

[Peter D. (peda)]

Karl heinz Buchegger wrote:
> Allerdings: Meine persönliche Meinung. Wer einen Buffer so dermassen
> knapp dimensioniert, dass ein Übertragungsblock da nur auf knirsch
> reinpasst, der hat es eh nicht besser verdient :-)

Nö, das kann ja zum Protokoll gehören.
D.h. der Master fragt ab, wieviel Puffer hast Du und dann schickt er 
immer exakt Häppchen von dieser Größe.
Gibt der Slave aber eine falsche Größe zurück, kracht es.
Z.B. in meinem Bootloader habe ich ein derartiges Protokoll.


Allerdings kann man auch die Hardwarepuffer der UART mit ausnutzen, und 
dann sinds sogar 1-2 Zeichen mehr, als die FIFO alleine, z.B.:

Beitrag "AVR-GCC: UART mit FIFO"

                                        // size must be in range 2 .. 256
#define RX0_SIZE        10              // usable: RX0_SIZE + 2 (4 .. 258)
#define TX0_SIZE        8               // usable: TX0_SIZE + 1 (3 .. 257)

Peter

[Peter D. (peda)]

chillfactor wrote:
> Gerade die Bedingung, dass die definierte Size 2^n haben muss ist
> problematisch, da man meist genau einen solch großen Puffer benötigt
> (blocktransfer), und nicht einen mit einem byte weniger.

Deshalb hat mein Code diese Einschränkung nicht.
Es scheint nur so, als ob man damit viel Code spart.


Peter

[Karl H. (kbuchegg)]

Peter Dannegger wrote:

> Allerdings kann man auch die Hardwarepuffer der UART mit ausnutzen, und
> dann sinds sogar 1-2 Zeichen mehr, als die FIFO alleine, z.B.:

Richtig!
An die hatte ich gar nicht mehr gedacht :-)