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

Wrong operator for eq 00_SIGNALduino #835

Closed
2 tasks
sidey79 opened this issue Apr 15, 2020 · 14 comments · Fixed by #864
Closed
2 tasks

Wrong operator for eq 00_SIGNALduino #835

sidey79 opened this issue Apr 15, 2020 · 14 comments · Fixed by #864

Comments

@sidey79
Copy link
Contributor

sidey79 commented Apr 15, 2020

if (length($rvosv2byte) eq 8) {

https://perldoc.perl.org/perlop.html#Equality-Operators
Binary "eq" returns true if the left argument is stringwise equal to the right argument.

Todo:

  • Test erstellen
  • Fehler beheben
@HomeAutoUser
Copy link
Contributor

Für was möchtest du da einen Test erstellen? Ein Test für diese Zeile ist überflüssig. Es würde nur Sinn machen, wenn ein Test sämtliche „ eq number“ aufdecken würde.

Korrigieren ja.

@sidey79
Copy link
Contributor Author

sidey79 commented Apr 15, 2020

Naja erstellen trifft es vielleicht nicht, aber den Test für diese Sub SIGNALduino_OSV2 ansehen und entscheiden ob dieser Fehler testbar ist.

@sidey79
Copy link
Contributor Author

sidey79 commented Apr 15, 2020

Hab auch noch Kandidaten für Nummern gefunden

RFFHEM/FHEM/00_SIGNALduino.pm

Lines 4005 to 4010 in a848db7

if ($channel == "00") { # in 0 LSB first
$newBitData .= "0001"; # out 1 MSB first
} elsif ($channel == "10") { # in 4 LSB first
$newBitData .= "0010"; # out 2 MSB first
} elsif ($channel == "01") { # in 4 LSB first
$newBitData .= "0011"; # out 3 MSB first

@HomeAutoUser
Copy link
Contributor

Bitte testen @sidey79 was Perl interpretiert.

Channel == 00 könnte als Channel == 0 gedeutet werden.
Nicht das Channel eq ´00´ gemeint ist. (Doppelte 0). Verstehst du mein Ansinnen?

@HomeAutoUser
Copy link
Contributor

HomeAutoUser commented Apr 15, 2020

Im Falle von #835 (comment) wäre es ‚eq‘ weil die Prüfung auf einen String mit der Länge 2 stattfindet.

my $channel = substr($bitData,6,2); # Byte 2 h: Channel

@sidey79
Copy link
Contributor Author

sidey79 commented Apr 15, 2020

@HomeAutoUser

Perl überrascht mich. Alle Varianten, auch die, welche nicht korrekt sind, liefern ein korrektes Ergebnis:

https://rextester.com/live/ALCOOC96500

@sidey79
Copy link
Contributor Author

sidey79 commented Apr 15, 2020

Damit hat sich das Thema 'Test anpassen" irgendwie erledigt

@HomeAutoUser
Copy link
Contributor

HomeAutoUser commented Apr 15, 2020

:) ja, irgendwie ist es nur eine Ansichtssache wie man persönlich argumentiert.

Da kannst du dich eher den anderen Tests widmen ;-)

@sidey79
Copy link
Contributor Author

sidey79 commented Apr 15, 2020

Laut Perdoc ist die aktuell verwendete Variante defintitv falsch oder seht ihr das anders?

@HomeAutoUser
Copy link
Contributor

HomeAutoUser commented Apr 15, 2020

Laut Erläuterungen sind

  1. Strings im Vergleich mit Bsp. eq zu vergleichen
  2. Nummerische Werte mit Bsp. == zu vergleichen.

Hier #835 (comment) wäre es ein Misch aus beiden. (Somit nicht ‚eindeutig‘ aber geht)

@sidey79
Copy link
Contributor Author

sidey79 commented Apr 15, 2020

Hier sind ein paar ganz gute Beispiele wie ich finde:
https://perlmaven.com/comparing-scalars-in-perl

sidey79 added a commit that referenced this issue Apr 15, 2020
some operator fixes #835
@sidey79
Copy link
Contributor Author

sidey79 commented Apr 15, 2020

wir haben noch etliche mehr solcher Stelle in unseren Modulen :(

@HomeAutoUser
Copy link
Contributor

Stellen sammeln / notieren und ändern. Sowas bleibt nicht aus bei Übernahme / Erweiterung / C&P Zeiten ...

@sidey79
Copy link
Contributor Author

sidey79 commented Apr 15, 2020

Hab jetzt alle die ich mittels grep finden konnte korrigiert:

https://github.com/RFD-FHEM/RFFHEM/tree/dev-r34_operatorFixes

@sidey79 sidey79 mentioned this issue Apr 21, 2020
3 tasks
@sidey79 sidey79 closed this as completed May 25, 2020
@sidey79 sidey79 mentioned this issue Jul 4, 2020
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants