Bugfix define with hostname 901 #904

sidey79 · 2020-10-27T20:39:55Z

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 and improve error handling
What is the current behavior? (You can also link to an open issue here)

Defining a SIGNALduino with a hostname and not with a path or ip address causes an error because the baudrate is wrongly added to the hostname, because it is not detected as a network reachable hostname.

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

The new behavior is, that the regex to detect direct serial attatched devices are detected with a valid regex.
Network attatched devices a checked if the complain to rfcs (ipv4 address valid / hostname valid)

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

No no breaking change

Other information:
issue Network connected Signalduino does not connect #901

Added serval testcases for define options #901

Added some regex checks to catch most errors with in definition

codecov · 2020-10-27T20:40:06Z

Codecov Report

Merging #904 into master will increase coverage by 0.10%.
The diff coverage is 91.66%.

@@            Coverage Diff             @@
##           master     #904      +/-   ##
==========================================
+ Coverage   54.17%   54.27%   +0.10%     
==========================================
  Files          61       62       +1     
  Lines        7943     7954      +11     
  Branches     1279     1279              
==========================================
+ Hits         4303     4317      +14     
+ Misses       2846     2842       -4     
- Partials      794      795       +1

Flag	Coverage Δ
#fhem	`91.37% <ø> (ø)`
#modules	`91.37% <ø> (ø)`
#perl	`91.37% <ø> (ø)`
#unittest	`54.27% <91.66%> (+0.10%)`	⬆️
#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	`61.09% <80.00%> (+0.24%)`	⬆️
t/FHEM/00_SIGNALduino/01_SIGNALduino_Define.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 909bdb2...bb8514e. Read the comment docs.

coveralls · 2020-10-27T20:43:07Z

Pull Request Test Coverage Report for Build 3902

0 of 5 (0.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.2%) to 53.477%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
FHEM/00_SIGNALduino.pm	0	5	0.0%

Totals
Change from base Build 3895:	-0.2%
Covered Lines:	3499
Relevant Lines:	6543

💛 - Coveralls

elektron-bbs · 2020-10-27T21:02:26Z

FHEM/00_SIGNALduino.pm

+
+  if ($dev ne 'none' && ($dev =~ m~^(?:/[^/ ]*)+?$~xms || $dev =~ m~^COM\d$~xms) && $dev !~ m/\@/)  # bei einer IP oder hostname wird kein \@57600 angehaengt
+  {
+    $dev .= '\@57600' 


Den führenden Backslash hast du wieder übernommen. M.E. passt der nicht.

Extended regex to match all RFC valid hostnames and ipv4 addresses Added support for directio and none. Optimized code a little bit

elektron-bbs · 2020-10-28T16:51:18Z

Jetzt hast du zwar jede Menge Tests geschrieben, die deine Änderungen für gut befinden, aber vielleicht solltest du so etwas doch besser vorher am lebenden Objekt prüfen :-)

DEF  /dev/ttyUSB0
DeviceName /dev/ttyUSB0\@57600

2020.10.28 17:23:24 3: Opening sduinoUSB0 device /dev/ttyUSB0\
2020.10.28 17:23:24 1: sduinoUSB0: Can't open /dev/ttyUSB0\: No such file or directory

define sduinoIP SIGNALduino ESP-DB7D13-Testboard:23

2020.10.28 17:23:24 2: Define, wrong hostname/port syntax: define <name> SIGNALduino {none | devicename[\@baudrate] | devicename\@directio | hostname:port}
2020.10.28 17:23:24 1: define sduinoIP SIGNALduino ESP-DB7D13-Testboard:23: Define, wrong hostname/port syntax: define <name> SIGNALduino {none | devicename[\@baudrate] | devicename\@directio | hostname:port}

sidey79 · 2020-10-28T18:22:50Z

Ich war der Meinung alles enthalten zu haben.
Gut bemerkt. Ich ergänze das gerne :)

HomeAutoUser · 2020-10-28T19:47:26Z

Ich war der Meinung alles enthalten zu haben.
Gut bemerkt. Ich ergänze das gerne :)

Den Fokus vermutlich auf das Schreiben der Tests gesetzt anstatt TestItLive ;-)

sidey79 · 2020-10-28T21:09:38Z

Hmm, gewundert habe ich mich da schon, alledings dachte ich dass muss doch richtig sein so, auch wenn nicht wusste wieso ;)

War dann aber schon länger falsch. das '@57600' haben wir ja schon vor diesem PR gehabt.

