RFFHEM icon indicating copy to clipboard operation
RFFHEM copied to clipboard

Added Module to Support LTECH

Open Devirex opened this issue 2 years ago • 18 comments

  • Please check if the PR fulfills these requirements
  • [x] Tests for the changes have been added / modified (needed for for bug fixes / features)
  • [X] commandref has been added / updated (needed for bug fixes / features)
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
  • [ ] Bugfix (please link issue)
  • [X] Feature enhancement
  • [ ] Documentation update
  • [ ] Unittest enhancement
  • [ ] other
  • What is the current behavior? No Modul for uknown Protocoll 31

  • What is the new behavior (if this is a feature change)? Module LTECH for known Protokol P31

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?) Module LTECH was added

  • Other information:

Devirex avatar Apr 22 '22 07:04 Devirex

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (910f5eb) 89.25% compared to head (2d02ef0) 88.92%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1098      +/-   ##
==========================================
- Coverage   89.25%   88.92%   -0.33%     
==========================================
  Files          43       43              
  Lines        2466     2466              
  Branches      170      170              
==========================================
- Hits         2201     2193       -8     
- Misses        111      119       +8     
  Partials      154      154              
Flag Coverage Δ
fhem 88.92% <ø> (-0.33%) :arrow_down:
modules 88.92% <ø> (-0.33%) :arrow_down:
perl 88.92% <ø> (-0.33%) :arrow_down:
unittests 88.92% <ø> (-0.33%) :arrow_down:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 22 '22 07:04 codecov[bot]

@Devirex

Danke für den PR.

In dem Modul finden sich noch Reste von einem Siri Modul. Die wolltest Du sicher noch ändern. Die Perl critic Meldungen beziehen sich hauptsächlich auf die Prototypen aber eine auch wegen der Verwendung von Switch.

Im Detail habe ich mir den Rest jetzt nicht angesehen, grundsätzlich ist es aber so, dass Module unittests benötigen. Vieles kann in bestehende Tests integriert werden. Manches ist aber speziell. Dafür werden wir Daten von dir benötigten.

sidey79 avatar Apr 22 '22 20:04 sidey79

Wow in der ersten Zeile gleich, -> zu lange davor gesessen. Ich passe das an. Das mit den Tests schaue ich mir an.

Devirex avatar Apr 23 '22 09:04 Devirex

Wow in der ersten Zeile gleich, -> zu lange davor gesessen. Ich passe das an. Das mit den Tests schaue ich mir an.

Der einfachste Test prüft nur, ob das Modul geladen werden kann. Den kannst Du aus einem anderen Modul 1:1 kopieren, dann funktioniert der schon, da er das Modul aus dem Ordnernamen verwendet. Damit wissen wir dann, dass es keine Syntaxfehler gibt.

Die Parse Funktion wird über Testdaten geprüft. Das suche sich dir raus, wie das geht.

Einzelne Subs bekommen in der Regel einen eigenen Test. Je nach komplexit der Sub ist das dann einfacher oder aufwändiger.

sidey79 avatar Apr 23 '22 09:04 sidey79

Also

  1. Basistest ob sich das Modul überhaupt laden lässt kannst Du die folgende Datei in einen Ordnernamen identisch zu deinem Modul kopieren. Damit erhalten wir auch gleich die Information ob wir einen Syntaxfehler haben oder nicht:

https://github.com/RFD-FHEM/RFFHEM/blob/master/t/FHEM/10_SD_Rojaflex/00_load.t

In diesem Fall kommen alle Tests in den Ordner t/FHEM/14_LTECH

Damit Tests überhaupt laufen muss auch eine fhem.cfg im Testordner liegen

Diese von hier, könnte ausreichend sein: https://github.com/RFD-FHEM/RFFHEM/blob/master/t/FHEM/10_SD_GT/fhem.cfg

2a) Testdaten kommen hier rein: (JSON) https://github.com/RFD-FHEM/SIGNALduino_TOOL/blob/master/FHEM/lib/SD_Device_ProtocolList.json#L2975

Sobald hier Testdaten für das Modul enthalten sind, kann ein Test diese validieren. Im groben, werden Internals und Readings deines Moduls damit validiert.

INPUT und OUTPUT sind je nach Test unterschiedlich. z.B. wird validiert ob die RMSG als input zu der angegebenen DMSG führt. Die DMSG wiederum, wird als Input für andere Tests verwendet um zu validieren, ob ein Reading gesetzt wurde.

