Fixes perlcritic prototypes in 14_SD_WS part 2 #1013

elektron-bbs · 2021-08-30T19:05:08Z

Please check if the PR fulfills these requirements

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, ...)

What is the current behavior?
(You can also link to an open issue here, if this describes the current behavior)
Perlcritic criticizes used subroutine prototypes.
What is the new behavior (if this is a feature change)?
Perlcritic found no violations in level 4.
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
no
Other information:

codecov · 2021-08-30T19:08:56Z

Codecov Report

Merging #1013 (327ecce) into master (ed33e93) will increase coverage by 0.00%.
The diff coverage is 62.50%.

@@           Coverage Diff           @@
##           master    #1013   +/-   ##
=======================================
  Coverage   50.04%   50.05%           
=======================================
  Files         218      218           
  Lines       15276    15282    +6     
  Branches     2856     2859    +3     
=======================================
+ Hits         7645     7649    +4     
+ Misses       5995     5994    -1     
- Partials     1636     1639    +3

Flag	Coverage Δ
fhem	`50.24% <62.50%> (-0.01%)`	⬇️
modules	`50.05% <62.50%> (+<0.01%)`	⬆️
perl	`51.01% <ø> (+0.01%)`	⬆️
unittests	`50.05% <62.50%> (+<0.01%)`	⬆️

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

Impacted Files	Coverage Δ
FHEM/14_SD_WS.pm	`64.16% <62.50%> (-0.13%)`	⬇️
local/lib/perl5/Test2/Util/HashBase.pm	`46.87% <0.00%> (+1.56%)`	⬆️

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 ed33e93...327ecce. Read the comment docs.

sidey79 · 2021-08-30T19:52:06Z

FHEM/14_SD_WS.pm

@@ -1511,7 +1512,7 @@ sub SD_WS_LFSR_digest8_reflect($$$$) {
 }

 #############################
