Universalclient

[teasy1]

Hallo zusammen,

kann mir jemand erklären was das Tool genau macht? Ich denke, daß ich das gut gebrauchen kann.

Zum Hintergrund:
Ich betreibe mein OpenHab 1.7.1 auf einem Odroid U3 und steuere damit diverse Homematic-Komponenten. An meiner Wand hängt ein ausgedientes Samsung Galaxy Tab mit der CometVisu. Das Tablet ist so eingestellt, daß es nach 10 Minuten in den Tiefschlaf geht und erst wieder erwacht, wenn sich jemand nähert. Dort habe ich dann öfters das Problem, daß CometVisu nicht die letzten Statusänderungen mitbekommen hat, meistens bei den Rolladenaktoren. Ich muß dann immer einen Refresh der Webseite machen.
Falls das damit gelöst werden kann, würde ich mich über weitere Infos und Installationsanweisungen freuen.

Gruß Teasy

[peuter]

Hauptziel des Universalclient ist es die Kommunikation mit unterschiedlichen Backends zu vereinheitlichen. Für den Enduser gibt's damit also erstmal keine Verbesserungen. Ich gehe mal davon aus, dass Du das Default-Backend benutzt. Damit sollte sich der Universalclient exakt genauso Verhalten, wie der alte Client (sofern ich keine Fehler eingebaut habe). Du bis natürlich herzlich eingeladen zu testen, aber bei dem von Dir beschriebenen Problem wird das nicht helfen.

[Chris M.]

Läuft und läuft. Hatte es bis jetzt nur ein weiteres mal wo der Reconnect nicht funktioniert hat. Hab jedoch nicht herausgefunden an was das liegt.

Probier mal den Code von
http://stackoverflow.com/questions/1...rom-sleep-mode - evtl. kann der das Aufwachen detektieren.
Falls ja, dann sollten wir den Watchdog entsprechend pimpen.
Falls nein, ein Versuch war's wert...

Außerdem wäre spannend, ob

window.addEventListener("focus", function(event) { /* ... */ } )

anspringt - ggf. gekoppelt mit Code vom o.g. Link.

[Chris M.]

Mit dem Code bin ich jetzt erst mal durch. Diskussion besser erst mal hier, da a) in deutsch und b) eher strategisch als auf spezifische Zeilen bezogen (den Kommentar zu spezifischen Zeilen hab ich in den Pull-Request eingetragen).

Ganz allgemeine Betrachtung:

• Leider wurde der Code des cometvisu-client.js durch einen automatischen Code-Formatierer gejagt. Das Ergebnis ist ähnlich wie bei einer Text-Verarbeitung wo man bei der Rechtschreibprüfung "alles annehmen" klickt: manchmal gut, manchmal knapp daneben und manchmal unbrauchbar.
  Ein paar Stellen hab ich im Code kommentiert, ist mir dann aber zu blöd geworden.
• Zu meiner Frage ob universal-client.js nicht besser nur eine Weiterentwicklung des cometvisu-client.js sein sollte und folglich dort in der Datei landen sollte, gab es ja die Antwort: "But technically, is a clean, fresh and well structured implementation that does not really share a lot of code with the old implementation."
  Da das meiste noch auf dem alten Code besteht (man wegen der Umformatierung aber immer etwas hin und her schauen muss), denke ich weiterhin, dass es eigentlich in die bestehende Datei gehört.

Das führt uns gleich zu dem Thema, dass ich neulich im Pull-Request Kommentar angesprochen hatte: wie weit soll der Client wissen mit wem er kommuniziert (d.h. knxd, OpenHAB ...)?
Ich denke, das sollte dem Client egal sein. Der Client soll nur wissen, ob er per Long-Polling oder per SSE sprechen soll. Ob das Backend nun knxd oder OpenHAB ist, gehört in eine höhere Schicht die auch weiß, dass wir hier eine Visu haben. Also im ersten Ansatz wohl in die templateengine.js
(Hintergrund: der Client wird auch in ganz anderen Projekten gegen ganz andere Backends eingesetzt. Also keine Visu und kein knxd. Und der Client macht genau eine Sache: Kommunizieren. Nicht mehr und nicht weniger).

Ich vermute, dass die `long-polling` und die `sse` Welt noch mehr vereinheitlicht werden können. So ist z.B. der Watchdog getrennt implementiert - was aber erst mal keinen offensichtlichen Sinn macht, da die Aufgabe bei beiden Transportschichten die gleiche ist: schauen ob die Kommunikation nicht mehr läuft und wenn ja, neu anstoßen.

Nach dieser Kritik (die positiv gemeint ist!) zum Ansatz des Universal-Clients an sich: finde ich sehr gut! Und ich denke, wir sind schon fast am Ziel.

