Falsche Nutzung von min/max in 88_HMCCUDEV.pm (u.a.)

Begonnen von RichardCZ, 18 März 2020, 13:52:19

Vorheriges Thema - Nächstes Thema

RichardCZ

Habe mich mal nach den Nutzern von 99_Utils.pm umgesehen.

88_HMCCUDEV.pm benutzt ziemlich intensiv min/max. Allerdings werden alle Aufrufe - so wie es aussieht - im falschen Kontext abgesetzt.

min/max machen Stringvergleiche

die in 99_Utils ebenfalls vorhandenen minNum/maxNum machen numerische Vergleiche.

nun ruft 88_HMCCUDEV.pm min/max ganz überwiegend (habe noch kein Gegenbeispiel gefunden) im numerischen Kontext auf.

Das kann u.U. ziemlich in die Hose gehen, weil im Stringkontext eben "11" kleiner ist als "2".




Entweder man knickt minNum und ersetzt min durch den Code (weil ich glaube, jeder sieht min intuitiv numerisch), oder 88_HMCCUDEV.pm sollte die min/max Aufrufe durch minNum/maxNum Aufrufe ersetzen.

Ich wäre für ersteres.
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

rudolfkoenig

Wie man es vermutlich aus anderen Beitraegen auslesen kann: ich bin konservativ mit globalen Aenderungswuenschen: auch wenn ich gerne FHEM komplett neu schreiben wuerde, ich (und all die anderen Maintainer) haben nicht die Energie es zu tun, vor allem den Support dafuer zu leisten.

Die Semantik von min/minNum behalten wir also erstmal, und damit wird das zu einer Fehlermeldung, und gehoert in dem HomeMatic Forumsbereich.



RichardCZ

Zitat von: rudolfkoenig am 18 März 2020, 14:13:28
Wie man es vermutlich aus anderen Beitraegen auslesen kann: ich bin konservativ mit globalen Aenderungswuenschen: auch wenn ich gerne FHEM komplett neu schreiben wuerde, ich (und all die anderen Maintainer) haben nicht die Energie es zu tun, vor allem den Support dafuer zu leisten.

Die Semantik von min/minNum behalten wir also erstmal, und damit wird das zu einer Fehlermeldung, und gehoert in dem HomeMatic Forumsbereich.

Gut, aber wenn gewünscht, kann man minNum/maxNum relativ schnell vom Antlitz der Erde fegen. So viel ist das nicht:

FHEM/Color.pm:  my $m = ::minNum( $r, $g, $b );
FHEM/00_THZ.pm:my $v0min = minNum(15, ($ycurvevalues->[33][1]), ($ycurvevalues->[33][2]), $heatSetTemp);        #lower offset than 15, if out of scale
FHEM/98_logProxy.pm:    $to = minNum( $time, $now );
FHEM/#99_Utils.pm#:sub minNum($@)
FHEM/#99_Utils.pm#:    <li><b>minNum(num1, num2, ...)</b><br>returns the lowest value from a given
FHEM/95_Astro.pm:    my $n = minNum( $next[0], @next );
FHEM/10_SOMFY.pm:                               $updateState = minNum( 100, $updateState );
FHEM/10_SOMFY.pm:                       $newState = minNum( 100, $posRounded );
FHEM/10_SOMFY.pm:  $v = minNum( 100, maxNum( 0, $v ) );
FHEM/10_SOMFY.pm:  $v = minNum( 200, maxNum( 0, $v ) );
FHEM/10_SOMFY.pm:                       $pos = minNum( 100, $pos );
FHEM/10_SOMFY.pm:                               $newPos = minNum( 100, $pos );
FHEM/10_SOMFY.pm:                                       $newPos = minNum( 200, $pos + $newPos );
FHEM/RESIDENTStk.pm:      minNum( ReadingsVal( $name, "lastLocationLong", 0 ), $currLong )
FHEM/RESIDENTStk.pm:      minNum( ReadingsVal( $name, "lastLocationLat", 0 ), $currLat )
FHEM/98_apptime.pm:    my $nice = minNum(19,keys %prioQueues);
FHEM/99_Utils.pm:minNum($@)
FHEM/99_Utils.pm:    <li><b>minNum(num1, num2, ...)</b><br>returns the lowest value from a given