Fixed baud rate added without specail chars Regex is now case insensitive Tests reworked to a loop with mockdata array

…gfix-901

sidey79 · 2020-10-28T21:32:03Z

Ich hatte all meine Sachen mit Kleinbuchstaben :) Jetzt hab ich das ausgemerzt

elektron-bbs · 2020-10-29T16:28:07Z

Einen habe ich noch:

DEF ESP-0CAD2F:23

Define, wrong hostname/port syntax: define <name> SIGNALduino {none | devicename[\@baudrate] | devicename\@directio | hostname:port}

Improvement of regex

sidey79 · 2020-10-29T20:32:24Z

@elektron-bbs

Die Regex hat es in sich :) Aber nun habe ich auch diese Fälle korrigiert.
Ein wunder, dass das wohl alles schon mal mehr oder minder funktioniert hat.

elektron-bbs · 2020-10-29T20:37:14Z

Eine Plausibilität wurde ja bisher nicht geprüft und es ist wohl auch noch niemand auf die Idee gekommen, die Baudrate nicht anzugeben oder den Hostnamen zu verwenden...
Das Regex ist tatsächlich "abenteuerlich" :-)

sidey79 · 2020-10-29T20:44:33Z

Hier kann die Regex getestet werden:
https://regex101.com/r/l5DTxy/2/

Ja ziemlich groß ist sie geworden. Dafür fischt sie aber auch Domänen raus, die nur aus einem Buchstaben bestehen (sowas gibt es nicht) und IP-Addressen die nicht vollständig sind.
Vielleicht packe ich die mal in eine Bibliothek um eine Funktion isValidHostDNSIP oder sowas in der Art :)

Release 3.5.2 10_SD_Rojaflex.pm new: Module for rojaflex remote controls 90_SIGNALduino_un.pm changed: fix some PerlCritic hints (#914) changed: revised commandref 00_SIGNALduino.pm: feature: xFSK processing feature: Added support for directio and none. feature: Extension for "get sduino ccreg" (#918) feature: parse subs optimized (#926) feature: update reading config when change settings (#948) feature: Allow incremental addition of match list entries (#1026) change: added N to send SN xFSK sendCommand change: added new sub SIGNALduino_calcRSSI to simplification code change: revised Parse_MN and loglevel change: revised logoutput text SIGNALduino_Get_Command change: rename "get raw" to "get rawmsg" (#925) feature: added commandref rfmode & cc1101_reg_user feature: added hardware ESP32cc1101, MAPLEMINI_F103CB on attribute feature: added new attrib rfmode to changed to xFSK & revised commandref feature: added separat sub SIGNALduino_Attr_rfmode feature: added set cmd LaCrossePairForSec (for LaCrosse bugfix: SIGNALduino_CheckccConfResponse is more robust #1015 (#1031) bugfix: fix PERL WARNING (#895) (#972) bugfix: get ccreg command caused stacktrace #898 bugfix: Bugfix define with hostname 901 (#904) bugfix: Wrong version assignment fixed Bugfix, module runs now without fhemweb instance bugfix: display protocol list (#947) bugfix: require 99_Utils only if really needed (#950) bugfix: corrected incorrect logoutput (#951) bugfix: Fix Multiple send delay (#941) bugfix: Fixes high CPU and MEM usage in patternExists (#988) SD_Protocols.pm: change: moved subs for converting in own package ConvLaCrosse, ConvKoppFreeControl and ConvPCA301 bugfix: Hideki fix inverted message (#974) SD_ProtocolData.pm feature: added rfmode, register rubric & comments change: fix perlcritic Severity 3 - hard tabs feature: Added crc checksum verification Revolt (#956) 14_SD_WS.pm: feature: protocol 27 for sensor EFS-3110A (#890) feature: protocol 106 for GT-TMBBQ-0 feature: protocol 110 for ADE WS1907 Weather station (#970) feature: protocol 111 for TS-FT002 water tank level (#1000) feature: protocol 113 for GFGT 433 B1 Wireless Grill sensor (#1003) feature: new protocol 108 for BRESSER 5-in-1 Weather Center and BRESSER Professional Rain Gauge (#973) feature: protocol 115 for Bresser 6-in-1 and 5-in-1 Comfort Wetter Center (#1010) feature: new protocol 107 for Fine Offset WH51 (#1055) feature: new protocol 116 for Fine Offset WH57 (#1061) bugfix: Update protocol 64 for sensor WH2A (#1009) bugfix: Conrad S522 protocol 33 no reading batteryState (#1042) 14_SD_WS07.pm: feature: protocol definition 7.1 for Mebus HQ7312-2 (#1050) 14_FLAMINGO.pm: change: Perlcritic (#887) 14_SD_UT.pm: change: PerlCritic (#877) feature: new protocol 105 for remote control BF-301 feature: decode and send protocol 56 remote control AC114-01B (#910) feature: decode and send protocol for Visivo remote control feature: Remote control SEAV BeSmart S4 (#933) feature: new protocol 114 for TR401 (#1002) 14_SD_BELL.pm: change: PerlCritic (#877) change: Adjusted little things (#937) feature: added AVANTEK Wireless doorbell & LED night light (#981)

sidey79 added 2 commits October 26, 2020 22:55

t\FHEM\00_SIGNALduino\01_SIGNALduino_Define.t

bb590b2

Added serval testcases for define options #901

FHEM\00_SIGNALduino.pm

934420e

Added some regex checks to catch most errors with in definition

sidey79 added the bug label Oct 27, 2020

sidey79 force-pushed the bugfix-901 branch from 45f645c to 3585cac Compare October 27, 2020 20:57

Automatic updated controls and CHANGED

415c34e

elektron-bbs reviewed Oct 27, 2020

View reviewed changes

sidey79 marked this pull request as ready for review October 27, 2020 21:06

FHEM\00_SIGNALduino.pm

3585cac

Extended regex to match all RFC valid hostnames and ipv4 addresses Added support for directio and none. Optimized code a little bit

sidey79 and others added 3 commits October 28, 2020 22:30

00_SIGNALduino.pm

860b049

Fixed baud rate added without specail chars Regex is now case insensitive Tests reworked to a loop with mockdata array

Merge branch 'bugfix-901' of ssh://github.com/RFD-FHEM/RFFHEM into bu…

a00c4b4

…gfix-901

Automatic updated controls and CHANGED

22286eb

sidey79 and others added 2 commits October 29, 2020 21:31

FHEM\00_SIGNALduino.pm

a18875f

Improvement of regex

Automatic updated controls and CHANGED

9696a30

Merge branch 'master' into bugfix-901

bb8514e

elektron-bbs approved these changes Oct 30, 2020

View reviewed changes

sidey79 merged commit 9c3b2c6 into master Oct 31, 2020

sidey79 deleted the bugfix-901 branch October 31, 2020 15:56

sidey79 linked an issue Oct 31, 2020 that may be closed by this pull request

Network connected Signalduino does not connect #901

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bugfix define with hostname 901 #904

Bugfix define with hostname 901 #904

sidey79 commented Oct 27, 2020 •

edited

Loading

codecov bot commented Oct 27, 2020 •

edited

Loading

coveralls commented Oct 27, 2020 •

edited

Loading

elektron-bbs Oct 27, 2020

elektron-bbs commented Oct 28, 2020

sidey79 commented Oct 28, 2020

HomeAutoUser commented Oct 28, 2020

sidey79 commented Oct 28, 2020 •

edited

Loading

sidey79 commented Oct 28, 2020

elektron-bbs commented Oct 29, 2020

sidey79 commented Oct 29, 2020

elektron-bbs commented Oct 29, 2020

sidey79 commented Oct 29, 2020

Bugfix define with hostname 901 #904

Bugfix define with hostname 901 #904

Conversation

sidey79 commented Oct 27, 2020 • edited Loading

codecov bot commented Oct 27, 2020 • edited Loading

Codecov Report

coveralls commented Oct 27, 2020 • edited Loading

Pull Request Test Coverage Report for Build 3902

💛 - Coveralls

elektron-bbs Oct 27, 2020

Choose a reason for hiding this comment

elektron-bbs commented Oct 28, 2020

sidey79 commented Oct 28, 2020

HomeAutoUser commented Oct 28, 2020

sidey79 commented Oct 28, 2020 • edited Loading

sidey79 commented Oct 28, 2020

elektron-bbs commented Oct 29, 2020

sidey79 commented Oct 29, 2020

elektron-bbs commented Oct 29, 2020

sidey79 commented Oct 29, 2020

sidey79 commented Oct 27, 2020 •

edited

Loading

codecov bot commented Oct 27, 2020 •

edited

Loading

coveralls commented Oct 27, 2020 •

edited

Loading

sidey79 commented Oct 28, 2020 •

edited

Loading