Dicker Fehler in der RP6I2CmasterTWI.h der RP6Lib + Bugfix

[RolfD]

Ich bin dabei die TWI-Funktion für Master/Salve und Multimaster umzuschreiben und dabei ist mir ein dicker Fehler in den RP6Libs aufgefallen. Er führt zu einem instabilen TWI System.

Es betrifft in der Datei RP6I2CmasterTWI.h die Zeile:

#define I2CTWI_isBusy() ((TWCR & (1<<TWIE)))

Dort wird durch Abfrage des TWIE Bit im TWCR geprüft ob die TWI Hardware frei ist bevor ein weiterer TWI-Befehl abgesetzt wird. Das TWIE Bit ist aber laut Datenblatt "nur" das TWI Interrupt Enable Bit. Die Abfrage ist recht unsinnig. Der Entwickler hat - weil er so laufend die Hardware überrannte - an allen möglichen und unmöglichen Stellen des Code Delays eingefügt. Durch die statischen Delays wird das Risiko vermidert, die Hardware zu überrennen aber es macht große Timingprobleme auf dem TWI Bus weshalb dieser instabil, langsam und manchmal arg unwillig ist.

Das ganze ist einfach zu fixen in dem man oben die Zeile durch folgende ersetzt:

#define I2CTWI_isBusy() ((TWCR & (1<<TWINT)))

Hier wird nun korrekt das TWINT Bit geprüft. Das Datenblatt sagt dazu:
"TWINT: TWI Interrupt Flag ; This bit is set by hardware when the TWI has finished its current job and expects application software response."

Weiterhin kann man dann auch alle Aufrufe von I2CTWI_delay in der RP6I2CmasterTWI.c entfernen. Als Folge läuft die TWI Hardware deutlich stabiler und schneller. Wer viel auf dem TWI Bus zu tun hat wird das schätzen wissen. Im Treiber sind noch mehr Bugs aber der erscheint mir wichtig genug um vorab eine Meldung zu machen.

Mehr dann vielleicht wenn meine Lib bzw. die Änderungen fertig sind.

EDIT: Also das mit dem "Delays entfernen" klappt nicht so ganz wie gedacht, bei der Aussage zum TWIE/TWINT bleibe ich aber.
LG RolfD

[Fabian E.]

Das sieht doch nicht schlecht aus.
Jetzt weiß ich auch wo machmal die unerklärlichen I2C-Fehler herkamen, die sich nur mit einem Dealy beheben ließen...

Wirst du deine Lib hier veröffentlichen?
Hätte ein nicht grade kleines Interesse an einer Multimaster Lib.

[RolfD]

Wenn ich sie so hinbekomme wie ich mir das vorstelle werde ich sie natürlich hier veröffentlichen. Es schaut gut aus aber sie ist noch nicht fertig. Interupts und Callbacks zu debuggen ist viel frickelei bis alles richtig läuft. Es sei schon mal verraten, das die Lib 99% Befehlscompatibel zum alten System ist, man kann also einfach die .c und .h Dateien des TWI System mit den neuen ersetzen, neu compilieren und man hat alle zusätzlichen Möglichkeiten und Fixes.
Ja an Dich denke ich auch bei der Lib. Aber bau du erst mal an Remotrol weiter
Ich melde mich wenn ich soweit bin.
LG Rolf

[Fabian E.]

Vielleicht könnte SlyD mal etwas dazu sagen.
Mir erscheint diese Abfrage in der Tat auch ziemlich sinnlos, aber vielleicht hat er sich ja in irgendeiner Art etwas dabei gedacht und es ist kein Bug.
Nicht, dass hier etwas gefixt wird, was eigentlich nie falsch war.

Ansonsten freue ich mich schon sehr auf die neuen Möglichkeiten und kann mich auch gerne als Beta-Tester anbieten

[RolfD]

Neues vom TWI Mastermode

Also zunächst bitte mein Edit im 1. Post beachten:
"Das mit dem "Delays entfernen" klappt nicht so ganz wie gedacht, bei der Aussage zum TWIE/TWINT bleibe ich aber."