FHEM/31_HUEDevice.pm:        my $bri  = maxNum($r,$g,$b);
FHEM/31_HUEDevice.pm:      my $f = maxNum($X,$Y,$Z);
FHEM/31_HUEDevice.pm:      my $f = maxNum($r,$g,$b);
FHEM/10_WS980.pm:                               if ($srcValue <= maxNum(0, $limit - $hyst)) {
FHEM/10_WS980.pm:                               } elsif ($srcValue >= maxNum(0, $limit - $hyst)) {
FHEM/00_MQTT2_SERVER.pm:    MQTT2_SERVER_out($hash, pack("CCnC*", 0x90, 3, $pid, maxNum(@ret)), $dump);
FHEM/Color.pm:  my $M = ::maxNum( $r, $g, $b );
FHEM/Color.pm:      my $f = main::maxNum($X,$Y,$Z);
FHEM/Color.pm:      my $f = main::maxNum($r,$g,$b);
FHEM/34_SWAP.pm:            $max_pos = maxNum( $max_pos, int($endpoint->{position}) );
FHEM/34_SWAP.pm:          $len = maxNum( $len, $max_pos+$max_pos_size );
FHEM/00_THZ.pm:   $Simul_heatSetTemp             = sprintf("%.1f", maxNum(5,( $tmp + $a)));
FHEM/00_THZ.pm:   $Simul_heatSetTemp_simplified = sprintf("%.1f", maxNum(5,($tmp + $a1)));
FHEM/00_THZ.pm:$v0min = maxNum(5, nearest_ceil(5, $v0min));                                     #start only from a multiple of 5, but do not go below 5
FHEM/98_Installer.pm:                    my $v = maxNum( 0, @{ $pkgStatus{$area}{$t}{$pkg} } );
FHEM/#99_Utils.pm#:sub maxNum($@)
FHEM/#99_Utils.pm#:    <li><b>maxNum(num1, num2, ...)</b><br>returns the highest value from a
FHEM/31_Aurora.pm:      my $f = maxNum($X,$Y,$Z);
FHEM/31_Aurora.pm:      my $f = maxNum($r,$g,$b);
FHEM/50_HP1000.pm:        return maxNum( 0, @a );
FHEM/10_SOMFY.pm:  $v = minNum( 100, maxNum( 0, $v ) );
FHEM/10_SOMFY.pm:  $v = minNum( 200, maxNum( 0, $v ) );
FHEM/10_SOMFY.pm:                               $newPos = maxNum( 0, $pos );
FHEM/10_SOMFY.pm:                                       $newPos = maxNum( 0, ($pos - $newPos) );
FHEM/10_SOMFY.pm:                                       $newPos = maxNum( 0, $pos - $newPos );
FHEM/10_SOMFY.pm:    my $nstt = maxNum($nowt+$utime-0.01, gettimeofday()+.1 );
FHEM/RESIDENTStk.pm:      maxNum( ReadingsVal( $name, "lastLocationLong", 0 ), $currLong ) -
FHEM/RESIDENTStk.pm:      maxNum( ReadingsVal( $name, "lastLocationLat", 0 ), $currLat ) -
FHEM/99_Utils.pm:maxNum($@)
FHEM/99_Utils.pm:    <li><b>maxNum(num1, num2, ...)</b><br>returns the highest value from a


Und das Problem betrifft auch andere Module.
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

zap

Ich korrigiere es in HMCCU. Nach dem Motto "wenn es gut werden soll, mach es selbst" baue ich eine eigene Funktion dafür. In jeder Programmiersprache ist min/max numerisch, daher der Fehler.
2xCCU3, Fenster, Rollläden, Themostate, Stromzähler, Steckdosen ...)
Entwicklung: FHEM auf AMD NUC (Ubuntu)
Produktiv inzwischen auf Home Assistant gewechselt.
Maintainer: FULLY, Meteohub, HMCCU, AndroidDB

RichardCZ

#4
Zitat von: zap am 18 März 2020, 19:12:30
Ich korrigiere es in HMCCU. Nach dem Motto "wenn es gut werden soll, mach es selbst" baue ich eine eigene Funktion dafür. In jeder Programmiersprache ist min/max numerisch, daher der Fehler.

