Verbesserungsvorschlag: 'column' - speziellen Raum default einführen

[Manul]

Ich bin neu bei FHEM, aber ich denke, daß ich in den meisten Räumen, die ich anlege, die gleichen Gruppen im gleichen Layout anzeigen lassen möchte. Wäre es nicht sinnvoll, im Attribut 'column' einen Raum 'default' angeben zu können, dessen Layout für alle Räume verwendet wird, für die kein eigenes explizit angegeben ist? Oder gibt's dafür schon eine andere Möglichkeit?

[rudolfkoenig]

Findet das sonst noch jemand sinnvoll?

[KernSani]

Findet das sonst noch jemand sinnvoll?

Ja, wenn zusätzlich eine wildcard möglich wäre, also sowas;

Code Auswählen Erweitern

attr WEB column ZimmerA:Wetter,Licht,TV,.* .*:Licht,Rollladen,Heizung,.*

Edit: Regex muss nicht sein, wäre aber schön

[Manul]

Mit regex wäre natürlich noch besser - und vermutlich einfacher umzusetzen. M.E. müsste es das hier schon tun (erfolgreich, aber nicht gründlich getestet) - zumindest für den Raumnamen:

Code Auswählen Erweitern


--- 01_FHEMWEB.pm.dist  2017-04-29 19:40:14.024081928 +0200
+++ 01_FHEMWEB.pm       2017-05-02 11:01:04.280064895 +0200
@@ -1851,7 +1851,7 @@
   foreach my $roomgroup (split("[ \t\r\n]+", AttrVal($FW_wname,"column",""))) {
     my ($room, $groupcolumn)=split(":",$roomgroup,2);
     $room =~ s/%20/ /g; # Space
-    next if(!defined($groupcolumn) || $room ne $FW_room);
+    next if(!defined($groupcolumn) || $FW_room !~ $room);
     $colNo = 1;
     foreach my $groups (split(/\|/,$groupcolumn)) {
       my $lineNo = 1;
@@ -1861,6 +1861,7 @@
       }
       $colNo++;
     }
+    last;
   }
   return (\%columns, $colNo);
}


[rudolfkoenig]

Hab den Patch uebernommen, Doku ergaenzt und eingecheckt.
Regexp fuer die Gruppen ist leider nicht so trivial.

[Manul]

Hab den Patch uebernommen, Doku ergaenzt und eingecheckt.

Super, danke! Ergänzung der Doku hätte ich auch noch übernommen, wollte nur erst mal abwarten, ob's überhaupt Anklang findet.

Regexp fuer die Gruppen ist leider nicht so trivial.

Ist mir auch aufgefallen. Ich hab noch ein, zwei Ideen, ich schau mal, ob's was wird. Gute Nacht!

[Manul]

Vorschlag:

Code Auswählen Erweitern

--- 01_FHEMWEB.pm.rooms 2017-05-03 13:25:33.722296990 +0200
+++ 01_FHEMWEB.pm 2017-05-03 13:28:39.630878128 +0200
@@ -1794,38 +1794,46 @@
   my $nameDisplay = AttrVal($FW_wname,"nameDisplay",undef);

   my ($columns, $maxc) = FW_parseColumns();
