Vorschlag für SetExtensionsCancel

[DeeSPe]

Hallo Rudi,

ich habe mich heute eine Weile mit SetExtensionsCancel herumgeschlagen weil es das durch SetExtensions erstellte at bei z.B. on/off-till nicht mit löscht.
Musste auch Andre eine Weile damit nerven. 

Dafür habe ich mal einen kleinen Patch erstellt.
Das dürfte ohne weitere Nebenwirkungen funktionieren, denke ich.
Oder hat es einen tieferen Sinn dass das at nicht gelöscht wird?

Vielleicht könntest Du das bei Gefallen mit einchecken?

Danke im Voraus.

Gruß
Dan

EDIT: Dateianhang entfernt.

[justme1968]

@rudi: wäre es eventuell sinnvoll die -till fälle genau so zu behandeln wie die -for-timer fälle und auch InternalTimer zu verwenden statt dem at?

[rudolfkoenig]

@DeeSPe: tieferen Grund hat es nicht, mir kam es bisher nur nicht im Sinn, dass jemand, der eine Lampe mit on-till einschaltet, sie mit on-for-timer nochmal anders programmiert haben will, und dann darauf besteht, dass die vom on-till eingeplante Ausschaltbefehl automatisch entfernt wird. Ich kann mir Szenarien vorstellen, wo das nicht sinnvoll ist (z.Bsp. Mischung von Handbetrieb mit FHEM-Automatik), und deswegen moechte ich Gegenargumente haben. Gegen dem Patch an sich habe ich nichts.

@andre: on-till ist (bei mir) was laenger laufendes, und ein FHEM-Neustart wuerde ein InternalTimer nicht ueberleben.

[DeeSPe]

Es war aufgefallen weil jemand on-till getestet hatte und danach mit manuellem Schalten eben diese SetExtension abbrechen wollte, was nicht klappte.
Sollte ein manuelles Schalten nicht sowas wie ein Override für alle SetExtensions sein?
Habe es in mein Modul erst einmal so mit aufgenommen dass das at bei SetExtensionsCancel gelöscht wird.

Gruß
Dan

[rudolfkoenig]

Ist halt 'ne Diskussionsfrage: wenn ich Licht draussen bis Abends 11 haben will, dann kommt wer, schaltet es aus, dann einer wieder an, und dann geht das Licht um 11 nicht aus, dann bin ich traurig. Kann man natuerlich auch andere Szenarien kosnstruieren.

[justme1968]

aber das nicht wiederholende at überlebt doch den neustart auch nicht?

[rudolfkoenig]

Sischer doch: in fhem.state

[justme1968]

stimmt. da war noch was

[justme1968]

SetExtensionsCancel hat noch ein kleines problem. aktuell muss man den aufruf von SetExtensionsCancel mit einem flag etwa so schützen:http://www.fhemwiki.de/wiki/DevelopmentModuleAPI#SetExtensionsCancel. der grund dafür ist folgender: da die SetExtension selber auch die SetFn eines moduls verwenden wird ohne dieses flag SetExtensionsCancel auch dann aufgerufen wenn .

beispiel:
- in einem modul wird SetExtensionsCancel bei den normalen selbst implementierten set funktionen verwendet
- ein anwender verwendet mit diesem modul on-for-timer
- das modul ruft SetExtentsions auf da es on-for-timer nicht selber implementiert
- die SetExtentsions füllen die TIMED_OnOff daten, rufen die SetFn des moduls mit on auf und starten den timer
- beim diesem on aufruf ruft das modul natürlich SetExtensionsCancel auf was TIMED_OnOff löscht
- der anwender verwendet set on und das modul ruft SetExtensionsCancel
- die TIMED_OnOff daten sind aber durch das vorherige falsche SetExtensionsCancel schon weg und SetExtensionsCancel tut nichts
- der timer läuft weiter und trotz in geht das device nach einer weile aus

die sicherste lösung wäre vermutlich das flag nicht jeweils in den SetFn der modul zu haben sondern direkt in den SetExtensions.

unabhängig davon: sollen wir das löschen des -till timers nicht auch noch in SetExtensionsCancel packen?

der vorgeschlagene patch macht beides:

Code Auswählen Erweitern

Index: SetExtensions.pm
===================================================================
--- SetExtensions.pm (revision 12682)
+++ SetExtensions.pm (working copy)
@@ -15,6 +15,7 @@
   $hash = $defs{$hash} if( ref($hash) ne 'HASH' );

   return undef if( !$hash );
+  return undef if( $hash->{InSetExtensions} );
   my $name = $hash->{NAME};

   return undef if( !$hash->{TIMED_OnOff} );
@@ -24,6 +25,9 @@

   delete $hash->{TIMED_OnOff};

+  my $at = $name ."_till";
+  CommandDelete(undef, $at) if($defs{$at});
+
   return undef;
}

@@ -30,6 +34,18 @@
sub
SetExtensions($$@)
{
+  my ($hash) = @_;
+
+  $hash->{InSetExtensions} = 1;
+  my $ret = SetExtensions_(@_);
+  delete( $hash->{InSetExtensions} );
+
+  return $ret;
+}
+
+sub
+SetExtensions_(@)
+{
   my ($hash, $list, $name, $cmd, @a) = @_;

   return "Unknown argument $cmd, choose one of " if(!$list);


[rudolfkoenig]

- in einem modul wird SetExtensionsCancel bei den normalen selbst implementierten set funktionen verwendet

Ich glaube das ist falsch. Richtig waere mAn, wenn SetExtensions selbst Cancel aufruft, sobald es um mehr als die Ausgabe der Hilfetexte geht. Cancel muss die intervals-ats und den blink-timer auch entfernen. Ich brauche dafuer nicht unbedingt einen Patch, aber eine Meinung, ob ich was uebersehe.

[justme1968]

Ich glaube das ist falsch. Richtig waere mAn, wenn SetExtensions selbst Cancel aufruft,

ich glaube nicht.

SetExtensions ruft SetExtensionsCancel jetzt schon in dem fall auf das z.b. zwei mal hintereinander on-for-timer aufgerufen wird. das ist richtig. aber ein anderer fall.

wenn ein modul ein set (wie z.b. set on) selber behandelt dann wird SetExtensions doch garnicht aufgerufen und bekommt nichts mit. SetExtensions kommt erst dann ins spiel wenn ein set (wie z.b. on-for-timer) nicht im modul direkt behandelt wird.


ja. die anderen fehlenden ats und timer müssen auch noch mit ins SetExtensionsCancel.

[Markus Bloch]

wenn ein modul ein set (wie z.b. set on) selber behandelt dann wird SetExtensions doch garnicht aufgerufen und bekommt nichts mit. SetExtensions kommt erst dann ins spiel wenn ein set (wie z.b. on-for-timer) nicht im modul direkt behandelt wird.

So verstehe ich das auch. Daher müsste meiner Meinung nach in jedem Modul, welches SetExtensions verwendet zu Beginn in der SetFn ein SetExtensionsCancel() aufgerufen werden, falls ein reines on/off ausgeführt wird.

Gruß
Markus

[justme1968]

genau.

damit das aber problemlos geht muss man mit dem flag arbeiten oder das flag in die SetExtensions einbauen.

[rudolfkoenig]

Danke fuer die Hinweise.
Flag in SetExtensions reicht nicht, siehe blink oder intervals.

Ich habe SE_DoSet eingefuehrt, der den Flag setzt, SetExtensionCancel auch fuer die anderen Parameter implementiert, blink umgebaut, damit es nur einen Timer verwendet, und SetExtensionsCancel in den von mir betreuten Modulen aufgerufen.