Beta : 10_MAX

[Wzut]

Ich habe in meineir heutigen Beta von 10_MAX,pm sowohl die Protoypen als auch die geliebten return undef rausgeworfen -> https://forum.fhem.de/index.php/topic,106577.msg1036378.html#msg1036378
Da ich an den beiden MAX Modulen nun schon seit November schraubem, muß ich das nicht gleich via commit auf ein paar hundert arme User loslassen,
sondern habe da eine kleine überschaubare Tester Gemeinde (20-30 User) die es i.d.R. gelassen sehen wenn ich mal wieder was verschlimmbessert habe
Bei mir läuft es, mal schauen was so an User Feedback kommt. 

[Wzut]

Nachdem es bis jetzt so ruhig an dieser Front war bin ich etwas mutiger geworden und habe versucht noch andere Punkte aus PBP umzusetzen.

Womit ich allerdings immer noch kämpfe sind die Subs Set und Parse. Beide waren wahre Monster was das Thema if-elsif betrifft und die Gesammtlänge hat mich beim scrollen oder editieren immer schwindelig gemacht. Hier war(ist)  also in jedem Fall Handlungbedarf da ich mich ab und zu in dem Dschungel selbst verirrt habe.

Bei der Set sub habe ich es fast geschafft sie zu einem reinen Dispatcher umzubauen mit Hilfe von neuen Subs.
Blöderweise sind die auch nicht gerade klein geworden was nun perlcritic auch wieder anmeckert (high complexity score). Nuja zumindest blicke ich nun bei den Sets wieder durch.
Bei der sub Parse sieht das allerdings noch düster aus, den if-elsif Bandwurm konnte ich zumindest in einzelne ifs zerlegen, aber was mir schon seit Anfang an gegen den Strich geht ist das Parse sich bei einem Ack Telegramm wieder selbst aufruft.
An einem Punkt bin ich allerdings bisher gescheitert : use utf8 , sobald ich das einsetze wird im FHEMWEB das °C nicht mehr richtig dargestellt.

Um das vorher / nachher besser vergleichen zu können hänge ich beide Versionen an, die 10_ ist die heutige Beta , die 20_ sollte mal die Zukunft werden. 

Achso , bitte keine Kommentare  wegen meiner deutschen Kommentare , ich brauch die einfach solange ich da aktiv schraube sonst habe ich teilweise in einer Woche vergessen warum ich was gemacht habe. Ebenso die ToDo Erinnerungen die da auftauchen.
   

[RichardCZ]

Hinsichtlich Parse/MAX_Parse:

Für gewöhnlich nimmt man

Code Auswählen Erweitern


...
_handle_ThermostatState(...) if ($msgtype eq 'ThermostatState');
_handle_WallThermostatX(..) if (($msgtype eq 'WallThermostatState') || ($msgtype eq 'WallThermostatControl'));
...


Als Zwischenschritt um diese Riesen if(elsif) Oschis zu zerstückeln.
Teile und Herrsche.

So bearbeitet, kann man das eventuell später noch weiter zu

Code Auswählen Erweitern

my %dispatch = (
    'ThermostatState' => \&_handleDies,
    'WallThermostat' => \&_handleDas,
...
);


umformen. Parse sollte dann locker unter 70 LOC bleiben. 

Man kann es dann sogar ein wenig effizienter haben, weil man dann künftig keine stille Post mehr braucht a la

return Parse($hash, "MAX2,$isToMe,WallThermostatState,$addr,$new_args") if ($shash->{type} eq 'WallMountedThermostat');

die sich Textmessages hin und herschickt, die man dann wieder parst, sondern direkt

return _handle_WallThermostatState(blubber) if ($shash->{type} eq 'WallMountedThermostat');

Von der Testbarkeit ganz zu schweigen.



Ein paar Beobachtungen:

Code Auswählen Erweitern

    my $name   = shift;
    my $action = shift // return;

    my $fname  = shift;
       $fname  //= $name;
->

Code Auswählen Erweitern

    my $name   = shift;
    my $action = shift // return;
    my $fname  = shift // $name;

my ($bits2,$valveposition,$desiredTemperature,$until1,$until2,$until3) = unpack("aCCCCC",pack("H*",$args[0]));
->
my ($bits2,$valveposition,$desiredTemperature,@until) = unpack("aCCCCC",pack("H*",$args[0]));

Ansonsten ist da echt noch viel zu tun. Vor allem die Redundanzen müssen weniger werden.
Vorher kann man das gar nicht sinnvoll kommentieren.

Code Auswählen Erweitern

    if (($shash->{type} eq 'HeatingThermostatPlus') && ($hash->{TYPE} eq 'MAXLAN'))
    {
readingsBulkUpdate($shash, 'valveposition', int($valveposition * MAX_ReadingsVal($shash, 'maxValveSetting')/100));
    }
    else {
readingsBulkUpdate($shash, 'valveposition', $valveposition);
    }
->

Code Auswählen Erweitern

    if (($shash->{type} eq 'HeatingThermostatPlus') && ($hash->{TYPE} eq 'MAXLAN')) {
$valveposition = int($valveposition * MAX_ReadingsVal($shash, 'maxValveSetting') / 100);
    }
            readingsBulkUpdate($shash, 'valveposition', $valveposition);

Code Auswählen Erweitern

my ($sensor,$t,$snotify) = split(':', AttrVal($shash->{NAME}, 'externalSensor', '::'));
$snotify = 0 if (!defined($snotify));
my $ext = ReadingsNum($sensor, $t, 0);
readingsBulkUpdate($shash, 'deviation', sprintf('%.1f',($ext-$shash->{'.desiredTemperature'}))) if ($shash->{'.desiredTemperature'} && $ext);
readingsBulkUpdate($shash, 'externalTemp', $ext) if ($ext && !$snotify);
->

Code Auswählen Erweitern

my ($sensor,$t,$snotify) = split(':', AttrVal($shash->{NAME}, 'externalSensor', '::'));
my $ext = ReadingsNum($sensor, $t, 0);
                if ($ext) {
                    my $desiredTemp = $shash->{'.desiredTemperature'};
                    readingsBulkUpdate($shash, 'deviation',
                                       sprintf('%.1f',($ext - $desiredTemp)))  if ($desiredTemp);
                    readingsBulkUpdate($shash, 'externalTemp', $ext)           if (!defined $snotify);
                }

Code Auswählen Erweitern

readingsBulkUpdate($shash, 'ecoTemperature',     SerializeTemperature($args[0]));
readingsBulkUpdate($shash, 'comfortTemperature', SerializeTemperature($args[1]));
readingsBulkUpdate($shash, 'maximumTemperature', SerializeTemperature($args[2]));
readingsBulkUpdate($shash, 'minimumTemperature', SerializeTemperature($args[3]));

Da kapiere ich bis heute nicht was das für ein Bulk ist wenn

Code Auswählen Erweitern

    readingsBulkUpdate($shash, {
        ecoTemperature     => SerializeTemperature($args[0]),
        comfortTemperature => SerializeTemperature($args[1]),
        maximumTemperature => SerializeTemperature($args[2]),
        minimumTemperature => SerializeTemperature($args[3]),
    });

nicht geht.

[Wzut]

Ansonsten ist da echt noch viel zu tun. Vor allem die Redundanzen müssen weniger werden.

gern, leider habe da nicht so den Blick für. Deine Beispiele sind aber schon mal ein Anfang, vllt werd ich da mit der Zeit noch sensibler.
Meinem Chef sage ich eigentlich immer "einem alten Hund bringt man keine Kunststückchen mehr bei" , ist aber in Bezug auf Perl für die letzten paar Wochen gelogen

Dein %dispatch Beispiel ist  genau das wo ich hin will, aber da werd ich vermutlich auch noch extra Hilfe benötigen.
Bzw. das Problem mit der Set (oder Get) Funktion müsste eigentlich jeder Modul Autor haben wenn seine Funktion eine ganze Latte von Kommandos abzuarbeiten hat plus der Zusammenstellung für FHEMWEB.

[RichardCZ]

Dein %dispatch Beispiel ist  genau das wo ich hin will, aber da werd ich vermutlich auch noch extra Hilfe benötigen.

Deswegen der Zwischenschritt über das Zerstückeln in einzelne "interne" _xxx subs. Das wird Dir weitere Optionen öffnen.