Ich wollte Rudolf vorschlagen, dass ich gerne Maintainerschaft für 99_Utils.pm übernehmen kann - aber irgendwie sind die PMs geblockt. Das sieht relativ OS-unabhängig aus (bzw. muss es sein) und wenn man da ansetzen kann allen Modulmaintainern einen guten Grundstock zu liefern, dann wäre das doch ein guter erster Schritt.

Momentan habe ich in meinem Repo

sub _sortAlphNum {
    return [sort { $a cmp $b } @_ ];
}

sub _sortNum {
    return [sort { $a <=> $b } @_ ];
}

sub min {
    return _sortAlphNum(@_)->[0];
}

sub max {
    return _sortAlphNum(@_)->[-1];
}

sub minNum {             
    return _sortNum(@_)->[0];
}

sub maxNum {             
    return _sortNum(@_)->[-1];
}


und die entsprechende Testsuite. Die Bedenken bzgl. CPAN verstehe ich, mittel-langfristig glaube ich aber dass man da noch bessere Wege finden wird, aber bis dahin wäre ich gerne bereit Utils zu einer "netten Toolbox" zu machen.
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

zap

Gefixt für HMCCU-Module. Die FHEM min/max Funktionen wurden durch Modul interne Funktionen ersetzt.
2xCCU3, Fenster, Rollläden, Themostate, Stromzähler, Steckdosen ...)
Entwicklung: FHEM auf AMD NUC (Ubuntu)
Produktiv inzwischen auf Home Assistant gewechselt.
Maintainer: FULLY, Meteohub, HMCCU, AndroidDB

justme1968

die min/max geschichten aufzuräumen finde ich gut.

zusätzlich neue eigene routinen in die module einzubauen aber nicht. das widerspricht dem gedanken einer zentralen toolbox.

hue, tradfri, alexa-fhem, homebridge-fhem, LightScene, readingsGroup, ...

https://github.com/sponsors/justme-1968

zap

Zitat von: justme1968 am 20 März 2020, 10:49:56
die min/max geschichten aufzuräumen finde ich gut.

zusätzlich neue eigene routinen in die module einzubauen aber nicht. das widerspricht dem gedanken einer zentralen toolbox.

Was soll man machen, wenn sich die zentrale Toolbox nicht an Konventionen in verschiedenen Programmiersprachen hält? Sollte sich das in FHEM ändern, bin ich gerne bereit, wieder min und max zu verwenden.

Ich bin RichardCZ sehr dankbar für diesen Thread. Nun weiß ich endlich, woher die teilweise seltsamen Effekte bei der Nutzung meiner Module kommen.
2xCCU3, Fenster, Rollläden, Themostate, Stromzähler, Steckdosen ...)
Entwicklung: FHEM auf AMD NUC (Ubuntu)
Produktiv inzwischen auf Home Assistant gewechselt.
Maintainer: FULLY, Meteohub, HMCCU, AndroidDB

justme1968

erst mal minNum und maxNum verwenden. die gibt es ja schon. zentral und ziemlich lange.
hue, tradfri, alexa-fhem, homebridge-fhem, LightScene, readingsGroup, ...

https://github.com/sponsors/justme-1968

RichardCZ

Zitat von: zap am 19 März 2020, 14:18:54
Gefixt für HMCCU-Module. Die FHEM min/max Funktionen wurden durch Modul interne Funktionen ersetzt.

Das ist halt der Trugschluss in FHEM.

Das sind keine Modul-internen Funktionen, Du hast einfach nur neue Implementierungen in den main Namespace rein.
Somit sind HMCCU_Min/HMCCU_Max nun für alle sichtbar/verfügbar.

Ist aber egal, der Namespace kann das ja ab, solange man brav das "HMCCU_" Präfix, bzw. sein Modulpräfix im Namen verwendet.
Kapselung/Modularität ist dann zwar ein Fremdwort, die Geschichte wird unwartbar, untestbar. Aber auch das ist egal, denn dann
kann man wenigstens unwartbar/untestbar als Argument gegen Änderungen aus der Schublade holen.

Ab jetzt kann halt jeder entweder min, minNum, HMCC_Min und noch ein paar andere Implementierungen ein- und derselben Sache verwenden.
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