Einfach mal versuchen einen Testdatensatz aus RMSG und DMSG zu erstellen. Lokal innerhalb FHEM kannst Du auch mittels des Moduls SIGNALduino_TOOL testen.

2b) Damit die Testdaten aus 2a getestet werden, braucht es noch einen weiteren Test. https://github.com/RFD-FHEM/RFFHEM/blob/master/t/FHEM/14_SD_UT/09_parseDatat.t

Optional können dort auch korrupte Daten getestet werden. Den Teil kommentiere ich aber erst einmal aus

Ich werde dir das gleich mal in deinen PR reinlegen, dann hast Du einen Ausgangspunkt.

sidey79 avatar Apr 24 '22 09:04 sidey79

@Devirex

Wie Du siehst, ist der Test nicht erfolgreich, das Modul kann nicht geladen werden und es bricht mit einem Fehler ab:

# Failed test '14_LTECH loaded'
Error: # at /home/runner/work/RFFHEM/RFFHEM/t/FHEM/14_LTECH/00_load.t line 13.
# +---------------------------------------------------------+----+---------+
# | GOT                                                     | OP | CHECK   |
# +---------------------------------------------------------+----+---------+
# | Can't locate Switch.pm in @INC (you may need to install | IS | <UNDEF> |
# |  the Switch module) (@INC contains: ./lib ./FHEM . /hom |    |         |
# | e/runner/work/RFFHEM/RFFHEM/local/lib/perl5/5.26.3/x86_ |    |         |
# | 64-linux /home/runner/work/RFFHEM/RFFHEM/local/lib/perl |    |         |
# | 5/5.26.3 /home/runner/work/RFFHEM/RFFHEM/local/lib/perl |    |         |
# | 5/x86_64-linux /home/runner/work/RFFHEM/RFFHEM/local/li |    |         |
# | b/perl5 /home/runner/work/_actions/shogo82148/actions-s |    |         |
# | etup-perl/v1.15.1/scripts/lib /opt/hostedtoolcache/perl |    |         |
# | /5.26.3/x64/lib/site_perl/5.26.3/x86_64-linux /opt/host |    |         |
# | edtoolcache/perl/5.26.3/x64/lib/site_perl/5.26.3 /opt/h |    |         |
# | ostedtoolcache/perl/5.26.3/x64/lib/5.26.3/x86_64-linux  |    |         |
# | /opt/hostedtoolcache/perl/5.26.3/x64/lib/5.26.3) at ./F |    |         |
# | HEM/14_LTECH.pm line 57.\n                              |    |         |
# | BEGIN failed--compilation aborted at ./FHEM/14_LTECH.pm |    |         |
# |  line 57.\n                                             |    |         |
# +---------------------------------------------------------+----+---------+
/home/runner/work/RFFHEM/RFFHEM/t/FHEM/14_LTECH/00_load.t ........................................ 

Wie schon erwähnt, sollte switch ersetzt werden. Da bietet sich eine Lookup Tabelle an um alle Kommandos abzubilden:

my %set_commands = (
# cmd switches
rgbcolor => sub { readingsSingleUpdate($hash, 'rgbcolor_sel', uc $value, 1 },
white => sub { readingsSingleUpdate($hash, 'white_sel', $value , 1},
);

Anstelle switch wird dann einfach die passende anonyme sub aufgerufen:

$set_commands {$cmd};

GGf. müssen aber noch Variable übergeben und eingelesen werden.

sidey79 avatar Apr 24 '22 09:04 sidey79

@sidey79

Habe jetzt Tests für das Laden, Autocreate und und druchspielen lokaler Testdaten ausprobiert. Wenn ich alles habe baue ich es dann hier ein: https://github.com/RFD-FHEM/SIGNALduino_TOOL/blob/master/FHEM/lib/SD_Device_ProtocolList.json#L2975

Devirex avatar Apr 25 '22 16:04 Devirex

@sidey79

Habe jetzt Tests für das Laden, Autocreate und und druchspielen lokaler Testdaten ausprobiert. Wenn ich alles habe baue ich es dann hier ein: https://github.com/RFD-FHEM/SIGNALduino_TOOL/blob/master/FHEM/lib/SD_Device_ProtocolList.json#L2975

Die lokalen Testdaten werden aber nicht geladen, dafür müsstest Du die auskommentierten Zeilen im Test wieder aktivieren :)

sidey79 avatar Apr 25 '22 17:04 sidey79

@sidey79 Habe jetzt Tests für das Laden, Autocreate und und druchspielen lokaler Testdaten ausprobiert. Wenn ich alles habe baue ich es dann hier ein: https://github.com/RFD-FHEM/SIGNALduino_TOOL/blob/master/FHEM/lib/SD_Device_ProtocolList.json#L2975

Die lokalen Testdaten werden aber nicht geladen, dafür müsstest Du die auskommentierten Zeilen im Test wieder aktivieren :)