[peuter]

Leider wurde der Code des cometvisu-client.js durch einen automatischen Code-Formatierer gejagt. Das Ergebnis ist ähnlich wie bei einer Text-Verarbeitung wo man bei der Rechtschreibprüfung "alles annehmen" klickt: manchmal gut, manchmal knapp daneben und manchmal unbrauchbar.

Ja nachdem ich angefangen hatte manuell die Tabs zu entfernen ist mir das recht schnell zu blöd geworden und ich habs durch den automatischen Code-Formatierer von Eclipse machen lassen. Ich werde da manuell nochmal drüber gehen.

Zu meiner Frage ob universal-client.js nicht besser nur eine Weiterentwicklung des cometvisu-client.js sein sollte und folglich dort in der Datei landen sollte, gab es ja die Antwort: "But technically, is a clean, fresh and well structured implementation that does not really share a lot of code with the old implementation."
Da das meiste noch auf dem alten Code besteht (man wegen der Umformatierung aber immer etwas hin und her schauen muss), denke ich weiterhin, dass es eigentlich in die bestehende Datei gehört.

Ist mir eigentlich egal wie der Dateiname lautet, ich kann die gerne in den alten Namen umbenennen.

Das führt uns gleich zu dem Thema, dass ich neulich im Pull-Request Kommentar angesprochen hatte: wie weit soll der Client wissen mit wem er kommuniziert (d.h. knxd, OpenHAB ...)?
Ich denke, das sollte dem Client egal sein. Der Client soll nur wissen, ob er per Long-Polling oder per SSE sprechen soll. Ob das Backend nun knxd oder OpenHAB ist, gehört in eine höhere Schicht die auch weiß, dass wir hier eine Visu haben. Also im ersten Ansatz wohl in die templateengine.js
(Hintergrund: der Client wird auch in ganz anderen Projekten gegen ganz andere Backends eingesetzt. Also keine Visu und kein knxd. Und der Client macht genau eine Sache: Kommunizieren. Nicht mehr und nicht weniger).

Im Grund ist das ja schon so, lediglich bei den Hardcodierten Backend-Konfigurationen tauchen die Namen der Backends auf. Diese könnte man in die templateengine schieben.

Ich vermute, dass die `long-polling` und die `sse` Welt noch mehr vereinheitlicht werden können. So ist z.B. der Watchdog getrennt implementiert - was aber erst mal keinen offensichtlichen Sinn macht, da die Aufgabe bei beiden Transportschichten die gleiche ist: schauen ob die Kommunikation nicht mehr läuft und wenn ja, neu anstoßen.

Klar machen die beiden Watchdogs das selbe, aber der Weg wie man feststellen kann ob die Kommunikation noch läuft in nunmal abhängig von der Transportschicht. Jetzt könnte man das Ganze so umbauen, dass die Transportschichten z.B. checkConnection() und restartConnection() zur Verfügung stellen und ein globaler Watchdog würde dann eigentlich nur noch aus einem Timer bestehen der regelmäßig checkConnection() prüft und ggf. restartConnection() aufruft. Allzu viel vereinheitlicht hat man damit aber nicht, da kann mans auch so lassen wie es ist.

Ich werde versuchen Deine Code-Review Anmerkungen heute noch durchzugehen.

[peuter]

Einige Dinge bzg. des Reviews habe ich eingecheckt. Leider scheinen alle meine Testsetups mich heute nicht zu mögen, daher ist das ein ungetesteter Blinder Commit. Da die meisten Sachen eher kosmetischer Natur sind, ist dies nicht weiter tragisch. Lediglich beim long-polling Watchdog, habe ich das versucht das anders zu lösen, da die Ursache des Problems am falschen Context liegt. Das hätte ich natürlich lieber selbst getestet, aber mein altes Testsystem fürs default Backend läst die Visu garnicht mehr. Und ich habe jetzt keine Lust mehr nach der Ursache zu forschen (hab das auch Jahre lang nicht mehr benutzt).
Chris M. Bitte teste das mal bei Dir.

[Chris M.]

Lädt aktuell nicht, in Zeile 151:

Uncaught TypeError: Cannot read property 'long-polling' of undefined

Vermutlich liegt es daran, dass die Methode wird in Zeile 153 aufgerufen wird, `this.transport` aber erst in Zeile 235 befüllt wird.

Um hier besser voran zu kommen, schlage ich vor:

1. Den Code auf den aktuellen develop-Stand zu basieren
2. Einen Branch von CometVisu/CometVisu mit diesem Code aufzumachen.