zap

Leider ist Perl halt eine sehr antiquierte Sprache, der so ziemlich alle netten Features moderner Programmiersprachen fehlen bzw. man muss einen ziemlichen Aufwand betreiben, um sie zu simulieren. Im Zweifel muss man eben damit Leben.

2xCCU3, Fenster, Rollläden, Themostate, Stromzähler, Steckdosen ...)
Entwicklung: FHEM auf AMD NUC (Ubuntu)
Produktiv inzwischen auf Home Assistant gewechselt.
Maintainer: FULLY, Meteohub, HMCCU, AndroidDB

RichardCZ

Zitat von: zap am 20 März 2020, 17:20:00
Leider ist Perl halt eine sehr antiquierte Sprache, der so ziemlich alle netten Features moderner Programmiersprachen fehlen bzw. man muss einen ziemlichen Aufwand betreiben, um sie zu simulieren. Im Zweifel muss man eben damit Leben.

:) Das ist der alternativste Fakt, seit ich das lezte Mal Donald Trump zugehört habe.

Was fehlt Dir denn genau? Objektorientierung? Introspektion? Asynchronizität? Nenn' mir bitte eines der netten Features von denen Du sprichst, das Dir fehlt.

Nur weil "ihr hier" einen Code produziert, der eher an eine volle Kinderwindel erinnert als an modernes Perl, muss man nicht gleich die Sprache zum Sündenbock machen. Oder man muss - keine Ahnung, das ist dann eher ne Frage an den Psychologen.
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

Wzut

Zitat von: RichardCZ am 20 März 2020, 18:06:57
Nur weil "ihr hier" einen Code produziert, der eher an eine volle Kinderwindel erinnert
Früher suchten sie dafür den Stein der Weisen, wir machen heute halt aus Scheisse Gold ....  ich versteh nur noch nicht warum die volle Windel bei mir nach über 4 Jahren noch so einen hohen Tragekomfort hat. 
Maintainer der Module: MAX, MPD, UbiquitiMP, UbiquitiOut, SIP, BEOK, readingsWatcher

RichardCZ

#13
Zitat von: Wzut am 20 März 2020, 19:23:15
Früher suchten sie dafür den Stein der Weisen, wir machen heute halt aus Scheisse Gold ....  ich versteh nur noch nicht warum die volle Windel bei mir nach über 4 Jahren noch so einen hohen Tragekomfort hat.

Ganz einfach! Weil sie sich nach 4 Jahren perfekt an Körpertemperatur und -Form angepasst hat.
Ich warte immer noch auf ein fehlendes Feature der antiquierten Sprache Perl.

edit:

Das mit der Kinderwindel ist natürlich eine bewusst provokative Formulierung - im wesentlichen als Antwort auf die Behauptung Perl sei ja so antiquiert. Beide Aussagen sind in etwa auf dem gleichen Niveau.

Generell sind pauschalisierende Aussagen "Code ist super", "Code ist scheisse" eher unwahr. Bzw. jemand, der so eine Aussage macht und darauf beharrt sieht die Sache vermutlich zu undifferenziert. Ich hoffe, das unterstellt man mir nicht.

Codequalität ist eine Multidimensionale Angelegenheit und "code ist scheisse" würde bedeuten, der Code ist in allen denkbaren Metriken "schlecht". Das ist selten der Fall. Meist gibt es irgendwas, was der Code gut macht, und etwas was er nicht gut macht.

HMCCU schwächelt beim Namespace handling, dafür gehört es hinsichtlich Funktionsumfang zu den eher weiterentwickelten.
AutoShuttersControl z.B. ist relativ gut beim Namespace, leistet sich aber ein ziemliches eval/require Massaker gleich zu Beginn.

Und so könnte man sich durch die 500+ Module hangeln.
Witty House Infrastructure Processor (WHIP) is a modern and
comprehensive full-stack smart home framework for the 21st century.

CoolTux

Zitat von: RichardCZ am 21 März 2020, 09:27:21
AutoShuttersControl z.B. ist relativ gut beim Namespace, leistet sich aber ein ziemliches eval/require Massaker gleich zu Beginn.

Spielst Du auf die Abfrage der JSON Wrapper an?
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