[erledigt] OWDevice feature request

[erwin]

Ich hab eine Bitte zum OWDevice Modul:

könnte man ein disable Attr. supporten?
Background: Ich habe etliche OW-devices (nicht mit Bus-Power), die "saisonal" nicht in Betrieb sind.. (Stichwort: z.B.: Swimmingpool, Heizung) und daher bei jedem poll unschöne Log messages produzieren...
Es sollte ja reichen, wenn die devices nicht mehr gepollt werden.
Alternativ set <device> poll 0, das produziert dzt. 5-10 Log msg/sec.
Es ist mühsam und fehleranfällig, jedesmal die devices aus der config zu löschen und wieder neu anzulegen...
Mit einem disable attr. könnte ich das automatisieren (und WAF freundlich machen)
PS: in der OWdevice_Notify Fn sind schon Ansätze für disable-attr drin...
l.g. erwin

edit: auf erledigt gesetzt - DANKE!

[eldrik]

eine kurzfristige Abhilfe zur Verringerung der Log Meldungen würde das erhöhen des Abfrageintervals auf z.B 10000 bringen.

Dazu ein paar passende notifies zum bequemen setzen der Werte.

Greetz
Eldrik

[Dr. Boris Neubert]

Hallo Erwin,

kannst Du bitte das beigefügte schnell zusammengehackte und ungetestete überarbeitete Modul  ausprobieren? Ich habe ihm das gewünschte disable-Attribut spendiert.

Grüße
Boris

[erwin]

Danke Boris,
...Test läuft, bisher ist mir folgendes aufgefallen (verbose 5):
1) disable 1 funktioniert
2) ein defmod (mit disable=1) bringt folgendes:

Code Auswählen Erweitern

2021.08.23 11:31:11.368 4: DS18S20_30BDDD010800: I/O device is myOWServer
2021.08.23 11:31:11.369 5: DS18S20_30BDDD010800: polling every 60 seconds
2021.08.23 11:31:11.370 5: DS18S20_30BDDD010800: interfaces: temperature
2021.08.23 11:31:11.371 5: DS18S20_30BDDD010800: getters: address crc8 family id locator power r_address r_id r_locator temperature temphigh templow type
2021.08.23 11:31:11.371 5: DS18S20_30BDDD010800: setters: temphigh templow
2021.08.23 11:31:11.372 5: DS18S20_30BDDD010800: polls: temperature
2021.08.23 11:31:11.372 5: DS18S20_30BDDD010800: state: temperature
2021.08.23 11:31:11.373 5: DS18S20_30BDDD010800: alerting: 1
2021.08.23 11:31:11.610 3: DS18S20_30BDDD010800: reading type did not return a value
...wobei mich nur die letzte Zeile stört!
3) ein disable 0 started das polling offensichtlich nicht.
4) löschen disable attr - kein polling - erst ein defmod <device> hilft, wobei dann wieder folgender Log kommt:

Code Auswählen Erweitern

2021.08.23 11:42:43.026 3: DS18S20_30BDDD010800: reading type did not return a value
2021.08.23 11:42:43.094 3: DS18S20_30BDDD010800: reading temperature did not return a value
2021.08.23 11:43:43.086 3: DS18S20_30BDDD010800: reading temperature did not return a value
...reading type did.. kommt allerdings nicht bei jedem poll, sondern nur beim defmod!

danke nochmal für deinen support, es ist jedoch nicht wirklich dringend. Die devices funktionieren ja problemlos.
l.g. erwin

[Dr. Boris Neubert]

Hallo Erwin,

Danke für Deinen Test. Wie gesagt, es war ein schneller Hack. Mangels aktivem Equipment nach Umzug kann ich selber nicht weiter testen und entwickeln, schon gar nicht diese Randfälle.

Der Hack unterbindet einfach das Lesen vom Bus, entsprechend kommen beim Öffnen der Verbindung vom Gerät keine brauchbaren Werte zum Typ, was zur Fehlermeldung führt. Das ganze braucht bzgl. der Zustände eine dokumentierte Überlegung, um es vollständig richtig hinzubekommen. Das kann ich nicht leisten.

Wenn ich weiter nichts Negatives höre, checke ich diese Fassung die Tage mal ein. Besser als nix wär das dann hoffentlich.

Viele Grüße
Boris

[erwin]

Hallo Boris,

ich hab jetzt noch 2 Änderungen versucht und damit läuft alles wie es soll:
in OWDevice_UpdateValues:

Code Auswählen Erweitern

#     return if  AttrVal($name, 'disable', 0);
      return if (AttrVal($name, 'disable', 0) == 1);
und in OWDevice_Attr:

Code Auswählen Erweitern

        elsif($attrName eq "disable" && ($cmd eq 'del' || $attrVal == 0)) { # restart poll after disable
                RemoveInternalTimer($hash);
                InternalTimer(int(gettimeofday()) + $hash->{fhem}{interval}, "OWDevice_UpdateValues", $hash, 0)
                   if(defined($hash->{fhem}{interval}));
        }
ans ende der sub.
Danke nochmal für den support!
l.g. erwin

[Dr. Boris Neubert]

Danke, Erwin.

Kannst Du bitte die Datei hier anhängen. Dann checke ich sie ein.

Viele Grüße
Boris

[erwin]

Hallo Boris,
bitte schön, die Datei - und danke!
l.g. erwin

[eldrik]

Moin zusammen,

ich habe die Ergänzungen auch einmal für meine 1Wire Instanzen eingepflegt und bisher keine Probleme festgestellt.

Für Devices die ich nicht Pollen muss, da sie rein zum schalten verwendet werden (DS2408, DS2413 etc.) oder es sich um 1Wire RGBW Controller handelt, hatte ich bisher Intervalle von 100000 eingetragen, für den Busverkehr kann es aber ja nur von Vorteil sein, diese in Gänze aus dem Polling zu nehmen.

Danke für die Anregung und Erweiterung.

Greetz
Eldrik

[Dr. Boris Neubert]

Überarbeitete Version eingecheckt, wird ab morgen früh per Update verteilt
Boris

[erwin]

Sorry Boris,
ich hab jetzt noch einen kleinen Bug entdeckt - (..meine Änderungen...) während startup kommt:

Code Auswählen Erweitern

PERL WARNING: Use of uninitialized value in numeric eq (==) at ./FHEM/11_OWDevice.pm line 725
Kannst du bitte die Zeile 725 ändern:

Code Auswählen Erweitern

#  return if($attr{$name} && $attr{$name}{disable} == 1);
  return if(ReadingsVal($name,'disable',1) == 1);
danke Erwin

[Dr. Boris Neubert]

Klar!

Kannst Du mir bitte wieder Deine getestete Version anhängen? Das minimiert das Fehlerpotential bei mir 

Viele Grüße
Boris

[Beta-User]

Zwischenfragen:
- warum "1" als default?
- warum ReadingsVal() bei nummerischem "Zwangsergebnis"? (=>ReadingsNum())
- warum überhaupt nummerisch?
- warum nicht gleich IsDisabled() aus fhem.pl? Das würde ggf. direkt auch "disabledForIntervals" erlauben, was sich aber mit dem Löschen des Timers beißt (solange das Attribut aber nicht zugelassen ist, dürfte es auch egal sein...). Jedenfalls wäre es dann von den syntaktischen Anforderungen an den Attributwert 1:1 mit anderen Modulen vergleichbar...
(- warum die Klammern bei "nachgelagertem if"? Da kann man die genausogut weglassen)

[erwin]

Hi Beta-User!

ReadingsVal vs. ReadingsNum -> du hast natürlich recht!

- warum überhaupt nummerisch?
- warum nicht gleich IsDisabled()

..disabled attr hat 0,1 als erlaubte werte, minimale Änderung an bestehendem code - der gehört Boris!
Das heutige Problem war ausschließlich im startup schlagend. (attr noch nicht verfügbar...)
l.g. erwin

edit: Readings(Val|Num) ist natürlich Blödsinn, mein Fehler, muss AttrVal heissen... Danke fürs anstupsen!

[Beta-User]

disabled attr hat 0,1 als erlaubte werte, minimale Änderung an bestehendem code - der gehört Boris!

Jein: Das "Problem" ist, dass manche "Schlauberger" die cfg editieren oder ggf. über die Kommandozeile Werte setzen. Wenn dann der Modulcode nicht eine Validierung durchführt (?), kommt "Käse" raus, der schwer zu entdecken ist. Die Validierung kann man sich schenken, wenn man einfach nur prüft, ob "wahr" (also nicht 0 oder undef).