Ohne Delays sind sind Befehlsfolgen wie:
I2CTWI_transmit3Bytes(I2C_RP6_BASE_ADR, 0, CMD_SET_ACS_POWER, ACS_PWR_MED);
I2CTWI_transmit3Bytes(I2C_RP6_BASE_ADR, 0, CMD_SET_WDT, true);
I2CTWI_transmit3Bytes(I2C_RP6_BASE_ADR, 0, CMD_SET_WDT_RQ, true);
problematisch da sie ohne Delay hintereinander weg auf das TWI geschrieben werden können wärend es noch aktiv ist (was nicht passieren darf!). Schüzen tut da ohne Delay nur das TWINT (ehemals das funktionslose TWIE) bit, was jedoch nach jedem übertragenem Zeichen "frei" meldet. Damit kann eine angefangene Befehlsfolge vom nächsten Transmit Aufruf nach Abschluß eines Bytes (aus der angefangene Befehlsfolge) platt geschrieben werden. Das passiert in der ungefixten Version scheinbar ab und zu auch wenn die Delays ansich zu kurz oder aus irgendeinem Grund die Übertragung zu lange für's Delay ist. (Z.B. wenn 2 Master Busarbitierung durchführen müssen)
Man kann sich an 3 Fingern abzählen das es da schon mal knallt wenn sich der Master sozusagen selbst ins Wort fällt.

Man hatte in den Request Funktionen schon mal versucht das Problem zu erschlagen in dem man ein Hilfsflag lastTransOK verwendet, in den Transmissionfunktionen fehlt das allerdings.

Zudem sind die Delays auf der Base und M32 wegen unterschiedlicher CPU-Clocks unterschiedlich lang. Ansich kein Problem aber die M32 wartet eben nur halb so lange bis es ein weiteres Zeichen sendet.

In meiner Version knallt es hier ohne Delays grade laufend... weshalb mir das auch aufgefallen ist. Das als Zwischenbericht meiner fummeleien hier - es erklärt vielleicht einige Probleme mit dem TWI.

@Fabian E.
Ja ich bin auch gespannt was SlyD sagt. Ich bin noch am debuggen aber du bekommst die Lib als erster zum testen.

LG Rolf

[Jaecko]

Hm, könnte das ein Grund sein, warum man von einem Slave oft nur ständig 0x00 statt brauchbare Werte liest?
Würde ja dann einiges erklären...

[Fabian E.]

Wie kompliziert wäre es denn das Flag fertig zu stellen?
Hat man Zugriff darauf, wann der Befehl eindeutig übertragen wurde?
Lösungen die auf Warten basieren sind mir nämlich ziemlich unsympathisch...

Habe gerade selbst mal in den Source reingeguckt, das passiert ja tatsächlich ziemlich oft...
Und 150 Cycles sind auch nicht grade wenig, wenn mans etwas öfter aufruft...

Soweit ich das sehe wird in der ISR aber doch immer schön lastTransOK gesetzt?
Und in den Sende-Methoden auch...

[RolfD]

Da bin ich grade dabei Fabian...

Es ist nicht sonderlich kompliziert denke ich.. und das Kriterium ist schlicht und einfach die Stop condition. In der ISR und in den Request Funktionen ist das vorgesehen aber eben nicht in den Transmisson Funktionen.

Keine Bange... ich krieg das schon hin Musst nur noch ein bischen warten *breitrinz
LG Rolf

[Fabian E.]

xD Da bin fest von überzeugt
Ich habe demnächst übrigens ein neues Design bei Remotrol für dich =P
Ist zwar sicher noch nicht perfekt dein Geschmack, aber es wird dir (vielleicht) besser gefallen.

[SlyD]

Hallo,

zunächst vorweg allgemein zur Erläuterung:
Es kann durchaus sein, dass einige Dinge alles andere als optimal implementiert sind - war auch nicht das Ziel und dazu war auch keine Zeit.
Es gab damals (2007) eine Deadline und 10 Baustellen (bei Hard- und Software)
gleichzeitig.
Nachdem es funktionsfähig war, kamen dann schon wieder andere Dinge...

Steht in der Baselib auch dabei
"This Code works OK, but it is not optimal! There is a lot potential for tuning!"
Und das gilt für die gesamte Lib!


Ziel war eine bessere Software Ausgangsbasis für den Anwender
als beim ASURO und beim alten CCRP5 Vorgänger.
Nicht mehr und nicht weniger. Dem bisherigen Feedback der Mehrzahl der Anwender nach zu urteilen ist das auch gut gelungen - von kleinen Details mal abgesehen.

Naja, wenn sofort alles perfekt wäre, bliebe für fortgeschrittene Anwender
wie euch auch nix interessantes mehr zu tun.



Zum Thema:
Ja das mit den Delays habe ich aus genau dem Grund eingebaut wie RolfD
oben schon rausgefunden hat. Natürlich nur eine "Notlösung" hat aber
für das was ich getestet hatte erstmal funktioniert.
Ich weiss nicht mehr genau, da es schon sehr lange her ist, aber ich meine
ich hätte es auch mit dem TWINT Bit getestet hat aber nicht
zufriedenstellend funktioniert. Hatte dann auch keine Zeit mir
was besseres auszudenken also ists einfach so ein unschönes Delay geworden.

Könnt ihr also gerne versuchen besser zu lösen


MfG,
SlyD