knx-Plugin - Hostname?

[Morg]

Hallo,

in meiner Konfiguration (plugin.yaml) ist das knx-Plugin konfiguriert:

knx:
     class_name: KNX
     class_path: plugins.knx
     host: hauspi
     port: 6720
     send_time: 60
     time_ga: 5/2/0
     date_ga: 5/2/1
     busmonitor: 'true'

Bei jedem Start erhalte ich eine Fehlermeldung:

2019-11-14 17:49:26 ERROR    metadata     Main         plugin 'knx': Found invalid value 'hauspi' for parameter 'host' (type ipv4) in /etc/plugin.yaml, using default value '127.0.0.1' instead -- metadata.py:check_parameters:937

Wenn das Plugin nur IP-Adressen akzeptieren soll, dann sollte das so in der Doku angepasst werden (nicht "hostname", sondern "IP address"). Das kann ich bei Bedarf gern machen.

Ansonsten (fände ich schöner) sollte das Plugin hier auch Hostnamen akzeptieren. Wenn die nicht aufgelöst werden können, kann man ja immer noch eine Fehlermeldung werfen.

Leider komme ich im Quelltext nicht weiter - im knx-Plugin wird die Funktion self.get_parameter_value('host') aufgerufen. Die habe ich als class-member zur Klasse SmartPlugin verfolgen können. Dort finde ich aber nur eine Funktion, die die Member-Funktion _parameters.get(parameter_name, None) auf ein Dictionary _parameters aufruft.

Ich habe weder finden können, wo _parameters initialisiert wird, noch - und das scheint mir wichtiger -, wo die offensichtlich stattfindende Typprüfung stattfindet (und wo der Typ "ipv4" festgelegt wird, vgl. Fehlermeldung oben).

Kann mich jemand erleuchten?

[edit]

Super, wenn einem nach dem Posten die Augen aufgehen...

metadata.py habe ich gefunden, auch wenn ich nicht genau weiß, von wo aus das aufgerufen wird, kann ich mir den weiteren Weg vorstellen.

Gibt es einen Grund, warum das Plugin den Parameter "host" nur als IP akzeptieren soll und nicht als Hostnamen?

[bmx]

Ich vermute aus dem Bauch heraus, das der Parser zwei verschiedene Typen für ein und demselben Parameter nicht verarbeiten kann.

[Morg]

Ich habe mal versucht, den weiteren Weg der Argumente nachzuverfolgen - es landet in lib.connection in der Funktion Client->connect, welche socket.connect aufruft. socket wird - soweit ich das sehen kann - mit import socket eingebunden; die Python-Library socket kann problemlos auch mit Hostnamen umgehen.

Es müsste ja reichen, in der plugin.yaml den Typ (und die Beschreibung) anzupassen.

-> kurzer Test zeigt: es funktioniert.

Daher nochmal meine Frage: spricht etwas dagegen, hier nur IP-Adressen zu akzeptieren? (Die Formatprüfung IPv4 ist zwar schön, aber ob er default auf 127.0.0.1 zurückfällt, was möglicherweise auch falsch ist, oder ob er den Fehler schmeißt, wenn er die Verbindung aufbauen will, ist wahrscheinlich mehr eine Designfrage als eine funktionale...)

[Msinn]

1. Der Parameter heisst host und nicht hostname
2. Hauspi ist ohne folgene Domain kein Name der sicher aufgelöst werden kann.
3. Doku lesen hilft: http://smarthomeng.de/user/plugins_doc/config/knx.html Dort steht explizit IP Adresse.
4. Wenn die Doku verbesserungswürdig ist: #machen

[Morg]

Schön, hier fühlt man sich konstruktiv aufgehoben...

1. Ich habe nirgendwo geschrieben, dass der Parameter "hostname" heißt. Ich habe "host" geschrieben.
2. Ob und wie ein Name aufgelöst werden kann, hängt von der jeweiligen Netzwerkumgebung ab. hauspi ist bei mir problemlos auflösbar, aber das muss woanders ja nicht so sein. knxgw.local wäre genauso möglich, je nach Setup. Und nur weil eine Domain auf den Namen folgt, ist eine falsche Domain auch nicht auflösbar...
3. Doku lesen hilft nicht. http://smarthomeng.de/user/plugins_doc/config/knx.html und http://smarthomeng.de/user/plugins/knx/README.html sind nicht einheitlich. In letztere steht ausdrücklich "hostname".
4. Habe ich schon angeboten, aber...