Da ist wohl beim merge was schief gelaufen. In meiner Branch waren die Zeilen nicht auskommentiert :) habe eine Zeit gebraucht um festzustellen dass in meinen Testdaten das DEF bei Internals gefehlt hat und er deshalb das device nicht temporär für den Test erstellt hatte :D

Devirex avatar Apr 25 '22 19:04 Devirex

@sidey79 Tests für Set und Define hinzugefügt, Daten werden jetzt auch geladen. https://github.com/RFD-FHEM/RFFHEM/runs/6176937353?check_suite_focus=true#step:8:30892

Devirex avatar Apr 26 '22 14:04 Devirex

Wie ist hier der Stand @Devirex ?

HomeAutoUser avatar Jul 07 '22 04:07 HomeAutoUser

Ist noch was offen? Tests sind drin, config wurde geändert. Benutze das Modul regelmäßig und bin soweit sehr zufrieden.

Devirex avatar Jul 08 '22 19:07 Devirex

Ist noch was offen? Tests sind drin, config wurde geändert. Benutze das Modul regelmäßig und bin soweit sehr zufrieden.

Da sind noch einige Anmerkungen und perlcritic Fehler. Haben es die Testdaten bereits in das SIGNALDuino Tool geschafft?

sidey79 avatar Jul 08 '22 20:07 sidey79

Ist noch was offen? Tests sind drin, config wurde geändert. Benutze das Modul regelmäßig und bin soweit sehr zufrieden.

Da sind noch einige Anmerkungen und perlcritic Fehler. Haben es die Testdaten bereits in das SIGNALDuino Tool geschafft?

Ich weiß nicht, ob @Devirex weiß was mit SIGNALDuino Tool gemeint ist. Aus die Schnelle habe ich nichts gesehen. Wenn ich RAWMSG´s erhalte, so kann ich diese eintragen.

Fehlt sonst noch was hier?

HomeAutoUser avatar Jul 19 '22 20:07 HomeAutoUser

Habe einen PR für die Testdaten erstellt:

https://github.com/RFD-FHEM/SIGNALduino_TOOL/pull/76

Devirex avatar Jul 21 '22 10:07 Devirex

Ich habe den Branch aktualisiert und dabei kamen ein paar Abweichungen in den Tests zutage:

ok 1 - 14_LTECH loaded
ok
    # Failed test 'check reading for devices'
Error:     # at /home/runner/work/RFFHEM/RFFHEM/t/FHEM/14_LTECH/01_set.t line 105.
    # +--------+----+--------+
    # | GOT    | OP | CHECK  |
    # +--------+----+--------+
    # | 7F1900 | eq | 7F7F7F |
    # +--------+----+--------+
# Failed test 'Protocol 31 - set LTECH_Test_11 brightness 50'
Error: # at /home/runner/work/RFFHEM/RFFHEM/t/FHEM/14_LTECH/01_set.t line 106.
    # Failed test 'check reading for devices'
Error:     # at /home/runner/work/RFFHEM/RFFHEM/t/FHEM/14_LTECH/01_set.t line 139.
    # +--------+----+--------+
    # | GOT    | OP | CHECK  |
    # +--------+----+--------+
    # | 7F1700 | eq | 7F0000 |
    # +--------+----+--------+
# Failed test 'Protocol 31 - set LTECH_Test_11 saturation 100'
Error: # at /home/runner/work/RFFHEM/RFFHEM/t/FHEM/14_LTECH/01_set.t line 140.

sidey79 avatar Jul 24 '22 16:07 sidey79

@Devirex @sidey79 , ich erhebe mal die Hand, um anzufragen wo es klemmt, das es nicht weiter geht ;-)

HomeAutoUser avatar Dec 07 '22 17:12 HomeAutoUser

@HomeAutoUser Ok, wenn von meiner Seite aus noch was fehlt dann bitte Bescheid geben

Devirex avatar Apr 26 '23 11:04 Devirex