Peer Review von Richards Gewurschtel

Begonnen von RichardCZ, 31 März 2020, 22:17:53

Vorheriges Thema - Nächstes Thema

RichardCZ

# compute the list of defined logical modules for a physical module
sub
computeClientArray($$)
{
  my ($hash, $module) = @_;
  my @a = ();
  my @mRe = split(":", $hash->{Clients} ? $hash->{Clients}:$module->{Clients});

  foreach my $m (sort { $modules{$a}{ORDER}.$a cmp $modules{$b}{ORDER}.$b }
                  grep { defined($modules{$_}{ORDER}) } keys %modules) {
    foreach my $re (@mRe) {
      if($m =~ m/^$re$/) {
        push @a, $m if($modules{$m}{Match});
        last;
      }
    }
  }

  $hash->{".clientArray"} = \@a;
  return \@a;
}


* foreach -> for, @a -> @client_array, prototypen weg ein wenig umsortieren und ich glaube, es ist:

sub computeClientArray {
    my $hash   = shift;
    my $module = shift;

    my @client_array = ();
    my @mRe          = split ":", $hash->{Clients} ? $hash->{Clients}:$module->{Clients};

    for my $m (sort { $modules{$a}{ORDER}.$a cmp $modules{$b}{ORDER}.$b }
                   grep { defined($modules{$_}{ORDER}) } keys %modules) {

      CCA_INNER1:
        for my $re (@mRe) {
            next CCA_INNER1 if ($m !~ m/^$re$/);

            push @client_array, $m if ($modules{$m}{Match});

            last CCA_INNER1;
        }
    }

    $hash->{".clientArray"} = \@client_array;

    return \@client_array;
}


So. Das ist wohl noch nicht das Ende der Fahnenstange, denn jetzt erst kapiere ich ein wenig mehr was das Ding macht.

Die Inner-Loop tut vermutlich

         if (first { $m =~ m/^$_$/ } @mRe) {
             push @client_array, $m if ($modules{$m}{Match});
         }



Wir erinnern uns first aus List::Util. Falls die aber echt das tut, kann man auch gleich

push @client_array, $m  if ($modules{$m}{Match}
                            && first { $m =~ m/^$_$/ } @mRe);


machen, weil der push geschieht ja nur wenn eine Regex gefunden wurde UND dieser "Match" hash-lookup tut.
Da UND kommutativ ist, kann man das letztere UND vorziehen. Falls das fehlschlägt macht der interpreter einen shortcut und evaluiert den zweiten ausdruck (first ...) gar nicht mehr.

Ich würde meinen, das ist im Schnitt so ne Größenordnung schneller. Ok, können wieder nur ein paar Mikrosekunden hier und da sein, aber hey! Eichhörnchenernährung und so...

Also dann bin ich erstmal bei

sub computeClientArray {
    my $hash   = shift;
    my $module = shift;

    my @client_array = ();
    my @mRe          = split m{:}xms, $hash->{Clients} ? $hash->{Clients}:$module->{Clients};

    for my $m (sort { $modules{$a}{ORDER}.$a cmp $modules{$b}{ORDER}.$b }
                   grep { defined($modules{$_}{ORDER}) } keys %modules) {

        push @client_array, $m if (   $modules{$m}{Match}
                                   && first { $m =~ m/^$_$/ } @mRe);
    }

    return $hash->{'.clientArray'} = \@client_array;
}


split und die äußere Schleife schaue ich mir ein anderes mal an. Mal schauen, was mir jetzt um die Ohren fliegt.  ;)

Hm...

Offensichtlich dient das hier dazu $hash->{'.clientArray'} zu setzen, warum machen wir das nicht gleich?


sub computeClientArray {
    my $hash   = shift;
    my $module = shift;

    my @mRe = split m{:}xms, $hash->{Clients} ? $hash->{Clients}
                                              : $module->{Clients};

    $hash->{'.clientArray'} = [];

    for my $m (sort { $modules{$a}{ORDER}.$a cmp $modules{$b}{ORDER}.$b }
                   grep { defined($modules{$_}{ORDER}) } keys %modules) {

        if ($modules{$m}{Match} && first { $m =~ m{\A$_\z}xms } @mRe) {
            push @{$hash->{'.clientArray'}}, $m;
        }
    }

    return $hash->{'.clientArray'};
}


Ich denke, so lasse ich das jetzt erstmal.
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

RichardCZ

#16
globale Variablen sind böse. Aber bevor ich auf die losgehe, suche ich mir - gemein wie ich bin - erstmal ein schwächeres Opfer:

my $srandUsed;

Dem werd' ich's jetzt zeigen! Erst schaut man nach, ob er einen Hinterhalt (im Sinne von Support, nicht im Sinne von Hinterhalt)
in der Community hat:

$ grep -r '$srandUsed' *
FHEM/98_todoist.pm:my $srandUsed;
hobo.pl:my $srandUsed;
hobo.pl:    srand(gettimeofday()) if (!$srandUsed++);
hobo.pl:    srand(gettimeofday()) if (!$srandUsed++);


Nicht viel. Die Nutzung in 98_todoist.pm ist komplett fake und die drei Zeilen in hobo.pl/fhem.pl haben keine Chance gegen mich.
$srandUsed wird verwendet um "nur einmal" srand aufzurufen bevor man dann im weiteren Verlauf des Programms rand nutzt
(um UUIDs zu generieren)

Erstmal schaut man nach, wofür srand überhaupt da ist https://perldoc.perl.org/functions/srand.html

Dann stellt man fest es dient dazu, dass rand() ein wenig zufälliger wird. Ansonsten ist rand ja nur ein Pseudozufallsgenerator.
Nun ist die Ganze UUID Geschichte keine Kryptographieanwendung, aber es wäre ziemlich uncool, wenn da immer nur gleiche
UUIDs aus den Funktionen purzeln würden.

Theoretisch könnte man sich nun auf den Standpunkt stellen, dass stets ein srand vor einem rand in diesem Fall nix ausmacht,
insbesondere wenn man als seed Mikrosekunden reinjagt, aber wer weiß was das mit der Performance macht. Auf der anderen
Seite ist einfach nur die Zeit zu verwenden bei einem srand nicht unbedingt best practice. Am besten wir knödeln dort noch die
Prozess-Id mit rein. srand(gettimeofday() . $$)

So. Zurück zu unserem Opfer, $srandUsed. Sich da einfach mitten im Quelltext als modulglobale Variable breitmachen
geht ja nun gar nicht. Könnte man da nicht ein tolles Feature von Perl 5.10.1 nutzen und statische Variablen benutzen?
Also sowas:

sub createUniqueId {
    state $srandUsed = 0;

    srand(gettimeofday() . $$) if (!$srandUsed++);

    my $uniqueID = join '', map { unpack "H*", chr(rand(256)) } 1..16;
    return $uniqueID;
}



Damit hat nun zwar jeder UUID-Generator sein eigenes, privates srandUsed Flag, aber ist doch auch ganz schön.
Und somit kann ich mich endlich meinem Opfer zuwenden:

my $srandUsed;

Muhahahaha.... aha ... doch halt, eine Sache hab' ich noch:

my $uniqueID = join '', map { unpack "H*", chr(rand(256)) } 1..16;

Das ist doch irgendwie komischer Code. "16 mal eine beliebig lange Bytefolge von einem byte ... what?"
Ich mach' das jetzt so:

sub createUniqueId {
    state $srandUsed = 0;

    srand(gettimeofday() . $$) if (!$srandUsed++);

    return sprintf("%02x" x 16, map { rand(256) } 1..16);
}


Und Benchmark meint das Eichhörnchen bekommt wieder was zum Essen:

Benchmark: timing 1000000 iterations of my, orig...
        my:  3 wallclock secs ( 2.49 usr +  0.00 sys =  2.49 CPU) @ 401606.43/s (n=1000000)
      orig:  3 wallclock secs ( 3.06 usr +  0.00 sys =  3.06 CPU) @ 326797.39/s (n=1000000)

Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

RichardCZ

Tja ... schön wär's. Erstmal lautet das Ziel überhaupt Tests zu haben.

Dank der Auslagerung von einigem Code in echte Perl Module (also im Gegensatz zu den meisten Modulen in FHEM) und noch dazu in dafür Vorgesehene Verzeichnisse lib/t kann ich endlich einen "prove" laufen lassen.

$ prove -I lib -r t
t/Export/Attrs/00.load.t ............... 1/1 # Testing Export::Attrs v0.1.0
t/Export/Attrs/00.load.t ............... ok   
t/Export/Attrs/01export.t .............. ok   
t/Export/Attrs/author-pod-syntax.t ..... skipped: these tests are for testing by the author
t/Export/Attrs/release-distribution.t .. skipped: these tests are for release candidate testing
t/Gentoo/Version/20-version.t .......... ok       
All tests successful.
Files=5, Tests=781,  0 wallclock secs ( 0.06 usr  0.01 sys +  0.27 cusr  0.05 csys =  0.39 CPU)
Result: PASS


Hey! 781 Tests.  ;)
Gut, die meisten sind noch von Gentoo::Version, aber immerhin und bald schmeisse ich auch einige von Hobo mit rein.




In Other News:

Ich versuche einen Weg zu finden wie man mit dem ganzen Namespace Gewusel fertig wird.
Dazu habe ich mir erstmal 59_WUup.pm vorgenommen und versuche da den use von HttpUtil und UConv sauber hinzubekommen.
Um damit nicht alle anderen Module zu schrotten, habe ich mir erstmal Kopien dieser Files unter lib/HomeBot/* angelegt.
Aber ist noch alles sehr brüchig...
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

RichardCZ

Ja, es ist soweit, ich schreibe mein erstes Modul. Natürlich nehme ich kritisch jeden Punkt in
https://wiki.fhem.de/wiki/DevelopmentModuleIntro
unter die Lupe.

So dachte mir ja heute, man könnte XXX_Initialize eleganter machen. Bislang (nur Beispiel):

sub HMCCU_Initialize ($)
{
my ($hash) = @_;

$hash->{DefFn} = "HMCCU_Define";
$hash->{UndefFn} = "HMCCU_Undef";
$hash->{SetFn} = "HMCCU_Set";
...
}


künftig - so dachte ich:

sub XXX_Initialize {

    return {
        DefFn   => 'XXX_Define', # besser: coderef
        UndefFn => 'XXX_UNdef',
...
    };
}


Ja denkste!

CommandReload in fhem.pl habe ich um ein $init_hr erweitert, den Initialize-Aufruf erweitert zu

    $init_hr = &{ "${fnname}_Initialize" }(\%hash);

und schliesslich das Modul verankern als

   $modules{$m} = ref $init_hr eq 'HASH' ? $init_hr : \%hash;

Das sollte - dachte ich - schön rückwärtskompatibel sein mit dem alten Weg (\%hash) und mit dem neuen weg (argumentlos, direkt anonymes hash zurück.
Denn niemand liefert eine Hashref zurück - ist doch logisch. Dachte ich Naivling, ich sende Rudi einen Patch.
Als erstes ist mir natürlich Hobo um die Ohren geflogen.  ;)
Ich also Debugmeldungen rein und siehe da:

Hallo: $VAR1 = {
          'ClientFilter' => 'telnet',
          'Fn' => 'CommandTelnetInform',
          'Hlp' => '{on|onWithState|off|log|raw|timer|status},echo all events to this client'
        };



Ja klar! Musste ja sein.

98_telnet.pm beendet Initialize mit


  $cmds{inform} = { Fn=>"CommandTelnetInform",
          ClientFilter => "telnet",
          Hlp=>"{on|onWithState|off|log|raw|timer|status},".
                        "echo all events to this client" };
} # <--- end sub


Verfluchte "$&%""§ !!!

Klar beendet sich dann die Sub mit dem Wert des letzten Statements. Also besagter hashref. (Wert der Zuweisung)

Es kann doch nicht so schwer sein ans Ende der sub ein return; zu pflanzen - oder?
PBP hat's ja schon immer gesagt...

Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

RichardCZ

Ich möchte, dass FHEM und HoBo schön synchronisiert laufen. Gleichzeitig, möchte ich aber in HoBo kein "Kraut und Rüben". Daher wird aller FHEM source code bevor er ins HoBo Repository kommt einem automatischen Waschgang unterzogen.

Aus fachlicher Sicht erfolgt eine "Normalisierung des Source Codes"

bislang (kann künftig mehr sein):

* latin1 -> utf-8
* de-htmlentities-ierung (&uuml; -> ü)
* perltidy (http://perltidy.sourceforge.net/tutorial.html)

und ein paar andere Details, die man dem Source erstmal nicht groß ansieht, wie chmod 644 (einige .pm Module in FHEM sind executable)

D.h. jetzt wird erstmal ein Schwall commits im Hobo Repository kommen, die irrsinnig viel Diffs zeigen. Aber, sobald das erledigt ist und irgendjemand aktualisiert sein Modul und ändert nur ein Zeichen, dann wird beim nächsten Version bump im Hobo Repo auch nur dieses eine Zeichen im Diff angezeigt.

Was dann vielleicht magisch aussehen mag, denn dieses Diff erfolgt u.U. in einem source, der strukturell anders aussieht. Magic...

Für die allermeisten Module funktioniert das, ab und zu finde ich welche, die aus mir unerfindlichen Gründen noch zicken. So fällt z.B. bei 00_THZ.pm perltidy auf die Schnauze

Perltidy version is 20200110

/tmp/00_THZ.pm.f2h: Begin Error Output Stream
1: unexpected character decimal 239 (ï) in script
1: unexpected character decimal 187 (») in script
1: unexpected character decimal 191 (¿) in script
1: Giving up after error


Warum - keine Ahnung - ist mir erstmal wurst, solche Module passen halt momentan nicht in die Waschstrasse.  :)

Wenn natürlich jemand sein Modul 100 Jahre nicht angefasst hat und vielleicht die gewaschene Version im Hobo Repo als Grundlage für weiteres Hacking machen möchte - nur zu, be my guest.

Der Prozess ist automatisch und sicher bekommt er nicht alles 100%ig hin, aber 80% machen ist besser als 100% wollen - habe ich mal hier irgendwo wiederholt gelesen.  ;)
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

rudolfkoenig

ZitatWarum - keine Ahnung - ist mir erstmal wurst, solche Module passen halt momentan nicht in die Waschstrasse.  :)
UTF-8 BOM.

RichardCZ

Zitat von: rudolfkoenig am 05 April 2020, 14:02:24
UTF-8 BOM.

Ja, mir schwante schon sowas nachdem ich die Korrelation gesehen habe:

00_THZ.pm:                   Perl5 module source, UTF-8 Unicode (with BOM) text, with very long lines
10_Itach_IR.pm:              Perl5 module source, UTF-8 Unicode (with BOM) text
23_LUXTRONIK2.pm:            Perl5 module source, UTF-8 Unicode (with BOM) text, with very long lines
32_withings.pm:              Perl5 module source, UTF-8 Unicode (with BOM) text, with very long lines
38_netatmo.pm:               Perl5 module source, UTF-8 Unicode (with BOM) text, with very long lines
39_Talk2Fhem.pm:             Perl5 module source, UTF-8 Unicode (with BOM) text
55_DWD_OpenData.pm:          Perl5 module source, UTF-8 Unicode (with BOM) text, with very long lines
70_JSONMETER.pm:             Perl5 module source, UTF-8 Unicode (with BOM) text, with very long lines
70_NEUTRINO.pm:              Perl5 module source, UTF-8 Unicode (with BOM) text
70_Pushsafer.pm:             Perl5 module source, UTF-8 Unicode (with BOM) text, with very long lines
70_VIERA.pm:                 Perl5 module source, UTF-8 Unicode (with BOM) text
71_YAMAHA_MC.pm:             Perl5 module source, UTF-8 Unicode (with BOM) text, with very long lines
72_XiaomiDevice.pm:          Perl5 module source, UTF-8 Unicode (with BOM) text, with very long lines
73_PRESENCE.pm:              Perl5 module source, UTF-8 Unicode (with BOM) text, with very long lines
88_Itach_IRDevice.pm:        Perl5 module source, UTF-8 Unicode (with BOM) text
93_DbRep.pm:                 Perl5 module source, UTF-8 Unicode (with BOM) text, with very long lines
95_Alarm.pm:                 Perl5 module source, UTF-8 Unicode (with BOM) text
95_Babble.pm:                Perl5 module source, UTF-8 Unicode (with BOM) text
95_Dashboard.pm:             Perl5 module source, UTF-8 Unicode (with BOM) text
95_FLOORPLAN.pm:             Perl5 module source, UTF-8 Unicode (with BOM) text
95_PostMe.pm:                Perl5 module source, UTF-8 Unicode (with BOM) text
95_remotecontrol.pm:         Perl5 module source, UTF-8 Unicode (with BOM) text
95_YAAHM.pm:                 Perl5 module source, UTF-8 Unicode (with BOM) text
98_alarmclock.pm:            Perl5 module source, UTF-8 Unicode (with BOM) text
98_cloneDummy.pm:            Perl5 module source, UTF-8 Unicode (with BOM) text
98_DSBMobile.pm:             Perl5 module source, UTF-8 Unicode (with BOM) text
98_livetracking.pm:          Perl5 module source, UTF-8 Unicode (with BOM) text, with very long lines
98_statistics.pm:            Perl5 module source, UTF-8 Unicode (with BOM) text
98_todoist.pm:               Perl5 module source, UTF-8 Unicode (with BOM) text


Alles Perltidy Fehler:

/tmp/00_THZ.pm.f2h.ERR         /tmp/38_netatmo.pm.f2h.ERR       /tmp/59_PROPLANTA.pm.f2h.ERR  /tmp/70_VIERA.pm.f2h.ERR         /tmp/73_PRESENCE.pm.f2h.ERR
/tmp/10_Itach_IR.pm.f2h.ERR    /tmp/39_Talk2Fhem.pm.f2h.ERR     /tmp/70_JSONMETER.pm.f2h.ERR  /tmp/71_YAMAHA_MC.pm.f2h.ERR     /tmp/88_Itach_IRDevice.pm.f2h.ERR
/tmp/23_LUXTRONIK2.pm.f2h.ERR  /tmp/55_DWD_OpenData.pm.f2h.ERR  /tmp/70_NEUTRINO.pm.f2h.ERR   /tmp/72_FRITZBOX.pm.f2h.ERR
/tmp/32_withings.pm.f2h.ERR    /tmp/59_OPENWEATHER.pm.f2h.ERR   /tmp/70_Pushsafer.pm.f2h.ERR  /tmp/72_XiaomiDevice.pm.f2h.ERR



Was kann man da machen? Recode?
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

rudolfkoenig


CoolTux

Die Maintainer bitten sich das mit perltidy einmal an zu schauen.
Du musst nicht wissen wie es geht! Du musst nur wissen wo es steht, wie es geht.
Support me to buy new test hardware for development: https://www.paypal.com/paypalme/MOldenburg
My FHEM Git: https://git.cooltux.net/FHEM/
Das TuxNet Wiki:
https://www.cooltux.net

RichardCZ

#24
Ok. Also erstmal muss man rausfinden ob BOM ja oder nein. HIerzu fragt man bei dem Unicode Konsortium nach:

Zitat2.6 Encoding Schemes
... Use of a BOM is neither required nor recommended for UTF-8, but may be encountered in contexts where UTF-8 data is converted from other encoding forms that use a BOM or where the BOM is used as a UTF-8 signature. See the "Byte Order Mark" subsection in Section 16.8, Specials, for more information.

Das interpretiere ich jetzt mal als klares NEIN.

Dann muss man rausfinden, wie man es los wird.

$ dos2unix 00_THZ.pm
dos2unix: converting file 00_THZ.pm to Unix format...
$ file 00_THZ.pm
00_THZ.pm: Perl5 module source, UTF-8 Unicode text, with very long lines
$ perltidy 00_THZ.pm
(kein Fehler) -> Profit!


edit:

das sieht relativ gut aus, etliche rauschen jetzt auch durch. Aber nicht alle

00_THZ.pm...
10_Itach_IR.pm...
38_netatmo.pm...
59_PROPLANTA.pm...
## Please see file /tmp/59_PROPLANTA.pm.f2h.ERR
70_VIERA.pm...


Perltidy version is 20200110

/tmp/59_PROPLANTA.pm.f2h: Begin Error Output Stream
1: unexpected character decimal 239 (<EF>) in script
1: unexpected character decimal 187 (<BB>) in script
1: unexpected character decimal 191 (<BF>) in script
1: Giving up after error


Muss ich bei Gelegenheit gugge.

edit2:

PROPLANTA OPENWEATHER und FRITZBOX sind irgendwie stur.
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

Wzut

#25
IMHO beginnen alle diese Dateien nicht mit HEX23 = # , sondern mit einem HEX EF ?
Edit : ok hat Richard schon gefunden , bleiben noch BB & BF
Maintainer der Module: MAX, MPD, UbiquitiMP, UbiquitiOut, SIP, BEOK, readingsWatcher

RichardCZ

und 95_YAAHM bockt auch irgendwie, aber anders als alle anderen:

/tmp/95_YAAHM.pm.f2h: Begin Error Output Stream
1029: already saw definition of 'sub YAAHM_restore' in package 'main' at line 357
1991: already saw definition of 'sub YAAHM_setWeeklyTime' in package 'main' at line 358
4401: To see 1 non-critical warnings rerun with -w


Noch k.A. aber da ich mir bei der Gelegenheit auch den Source angesehen habe:

https://gl.petatech.eu/root/HomeBot/-/blob/master/FHEM/95_YAAHM.pm#L4163

möchte ich schon anmerken, dass es sowas wie HEREDOCs gibt.
(siehe z.B. https://perlmaven.com/here-documents)
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

RichardCZ

Zitat von: RichardCZ am 05 April 2020, 13:41:00
Aber, sobald das erledigt ist und irgendjemand aktualisiert sein Modul und ändert nur ein Zeichen, dann wird beim nächsten Version bump im Hobo Repo auch nur dieses eine Zeichen im Diff angezeigt.

Was dann vielleicht magisch aussehen mag, denn dieses Diff erfolgt u.U. in einem source, der strukturell anders aussieht. Magic...

Ich muss mich natürlich loben (macht ja sonst keiner) - das scheint zu funktionieren:

https://gl.petatech.eu/root/HomeBot/-/commit/295596c9328eecb738970a366350019d11987331
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

Wzut

Ich würde dir ja einen Keks geben wenn sich sehen könnte was die Waschstrasse abgewaschen hat.
Was ich da sehe sind lediglich die Diffs zum Vorgänger ? Gerade das Thema UTF8 hätte mich interessiert ob da nun alles stimmt,
Maintainer der Module: MAX, MPD, UbiquitiMP, UbiquitiOut, SIP, BEOK, readingsWatcher

KernSani

98_DSBMobile hatte offensichtlich kein Problem mit Perltidy (verwende ich ohnehin immer) aber falsches encoding. Hab das mal geändert und eingecheckt- macht sich das bei dir jetzt irgendwie bemerkbar?


Gesendet von iPhone mit Tapatalk
RasPi: RFXTRX, HM, zigbee2mqtt, mySensors, JeeLink, miLight, squeezbox, Alexa, Siri, ...