spricht etwas dagegen, Hostnamen zu erlauben?

Und nur als Hinweis: die Frage impliziert einen konstruktiven Vorschlag; ich würde den sogar umsetzen und die Doku entsprechend anpassen.

Wenn das nicht gewünscht ist, dann sag mir das, dann lasse ich sowas.

[Onkelandy]

Prinzipiell gibt es das "Problem" ja bei einer ganzen Menge an Plugins. Hier wäre sicher nicht schlecht, alles einheitlich hinzubekommen. Gerade auch, was die Benennung der Parameter anlangt, da diese manchmal auch "ip" heißen.

[bmx]

Ein hostname wäre als String ok. Aber eine IP kann zumindest syntaktische geprüft werden. Nun müsste es aber ein Möglichkeit geben sowohl das eine als auch das andere anzugeben und prüfen zu können.
Das ist mit dem aktuellen PR nicht gegeben.

[Morg]

Habe mal noch angepasst; allerdings auf zwei Parameter verteilt: "host" (str) ohne Prüfung und "ip" (ipv4) mit Prüfung. Wenn "host" angegeben ist, wird "ip" ignoriert. Die Priorisierung könnte man sicher ändern...
Mit ungültigen Hostnamen (connection failed) wird - im Code von socket - so umgegangen, wie mit falschen IPs auch.

Offen bleibt eine Prüfung, ob ein Wert ein Hostname oder eine ungültige IP ist. Ich glaube, das ist syntaktisch nicht sauber zu trennen, darum habe ich auch den zweiten Parameter angeführt.

Oder wäre es - um die Kontinuität zu wahren - besser, "host" nach wie vor als IP zu definieren und zB "hostname" als String, der ggf. den Parameter "host" überschreibt? Das wäre ja simpel umzusetzen, ist aber letztlich eine Policy-Entscheidung... (wobei "host" als ip und "hostname" als hostname für mich weniger eingängig wäre als "ip" und "host"...)

[Onkelandy]

Hier sollte eine Entscheidung für alle Plugins fallen, um das zu vereinheitlichen. Problem sehe ich bei der Trennung in 2 Parameter, dass man nicht beide als mandatory definieren kann, aber eines der beiden mandatory sein sollte.

[smai]

Ich würde auch eher bei einem Parameter bleiben, ein zweiter erhöht nur die möglichen Fehlerquellen. Es dürfte auch sehr unüblich sein, mir fällt jedenfalls spontan keine Software ein, wo ich das separat angeben könnte/müsste.

Der Name "Host" scheint mir passend für IP oder Hostname.
Auf die Validierung könnte man mMn auch einfach verzichten. Wenn jemand eine invalide IP eingibt, kommt halt ein Connection Error im Log. Es dürfte sowieso wahrscheinlicher sein, dass jemand sich in einer einzelnen Ziffer der IP vertippt, dann verhält es sich ja auch nicht anders.

[Msinn]

Ein neuer Parameter ip würde für bestehende Nutzer beim Update einen breaking Change darstellen, da sie dann den Parameter ip statt host konfigurieren müssten. Das Finde ich unglücklich.

Die Prüfung der Metadaten kann mit einem kombinierten Parameter für ip und Hostname umhehen. Esgibt folgende Datentypen:

ip. Erlaubt auch hostnamen (minimale Prüfung fürHostnamen). Ganz ohne Prüfung müsste man str als Typ für den Parameter angeben
ipv4 Nur ipv4 Adressen, keine Hostnamen
ip6 Nur ipv6 Adressen, keine Hostnamen.

Es müsste also nur eine Namensauflösung im Plugin ergänzt werden. Neue Parameter. Raucht es nicht.

[Morg]

Das ist doch okay - den Datentyp "ip" hatte ich nicht gefunden.

Namensauflösung braucht es hier nicht, das macht das Modul "socket" von allein. Also schreibe ich nochmal um...

[Msinn]

Das steht hier in der Doku genau über der Beschreibung für ipv4 und ipv6.

[Morg]

Gut zu wissen