search

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

wrong syntax in SIGNALduino_Parse_MU #1043

Closed HomeAutoUser closed 2 years ago

HomeAutoUser commented 2 years ago 
2021.12.08 17:50:33.457 4: sduino_dummy: get rawmsg: MU;P0=-392;P1=1285;P2=-1024;P3=506;P4=-27790;D=012121212321232123232321232321212321232121232123212323212121212121232123212323212123212141212121232123212323232123232121232123212123212321232321212121212123212321232321212321214121212123212321232323212323212123212321212321232123232AFZ212121212123212321232;CP=2;R=0;O;
2021.12.08 17:50:33.476 3: sduino_dummy: Parse_MU, faulty rawData D=: 012121212321232123232321232321212321232121232123212323212121212121232123212323212123212141212121232123212323232123232121232123212123212321232321212121212123212321232321212321214121212123212321232323212323212123212321212321232123232AFZ212121212123212321232
2021.12.08 17:50:33.495 4: sduino_dummy: Parse_MU, Fingerprint for MU protocol id 8 -> TX3 Protocol matches, trying to demodulate
2021.12.08 17:50:33.569 4: sduino_dummy: Parse_MU, Decoded matched MU protocol id 8 dmsg TXAECA560564 length 44 dispatch(1/4) RSSI = -74
2021.12.08 17:50:33.977 2: CUL_TX Unknown device 101, please define it
2021.12.08 17:50:34.001 4: autocreate: received 1 event(s) for 'CUL_TX 101' during the last 240 seconds
2021.12.08 17:50:34.001 4: autocreate: ignoring event for 'CUL_TX 101': at least 5 needed
2021.12.08 17:50:34.077 4: sduino_dummy: Parse_MU, Decoded matched MU protocol id 8 dmsg TXAECA560564 length 44 dispatch(2/4) RSSI = -74
2021.12.08 17:50:34.113 4: sduino_dummy: Dispatch, TXAECA560564, Dropped due to short time or equal msg
codecov[bot] commented 2 years ago

Codecov Report

Merging #1043 (2ff2443) into master (cd2cc86) will increase coverage by 0.13%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1043      +/-   ##
==========================================
+ Coverage   61.25%   61.39%   +0.13%     
==========================================
  Files         125      125              
  Lines        9323     9323              
  Branches     1476     1476              
==========================================
+ Hits         5711     5724      +13     
+ Misses       2541     2528      -13     
  Partials     1071     1071
Flag Coverage Δ
fhem 52.65% <100.00%> (+0.16%) :arrow_up:
modules 61.39% <100.00%> (+0.13%) :arrow_up:
perl 91.61% <ø> (ø)
unittests 61.39% <100.00%> (+0.13%) :arrow_up:

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

Impacted Files Coverage Δ
FHEM/00_SIGNALduino.pm 63.81% <100.00%> (+0.64%) :arrow_up:
t/FHEM/14_FLAMINGO/00_load.t
t/FHEM/14_BresserTemeo/00_load.t 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update cd2cc86...2ff2443. Read the comment docs.

sidey79 commented 2 years ago

hier fehlt noch ein kleiner Test oder?

HomeAutoUser commented 2 years ago

@sidey79 was möchtest du hier testen? Ich finde Dinge oder fehlerhafte Stellen im Code und du möchtest nur testen grins

sidey79 commented 2 years ago

@sidey79 was möchtest du hier testen? Ich finde Dinge oder fehlerhafte Stellen im Code und du möchtest nur testen grins

Die Motivation ist, den Fehler nicht wieder (später) einzubauen.

HomeAutoUser commented 2 years ago

Die Motivation ist, den Fehler nicht wieder (später) einzubauen.

Wie möchtest du dies sicherstellen, das der Fehler der Schreibweise nicht wieder später auftritt?

PS: Unabhängig von der Anpassung gibt es noch eine weitere Stelle im Code wo der Fehler noch drin ist ;-) Diesen hätte ich dann im nächsten Schritt eingereicht.

sidey79 commented 2 years ago

Die Motivation ist, den Fehler nicht wieder (später) einzubauen.

Wie möchtest du dies sicherstellen, das der Fehler der Schreibweise nicht wieder später auftritt?

Einfach einen Test für SIGNALduino_Parse_MU erweitern und fehlerhafte Daten übergeben. Der Rückgabewert sollte durch die Anpassung undef sein. Wenn sich dann daran etwas ändert, würde der Test einen Fehler feststellen

PS: Unabhängig von der Anpassung gibt es noch eine weitere Stelle im Code wo der Fehler noch drin ist ;-) Diesen hätte ich dann im nächsten Schritt eingereicht.

Ich bin halt Wiederholungstäter :)

HomeAutoUser commented 2 years ago

Die Motivation ist, den Fehler nicht wieder (später) einzubauen.

Wie möchtest du dies sicherstellen, das der Fehler der Schreibweise nicht wieder später auftritt?

Einfach einen Test für SIGNALduino_Parse_MU erweitern und fehlerhafte Daten übergeben. Der Rückgabewert sollte durch die Anpassung undef sein. Wenn sich dann daran etwas ändert, würde der Test einen Fehler feststellen

Dann haben wir aneinander vorbeigeschrieben. :-D Du möchtest expliziet die Sub prüfen durch einen Test und nicht expliziet die Schreibweise von einer solchen // Codezeile egeal wo im Code.

Wenn du diesen Test möchtest, ergänzt du diesen? Bitte aber ohne zusätzliche Warnings im Log smile

PS: Unabhängig von der Anpassung gibt es noch eine weitere Stelle im Code wo der Fehler noch drin ist ;-) Diesen hätte ich dann im nächsten Schritt eingereicht.

Ich bin halt Wiederholungstäter :)

Das kann ich bestätigen und ich würde dir sogar noch den Titel Testmeisterverleihen smile

HomeAutoUser commented 2 years ago

@sidey79, ich habe noch einmal über deine Worte nachgedacht. Wieso möchtest du genau diese PR Korrigierung testen wenn man nicht im Normalfall hinkommt.

Grund, das darüber liegende RegEx filtert und prüft. Sollte etwas nicht passen, fällt man dort schon raus. Ich habe die RegEx Prüfung manuell im Code auskommentiert um die Syntax / Korrektheit zu prüfen. Deine Tests würden nie an die Stelle gelangen, nur WENN du ein Beispiel bringst, was durch deine RegEx Prüfung durchfällt.

sidey79 commented 2 years ago

@sidey79, ich habe noch einmal über deine Worte nachgedacht. Wieso möchtest du genau diese PR Korrigierung testen wenn man nicht im Normalfall hinkommt.

Ich sehe das ganz simpel. Da ist ein Bug, völlig egal wie er sich bemerkbar macht. Du hast einen fix, das ist Klasse.

Den Bug haben wir ja nur weil ich mir mal sicher war, dass es so wie ich es machte fehlerfrei ist.

Der Test soll nur sicherstellen, dass wir diesen Bug dauerhaft beheben, mehr nicht. Da ist es völlig egal unter welchen Umständen er heute ausgelöst wird oder eben nicht.

HomeAutoUser commented 2 years ago

Geht in Ordnung @sidey79.

Der Test soll nur sicherstellen, dass wir diesen Bug dauerhaft beheben, mehr nicht. Da ist es völlig egal unter welchen Umständen er heute ausgelöst wird oder eben nicht.

Bitte ergänze du den Test, da du scheinbar schon eine Idee hast, wie du dies Umsetzen möchtest. Ich würde es sicherlich falsch umsetzen.

sidey79 commented 2 years ago

Bitte ergänze du den Test, da du scheinbar schon eine Idee hast, wie du dies Umsetzen möchtest. Ich würde es sicherlich falsch umsetzen.

Der Test ist schon vorhanden und wird erfolgreich absolviert:

https://github.com/RFD-FHEM/RFFHEM/blob/2ff2443e5708a9ebdad583a15f83d1f0b5036415/t/FHEM/00_SIGNALduino/01_SIGNALduino_Parse_MU.t#L56-L60

Es wird auf undef als Return Value verwiesen und auch noch auf Warnings im Code untersucht. Zu einem Fehler kann es in dieser Konstellation nur kommen, wenn Split_Message die Daten fehlerhaft liefert. Auch wenn das im Bereich des möglichen liegt, sehe ich davon ab hier einen Testfall zu erstellen.