-  FW_pO "<tr class=\"column\">" if($maxc != -1);
-  for(my $col=1; $col < ($maxc==-1 ? 2 : $maxc); $col++) {
-    FW_pO "<td><table class=\"column tblcol_$col\">" if($maxc != -1);
-
-    # iterate over the distinct groups 
-    foreach my $g (sort { $maxc==-1 ?
-                    $a cmp $b :
-                    ($columns->{$a} ? $columns->{$a}->[0] : 99) <=>
-                    ($columns->{$b} ? $columns->{$b}->[0] : 99) } keys %group) {
-
-      next if($maxc != -1 && (!$columns->{$g} || $columns->{$g}->[1] != $col));
-
-      #################
-      # Check if there is a device of this type in the room
-      FW_pO "\n<tr><td><div class=\"devType\">$g</div></td></tr>";
-      FW_pO "<tr><td>";
-      FW_pO "<table class=\"block wide\" id=\"TYPE_$g\">";
-
-      foreach my $d (sort { $sortIndex{$a} cmp $sortIndex{$b} }
-                     keys %{$group{$g}}) {
-        my $type = $defs{$d}{TYPE};
-        $extPage{group} = $g;
-
-        FW_makeDeviceLine($d,$row,\%extPage,$nameDisplay,\%usuallyAtEnd);
-        $row++;
+  # keep track of which groups were already handled
+  my %handled;
+  FW_pO "<tr class=\"column\">" if($maxc != 1);
+  for(my $col=0; $col < $maxc; $col++) {
+    FW_pO "<td><table class=\"column tblcol_$col\">" if($maxc != 1);
+
+    # $columns is an array of arrays, each inner array represents one column of room names/patterns
+    foreach my $pattern (@{$columns->[$col]}) {
+     
+      # make pattern match whole string to be consistent with pre-regexp behaviour
+      $pattern = "^".$pattern."\$";
+
+      # iterate over the distinct groups 
+      foreach my $g (keys %group) {
+
+        next if ($g !~ $pattern or exists $handled{$g});
+
+        $handled{$g}=undef;
+
+        #################
+        # Check if there is a device of this type in the room
+        FW_pO "\n<tr><td><div class=\"devType\">$g</div></td></tr>";
+        FW_pO "<tr><td>";
+        FW_pO "<table class=\"block wide\" id=\"TYPE_$g\">";
+
+        foreach my $d (sort { $sortIndex{$a} cmp $sortIndex{$b} }
+                       keys %{$group{$g}}) {
+          my $type = $defs{$d}{TYPE};
+          $extPage{group} = $g;
+
+          FW_makeDeviceLine($d,$row,\%extPage,$nameDisplay,\%usuallyAtEnd);
+          $row++;
+        }
+        FW_pO "</table>";
+        FW_pO "</td></tr>";
       }
-      FW_pO "</table>";
-      FW_pO "</td></tr>";
     }
-    FW_pO "</table></td>" if($maxc != -1); # Column
+    FW_pO "</table></td>" if($maxc != 1); # Column
   }
-  FW_pO "</tr>" if($maxc != -1);
+  FW_pO "</tr>" if($maxc != 1);

   FW_pO "</table><br>";

@@ -1845,25 +1853,30 @@
sub
FW_parseColumns()
{
-  my %columns;
-  my $colNo = -1;
+  my @columns = ([".*"]);
+  my $colNo = 1;

   foreach my $roomgroup (split("[ \t\r\n]+", AttrVal($FW_wname,"column",""))) {
     my ($room, $groupcolumn)=split(":",$roomgroup,2);
     $room =~ s/%20/ /g; # Space
     next if(!defined($groupcolumn) || $FW_room !~ $room);
-    $colNo = 1;
+    @columns = ();
+    $colNo = 0;
     foreach my $groups (split(/\|/,$groupcolumn)) {
+      # create one array of room names/patterns for each column
+      my @column = ();
       my $lineNo = 1;
       foreach my $group (split(",",$groups)) {
         $group =~ s/%20/ /g; # Forum #33612
-        $columns{$group} = [$lineNo++, $colNo]; # Forum #23212
+        push @column, $group;
       }
+      # add the column array as element to array columns
+      push @columns, [ @column ];
       $colNo++;
     }
     last;
   }
-  return (\%columns, $colNo);
+  return (\@columns, $colNo);
}

Bei der Gelegenheit ist mir aufgefallen, daß in FW_showRoom bei Anbruch einer neuen Spalte der rowcounter nicht auf 1 zurückgesetzt wird - ist das Absicht? Ich hab's jetzt mal so belassen.

[rudolfkoenig]

Danke fuer den Vorschlag, bin aber unsicher, ob ich es einbaue: es bedeutet eine zusaetzliche Schleife (d.h. langsamer fuer alle), etliches an Code Aenderung (d.h. potentiellen Aerger/Bugreports), und weiss nicht, ob es eine relevante Anzahl an Anwendern gibt, d.h. ob es sich lohnt, ich vermute nein.

in FW_showRoom bei Anbruch einer neuen Spalte der rowcounter nicht auf 1 zurückgesetzt wird

Nein, allerdings hatte ich bisher auch keine Beschwerde deswegen . Ich glaube es interessiert keinen, weil (entgegen meiner Erwartung), niemand diese Hilfen beim Styling braucht.

[Manul]

Danke fuer den Vorschlag, bin aber unsicher, ob ich es einbaue: es bedeutet eine zusaetzliche Schleife (d.h. langsamer fuer alle)

Stimmt natürlich, sehe ich aber bis jetzt auch keinen Weg drumrum, falls das Feature gewünscht wird.

etliches an Code Aenderung (d.h. potentiellen Aerger/Bugreports)

Stimmt auch, wobei ich ca. 25 Zeilen Änderung noch für einigermaßen überschaubar halte.

und weiss nicht, ob es eine relevante Anzahl an Anwendern gibt, d.h. ob es sich lohnt, ich vermute nein.

Das kann ich nicht beurteilen - mir persönlich reicht derzeit die regex für den Raumnamen - und falls ich sie für die Gruppe noch brauch, weiß ich ja jetzt, wie ich sie einbauen kann.

[Manul]

sehe ich aber bis jetzt auch keinen Weg drumrum, falls das Feature gewünscht wird.

Da war ich gestern möglicherweise etwas voreilig. Wie wär's denn hiermit?

Code Auswählen Erweitern

--- 01_FHEMWEB.pm.rooms 2017-05-03 13:25:33.722296990 +0200
+++ 01_FHEMWEB.pm       2017-05-04 12:06:40.079566411 +0200
@@ -33,7 +33,7 @@
sub FW_pH(@);
sub FW_pHPlain(@);
sub FW_pO(@);
-sub FW_parseColumns();
+sub FW_parseColumns(%);
sub FW_readIcons($);
sub FW_readIconsFrom($$);
sub FW_returnFileAsStream($$$$$);
@@ -1793,7 +1793,7 @@
   my %extPage = ();
   my $nameDisplay = AttrVal($FW_wname,"nameDisplay",undef);

-  my ($columns, $maxc) = FW_parseColumns();
+  my ($columns, $maxc) = FW_parseColumns(%group);
   FW_pO "<tr class=\"column\">" if($maxc != -1);
   for(my $col=1; $col < ($maxc==-1 ? 2 : $maxc); $col++) {
     FW_pO "<td><table class=\"column tblcol_$col\">" if($maxc != -1);
@@ -1845,6 +1845,7 @@
sub
FW_parseColumns()
{
+  my (%group) = @_;
   my %columns;
   my $colNo = -1;

@@ -1853,11 +1854,18 @@
     $room =~ s/%20/ /g; # Space
     next if(!defined($groupcolumn) || $FW_room !~ $room);
     $colNo = 1;
+    my @grouplist = keys %group;
+    my %handled;
     foreach my $groups (split(/\|/,$groupcolumn)) {
       my $lineNo = 1;
       foreach my $group (split(",",$groups)) {
         $group =~ s/%20/ /g; # Forum #33612
-        $columns{$group} = [$lineNo++, $colNo]; # Forum #23212
+        $group = "^".$group."\$";
+        foreach my $g (grep /$group/ ,@grouplist) {
+          next if exists $handled{$g};
+          $handled{$g}=undef;
+          $columns{$g} = [$lineNo++, $colNo]; # Forum #23212
+        }
       }
       $colNo++;
     }


Auf die Bemerkung mit der zusätzlichen Schleife hin habe ich mir den Code übrigens nochmal in Hinblick auf Effizienz angeschaut und hätte 2 Vorschläge:

- Als minimalen Eingriff würde ich das sort der keys von %group vor die Schleife über die Spalten ziehen.
- Wenn FW_parseColumns statt eines hashes einen array der Form [ [ Gruppe, Spalte ] [ Gruppe, Spalte ] ... ] zurückgeben würde, könnte man sich das anschließende sort und das Iterieren über alle Gruppen sparen und müsste nur noch einmal über den array iterieren.

Ich glaube es interessiert keinen, weil (entgegen meiner Erwartung), niemand diese Hilfen beim Styling braucht.

Das zeigt zumindest, daß die Einschätzung, wie attraktiv ein Feature ist, nicht ganz einfach zu sein scheint.

[rudolfkoenig]

Hab dein Diff mit leichten Aenderungen eingebaut, etwas getestet und eingecheckt.

[Manul]

Prima, wird ja dann sicher demnächst eintrudeln. Bei der Gelegenheit ist mir noch ein kleiner Tippfehler in der aktualisierten englischen commandref aufgefallen: "regeular" statt "regular" expression.

[KernSani]

Cool. Dann wsrte ich mal bis morgen mit dem nächsten update

[KernSani]

Cool. Dann wsrte ich mal bis morgen mit dem nächsten update

Wurde dann doch etwas später mit dem update, aber: Nach einem ersten schnellen Test scheint es genau so zu funktionieren, wie ich mir das gewünscht habe. Danke!

Edit: Eine Kleinigkeit: Die als Regex angegebenen Groups scheinen unsortiert zu kommen... Kriegen wir da noch eine alphabetische Sortierung (wie sonst auch) hin?

[Manul]

Ich hätte gedacht, daß das hier:

Code Auswählen Erweitern

--- 01_FHEMWEB.pm.dist 2017-05-07 17:14:10.950637642 +0200
+++ 01_FHEMWEB.pm 2017-05-07 17:29:43.263537930 +0200
@@ -1858,7 +1858,7 @@
     $room =~ s/%20/ /g; # Space
     next if(!defined($groupcolumn) || $FW_room !~ m/$room/);
     $colNo = 1;
-    my @grouplist = keys %$aGroup;
+    my @grouplist = sort keys %$aGroup;
     my %handled;

     foreach my $groups (split(/\|/,$groupcolumn)) {


ausreichen müsste, scheint es aber nach meinen Tests nicht zu tun. Ich denke nochmal drüber nach, aber vielleicht sieht @rudolfkoenig den Fehler ja auf Anhieb.

@rudolfkoenig: Bzgl. Deiner Änderungen: Hast Du mal einen link zu einer guten Erklärung des match-Operators? Ich hab mir die offizielle Doku dazu durchgelesen, habe aber den Unterschied zwischen "$var !~ $pattern" und $var !~ m/$pattern/" nicht verstanden.

edit: P.S. Wäre es vielleicht sinnvoll, für column den Seiteneditor zu öffnen? In der Eingabezeile wird's doch arg eng.

[rudolfkoenig]

Hast Du mal einen link zu einer guten Erklärung des match-Operators?

Gut ist relativ, "perldoc perlre" ist immerhin die offizielle Doku.

habe aber den Unterschied zwischen "$var !~ $pattern" und $var !~ m/$pattern/" nicht verstanden.

Die Schreibweise ohne m ist mir auch aufgefallen, ich verwende es aber nicht. Ich dachte es waere mit m equivalent.