sidey79 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)
[ ] commandref has been added / updated (needed for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
[x] Bugfix (please link issue)
[x] Feature enhancement
[ ] Documentation update
[ ] Unittest enhancement
[ ] other
What is the current behavior? (You can also link to an open issue here, if this describes the current behavior)

If Digest::CRC Module is not available, the Module is loaded and startup errors are logged

BEGIN failed--compilation aborted at ./FHEM/00_SIGNALduino.pm line 27, <$fh> line 49.
 Can't locate Digest/CRC.pm in @INC (you may need to install the Digest::CRC module) (@INC contains: ./lib ./FHEM . /etc/perl /usr/local/lib/arm-linux-gnueabihf/perl/5.28.1 /usr/local/share/perl/5.28.1 /usr/lib/arm-linux-gnueabihf/perl5/5.28 /usr/share/perl5 /usr/lib/arm-linux-gnueabihf/perl/5.28 /usr/share/perl/5.28 /usr/local/lib/site_perl /usr/lib/arm-linux-gnueabihf/perl-base ./FHEM/lib) at ./FHEM/00_SIGNALduino.pm line 27, <$fh> line 49.
2022.01.18 20:58:57.371 1: reload: Error:Modul 00_SIGNALduino deactivated:

What is the new behavior (if this is a feature change)?

The SIGNALduino Module no longer depends on the CRC Module

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

maybe it causes an error in the libs, because they require CRC for some subs

Other information:
https://forum.fhem.de/index.php/topic,125541.0.html
https://forum.fhem.de/index.php/topic,58397.msg1201448.html#msg1201448

927

codecov[bot] commented 2 years ago

Codecov Report

Merging #1066 (c9fa62f) into master (bfd6b06) will increase coverage by 0.27%. The diff coverage is 93.75%.

@@            Coverage Diff             @@
##           master    #1066      +/-   ##
==========================================
+ Coverage   62.18%   62.45%   +0.27%     
==========================================
  Files         126      129       +3     
  Lines        9332     9371      +39     
  Branches     1475     1478       +3     
==========================================
+ Hits         5803     5853      +50     
+ Misses       2429     2415      -14     
- Partials     1100     1103       +3

Flag	Coverage Δ
fhem	`53.83% <87.50%> (+0.35%)`	:arrow_up:
modules	`62.45% <93.75%> (+0.27%)`	:arrow_up:
perl	`91.85% <100.00%> (+0.01%)`	:arrow_up:
unittests	`62.45% <93.75%> (+0.27%)`	:arrow_up:

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

Impacted Files	Coverage Δ
t/FHEM/00_SIGNALduino/01_SIGNALduino_Parse_MN.t	`100.00% <ø> (ø)`
FHEM/00_SIGNALduino.pm	`64.10% <85.71%> (+1.10%)`	:arrow_up:
FHEM/lib/SD_Protocols.pm	`79.81% <100.00%> (+0.09%)`	:arrow_up:
t/FHEM/00_SIGNALduino/00_SIGNALduino_initialize.t	`100.00% <100.00%> (ø)`
FHEM/10_SD_Rojaflex.pm	`68.69% <0.00%> (-2.85%)`	:arrow_down:
t/FHEM/14_SD_WS07/09_parseDatat.t
t/FHEM/14_FLAMINGO/09_parseDatat.t	`80.00% <0.00%> (ø)`
t/FHEM/14_FLAMINGO/00_load.t	`100.00% <0.00%> (ø)`
t/FHEM/14_SD_AS/00_load.t	`100.00% <0.00%> (ø)`
t/FHEM/14_SD_BELL/09_parseDatat.t	`80.00% <0.00%> (ø)`
... and 1 more

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 bfd6b06...c9fa62f. Read the comment docs.

elektron-bbs commented 2 years ago

So ganz "Graceful" ist das noch nicht :-) So, wie es jetzt ist, schmiert FHEM bei jedem Versuch Digest::CRC zu nutzen, ab:

Can't locate object method "new" via package "Digest::CRC" at FHEM/lib/SD_Protocols.pm line 1989.
2022.01.19 15:02:25.304 1: Including fhem.cfg
2022.01.19 15:02:25.709 3: WEB: port 8083 opened

und Dank systemd passiert das dann alle paar Sekunden wieder:

2022.01.19 15:02:49.125 3: LaCrosseGateway device opened
Can't locate object method "new" via package "Digest::CRC" at FHEM/lib/SD_Protocols.pm line 1989.
2022.01.19 15:02:51.274 1: Including fhem.cfg
2022.01.19 15:02:51.589 3: WEB: port 8083 opened
...
2022.01.19 15:03:14.296 3: sduino_USB0: CheckVersionResp, enable receiver (XE) 
Can't locate object method "new" via package "Digest::CRC" at FHEM/lib/SD_Protocols.pm line 1989.
2022.01.19 15:03:15.634 1: Including fhem.cfg
2022.01.19 15:03:15.858 3: WEB: port 8083 opened

Es müsste auf jeden Fall in jeder sub, die Digest::CRC verwendet ein return in folgender Art hinein:

  return ( 1,'ConvLaCrosse, Modul CRC not loaded, please install modul Digest::CRC' )
    if (!HAS_DigestCRC);

Damit die Fehlermeldungen auch sichtbar werden, hatte ich in der 00_SIGNALduino.pm sub SIGNALduino_Parse_MN schon mal diese Zeilen angepasst:

    if ($#methodReturn != 0) {
      my $vb = $methodReturn[0];
      $hash->{logMethod}->($name, $vb, qq{$name: Parse_MN, Error! method $methodReturn[1]});
      next mnIDLoop;
    }

sidey79 commented 2 years ago

@elektron-bbs

Mach doch einen commit mit den Anpassung.

Ich war gestern ohnehin überrascht, dass es nicht mehr so ist, wie wir es in #927 hinterlassen hatten.

elektron-bbs commented 2 years ago

Wir hatten ja in dem Issue keine Übereinstimmung gefunden, deshalb war alles so geblieben, wie es war. Ich mache ein commit.

elektron-bbs commented 2 years ago

Falls wir das mit meiner Änderung so lassen wollen, müssten wir sicher in der SD_Protocols.pm noch die verbose-level anpassen.

sidey79 commented 2 years ago

Falls wir das mit meiner Änderung so lassen wollen, müssten wir sicher in der SD_Protocols.pm noch die verbose-level anpassen.

Was meinst Du damit? Der Returncode 1 ist ein Fehlercode und kein VerboseLevel :)

elektron-bbs commented 2 years ago

Ich habe diesen jetzt aber gleich für den Verbose-Level verwendet

    if ($#methodReturn != 0) {
      my $vb = $methodReturn[0];
      $hash->{logMethod}->($name, $vb, qq{$name: Parse_MN, Error! method $methodReturn[1]});
      next mnIDLoop;
    }

da wir sonst Fehlermeldungen nie zu sehen bekommen.

sidey79 commented 2 years ago

da wir sonst Fehlermeldungen nie zu sehen bekommen.

Habe ich gesehen, so ist das nicht gedacht. Eher so wie exit(0) kein Fehler exit(1) einen Fehlercode 1 liefert usw dann auch Fehlercode 2,3,4,5,6

sidey79 commented 2 years ago

Sieht für mich jetzt gut aus:

2022.01.22 01:16:12.899 4: dummyDuino: Parse_MN, 2-FSK Protocol id 116.1 WH57 msg 1E6C20B00C1618FF99FF0458FFFFA9FF015B0000 not match (?^:^57)
2022.01.22 01:16:12.893 4: dummyDuino: Parse_MN, 2-FSK Protocol id 116 WH57 msg 1E6C20B00C1618FF99FF0458FFFFA9FF015B0000 not match (?^:^57)
2022.01.22 01:16:12.887 1: dummyDuino: Parse_MN, Error! method ConvBresser_6in1, missing module , please install modul Digest::CRC
2022.01.22 01:16:12.881 4: dummyDuino: Parse_MN, Found 2-FSK Protocol id 115 -> Bresser 6in1
2022.01.22 01:16:12.875 4: dummyDuino: Parse_MN, Error! id 112 msg=1E6C20B00C1618FF99FF0458FFFFA9FF015B0000, message is to long
2022.01.22 01:16:12.870 4: dummyDuino: Parse_MN, Error! id 109 msg=1E6C20B00C1618FF99FF0458FFFFA9FF015B0000, message is to long
2022.01.22 01:16:12.864 4: dummyDuino: Parse_MN, Error! id 108 msg=1E6C20B00C1618FF99FF0458FFFFA9FF015B0000, message is to short
2022.01.22 01:16:12.858 4: dummyDuino: Parse_MN, 2-FSK Protocol id 107.1 WH51 868.35 MHz msg 1E6C20B00C1618FF99FF0458FFFFA9FF015B0000 not match (?^:^51)
2022.01.22 01:16:12.852 4: dummyDuino: Parse_MN, 2-FSK Protocol id 107 WH51 433.92 MHz msg 1E6C20B00C1618FF99FF0458FFFFA9FF015B0000 not match (?^:^51)
2022.01.22 01:16:12.846 4: dummyDuino: Parse_MN, 2-FSK Protocol id 103 Lacrosse mode 2 msg 1E6C20B00C1618FF99FF0458FFFFA9FF015B0000 not match (?^:^9)
2022.01.22 01:16:12.840 4: dummyDuino: Parse_MN, GFSK Protocol id 102 KoppFreeControl msg 1E6C20B00C1618FF99FF0458FFFFA9FF015B0000 not match (?^:^0)
2022.01.22 01:16:12.834 1: dummyDuino: Parse_MN, Error! method ConvPCA301, missing module , please install modul Digest::CRC
2022.01.22 01:16:12.828 4: dummyDuino: Parse_MN, Found 2-FSK Protocol id 101 -> PCA 301
2022.01.22 01:16:12.822 4: dummyDuino: Parse_MN, 2-FSK Protocol id 100 Lacrosse mode 1 msg 1E6C20B00C1618FF99FF0458FFFFA9FF015B0000 not match (?^:^9)

elektron-bbs commented 2 years ago

So geht es natürlich auch erstmal. Ich finde aber die Variante, die Zahl direkt als Verbose-Level zu verwenden, flexibler. Wenn wir mal eine andere Meldung im Log sehen wollen, müssen wir immer erst das Regex ergänzen.

sidey79 commented 2 years ago

@elektron-bbs

Ich würde es auf die schnelle erst einmal so belassen und dann ggf. auch heute noch SVN aktualisieren

RFD-FHEM / RFFHEM

Graceful load JSON and Digest:CRC Modules #1066

927

Codecov Report