DevIO.pm

[Dr. Boris Neubert]

Hallo,

ich wollte "eben mal" 66_ECMD.pm etwas robuster machen gegen fehlende Devices. Ich bin an DevIO_OpenDev() gescheitert.

Seit ich diese Funktion vor einigen Jahren mitentwickelt habe ist sie erheblich gewachsen und mangels Kommentaren mir nicht verständlich. Obwohl im Wiki steht, dass die Funktion eine Fehlermeldung oder undef im Erfolgsfall zurückliefert, wird alles mögliche zurückgeliefert (0, "", undef, ...).

Ich habe mich dann mal in die Funktion vertieft. Für TCP-Verbindungen heißt es ab Zeile 326

Code Auswählen Erweitern

      } else {
        Log3 $name, 3, "Can't connect to $dev: $!" if(!$reopen && $!);
        $readyfnlist{"$name.$dev"} = $hash;
        DevIo_setStates($hash, "disconnected");
        $hash->{NEXT_OPEN} = time() + $nextOpenDelay;
        return 0;
      }



Solte es nicht so gehen?

Code Auswählen Erweitern

      } else {
        my $errmsg= ($! ? "$name: can't connect to $dev: $!" : undef);
        Log3 $name, 1, $errmsg if(defined($errmsg));
        $readyfnlist{"$name.$dev"} = $hash;
        #DevIo_setStates($hash, "disconnected");
        $hash->{NEXT_OPEN} = time() + $nextOpenDelay;
        return $errmsg;
      }


Konkret sind mir in diesem Zusammenhang folgende Punkte aufgefallen:

• Unklar, warum nur dann eine Fehlermeldung ausgegeben werden kann, wenn es sich um ein reopen handelt.
• Loglevel für Fehlermeldungen soll 1 sein, ist 3.
• Log3 bekommt den Namen, loggt ihn aber nicht.
• Die Funktion setzt den State. Ist das wirklich gewollt oder sollte man das nicht grundsätzlich dem Aufrufer überlassen?
• In den State werden mal GROSSBUCHSTABEN, mal Kleinbuchstaben geschrieben.
• Es sollte nicht 0 sondern die Fehlermeldung oder undef zurückgegeben werden.

Ich plädiere dafür, für Standardsituationen Guidelines zu erlassen und den Shared Code daraufhin zu überarbeiten. Das kleine bisschen Code ruft nach Guidelines für:

• Wie sollen Fehler signalisiert werden? Z.B. durch Rückgabe einer Liste (undef, $errmsg).
• Format von Logmeldungen?
• Loglevel?
• In welchen Fällen darf Shared Code Inhalte im Device-$hash ändern?
• Standards für Standardzustände im State, z.B. connected vs. Connected vs. CONNECTED?
• Wie wird der Shared Code dokumentiert? Kommentare im Code? Im Wiki? Via Pod?

Und danach einen Code Cleanup. Ja, das wird Kollateralschäden geben, aber ansonsten bleibt die Codebasis nicht mehr beherrschbar. Können wir ja in einem Branch machen.

Viele Grüße
Boris

[rudolfkoenig]

Unklar, warum nur dann eine Fehlermeldung ausgegeben werden kann, wenn es sich um ein reopen handelt.

Andersherum: die Meldung wird ausgegeben, wenn es _kein_ reopen ist, damit bei einem nicht erreichbaren Geraet nicht einmal die Minute was im Log steht.

Loglevel für Fehlermeldungen soll 1 sein, ist 3.

Stimmt, das habe ich nur von der anderen Stellen in DevIo uebernommen. Habe jetzt alle auf 1 geaendert.

Log3 bekommt den Namen, loggt ihn aber nicht.

Habs auch angepasst.

Ist das wirklich gewollt oder sollte man das nicht grundsätzlich dem Aufrufer überlassen?

Wenn wir FHEM neu schreiben wuerden, dann wuerde ich das dem Aufrufer ueberlassen.
Jetzt ist mir Risiko/Nutzen zu gross, um es zu aendern.

Es sollte nicht 0 sondern die Fehlermeldung oder undef zurückgegeben werden.