Dann können wir gemeinsam das auf einen Stand bringen der für alle passt. Ich würde da auch das Commiten ohne Review erlauben wollen, denn das Review kommt dann ganz zu Schluss, wenn es nach develop gepullt wird.

[peuter]

Ok, hab den Branch auf develop rebased und auf CometVisu/CometVisu/ gepushed.

[Chris M.]

OK, danke. Ich schau gerade mal ob ich den Client stärker umstelle mit Kapselung u.ä.

[Chris M.]

So, jetzt hab ich im Code mal kräftig rum gerührt, stärker Session Layer vom Transport Layer getrennt, etc. pp.

Gegen den eibd auf dem Wiregate läuft der Code nun (wieder). Gut möglich, dass ich anderen Code zerbrochen habe (hm, kann man "break" hier so übersetzen?).
=> Bitte schaut mal gegen Euere Backends und stellt Fixes ein oder gebt hier Bescheid, dass es funktioniert.

Im Long-Polling Code würde ich noch gerne etwas weiter aufräumen, aber vorher sollten wir nun einen universell laufenden Code als Absprungbasis haben.

[peuter]

Mit einem Backend, welches "sich selbst konfiguriert" (also der CometVisu beim Login seine Konfiguration mitteilt) funktioniert das nicht, weil der Watchdog auf 2 Parameter zugreift, die an der Stelle noch nicht konfiguriert sind (maxConnectionAge und maxDataAge).

Wäre es nicht überhaupt sinnvoller den Watchdog nicht in der subscribe Methode sondern erst in der handleLogin Methode zu starten, da hier erst die Verbindung zu Stande kommt, die der Watchdog überwachen soll?

[Chris M.]

Stimmt, der Watchdog muss erst laufen wenn die Verbindung steht - denn das ist ja dass, was überwacht werden soll...
Hab's gerade mal verschoben.

[peuter]

Habe gerade zwei kleine Fixes für den SSE Transport commited, damit läuft der Client erstmal grundsätzlich. Nur der Watchdog scheint beim SSE Probleme zu machen. Ich denke, dass dort nur der isConnectionRunning() Teil nötig wäre und der Ganze Rest eigentlich überflüssig ist. Ich habe hier in meinem Testsetup recht wenig Updates, somit springt der Watchdog immer an und kappt die laufende SSE Verbindung (was ja unnötig ist), und danach läuft garnichts mehr. Muss das aber noch ein wenig testen. Ich denke ich werde das so umbauen im SSE-Teil, dass er beim restart prüft ob die Verbindung noch existiert und in dem Fall nichts macht.

[Chris M.]

Ich kenne die SSE nicht wirklich. Bei dem AJAX kann es aber passieren (sollte nicht, ist der Unterschied zwischen Theorie und Praxis...) dass die Verbindung nicht mehr läuft, der Client das aber nicht weiß. (IIRC z.B. wenn man den Server-Prozess abschießt und neu startet während der Client gerade die Verbindung offen hält).

Evtl. kann man hier auch ein aktiven Heartbeat in die Kommunikation mit einbauen, der das checkt.

Gerde der Watchdog hat (leider...) einige Lessons Learned drinnen um produktiv eine stabile Verbindung sicherzustellen. Es mag gut sein, dass man gewisse Zeiten bei einem anderen Transport-Layer anders einstellen muss, aber grundsätzlich sollten diese Prüfungen bei jedem Transport Sinn machen.

[peuter]

Langzeit-Erfahrungen mit SSE habe ich auch noch nicht. Vor allem die oft vorhandenen Unterschiede zwischen "müsste so funktionieren" und "funktioniert tatsächlich" kenne ich daher nicht. Zumindest bekommt der Client das mit wenn der Server weg ist und versucht sogar selbstständig sich neu zu Verbinden (so sollte es laut SSE-Definition auch sein). Was durch bisherige Tests aus der Praxis bereits bekannt ist, ist, dass manchmal kein event geworfen wird, wenn die Verbindung beendet wird (z.B. wenn Android Geräte schlafen gehen). Da reicht aber das regelmäßige Überprüfen ob die Verbindung noch steht (was der Watchdog ja macht).

Die Frage ist nun ob es passieren kann, dass die Verbindung als weiterhin existent gemeldet wird obwohl das nicht der Fall ist. Ist bisher nicht vorgekommen, kann man aber natürlich nicht ausschließen. Das muss die Praxis dann zeigen. Momentan ist es eher so dass ständiges neu Aufbauen der Verbindung mehr kaputt macht als es repariert und daher würde ich das erstmal so einbauen, wie bereits beschrieben. Wenn wir dann einen reproduzierbaren Fall haben, kann man das immer noch ändern.