Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix 898 #899

Merged
merged 5 commits into from
Oct 8, 2020
Merged

Bugfix 898 #899

merged 5 commits into from
Oct 8, 2020

Conversation

sidey79
Copy link
Contributor

@sidey79 sidey79 commented Oct 7, 2020

  • 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)
  • CHANGED has been updated (needed for bug fixes / features)
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Bugfix

  • What is the current behavior? (You can also link to an open issue here)

#898

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

Sub checks if parameters are provided

  • 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
Copy link

codecov bot commented Oct 7, 2020

Codecov Report

Merging #899 into master will increase coverage by 0.03%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #899      +/-   ##
==========================================
+ Coverage   54.14%   54.17%   +0.03%     
==========================================
  Files          60       61       +1     
  Lines        7933     7943      +10     
  Branches     1277     1279       +2     
==========================================
+ Hits         4295     4303       +8     
  Misses       2846     2846              
- Partials      792      794       +2     
Flag Coverage Δ
#fhem 91.37% <ø> (ø)
#modules 91.37% <ø> (ø)
#perl 91.37% <ø> (ø)
#unittest 54.17% <75.00%> (+0.03%) ⬆️
#unittests 91.37% <ø> (ø)

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

Impacted Files Coverage Δ
FHEM/00_SIGNALduino.pm 60.84% <25.00%> (-0.07%) ⬇️
.../00_SIGNALduino/01_SIGNALduino_Get_Command_CCReg.t 100.00% <100.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 9328a20...d6bdd1c. Read the comment docs.

@coveralls
Copy link

coveralls commented Oct 7, 2020

Pull Request Test Coverage Report for Build 3833

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.01%) to 53.68%

Totals Coverage Status
Change from base Build 3813: 0.01%
Covered Lines: 3530
Relevant Lines: 6576

💛 - Coveralls

@sidey79 sidey79 force-pushed the bugfix-898 branch 2 times, most recently from cd85be5 to 3f4004e Compare October 8, 2020 11:50
bugfix: get ccreg command caused stacktrace #898
@sidey79 sidey79 marked this pull request as ready for review October 8, 2020 11:53
@sidey79 sidey79 added the bug label Oct 8, 2020
@sidey79 sidey79 modified the milestone: 3.5.0 Oct 8, 2020
@elektron-bbs
Copy link
Contributor

Diese Änderung funktioniert bei mir nicht:

sub SIGNALduino_Get_Command_CCReg {
  my ($hash, @a) = @_;
  my $name=$hash->{NAME};
  Log3 ($hash->{NAME}, 3, "$name: a[0]=$a[0], a[1]=$a[1], $a[2]=$a[2],"); 
	return 'not enough number of arguments' if $#a < 1;	
  return 'Wrong command provided' if $a[0] ne 'ccreg';
  # if (exists($cc1101_register{uc($_[2])}) || $_[2] =~ /^99$/ ) {
  if (exists($cc1101_register{uc($_[1])}) || $_[1] =~ /^99$/ ) {
    return SIGNALduino_Get_Command(@_);
  } else {
   # return "unknown Register $_[2], please choose a valid cc1101 register";
    return "unknown Register $_[1], please choose a valid cc1101 register";
  }
}

Bei "get sduinoIP ccreg 13" erhalte ich:

unknown Register ccreg, please choose a valid cc1101 register

Vorher hat es funktioniert. Ich verwende Version 3.5.0 aus dem Master.

So funktioniert es bei mir wieder:

sub SIGNALduino_Get_Command_CCReg {
  my ($hash, @a) = @_;
  my $name=$hash->{NAME};
  return 'not enough number of arguments' if $#a < 1;	
  return 'Wrong command provided' if $a[0] ne 'ccreg';
  # if (exists($cc1101_register{uc($_[2])}) || $_[2] =~ /^99$/ ) {
  if (exists($cc1101_register{uc($a[1])}) || $a[1] =~ /^99$/ ) {
    return SIGNALduino_Get_Command(@_);
  } else {
   # return "unknown Register $_[2], please choose a valid cc1101 register";
    return "unknown Register $a[1], please choose a valid cc1101 register";
  }
}

my ($hash, @a) = @_;
return 'not enough number of arguments' if $#a < 1;
return 'Wrong command provided' if $a[0] != 'ccreg';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Das erzeugt eine Warnung - besser so:

return 'Wrong command provided' if $a[0] ne 'ccreg';

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ja da hast Du recht

@sidey79
Copy link
Contributor Author

sidey79 commented Oct 8, 2020

So funktioniert es bei mir wieder:

sub SIGNALduino_Get_Command_CCReg {
  my ($hash, @a) = @_;
  my $name=$hash->{NAME};
  return 'not enough number of arguments' if $#a < 1;	
  return 'Wrong command provided' if $a[0] ne 'ccreg';
  # if (exists($cc1101_register{uc($_[2])}) || $_[2] =~ /^99$/ ) {
  if (exists($cc1101_register{uc($a[1])}) || $a[1] =~ /^99$/ ) {
    return SIGNALduino_Get_Command(@_);
  } else {
   # return "unknown Register $_[2], please choose a valid cc1101 register";
    return "unknown Register $a[1], please choose a valid cc1101 register";
  }
}

Was genau möchtest Du mir damit mitteilen?
So recht verstanden, worin der Unterschied zu diesem PR besteht habe ich leider nicht .

@elektron-bbs
Copy link
Contributor

Der Unterschied besteht darin:
$_[1]
in deiner Variante wird:
$a[1]
in meiner.

@sidey79
Copy link
Contributor Author

sidey79 commented Oct 8, 2020

Der Unterschied besteht darin:
$_[1]
in deiner Variante wird:
$a[1]
in meiner.

Kannst Du die Zeilennummer referenzieren?
Ich habe da wohl Tomaten auf den Augen .

my $name=$hash->{NAME};
if (exists($cc1101_register{uc($_[2])}) || $_[2] =~ /^99$/ ) {
if (exists($cc1101_register{uc($a[1])}) || $a[1] =~ /^99$/ ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Die hier meinte ich. Aber sorry, ich habe mich vertan. Ich hatte bei mir nur die 2 in eine 1 geändert. Hat sich erledigt.

@sidey79 sidey79 merged commit 5c8a996 into master Oct 8, 2020
@sidey79 sidey79 deleted the bugfix-898 branch October 8, 2020 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants