[patch] Neues Modul: Venetian Blinds

[Christian.Kühnel]

Liebe FHEM Community,

ich habe über die letzten Monate ein neues Modul geschrieben, das meine Raffstore (aka. Außenlamellen,  engl. "ventian blinds") abhängig von diversen Parametern (Wind, Monat, Sonnenstand, Bewölkung, ...) verfährt. Die Dokumentation liegt im üblichen FHEM-Format im Code vor.

Der Code liegt als Perl Modul im CPAN Format auf github:
https://github.com/ChristianKuehnel/fhem-venetianblinds

Die Lösung läuft seit etwa 3 Monaten bei mir produktiv und funktioniert sehr zuverlässig. Aus meiner Sicht ist das Modul jetzt soweit, dass man es in die offizielle FHEM Distribution aufnehmen könnte.

Was soll ich dafür machen?

Beste Grüße
Christian

[rudolfkoenig]

Kurz: mir oder Boris via "richtigen" Email (nicht sourceforge) dein sourceforge-Account nennen, bereit sein dein Modul fuer eine Zeit hier im Forum zu Supporten, und Doku fuer commandref bereitstellen.

Lang: https://forum.fhem.de/index.php/topic,18962.0.html

[Christian.Kühnel]

OK, danke.

Das Modul ist jetzt eingecheckt: Revision 12226

[justme1968]

normale module sollten keine 99_ nummer verwenden. diese werden automatisch immer geladen. nicht nur bei bedarf.

es wäre gut wenn du das bis zum update morgen früh wieder änderst.

gruss
  andre

[CoolTux]

Siehe auch
https://forum.fhem.de/index.php/topic,58311.0.html

[Prof. Dr. Peter Henning]

Ich schlage  außerdem vor, das Modul nicht mit dem generischen Namen "Venetian" zu verzieren, denn es steuert eben nicht beliebige Sonnenschutzlamellen.

Laut enthaltener Dokumentation funktioniert es nur mit dem Controller Fibaro FGM-222, der generische Name ist also irreführend.

Es ist in FHEM in solchen Fällen üblich, die Hardwarebezeichung (mindestens teilweise, oder in einer sinnvollen Abkürzung) als Modulname zu verwenden.

Weitere Kommentare:

In der enthaltenen Dokumentation sind noch diverse sprachliche und Rechtschreibfehler, sowie "TODO"-Einträge. Wäre besser, das erst einmal zu komplettieren.

Das Modul verwendet diverse andere Devices, um Parameter zu holen - aber wie deren Daten genau zur Verfügung stehen müssen, fehlt in der Dokumentation leider auch.


LG

pah

[rudolfkoenig]

Ich habe das Modul aus FHEM entfernt, bevor es morgen frueh anfaengt sich per FHEM-update zu verbreiten. Nachtraegliches loeschen per FHEM-update ist noch aufwendig.

Weiterhin habe ich in das SVN pre-commit Hook eine Abfrage eingebaut, damit 99'er Dateien nicht ohne weiteres eingecheckt werden koennen.

@Christian.Kühnel: kannst du bitte die Datei unter einem anderen Prefix einchecken? Wenn das Modul nicht direkt ein Geraet bedient, sondern andere FHEM-Module benoetigt, dann schlage ich 98 vor. Weiterhin gebe ich pah Recht: das Modul sollte schon im Namen darauf hinweisen, dass es nicht beliebige Jalousien direkt bedient, z.Bsp. mit 98_FGM222_Venetian.pm. Oder wenn es doch generischer ist, dann 98_VenetianHelper.pm. Oder so.

[betateilchen]

Auch den Namen "Shared.pm" finde ich viel zu generisch, da er in der von "version" ausgegebenen Liste nicht direkt zuordenbar ist und den Eindruck erweckt, das gehöre generisch zu fhem.

[rudolfkoenig]

Seufz, lib/VenetianBlinds habe ich ganz uebersehen.

@Christian.Kühnel: wie ich in meiner Email im Abschnitt Standard Belehrung geschrieben habe:

- neue Verzeichnisse duerfen nur nach Ruecksprache mit mir angelegt werden, ....

Das hat auch einen Grund: fhemupdate verteilt Unterverzeichnisse nicht automatisch, diese muessen per manuellen Eingriff in fhemupdate.pl eingetragen werden.
Mir waere ganz-ganz-lieb, wenn du ohne diesen Unterverzeichnis auskommen wuerdest, sonst fangen alle anderen auch damit an. Und betateilchen hat recht: Shared.pm geht leider nicht. Ja, man koennte alles anders organisieren, habe bisher aber nicht genug Gruende dafuer.

[Christian.Kühnel]

Danke für die Hinweise.

- 99er Präfix brauche nicht nicht, ich habe einfach irgendwas genommen
--> baue ich auf 98 um

- Zum Namen:
Ich habe nur die FGRM-Schalter von Fibaro, entsprechend habe ich auch nur die unterstützt. Es wäre aber durchaus denkbar das Modul für andere Schalter zu erweitern.
--> Ich werde erst mal den Schalternamen in den Modulnamen übernehmen. Vielleicht findet sich irgendwann jemand, der das generischer gestalten will.

- Zur Doku:
Hier gehe ich nochmal drüber.

- zu shared.pm:
Über den Perl Namespace kann man die Datei dem Modul zuordnen FHEM/lib/VenetianBlinds/Shared.pm. Auch beim Import muss man den Namespace explizit angeben:
use VenetianBlinds::Shared
--> Änderung siehe nächster Absatz

- Ich fand es übersichtlicher das Modul in mehrere Teile zu zerlegen und mit Hilfe von Namespaces vom Rest zu trennen. Dafür brauche ich dann ein neues Unterverzeichnis unter FHEM/lib.  Das kann ich in ein Modul umbauen, da das vermutlich die Konvention ist. Das ist ein größere Refactoring, da ich dann die einzelnen Module in eine großes Modul zusammenführen muss. Dann funktionieren aber meine Tests nicht mehr ohne weiteres...

Danke für die Hinweise.

Kann etwas dauern, bis ich das alles umgesetzt habe. Ich melde mich nochmal, wenn ich damit durch bin

[Christian.Kühnel]

Liebe Community,

nochmal danke für die Hinweise.

Ich versuche gerade meinen Code in eine einzige Datei zu zusammenzuführen, werde damit aber aus folgenden Gründen nicht glücklich:

• Mir ist die Lesbarkeit und Wartbarkeit des Codes sehr wichtig. Daher finde ich eine Datei mit ca. 900 Zeilen ziemlich unpraktisch.
• Im Sinne von separation of concerns sollten verschiedene Teilfunktionen in verschiedene Dateien aufgeteilt werden:
  http://clean-code-developer.de/die-grade/orangener-grad/#Separation_of_Concerns_SoC
  
• Packagenames sind das Mittel der Wahl um Namenskonflikte zu verhindern. Insofern bin ich ein großer Fan davon.
  
  
  
  • PerlCritic fordert (aus gutem Grund) dass Modulname = Packagename :
    http://search.cpan.org/~thaljef/Perl-Critic/lib/Perl/Critic/Policy/Modules/RequireFilenameMatchesPackage.pm
  • Test::MockModule funktioniert nur, Modulname = Packagename. Das ist für mich ein Showstopper, da ich MockModule für meine Unittests brauche!
  [li]Da vermutlich niemand will, dass ich direkt in FHEM/lib 4 neue Dateien ablege, brauche ich dafür ein neues Unterverzeichnis, das aber gegen dis bisherige Konvention ist.

Insgesamt hätte ich also gerne folgendes einen Vorschlag, wie ich folgendes machen kann:

• eine Möglichkeit Packagenames im Sinne des Erfinders zu verwenden, mit Packagename = Modulname
• eine Möglichkeit mein Modul in mehrere Dateien aufzuteilen

P.S.: Soweit ich sehe haben einige Module schon eigene Packages und Unterverzeichnisse in FHEM/lib angelegt (MP3, Net, SWAP,...). Auch habe ich dort ein "Common.pm" gefunden. Scheinbar gab es also mal gute Gründe das zu erlauben...

Schönen Feiertag,
Christian

[rudolfkoenig]

Das man Code aufteilen kann, ist mir nicht ganz neu, sonst wuerde es auch keine Module geben. Allerdings halte ich in diesem Projekt _Einfachheit_ auch fuer wichtig, damit man potentielle Modulentwickler nicht abschreckt.

Ich bin nicht strikt gegen so eine Loesung, aber
- es bedeutet Arbeit fuer mich
- macht bisherige Gewohnheiten (z.Bsp. die Auflistung aller geaenderten Dateien beim update) ueber kurz oder lang sinnlos
- verschreckt potentielle Modulautoren, die anhand der Beispiele denken, fuer ein Modul muesste man 5+ Dateien anlegen.
- ich habe bisher keine stichhaltigen Argumente dafuer gefunden. "Jemand mit einer tollen Webseite haelt das generell fuer eine gute Idee" zaehlt nicht.

Wenn die Gegenargumente meine ueberwiegen, dann bin ich bereit fhemupdate anzupassen.

[justme1968]

und selbst die 'gute idee' der web seite sagt nur das code in kleine einheiten unterteilt werden soll. nicht das diese einheiten in eigene files sollen.

kleine routinen und klassen mit jeweils einem klar abgegrenzten zweck sind übersichtlich. diese dann auch noch auf x files zu verteilen nicht mehr unbedingt.

[Christian.Kühnel]

Hallo justme1968,
Hallo rudolfkoenig,

ich sehe, wir haben hier offenbar unterschiedliche Vorstellungen zur Strukturierung von Software. Ich will euch nicht zu etwas drängen, das ihr nicht haben wollt.

Auch will ich niemanden zwingen, seinen Code aufzuteilen, der es nicht will oder auch nicht braucht. Ich habe auch kleinere Module, die aus nur einer Datei mit <200 Zeilen bestehen. Da lohnt sich eine Aufteilung schlichtweg nicht. Aber ich hätte gerne die Möglichkeit das zu tun, wenn ich das für sinnvoll halte. Einzelne Module zu haben ist ein guter Anfang, aber wenn die Module zu groß werden, braucht man mehr.

Habt bitte Verständnis, dass ich im Gegenzug aber nicht bereit bin auf wesentliche Punkte meiner bisherigen Umgebung [2] zu verzichten:

• Ich kann eigene Perl Untermodule einchecken und Namespaces nutzen.
• ich kann meine Unit Tests für meine Module einchecken.
• Ich kann meine Unit Tests automatisch laufen lassen.
• ich kann die statisch Analyse mit Test::Perl::Critic automatisch laufen lassen (z.B. als Teil der Unit Tests).

Glaubt ihr, wir kommen hier auf einen grünen Zweig?
Oder wäre es doch besser meinen Code bei [2] zu belassen?

Versteht mich nicht falsch:
Ich sehe sehr wohl einen Mehrwert für uns alle, meinen Code in das FHEM-Repository zu überführen. Aber ich werde das nicht um jeden Preis machen...


P.S.: "Clean Code" [1] ist nicht nur irgendeine Webseite sondern aus meiner Sicht ein Standardwerk für moderne Softwareentwicklung in derselben Gewichtsklasse wie "The Art of Computer Programming".

[1] https://www.amazon.de/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882/ref=sr_1_1?ie=UTF8&qid=1475485071&sr=8-1&keywords=clean+code
[2] https://github.com/ChristianKuehnel/fhem-venetianblinds

[rudolfkoenig]

Mein Vorschlag ist, es erstmal bei [2] zu belassen.

Einen externen Server kann ein potentieller Benutzer mit relativ wenig Aufwand ins persoenliche update Prozess einbinden, und so von den Aenderungen direkt profitieren.