Das sollte im Fehlerfall mit doCb der Fall sein.

[michael.winkler]

Hi Kollegen,

da Ihr gerade an dem Modul arbeitet. An meinem Testsystem erhalte ich immer folgenden Meldungen im LOG.

Code Auswählen Erweitern


2018.03.09 13:07:49 1: CUL: Can't open /dev/ttyS2: Input/output error
2018.03.09 13:07:49 3: Probing CUL device /dev/ttyS3
2018.03.09 13:07:49 1: PERL WARNING: can't getattr: Input/output error at ./FHEM/DevIo.pm line 394.
2018.03.09 13:07:49 1: stacktrace:
2018.03.09 13:07:49 1:     main::__ANON__                      called by /usr/share/perl/5.24/Carp.pm (169)
2018.03.09 13:07:49 1:     Carp::carp                          called by /usr/lib/x86_64-linux-gnu/perl5/5.24/Device/SerialPort.pm (339)
2018.03.09 13:07:49 1:     Device::SerialPort::new             called by ./FHEM/DevIo.pm (394)
2018.03.09 13:07:49 1:     (eval)                              called by ./FHEM/DevIo.pm (392)
2018.03.09 13:07:49 1:     main::DevIo_OpenDev                 called by ./FHEM/98_autocreate.pm (587)
2018.03.09 13:07:49 1:     main::CommandUsb                    called by fhem.pl (1176)
2018.03.09 13:07:49 1:     main::AnalyzeCommand                called by fhem.pl (1026)
2018.03.09 13:07:49 1:     main::AnalyzeCommandChain           called by ./FHEM/91_notify.pm (104)
2018.03.09 13:07:49 1:     main::notify_Exec                   called by fhem.pl (3531)
2018.03.09 13:07:49 1:     main::CallFn                        called by fhem.pl (3451)
2018.03.09 13:07:49 1:     main::DoTrigger                     called by fhem.pl (601)
2018.03.09 13:07:49 1: CUL: Can't open /dev/ttyS3: Input/output error


Gruß
Michael

[rudolfkoenig]

Offensichtlich ist unter /dev/ttyS3 was komischen angeschlossen.
Wenn die Meldung stoert, dann wuerde ich wuerde auf dem initialen "usb scan" verzichten, mit "attr initialUsbCheck disable"

[jensb]

Bin beim Überarbeiten von FRM auch über den State-Zugriff von DevIO gestolpert. User hatten moniert, dass die States von FRM nicht transparent sind und unterschiedlich geschrieben werden (Initialized, disconnected, etc.). Bei FRM liegt das daran, dass mehrere Schnittstellen unterstützt werden, die jeweils ihr eigenes State-Meldeverhalten aufweisen, worauf auch der Code abhebt. Habe mich deshalb dazu entschieden, die States für jede Schnittstelle in der Modulhilfe zu beschreiben, damit die User wissen, welche Werte in Frage kommen.

Wenn man das jetzt Standardisieren würde, wäre es im Ergebnis konsequenter. Man könnte so etwas dann auch in einem zentralen Teil der Dokumentation unterbringen. Allerdings kann ich mir keinen weichen Übergang vorstellen. An dem Tag, wo DevIO oder TcpServerUtils geändert und freigegeben werden, müssten eine ganze Reihe Module ebenfalls in überarbeiteter Form vorliegen. Das Einführen eines DevIO2 (ggf. mit integrierter Abwärtskompatibilität) könnte dagegen einen Übergang ohne Bruch ermöglichen, wäre aber entsprechend aufwendiger in der Umsetzung.

Grüße,
Jens

[Dr. Boris Neubert]

Das sollte im Fehlerfall mit doCb der Fall sein.

Wie gesagt, der Code gibt alles mögliche zurück, 0, undef, "" oder das Ergebnis von doCb.

Wenn eine Änderung zuviele Nebeneffekte hätte, dann markieren wir die Funktion als deprecated und schreiben einen Wrapper, der (Ergebnis, undef) oder (undef, Fehlermeldung) zurückliefert oder worauf auch immer wir uns einigen, wie Funktionen in FHEM künftig Ergebnis oder Fehler zurückliefern sollen.