IsDisabled (an der richtigen Stelle, s.U.) hat den weiteren Vorteil, dass man "inactive" als state setzen könnte. (k.A., ob das zum sonstigen Modulkonzept paßt, nur eine Anmerkung).

Das heutige Problem war ausschließlich im startup schlagend. (attr noch nicht verfügbar...)

Dann sollte man m.E. die eigentliche Initialisierung nach "$init_done" erledigen (InternalTimer-Aufruf), weil mAn. zu diesem Zeitpunkt auch kein ReadingsVal ein irgendwie sinnvolles Ergebnis bringt (statefile wird erst nach der cfg geladen).

[erwin]

Hi Boris,
der korrigierte code im Attachment.
@Beta-User, klingt alles richtig, allerdings würde ich das Boris überlassen.
l.g. erwin

[Beta-User]

@Beta-User, klingt alles richtig, allerdings würde ich das Boris überlassen.

"prinzipiell" ist das schon die richtige Haltung, aber:

Mangels aktivem Equipment nach Umzug kann ich selber nicht weiter testen und entwickeln, schon gar nicht diese Randfälle.

Zum einen: die "disabled"-Reading-Auswertung _kann_ m.E. so nicht richtig sein, wenn sie vor $init_done erfolgt (=> neuer bug!)
Zum anderen: du hast gute Gründe, das selbst zu fixen, denn Boris ist auf passende Zuarbeit angewiesen . Falls du MySensors im Einsatz hast: z.B. 00_MYSENSORS.pm hat so einen verzögerten Start implementiert (man kann die statischen Teile auch gleich machen und nur den Rest nach "Start()" verlegen). Ist kein Hexenwerk.

[Beta-User]

Hallo zusammen,

nachdem ich mir den Code mal angesehen habe...

Die Initialisierung wird bisher via notifyFn gemacht, und dann aussortiert, was nicht relevant ist. Das ist hier nur die Erstinitialisierung und RereadCfg. Im laufenden Betrieb also mAn. völlig unnötiger overhead. Würde vorschlagen, das auf Timer-Basierte Initialisierung umzustellen, Code-Vorschlag (außer dem prinzipiellen Laden ungetestet!) anbei.

@erwin: vielleicht magst du testen?

[erwin]

Hi,
ich hab getestet, Funktion ok, alle devices pollen, disable/enable funktioniert, shutdown restart unauffällig!
Herzlichen Dank für deinen Input!
Ich möchte Boris bitten, deine Version einzuchecken.
l.g. erwin

[Beta-User]

Danke für die Rückmeldung.

Vielleicht magst du noch eine "aufgeräumte" Version erstellen? Ich hatte teils nur auskommentiert, was überflüssig wurde und auch die Funktion selbst nicht umbenannt. Das könnte/sollte man m.E. noch machen (hüftgeschossener Vorschlag für sowas wäre .*_Start oder .*_firstInit)

[erwin]

Hi Beta-User,
ich hab ein wenig aufgeräumt, die auskommentierten Zeilen im Init hab ich bewusst drin gelassen - damit man gleich sieht, dass es keine NotifyFn gibt.
Kurz nochmal getestet - funktioniert!
l.g. erwin

[Beta-User]

Thx. Habe grade noch gesehen, dass da NOTIFYDEF direkt (In Initialize) gesetzt wird, die Zeile sollte auch noch raus; sorry for inconvenience...

(@erwin: Falls du betr. der neu übernommenen Module Bedarf in Richtung der Initialisierung etc. hast oder sonst eine Frage: ggf. melden!).Edit: Kurzer Blick in den Code genügt: offenbar kein support erforderlich...)

EDIT 2, etwas intensiver nachgesehen: Warum hat 10_KNX.pm eigentlich überhaupt eine notifyFn?

[erwin]

hi,
NOTIFYDEF auch noch gelöscht, sollte passen...

10_KNX.pm NotifyFN - war/ist vorgesehen für delayed start und evtl. auch falls ich Attribute wärend des defines brauchen würde... - macht dzt. nix (ausser cpu-verschwenden  ).
Ich werde beim nächsten update ein frühes return einbauen...
PS: das komische IsDisabled($ownName) == 1 kommt daher, weil es KNX devices git, die als state disabled / enabled setzen. Bin damals darüber gestolpert...
l.g.erwin

[Beta-User]