-sub SD_WS_bin2dec($) {
+sub SD_WS_bin2dec {
  my $h = shift;


Willst Du hier nicht noch eine Validierung hinzufügen ob etwas übergeben wurde, bzw. einen definierten Abbruch, wenn nichts übergeben wurde.

my $h = shift // return ;

Ähnliche Dinge für die anderen subs, ich kommentiere es nicht überall.

Hast Du bei den subs mit zwei oder mehr Parametern auch auf dem Schirm?

Ja, aber bei diesen beiden fehlt mir eine Lösung:

$lbit=shift if @_;
my ($bytes, $gen, $key, $rawData) = @_;

Wie wäre es damit zum Beispiel?

my $bytes = shift // carp 'no bytes data proided'; my $gen = shift // carp 'no gen provided'; my $key = shift // carp 'no key provided'; my $rawData = shift // carp 'no raw msg provided';

Den Teil:
$lbit=shift if @_;

habe ich entweder nicht verstanden, oder später folgt ein warning wenn $lbit undef ist.
Aber selbst das ganze auf 0 setzen dürfte kein brauchbares Ergebnis am Ende liefern oder?

$lbit=shift // 0;

Den Teil:
$lbit=shift if @_;

habe ich entweder nicht verstanden, oder später folgt ein warning wenn $lbit undef ist.

Sorry, hier habe ich etwas übersehen. $lbit wird eine Zeile darüber schon initialisiert:

sub SD_WS_binaryToNumber { my $binstr=shift // return; my $fbit=shift // return; my $lbit=$fbit; $lbit=shift if @_; return oct("0b".substr($binstr,$fbit,($lbit-$fbit)+1)); }

Das dürfte sich damit erledigt haben.

sidey79 · 2021-08-30T19:52:21Z

FHEM/14_SD_WS.pm

@@ -1527,7 +1528,7 @@ sub SD_WS_binaryToNumber {
 }

 #############################
-sub SD_WS_WH2CRCCHECK($) {
+sub SD_WS_WH2CRCCHECK {
  my $rawData = shift;


Gleiche Frage wie zuvor.

sidey79 · 2021-08-30T19:52:29Z

FHEM/14_SD_WS.pm

@@ -1538,7 +1539,7 @@ sub SD_WS_WH2CRCCHECK($) {
 }

 #############################
-sub SD_WS_WH2SHIFT($){
+sub SD_WS_WH2SHIFT {
  my $rawData = shift;


sidey79 · 2021-09-01T20:17:29Z

FHEM/14_SD_WS.pm

-sub SD_WS_LFSR_digest8_reflect($$$$) {
-  my ($bytes, $gen, $key, $rawData) = @_;
+sub SD_WS_LFSR_digest8_reflect {
+  my $bytes = shift // carp 'no bytes data provided';


Das ganze geht auch mit nur einer Zeile ausreichend Informativ:

croak qq['Usage: SD_WS_LFSR_digest8_reflect ($bytes, $gen, $key, $rawData)';] if @_ != 4;

Ist das so gewollt, das bei Nutzung von croak bei fehlerhaften Übergaben dann FHEM neu startet???
Egal, wie ich das auch probiere, erfolgt immer ein Neustart.

my $bytes = shift // croak 'Error: called without $hexdata as input'; my $gen = shift // croak 'Error: called without $hexdata as input'; my $key = shift // croak 'Error: called without $hexdata as input'; my $rawData = shift // croak 'Error: called without $hexdata as input'; # croak 'Usage: SD_WS_LFSR_digest8_reflect' if @_ != 4; # croak qq['Usage: SD_WS_LFSR_digest8_reflect ($bytes, $gen, $key, $rawData)';] if @_ != 4;

Wenn dem so ist, dann geht das gar nicht!

carp wäre ja noch halbwegs zu ertragen, auch wenn es das Log ziemlich zumüllt.

Wenn wir schon solche (für mich eigentlich sinnlosen Prüfungen) einbauen wollen, dann wäre ein simples return noch das Beste. Das liefert mir eine PERL WARNING mit der Zeilennummer wo der Aufruf der sub erfolgt und gut ist:

2021.09.02 17:58:32 4: sduino_dummy: get rawmsg: �MU;P1=247;P2=-750;P3=722;P4=-489;P5=491;P6=-236;P7=-2184;D=1232141456565656145656141456565614141456141414145656141414141456561414141456561414145614561456145614141414141414145614145656145614141732321414565656561456561414565656141414561414141456561414141414565614141414565614141456145614561456141414141414141456141;CP=1;R=55;O;� 2021.09.02 17:58:32 4: sduino_dummy: Parse_MU, Fingerprint for MU protocol id 54 -> TFA 30.3233.01 matches, trying to demodulate 2021.09.02 17:58:32 4: sduino_dummy: Parse_MU, last part pair=1 reconstructed, bit=0 2021.09.02 17:58:32 4: sduino_dummy: Parse_MU, Decoded matched MU protocol id 54 dmsg W54#3D9C430618AA01340 length 68 dispatch(1/4) RSSI = -46.5 2021.09.02 17:58:32 4: sduino_dummy: SD_WS_Parse protocol 54, rawData 3D9C430618AA01340 2021.09.02 17:58:32 1: PERL WARNING: Use of uninitialized value $checksum in numeric eq (==) at ./FHEM/14_SD_WS.pm line 467. 2021.09.02 17:58:32 1: PERL WARNING: Use of uninitialized value $checksum in concatenation (.) or string at ./FHEM/14_SD_WS.pm line 470. 2021.09.02 17:58:32 3: sduino_dummy: SD_WS_54 Parse msg W54#3D9C430618AA01340 - ERROR checksum != 52 2021.09.02 17:58:32 4: sduino_dummy: SD_WS_Parse 3D9C430618AA01340 protocolid 54 (TFA 30.3233.01) - ERROR CRC

Ja, croak beendet den aktuellen scope.
Das hatte ich nicht bedacht und da das Modul in main läuft, wird eben main beendet.
Croak funktioniert hingegen gut, wenn Module nicht in main laufen.

Für diesen Fall ist carp besser geeignet, da es den laufenden Prozess nicht beendet aber den Aufrufer der Funktion bekannt gibt.

FHEM/14_SD_WS.pm

sidey79 · 2021-09-01T20:20:08Z

FHEM/14_SD_WS.pm


-sub SD_WS_Initialize($)
+sub SD_WS_Initialize
 {
  my ($hash) = @_;


Alternatv:
my $hash = shift // return;

sidey79 · 2021-09-01T20:21:10Z

FHEM/14_SD_WS.pm

-  my $binstr=shift;
-  my $fbit=shift;
+  my $binstr=shift // return;
+  my $fbit=shift // return;
  my $lbit=$fbit;
  $lbit=shift if @_;


Ob das zum gleichen Ergebnis führt?
$lbit = shift // $lbit;

Scheint zu funktionieren.

Perlcritic: Always unpack @_ first

elektron-bbs · 2021-09-08T15:29:10Z

@sidey79 bitte nochmal ein review, Github wollte erst noch ein Merge branch 'master'.

elektron-bbs and others added 3 commits August 30, 2021 20:48

Fixes perlcritic prototypes in 14_SD_WS

ed93766

Automatic updated controls and CHANGED

6017e0d

Update Versiondate

c07eb1a

elektron-bbs requested a review from sidey79 August 30, 2021 19:20

sidey79 reviewed Aug 30, 2021

View reviewed changes

elektron-bbs and others added 4 commits August 31, 2021 20:39

Update 14_SD_WS.pm

0897d20

Update Versiondate

ed619d0

Update 14_SD_WS.pm

e9c143f

Update Versiondate

d9ba636

sidey79 reviewed Sep 1, 2021

View reviewed changes

FHEM/14_SD_WS.pm Show resolved Hide resolved

sidey79 reviewed Sep 1, 2021

View reviewed changes

FHEM/14_SD_WS.pm Show resolved Hide resolved

sidey79 reviewed Sep 1, 2021

View reviewed changes

elektron-bbs and others added 4 commits September 4, 2021 13:15

Update 14_SD_WS.pm

dfbcf48

Update Versiondate

7b4d7e0

Update 14_SD_WS.pm

df649b5

Perlcritic: Always unpack @_ first

Update Versiondate

da32e02

sidey79 previously approved these changes Sep 7, 2021

View reviewed changes

elektron-bbs and others added 2 commits September 8, 2021 17:08

Merge branch 'master' into master_critic_SD_WS_part2

619367d

Update Versiondate

327ecce

sidey79 dismissed their stale review via 327ecce September 8, 2021 15:09

sidey79 approved these changes Sep 8, 2021

View reviewed changes

elektron-bbs merged commit b934d4c into master Sep 8, 2021

elektron-bbs deleted the master_critic_SD_WS_part2 branch October 15, 2021 15:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes perlcritic prototypes in 14_SD_WS part 2 #1013

Fixes perlcritic prototypes in 14_SD_WS part 2 #1013

elektron-bbs commented Aug 30, 2021

codecov bot commented Aug 30, 2021 •

edited

Loading

sidey79 Aug 30, 2021 •

edited

Loading

sidey79 Aug 31, 2021

elektron-bbs Aug 31, 2021

sidey79 Aug 31, 2021

sidey79 Aug 31, 2021

elektron-bbs Sep 1, 2021

sidey79 Aug 30, 2021

sidey79 Aug 30, 2021

sidey79 Sep 1, 2021

elektron-bbs Sep 2, 2021

sidey79 Sep 4, 2021

sidey79 Sep 1, 2021

sidey79 Sep 1, 2021

elektron-bbs Sep 4, 2021

elektron-bbs commented Sep 8, 2021

Fixes perlcritic prototypes in 14_SD_WS part 2 #1013

Fixes perlcritic prototypes in 14_SD_WS part 2 #1013

Conversation

elektron-bbs commented Aug 30, 2021

codecov bot commented Aug 30, 2021 • edited Loading

Codecov Report

sidey79 Aug 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elektron-bbs commented Sep 8, 2021

codecov bot commented Aug 30, 2021 •

edited

Loading

sidey79 Aug 30, 2021 •

edited

Loading