Added Module to Support LTECH

Devirex commented 2 years ago

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:

codecov[bot] commented 2 years ago

Codecov Report

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

Project coverage is 88.92%. Comparing base (910f5eb) to head (2d02ef0). Report is 58 commits behind head on master.

Additional details and impacted files

```diff @@ 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](https://app.codecov.io/gh/RFD-FHEM/RFFHEM/pull/1098/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RFD-FHEM) | Coverage Δ | | |---|---|---| | [fhem](https://app.codecov.io/gh/RFD-FHEM/RFFHEM/pull/1098/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RFD-FHEM) | `88.92% <ø> (-0.33%)` | :arrow_down: | | [modules](https://app.codecov.io/gh/RFD-FHEM/RFFHEM/pull/1098/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RFD-FHEM) | `88.92% <ø> (-0.33%)` | :arrow_down: | | [perl](https://app.codecov.io/gh/RFD-FHEM/RFFHEM/pull/1098/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RFD-FHEM) | `88.92% <ø> (-0.33%)` | :arrow_down: | | [unittests](https://app.codecov.io/gh/RFD-FHEM/RFFHEM/pull/1098/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RFD-FHEM) | `88.92% <ø> (-0.33%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RFD-FHEM#carryforward-flags-in-the-pull-request-comment) to find out more.

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

sidey79 commented 2 years ago

@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.

Devirex commented 2 years ago

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

sidey79 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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.

Devirex commented 2 years ago

@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

sidey79 commented 2 years ago

@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 :)

Devirex commented 2 years ago

@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 commented 2 years ago

@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

HomeAutoUser commented 2 years ago

Wie ist hier der Stand @Devirex ?

Devirex commented 2 years ago

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

sidey79 commented 2 years ago

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?

HomeAutoUser commented 2 years ago

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?

Devirex commented 2 years ago

Habe einen PR für die Testdaten erstellt:

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

sidey79 commented 2 years ago

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.

HomeAutoUser commented 1 year ago

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

Devirex commented 1 year ago

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

RFD-FHEM / RFFHEM

Added Module to Support LTECH #1098

Codecov Report