[Refactoring] MySensors-Module

[RichardCZ]

Bei Gelegenheit werde ich mal den Vorschlag mit dem Vorabtest auf den richtigen ref einbauen. Ist irgendwie seltsam, denn lt. Doku unter https://www.mysensors.org/download/serial_api_20#command sollte es wirklich nur diese $type geben; kann aber natürlich sein, dass da sehr viel mehr garbage über die Serielle Schnittstelle reinkommt, als ich bisher dachte...

Ich habe ja schon ein wenig über "defensives Programmieren" schwadroniert, da hätte man natürlich im Beispiel den test auf coderef schon gleich mitliefern können, aber da behaupte ich jetzt einfach mal sowas sollte man autonom-automatisch zu meinen Skelettbeispielen dazutun. 

Vermutlich werden einige neue kreative Messages bzw. Werte für $type im Log auftauchen.

[Beta-User]

Ja, manche brauchen halt für die Defensive länger wie für Offensive...

Das Problem waren weniger "kreative" neue Message-types, sondern vielmehr die Auflösung der Constants. Die logs erspare ich euch, aber es wurden bei dem kurzen Test nur 0-4 ausgeworfen...

Habe daher erst mal nur schlicht und ergreifend  die nummerischen Werte als Referenz vorne eingetragen. Nicht so aussagekräftig, aber funktional.

Anbei zur Abwechslung dann mal wieder  Code, der tut, was er soll (zumindest scheint das erst mal so...).

[Sidey]

Kleine Info was das Thema Testen ohne Hardware betrifft.

Ich teste mein physisches Modul auch gänzlich ohne Hardware. Das geht durchaus.

Beispielsweise, wenn man über httpmod etwas von einem Webserver abruft, dann präpariere ich mir einen repräsentativen Output. Z.B. kann das der json Body sein.
Der routine, welche dann den Output prüft, übergebe ich den präparierten json und verifiziere ob das Ergebnis passt.

Das kann man dann variieren und auch Daten übergeben mit denen man erst mal nicht gerechnet hat.

Naja an alles zu denken ist schwer, aber so ein paar Standard Situationen wiederholen sich vom Prinzip immer wieder.

Im Endeffekt kann man fast alles ohne Hardware testen.
Sogar das DevIO Modul kann man ohne Hardware testeten.

Grüße Sidey

[Beta-User]

"Kann" ist klar, hatte ja auch schon gleich geschrieben, dass man das vermutlich simulieren kann, wenn man ohne Hardware testen will.

Für mich war nur die Frage, wie es _für mich_ einfacher ist. Da habe ich für Funktionstests die Optionen Echtsystem, Testsystem mit Hardware oder Software.
Meistens kommt da in meinem Fall "Echtsystem" (nach Test des Moduls auf Lauffähigkeit in einem Testsystem ohne Hardware) raus. Hat halt den Nachteil, dass ich nur dann teste, wenn ich notfalls direkt auch reparieren kann, aber den Vorteil, dass ich gleich sehe, ob es im realen Leben auch geht (ich weiß ja in etwa, wann welche Node was abliefern müßte).

Würde ich das in Software machen wollen, wäre das noch eine Baustelle, und die tue ich mir im Moment schlicht und ergreifend nicht an, selbst wenn es einfach sein sollte. Wenn, dann will ich auch verstehen, was da "Sache ist", sonst nützt dem Bauchgefühl nach auch das Testen wenig.

Für "Sonderfälle" hatte ich mal ein Testsystem mit Hardware, das ich aber schon länger nicht mehr angefaßt hatte (deswegen werden die elsif-Kaskaden bei den diversen Debug-Dingern wohl auch noch eine ganze Zeit Bestand haben, falls sich nicht jemand erbarmt, der grade zufällig was passendes rumliegen hat; das erfordert nämlich zu allem Überfluß auch noch speziellen Code auf der abzufragenden Node bzw. funktioniert nicht mit allen Transceiver-Varianten...)

[RichardCZ]

Device::MySensors::Message

sub gettime ...

kann das weg? Ich sehe nirgends eine Verwendung und es wird nichtmal exportiert.

