RFD-FHEM / RFFHEM

Counterpart of SIGNALDuino, it's the code for FHEM to work with the data received from the uC
GNU General Public License v3.0
44 stars 33 forks source link

SD_UT RC Buttons_six #1114

Closed elektron-bbs closed 1 year ago

elektron-bbs commented 1 year ago
codecov[bot] commented 1 year ago

Codecov Report

Merging #1114 (4fc5664) into master (2d35b42) will increase coverage by 0.82%. The diff coverage is 71.42%.

@@            Coverage Diff             @@
##           master    #1114      +/-   ##
==========================================
+ Coverage   64.35%   65.18%   +0.82%     
==========================================
  Files         132      137       +5     
  Lines        9643     9711      +68     
  Branches     1530     1536       +6     
==========================================
+ Hits         6206     6330     +124     
+ Misses       2244     2174      -70     
- Partials     1193     1207      +14     
Flag Coverage Δ
fhem 57.19% <71.42%> (+1.01%) :arrow_up:
modules 65.18% <71.42%> (+0.82%) :arrow_up:
perl 90.46% <ø> (+0.28%) :arrow_up:
unittests 65.18% <71.42%> (+0.82%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
FHEM/14_SD_UT.pm 40.54% <60.00%> (+2.58%) :arrow_up:
t/FHEM/14_SD_UT/01_attr.t 100.00% <100.00%> (ø)
t/FHEM/14_SD_WS09/00_load.t
t/FHEM/14_SD_BELL/00_load.t 100.00% <0.00%> (ø)
t/FHEM/14_FLAMINGO/09_parseDatat.t 80.00% <0.00%> (ø)
t/FHEM/10_SD_GT/00_load.t 100.00% <0.00%> (ø)
t/FHEM/14_SD_BELL/09_parseDatat.t 80.00% <0.00%> (ø)
t/FHEM/14_FLAMINGO/00_load.t 100.00% <0.00%> (ø)
FHEM/lib/SD_Protocols.pm 79.20% <0.00%> (+0.20%) :arrow_up:
FHEM/00_SIGNALduino.pm 64.19% <0.00%> (+0.44%) :arrow_up:
... and 2 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

elektron-bbs commented 1 year ago

Mhmm, was soll mir das sagen???

codecov/patch Failing after 1s — 30.00% of diff hit (target 55.35%)

HomeAutoUser commented 1 year ago

Mhmm, was soll mir das sagen???

codecov/patch Failing after 1s — 30.00% of diff hit (target 55.35%)

Keine Ahnung was er uns mitteilen möchte. Hier https://github.com/RFD-FHEM/RFFHEM/pull/1111 lief es noch durch. Vielleicht weiß @sidey79 darauf eine Antwort. Ich vermute, das die Meldung nur eine Prüfung der Codeabdeckung meldet ... Können wir dies so übernehmen @sidey79 oder hat es gravierende o. keine Auswirkungen?

sidey79 commented 1 year ago

Der Fehler besagt, dass 30% der veränderten Zeilen nicht durch einen Test verifiziert werden.

Also jede 3. in etwa: Hier zu sehen, es geht vornehmlich um die roten: https://app.codecov.io/gh/RFD-FHEM/RFFHEM/compare/1114/diff

Ich könnte mir vorstellen, dass mit relevanten Testdaten die Abdeckung steigt.

elektron-bbs commented 1 year ago

Heißt das jetzt, das ich mehr Testnachrichten hochladen müsste, oder du mehr Tests einbauen musst, oder das das eigentlich egal ist, oder, oder...?

sidey79 commented 1 year ago

Ich habe jetzt nicht nachgesehen, aber es sieht so aus, als ob wir machen Modelle nicht in den Testdaten haben und die sollten wir dort hinterlegen.

HomeAutoUser commented 1 year ago

Ich habe jetzt nicht nachgesehen, aber es sieht so aus, als ob wir machen Modelle nicht in den Testdaten haben und die sollten wir dort hinterlegen.

Ich habe nun mal nachgesehen im Modul und im Tool. @elektron-bbs, haben wir RAW Msg´s von folgenden Modellen?

Das sind die Modelle, von welchen wir keine Daten im JSON besitzen. Scheinbar ist das codecov/patch noch pingeliger gewurden weil es bis jetzt nichts bemängelt hat grins. Ansonsten kann ich den von @sidey79 obigen markierten Link nicht deuten.

sidey79 commented 1 year ago

Im wesentlichen sagt der Link weiter oben, dass die Bedingungen $model eq 'Buttons_five' || $model eq 'Buttons_six') im Test nie ausgeführt wurden. Es fehlen also Testbedingungen dafür.

elektron-bbs commented 1 year ago

Ich finde in meinen Aufzeichnungen (etwa 3,5 Jahre alt) nur diese:

P46_TEDSEN_SKX1MD,Button:1,MU;P0=-15829;P1=-3580;P2=1962;P3=-330;P4=245;P5=-2051;D=1234523232345234523232323234523234540023452323234523452323232323452323454023452323234523452323232323452323454023452323234523452323232323452323454023452323234523452323232323452323454023452323234523452323;CP=2;
P46_GEIGER_GF0001,Ch:1_Button:Up,MU;P0=-32001;P1=2072;P2=-260;P3=326;P4=-2015;P5=-15769;D=01212123412123434121212123434123412351212123412123434121212123434123412351212123412123434121212123434123412351212123412123434121212123434123412351212123412123434121212123434123412351212123412123434121212123434123412351212123412123434121212123434123412351;CP=3;R=37;O;
P46_GEIGER_GF0001,Ch:1_Button:Down,MU;P0=-15694;P1=2009;P2=-261;P3=324;P4=-2016;D=01212123412123434121212123434123434301212123412123434121212123434123434301212123412123434121212123434123434301212123412123434121212123434123434301212123412123434121212123434123434301;CP=3;R=30;

Für Meikee_21 habe ich nichts. Wir haben da nur die Tastencodes abgeglichen, da die Nachrichtenformate identisch sind. Siehe https://forum.fhem.de/index.php/topic,126110.msg1210715.html#msg1210715

sidey79 commented 1 year ago

@elektron-bbs

Ich habe Tests für die Attribute eingefügt. Beim Attribut repeats haben die Tests einen Fehler im Code identifiziert.

Ob beim Attribut model alles passt, bin ich mir nicht sicher.

elektron-bbs commented 1 year ago

Das Attribut repeats habe ich korrigiert. Da passte das regex nicht.

Für das Attribut model habe ich keine Lösung gefunden. Ich dachte erst, das es an der falschen bitMSG liegt, aber das war es offensichtlich nicht:

2022.09.06 14:52:06 3: Please press button again or receive more messages!
Only with another message can the model be defined.
We need bitMSG from message.
not ok 5 - Change module attribute to buttons_five {
    1..2
    ok 1 - check attribute model isnt Buttons_five
    not ok 2 - check attribute model is Buttons_five
}
2022.09.06 14:52:06 3: SD_UT_Attr set model to Buttons_six
not ok 6 - Change module attribute to buttons_six {
    1..2
    not ok 1 - check attribute model is not Buttons_six
    ok 2 - check attribute model is Buttons_six
}
sidey79 commented 1 year ago

@elektron-bbs Ich verstehe nicht, welche Bedingungen erfüllt sein muss die nicht erfüllt ist. Verstehst Du denn, warum das Attribut nicht geändert wird?

elektron-bbs commented 1 year ago

Ich denke, ich kann es mir schon vorstellen, warum das so nicht funktioniert. Nach Umschalten des Attributes model von Buttons_five zu Buttons_six ändert sich ja auch der Sensorname von Buttons_five zu Buttons_six. Die zweite Abfrage muss also fehlschlagen, weil der Sensorname jetzt Buttons_six ist. Ich habe aber keinen Plan, wie ich deinen Test abändern muss.

sidey79 commented 1 year ago

Ich denke, ich kann es mir schon vorstellen, warum das so nicht funktioniert. Nach Umschalten des Attributes model von Buttons_five zu Buttons_six ändert sich ja auch der Sensorname von Buttons_five zu Buttons_six. Die zweite Abfrage muss also fehlschlagen, weil der Sensorname jetzt Buttons_six ist. Ich habe aber keinen Plan, wie ich deinen Test abändern muss.

Die Hypothese hat mir gefallen, aber ich konnte kein umbenanntes device finden.

Laut Logfile landen wir auch in Zeile 2346: return "Please press button again or receive more messages!\nOnly with another message can the model be defined.\nWe need bitMSG from message.";

An diese Stelle sollten wir aber nur gelanden, wenn bitMSG keine Bits entält. Das habe ich verifiziert, bitMSG ist mit Bits gesetzt. Ich verstand nicht, wieso diese Bedingung nicht wahr ist: if (InternalVal($name, 'bitMSG', 'no data') ne 'no data') {

Den Fehler habe ich in den Tests gefunden, commandAttr muss einfach noch mal aufgerufen werden und lastMSG wird auch benötigt. Umbenannt wird die Definition allerdings nicht, wäre auch seltsam oder?

elektron-bbs commented 1 year ago

Den Fehler habe ich in den Tests gefunden, commandAttr muss einfach noch mal aufgerufen werden und lastMSG wird auch benötigt. Umbenannt wird die Definition allerdings nicht, wäre auch seltsam oder?

Mhmm, dann läuft das hier wohl anders ab:

Das Attribut model ändern geht nur, wenn vorher eine Nachricht dispatcht wurde.

2022.09.07 10:35:03 4: sduino_dummy: get rawmsg: MU;P0=1836;P1=-497;P2=189;P3=-275;P4=416;P5=-8062;D=01212121212341212121212345212121212123412121212123452121212121234121212121234521212121212341212121212345212121212123412121212123452121212121234121212121234521212121212341212121212345212121212123412121212123452121212121234121212121234521212121212341212121;CP=2;R=49;O;
2022.09.07 10:35:03 4: sduino_dummy: Parse_MU, Fingerprint for MU protocol id 29 -> HT12e matches, trying to demodulate
2022.09.07 10:35:03 4: sduino_dummy: Parse_MU, Decoded matched MU protocol id 29 dmsg P29#FBE length 12 dispatch(1/4) RSSI = -49.5
2022.09.07 10:35:03 4: sduino_dummy: SD_UT protocol 29, bitData 111110111110, hlen 3
2022.09.07 10:35:03 4: sduino_dummy: Parse_MU, Decoded matched MU protocol id 29 dmsg P29#FBE length 12 dispatch(2/4) RSSI = -49.5
2022.09.07 10:35:03 4: sduino_dummy: Dispatch, P29#FBE, Dropped due to short time or equal msg
2022.09.07 10:35:03 4: sduino_dummy: Parse_MU, Decoded matched MU protocol id 29 dmsg P29#FBE length 12 dispatch(3/4) RSSI = -49.5
2022.09.07 10:35:03 4: sduino_dummy: Dispatch, P29#FBE, Dropped due to short time or equal msg
2022.09.07 10:35:03 4: sduino_dummy: Parse_MU, Decoded matched MU protocol id 29 dmsg P29#FBE length 12 dispatch(4/4) RSSI = -49.5
2022.09.07 10:35:03 4: sduino_dummy: Dispatch, P29#FBE, Dropped due to short time or equal msg
2022.09.07 10:35:03 4: sduino_dummy: Parse_MU, Fingerprint for MU protocol id 120 -> TFA 35.1077.54.S2 matches, trying to demodulate

Jetzt attr Buttons_five_E model Buttons_six

2022.09.07 10:35:34 3: SD_UT_Attr UNDEFINED sensor Buttons_six detected, code E (DoTrigger)
2022.09.07 10:35:34 2: autocreate: define Buttons_six_E SD_UT Buttons_six E
2022.09.07 10:35:34 2: autocreate: define FileLog_Buttons_six_E FileLog ./log/Buttons_six_E-%Y-%m.log Buttons_six_E
2022.09.07 10:35:34 3: SD_UT_Attr set model to Buttons_six
2022.09.07 10:37:53 4: sduino_dummy: get rawmsg: MU;P0=1836;P1=-497;P2=189;P3=-275;P4=416;P5=-8062;D=01212121212341212121212345212121212123412121212123452121212121234121212121234521212121212341212121212345212121212123412121212123452121212121234121212121234521212121212341212121212345212121212123412121212123452121212121234121212121234521212121212341212121;CP=2;R=49;O;
2022.09.07 10:37:53 4: sduino_dummy: Parse_MU, Fingerprint for MU protocol id 29 -> HT12e matches, trying to demodulate
2022.09.07 10:37:53 4: sduino_dummy: Parse_MU, Decoded matched MU protocol id 29 dmsg P29#FBE length 12 dispatch(1/4) RSSI = -49.5
2022.09.07 10:37:53 4: sduino_dummy: SD_UT protocol 29, bitData 111110111110, hlen 3
2022.09.07 10:37:54 3: SD_UT_Parse device Buttons_five_E deleted
2022.09.07 10:37:54 4: sduino_dummy: Parse_MU, Decoded matched MU protocol id 29 dmsg P29#FBE length 12 dispatch(2/4) RSSI = -49.5
2022.09.07 10:37:54 4: sduino_dummy: Dispatch, P29#FBE, Dropped due to short time or equal msg
2022.09.07 10:37:54 4: sduino_dummy: Parse_MU, Decoded matched MU protocol id 29 dmsg P29#FBE length 12 dispatch(3/4) RSSI = -49.5
2022.09.07 10:37:54 4: sduino_dummy: Dispatch, P29#FBE, Dropped due to short time or equal msg
2022.09.07 10:37:54 4: sduino_dummy: Parse_MU, Decoded matched MU protocol id 29 dmsg P29#FBE length 12 dispatch(4/4) RSSI = -49.5
2022.09.07 10:37:54 4: sduino_dummy: Dispatch, P29#FBE, Dropped due to short time or equal msg
2022.09.07 10:37:54 4: sduino_dummy: Parse_MU, Fingerprint for MU protocol id 120 -> TFA 35.1077.54.S2 matches, trying to demodulate

Dein Test fragt sicher das Attribut model ab, bevor das alte Device gelöscht wird. Zu dem Zeitpunkt existieren noch beide.

Aber egal, Hauptsache der Test läuft durch. Soll ich die Testdaten für Buttons_six wieder ins SIGNALduino_TOOL laden?

sidey79 commented 1 year ago

Ich habe die Testdaten wieder hinterlegt.

Tests laufen auch, wenn sie durch sind, könnte der Merge heute noch klappen :)

sidey79 commented 1 year ago

@elektron-bbs Brauchst Du den branch noch oder können wir ihn löschen?

elektron-bbs commented 1 year ago

Den Branch brauche ich eigentlich nicht mehr. Eines ist mir gestern nach dem PR noch aufgefallen: Kann es sein, das die Datei CHANGED nicht mehr automatisch aktualisiert wird? Der letzte Eintrag ist dieser:

2022-06-14 - Protocol 121 remote control Busch-Transcontrol HF
           - 14_Hideki.pm - fix (#1099)
sidey79 commented 1 year ago

@elektron-bbs

Da ist tatsächlich etwas nicht in Ordnung. Wenn Du den Branch nicht mehr benötigst, dann einfach hier löschen :)

elektron-bbs commented 1 year ago

Vielleicht wäre es ja doch besser, in dem Test 01_attr.t eine Originalnachricht statt der zu kurzen zu verwenden:

$defs{$sensorname}{lastMSG} = q[010];

Vermeidet dann sicher 4 Warnungen beim Test:

Error: 2022.09.10 11:58:56 1: PERL WARNING: substr outside of string at ./FHEM/14_SD_UT.pm line 2213.
Error: 2022.09.10 11:58:56 1: PERL WARNING: Use of uninitialized value $deviceCode in concatenation (.) or string at ./FHEM/14_SD_UT.pm line 2214.
Error: 2022.09.10 11:58:56 1: PERL WARNING: substr outside of string at ./FHEM/14_SD_UT.pm line 2218.
Error: 2022.09.10 11:58:56 1: PERL WARNING: Use of uninitialized value $deviceCode in concatenation (.) or string at ./FHEM/14_SD_UT.pm line 2219.
sidey79 commented 1 year ago

Hmm, wäre aber eher ein umgehen des Fehlers, denn da wird wohl nicht geprüft ob lastmsg ausreichend lang für diese Operation ist

elektron-bbs commented 1 year ago

Jetzt, bei genauerem Hinsehen, fällt mir auf, das die Warnungen sich ja auf bitMSG beziehen, im Test wird aber zuerst nur lastMSG gesetzt. lastMSG wird m.E. bei Änderung des Attributes model nicht benötigt. Ansonsten wird im Modul die Länge schon beim parsen geprüft, bevor bitMSG und lastMSG in den hash geschrieben werden.

HomeAutoUser commented 1 year ago

@elektron-bbs zur Info. Ich habe das SVN (SD_UT Modul) aktualisiert.