10_KNX.pm NotifyFN - war/ist vorgesehen für delayed start

nach der Diskussion hier nehme ich an, dass "war" die korrekte aktuelle Lesart ist...?
Initialisierung macht man besser Timer-basiert. (Zwischenzeitlich, früher war das anders in der Doku gestanden bzw. die ist in dem Punkt ggf. nicht mehr aktuell)

Ich werde beim nächsten update ein frühes return einbauen...

Bzgl. "return": Bei der kurzen Hineinsicht in KNX bin ich auch über die "Variable" "$UNDEF" gestolpert. Bin zwar kein Perl-Experte, aber alle meine bisherigen Erfahrungen mit dem Thema gehen in die Richtung, dass man (in eigenem Code) immer entweder was zurückgibt, oder direkt einen Strichpunkt nach dem return setzt .

PS: das komische IsDisabled($ownName) == 1 kommt daher, weil es KNX devices git, die als state disabled / enabled setzen. Bin damals darüber gestolpert...

Das Problem (bzw. was Ähnliches) hatte ich in RandomTimer auch; habe das irgendwie anders gelöst, mir scheint bisher entgangen zu sein, dass es auch andere "disabled-Level" gibt ... Wieder was gelernt!

[Dr. Boris Neubert]

Erstmal herzlichen Dank an Erwin und Beta-User für Eure Arbeit und dass Ihr Euch der Weiterentwicklung angenommen habt.


Da die Diskussion noch andauert, warte ich noch bis morgen oder übermorgen ab, bis ich mir das überarbeitete Modul auch ansehe und dann einchecke, wenn mir nichts mehr auffällt!

Liebe Grüße
Boris

[Beta-User]

Die weitere Diskussion ist/war hier eigentlich OT, sorry ...

(Wir (erwin und ich) hatten nur bzgl. Doku-Checks und 10_KNX.pm gestern noch eine Irritation, von daher wollte ich nur grundsätzliche Bereitschaft signalisieren, manche Punkte zu klären, die sich einem als "neuer" Maintainer ggf. nicht direkt erschließen, weil sie (noch) nicht unbedingt gut dokumentiert sind, und da wir hier jetzt schon mal dabei waren...).

[erwin]

Hi,
ich hab ja das Modul geerbt....
bez. return $UNDEF:
kommt aus der developer docu: etliche externe Funktionen wollten (vor ca. 1 Jahr) ein return undef; . Nachdem aber PBP darüber gemeckert hat, hab ich nachgegeben und irgendwo my $UNDEF = undef; definiert und ab da verwendet.
Ob die developer docu in dem Punkt (noch) korrekt ist, kann ich nicht sagen.
l.g. erwin
PS: Bin immer offen für reviews! (ich werd mir die 00_MYSENSORS.pm anschauen..)

[Beta-User]

  Das Unbehagen, überkommene "Anforderungen" über Bord zu werfen kann ich gut nachvollziehen...

MYSENSORS.* ist aber (wie MQTT-Classic) etwas "speziell", weil es - obwohl zweistufig - keine ParseFn kennt. (Das ist - im Sinne einer strukturierten Eventverarbeitung - nicht gut, aber schwer zu ändern!)

Fragen aber gerne - wo auch immer. Notfalls hier weiter OT, obwohl es Boris wohl davon abhält, das einzuchecken ...
(Ich habe bewußt weggesehen, was elsif-Kaskaden oder return undef; angeht, aber das jetzt hier auch noch abzuarbeiten, war/ist nicht mein Anliegen!)

@KNX.pm:
Die doku bzgl. "return undef;" dürfte auch schlicht dem bisher üblichen "common style" entsprechen, aber funktional kann man nach meiner Erfahrung auch ohne weiteres PBP-"pur" fahren. Soweit ich das im Kopf habe: Nur, wenn die aufrufende Funktion ein Array erwartet, ist "return undef;" funktional was anderes als "return;" - und das sind die allerwenigsten Funktionen...
Schon gleich, was die Frage der Aufrufe "von extern" angeht - du hast das eingepackt, außer "Initalize" ist keine Funktion mehr unter dem alten Namen aufrufbar, und wo du modulintern selbst ein Array erwartest, müsstest du recht schnell klären können.

Also "hau es weg", teste es eine Woche, und dann ab mit "return pur" ins svn.
Just my2ct.

[Dr. Boris Neubert]

eingecheckt