Viele Grüße
Boris

[rudolfkoenig]

Wie gesagt, der Code gibt alles mögliche zurück, 0, undef, "" oder das Ergebnis von doCb.

Kannst du das bitte an einem Beispiel zeigen?

[Dr. Boris Neubert]

Hallo,

ich habe mir einfach den Code von DevIO_OpenDev() auf return-Anweisungen hin durchgesehen:

Code Auswählen Erweitern


return $r;   
return undef;
return &$doCb(undef);
return &$doCb($@);
return &$doCb("");
return 0;
return 1;
return ""
return &$doCb(&$doTailWork());

Grüße
Boris

[rudolfkoenig]

Na so einfach ist das auch nicht , da einige dieser returns in lokalen "subs" verwendet werden, die DevIo_OpenDev nicht direkt verlassen, sondern vorher noch geprueft werden. Und das ist mAn "legal".

[Dr. Boris Neubert]

OK, jetzt sehe ich es. 

Bestätigt meine These, dass ich den Code nicht überblicke, weil die Routine zu lang ist und kaum kommentiert. Es gibt eine Doku im Wiki, der ich nicht vertrauen kann, weil ich nicht weiß, wie eng Wiki und Code beieinander gehalten werden.

Möglicherweise liegt es an meiner Unfähigkeit, DevIO.pm zu verstehen. Für mich ist das ein echtes Hindernis, den Code zu nutzen. Ich plädiere für Coding Guidelines und Doku im Code - das ist m.E. die bequemste Art für die Programmierer.

Und das hier:

Code Auswählen Erweitern

https://www.kernel.org/doc/html/v4.10/process/coding-style.html#functions

halte ich für ein gutes Beispiel.

Viele Grüße
Boris

[Markus Bloch]

Hallo Boris,

Es gibt eine Doku im Wiki, der ich nicht vertrauen kann, weil ich nicht weiß, wie eng Wiki und Code beieinander gehalten werden.

den Artikel habe ich erstellt, da es für Einsteiger in die Modulentwicklung nicht direkt ersichtlich ist, wie DevIo funktioniert und welche Möglichkeiten es bietet. Ich versuche dabei immer den Wiki-Artikel zu aktualisieren sobald es entsprechende Änderungen gibt. Gleiches gilt hier auch für alle anderen Artikel im Bereich "Development". Das mag mir natürlich nicht immer gelingen. Das bedeutet aber nicht, dass ich die Artikel für mich allein beanspruche. Jeder (sowohl User als auch Modulentwickler) ist hier aufgerufen mitzuhelfen und Änderungen herbei zu führen wo es notwendig erscheint. Alleine aufgrund der Tatsache das ich als Auto eine gewisse Betriebsblindheit habe, freue ich mich umso mehr, wenn Leute mit einem anderen Blickwinkel die Inhalte reflektieren.

Viele Grüße

Markus

[zap]

Wenn man Guidelines für Code einführt (was ich gut finde), braucht es auch eine Instanz oder ein Gremium, das neue Module vor dem ersten Einchecken prüft und freigibt.
D.h. es entsteht etwas Verwaltungsaufwand und es müssen sich Leute finden, die die Code Prüfung durchführen.

[tupol]

Ich vermute, es wird Modul-Entwickler (wie z. B. mich) demotivieren, wenn sie ihren Code dann noch "rechtfertigen" müssen.
An eine Vorgabe würde ich mich aber trotzdem gerne halten und finde die Artikel in der WIKI sehr hilfreich.

[Prof. Dr. Peter Henning]

Ich schließe mich aber Boris Neubert an: Es ist schon ziemlich zäh, die Wirkungsweise von DevIO.pm zu verstehen. Kommentare sind der erste Schritt.

LG

pah

[rudolfkoenig]

Es ist schon ziemlich zäh, die Wirkungsweise von DevIO.pm zu verstehen. Kommentare sind der erste Schritt.

Habs jetzt versucht.