[Beta-User]

Im ersten Post jetzt nochmal der komplette Satz mit dem aktuellen Stand.

Da ist dann auch "sub gettime" raus, danke für den Hinweis .

Noch nicht optimal:
Message.pm: der "Standort" der EXPORT-Teile. Muß wohl so sein, bzw. anderfalls braucht man wohl die {no ...}-lexical scopes analog Constants.pm.

MYSENSORS_DEVICE
- Die Reihenfolge im set-Command-Handler@(kommt bei Gelegenheit mal);
- "die eine" if-Kaskade (mal schauen).

Grüße, Beta-User

[Beta-User]

Im ersten Post nochmal eine Aktualisierung von  10_MYSENSORS_DEVICE.pm.

Habe die Command-Handler weiter reduziert und eine erste Testfasung für Stream-Type Messages (falls "jemand" an Bildübertragung interessiert ist...). Ist aber noch nicht groß getestet.

[RichardCZ]

Ich sehe bei Get ein löbliches

return sendClientMessage(...

(es fehlt ansonsten häufig in Modulen, dass Rückgabewerte weitergeschleift werden, da verliert man viel Information)
aber kurze Zeit später

        sendClientMessage($hash, cmd => C_INTERNAL, subType => I_SIGNAL_REPORT_REQUEST, ack => 0, payload => "R");
        return;

wasdasdenn?

Rein interessehalber: "=pod disable unused $types "

Kommen die nicht zur Verwendung oder ist das NYI?

Code Auswählen Erweitern

   if ($type == I_VERSION ||
      $type == I_ID_REQUEST ||
      $type == I_ID_RESPONSE ||
      $type == I_INCLUSION_MODE ||
      $type == I_FIND_PARENT ||
      $type == I_FIND_PARENT_RESPONSE ||
      $type == I_LOG_MESSAGE ||
      $type == I_SIGNAL_REPORT_REVERSE ||
      $type == I_LOCKED ||
      $type == I_REGISTRATION_REQUEST ||
      $type == I_REGISTRATION_RESPONSE ) {
        $hash->{$typeStr} = $msg->{payload};
        return;
    }

So lange wir hier nicht alle Allahu akbar oder Meschugge sagen und RTL (nein, nicht der Fernsehsender: https://en.wikipedia.org/wiki/Right-to-left) lesen, sollten die Operatoren immer am Anfang (Links) und nicht am Ende (Rechts) stehen. Alle. Auch Concats.

Die echt elegantere Lösung als

Code Auswählen Erweitern

if ($type == FOO) {
wurschtel wurschtel
}

if ($type == BAZ) {
wurschtel wurschtel
}


ist tatsächlich

Code Auswählen Erweitern

sub _onFoo {
wurschtel
}

sub _onBaz {
wurschtel
}

und dann Dispatch table. Wenn Du dann irgendwann eine Testsuite für das Modul schreiben willst, dankst Du Gott (oder mir), dass Dein Code bereits in schöne kleine testbare subs seziert ist.

Code Auswählen Erweitern

my $nextTrigger = main::gettimeofday()

Ne - echt nicht. Mach den use Time::HiRes selbst.

[Beta-User]

Hmm, also (im ersten Post wieder eine kleine Aktualisierung, nix wildes, aber da hatte sich noch ein "last;" versteckt, das hätte ggf. bei einem der vielen Tester schiefgehen können ).

Was return sendClientMessage() betrifft, war es teils schlicht mit dem guten Gefühl abgekupfert, dass es so flexibler sein könnte. Tatsächlich gibt die betreffende sub aber nix zurück und es ist mMn. in fast allen Fällen auch eine "fire&forget" Anweisung. Wenn man da was auswerten wollte, wird es schnell kompliziert, da ein Teil der messages nicht unbedingt gleich rausgeschickt werden (bei smartSleep-Nodes wird gequeued). Hab's jetzt trotzdem da auch umgebaut, wo ich drübergestolpert bin.

Wenn ich mal lustig bin, baue ich ggf. den "callback" für manche "get/set" noch ein, aber das ist etwas anderes: https://wiki.fhem.de/wiki/DevelopmentModuleIntro#X_AsyncOutput (@eventuelle kundige Mitleser: Das würde mich interessieren, wie das im Detail geht, kann ja eigentlich nix dramatisches sein...).


Was die deaktivierten Typen angeht, sind die mWn. tatsächlich nur zur Kommunikation node-2-node im C-code vorgesehen, werden aber nicht (außerhalb eventueller Debug-Meldungen an der seriellen Konsole der Arduinos) als regulärer Message-Inhalt ausgegeben. Das betrifft sogar einige der "älteren" Typen, die in dieser "or"-Struktur vergraben waren. War auch eher zur Vorbereitung gedacht, um da mal auszumisten und das löschen, was nichts bringt. (Es hatte mir jemand geraten, sukzessive vorzugehen ).

Und da hatte ich die verstreute "Soll ich damit was anfangen?"-Liste erst mal in "ja, und zwar speziell das...,", "ja, aber nur Internals updaten" und "wasndat?!?" sortiert, wobei der zweite Haufen vermutlich immer noch zu groß ist. Da gehört mind. die Hälfte auch in "wasndat?!?", aber das mußte ich mir nochmal im Detail ansehen... (und zwei -ziemlich unwichtige - Typen waren auch falsch umsortiert). Jetzt ist raus, was mAn. garbage ist...

Das mit dispatch hat zwar was, aber es hat auch seine Tücken, angefangen damit, dass das hier CONSTANT sind und das mit dem dispatchen daher nur geht, wenn man die nummerischen Entsprechungen verwendet. Das ist aber mMn. schlechter lesbar (den "Trick" mit dem "+" vorneweg, den hier jemand anderes propagiert hatte, hatte ich erfolglos getestet)...

In dem "Sammel-if" ist jetzt nicht mehr viel drin, und bei manchem (I_LOG_MESSAGE und I_LOCKED frage ich mich, warum das dann nicht als triggerndes Reading ausgeworfen wurde) (beides nie verwendet...). Nun ja, schön ist es nicht, aber andererseits ist die Frage, ob man
wegen der paar Werte einen hash aufmacht oder sonst einen Zinnober, zumal bei den Constants ja fraglich ist, ob der Wert dann wirklich sauber geprüft wird... .

Zu guter letzt ist es jetzt noch nach gefühlter Häufigkeit sortiert .

Da ich mich nicht in der Lage sehe, da eine Testsuite für zu entwickeln, braucht man mMn. den Code auch an der Stelle nicht weiter zu atomisieren, das ist zwar länglich, aber diese Teile sind so simpel, die verstehe sogar ich .

use Time::HiRes: Aye Sir! Hatte mich bisher nicht näher damit beschäftigt, wo die Funktion eigentlich ursprünglich mal herkam...

[krikan]

https://wiki.fhem.de/wiki/DevelopmentModuleIntro#X_AsyncOutput ([...] Das würde mich interessieren, wie das im Detail geht, kann ja eigentlich nix dramatisches sein...).

asyncOutput-Beispiele findest Du mehrere in https://svn.fhem.de/trac/browser/trunk/fhem/FHEM/10_ZWave.pm

[Beta-User]

Hmm, das bringt mich leider nur bedingt weiter... Ich hatte mir das mal vor langem schon im Signalduino-Code angesehen, neulich mal wieder im MQTT2_DEVICE und habe leider immer noch nicht so richtig den Anpack, was jetzt wo genau ergänzt werden müßte, dass das klappt. Habe jetzt nochmal in den MQTT2_DEVICE-Code geschaut, da weiß ich besser, was da abläuft und es sind weniger Stellen.

Mal die Bruchstücke:
- Wenn ein get abgesetzt wird, kommt der if-Code aus https://svn.fhem.de/trac/browser/trunk/fhem/FHEM/10_MQTT2_DEVICE.pm#L356 zum Einsatz.
- kommt irgendwas zurück, wird "checkForGet" dazwischengeschoben: https://svn.fhem.de/trac/browser/trunk/fhem/FHEM/10_MQTT2_DEVICE.pm#L125

Aus der Zusammenschau würde ich also schließen, dass mir als "Brücke" fehlt, wo "$hash->{CL}" herkommt, aber alles, was ich dazu im Modul selbst finde, wäre https://svn.fhem.de/trac/browser/trunk/fhem/FHEM/10_MQTT2_DEVICE.pm#L321, da wird ein hash gebaut und an fhem.pl übergeben. In fhem.pl findet sich dann zwar was, aber das klingt nach Zusammenhängen mit CommandSetReading, und ab da bin ich dann gedanklich weg...
Eigentlich müßte das eher was mit der Frage zu tun haben, ob der Befehl aus FHEMWEB oder telnet kam, denn dahin sollte ja die Rückmeldung gehen...

Vielleicht kannst du mir da dann noch den entscheidenden Tipp geben, wie der "Ring" zu schließen ist...?

[rudolfkoenig]

Von vorne:
- Benutzer will Antwort auf Frage
- Problem: die Antwort dauert laenger, das Modul will FHEM nicht blockieren
- Loesung: set/get/attr/define/modify setzt _fuer die Dauer des Modul-Funktionsaufrufs_ $hash->{CL} auf das "Client-Hash", d.h. die FHEMWEB/telnet/etc Verbindungsinstanz worueber der Befehl reingekommen ist.
- dieses $hash->{CL} muss man im modulspezifischen getFn/etc merken, die Abfrage ohne blockieren starten, und nichts zurueckliefern.
- wenn die Antwort eingetroffen ist (in ParseFn?), dann wird geprueft, ob das die Antwort auf die aussstehende Frage ist. Wenn ja, asyncOutput() wird mit der gespeicherten Variante von $hash->{CL} aufrufen.
- wenn die dazugehoerige Verbindung (FHEMWEB/telnet/etc) eine AsyncOutputFn implementiert, dann wird darueber dem Benutzer die Antwort mitgeteilt.

[Beta-User]

Ah, jetzt wird klarer, warum der Code in MQTT2_DEVICE sich so liest, als könnte man davon ausgehen, dass $hash-{CL} einfach da ist... Das ist der Fall, wenn der "Auslöser" aus FHEMWEB (&Co) heraus aufgerufen wird.

Bleibt die Frage, wie man es mit timeouts hält. MQTT2_DEVICE wartet 4 Sekunden, das finde ich eigentlich auch ganz ok, hier ist ähnlich unklar, ob und wann ggf. die Anfrage zugestellt werden kann. Eigentlich weiß man nur, wenn ein bestimmter Schlafmodus genutzt wird (was kaum einer macht), dass die Node nicht erreichbar ist...

Damit habe ich zumindest mal eine Blaupause, Danke! Mal sehen, wann ich dazu komme, diese "Verschönerung" einzubauen (da hat sich all die Jahre keiner wirklich dran gestört).

Eine abschließende Frage: Auf bestimmte Fragen gibt es mehrere Rückgabewerte bzw. manches läuft in einer Art "Schleife" (bestimmte Debug-Infos von den Nodes). "Schön" wäre es, diese Infos zu sammeln (falls die Node zeitnah Antwort liefert) und dann auch gebündelt auszugeben. Geht doch bestimmt auch?

[rudolfkoenig]

"Schön" wäre es, diese Infos zu sammeln (falls die Node zeitnah Antwort liefert) und dann auch gebündelt auszugeben

Dann sammele die Daten solange, bis der Timeout zuschlaegt, und gib sie dann in der Timeout-Routine alle auf einmal aus.
Oder Du kriegst vorher irgendwie mit, dass nichts mehr kommt.
Oder anders

[Beta-User]

Dann sammele die Daten solange, bis der Timeout zuschlaegt, und gib sie dann in der Timeout-Routine alle auf einmal aus.
Oder Du kriegst vorher irgendwie mit, dass nichts mehr kommt.
Oder anders

Danke. Ich glaube, der Groschen ist gefallen, siehe screenshot .

Aktualisiertes Modul im Anhang (heartbeat ist aber erst der Anfang...).