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

Update 14_SD_UT.pm - fix buttons P90 #1060

Merged
merged 11 commits into from
Jan 20, 2022
Merged

Update 14_SD_UT.pm - fix buttons P90 #1060

merged 11 commits into from
Jan 20, 2022

Conversation

HomeAutoUser
Copy link
Contributor

@HomeAutoUser HomeAutoUser commented Jan 13, 2022

  • 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, ...)
  • Bugfix (please link issue)
  • 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)

    • no abort processing and implausible buttons
  • What is the new behavior (if this is a feature change)?

    • abort processing if no suitable button
  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

  • Other information:

@codecov
Copy link

codecov bot commented Jan 13, 2022

Codecov Report

Merging #1060 (d0ad582) into master (5089869) will increase coverage by 0.17%.
The diff coverage is 90.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1060      +/-   ##
==========================================
+ Coverage   61.92%   62.10%   +0.17%     
==========================================
  Files         124      127       +3     
  Lines        9309     9357      +48     
  Branches     1474     1478       +4     
==========================================
+ Hits         5765     5811      +46     
+ Misses       2451     2443       -8     
- Partials     1093     1103      +10     
Flag Coverage Δ
fhem 53.48% <90.00%> (+0.25%) ⬆️
modules 62.10% <90.00%> (+0.17%) ⬆️
perl 91.61% <ø> (ø)
unittests 62.10% <90.00%> (+0.17%) ⬆️

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

Impacted Files Coverage Δ
FHEM/14_SD_UT.pm 37.97% <88.88%> (+1.22%) ⬆️
t/FHEM/14_SD_UT/09_parseDatat.t 94.11% <90.90%> (ø)
FHEM/10_SD_Rojaflex.pm 68.69% <0.00%> (-2.44%) ⬇️
t/FHEM/14_SD_WS07/09_parseDatat.t 80.00% <0.00%> (ø)
t/FHEM/14_FLAMINGO/00_load.t 100.00% <0.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 5089869...d0ad582. Read the comment docs.

@@ -1526,10 +1526,11 @@ sub SD_UT_Parse {

### Manax MX-RCS250 | mumbi [P90] ###
if (!$def && $protocol == 90) {
my $button = $models{RC_10}{buttons}{substr($bitData,20,3)};
if (!defined $button) {return ''}
Copy link
Contributor

Choose a reason for hiding this comment

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

Damit wird die Verarbeitung vollständig abgebrochen.
Dispatch wird hier ebenfalls aufhören zu versuchen ein anderes Modul zu finden, welches die Nachricht verarbeiten kann. Wenn das so gewollt ist, dann kann es so bleiben. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Im Normalfall war es so gedacht.
Oder spricht was dagegen @elektron-bbs ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Für den Fall, dass es so gewünscht ist (ohne Logmeldung etc., dann sollte das return an den Anfang gestellt werden.

Suggested change
if (!defined $button) {return ''}
return '' if (!defined $button)

Achso, hast Du zufällig so eine Nachricht in dem der button nicht enthalten ist?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ich habe die Diskussion im Forum mitverfolgt und glaube aber nicht, das ein return '' die weitere Verarbeitung komplett abbricht.

2022.01.16 16:42:15 4: sduino_dummy: get rawmsg: MN;D=5100C6BF107F1FF8BBFFFFFFEA22;R=14;
2022.01.16 16:42:15 4: sduino_dummy: Parse_MN, 2-FSK Protocol id 100 Lacrosse mode 1 msg 5100C6BF107F1FF8BBFFFFFFEA22 not match (?^:^9)
2022.01.16 16:42:15 4: sduino_dummy: Parse_MN, 2-FSK Protocol id 103 Lacrosse mode 2 msg 5100C6BF107F1FF8BBFFFFFFEA22 not match (?^:^9)
2022.01.16 16:42:15 4: sduino_dummy: Parse_MN, Found 2-FSK Protocol id 107 -> WH51 433.92 MHz with match (?^:^51)
2022.01.16 16:42:15 4: sduino_dummy: SD_WS_Parse protocol 107, rawData 5100C6BF107F1FF8BBFFFFFFEA22
2022.01.16 16:42:15 3: sduino_dummy: SD_WS_107 Parse msg 5100C6BF107F1FF8BBFFFFFFEA22 - ERROR CRC8
2022.01.16 16:42:15 4: sduino_dummy: SD_WS_Parse 5100C6BF107F1FF8BBFFFFFFEA22 protocolid 107 (WH51, DP100, MISOL/1) - ERROR CRC
2022.01.16 16:42:15 4: sduino_dummy: Parse_MN, Found 2-FSK Protocol id 107.1 -> WH51 868.35 MHz with match (?^:^51)
2022.01.16 16:42:15 4: sduino_dummy: Dispatch, W107#5100C6BF107F1FF8BBFFFFFFEA22, Dropped due to short time or equal msg
2022.01.16 16:42:15 4: sduino_dummy: Parse_MN, Error! id 108 msg=5100C6BF107F1FF8BBFFFFFFEA22, message is to short
2022.01.16 16:42:15 4: sduino_dummy: Parse_MN, Error! id 112 msg=5100C6BF107F1FF8BBFFFFFFEA22, message is to long
2022.01.16 16:42:15 4: sduino_dummy: Parse_MN, Error! id 115 msg=5100C6BF107F1FF8BBFFFFFFEA22, message is to short
2022.01.16 16:42:15 4: sduino_dummy: Parse_MN, 2-FSK Protocol id 116 WH57 msg 5100C6BF107F1FF8BBFFFFFFEA22 not match (?^:^57)
2022.01.16 16:42:15 4: sduino_dummy: Parse_MN, 2-FSK Protocol id 116.1 WH57 msg 5100C6BF107F1FF8BBFFFFFFEA22 not match (?^:^57)
2022.01.16 16:42:15 4: sduino_dummy: Parse_MN, Error! id 117 msg=5100C6BF107F1FF8BBFFFFFFEA22, message is to short
2022.01.16 16:42:15 4: sduino_dummy: Parse_MN, Found 2-FSK Protocol id 994 -> LaCrosse mode 4

Nach dem ERROR CRC der hervorgerufen wird durch diese Zeilen:

    my $retcrc=$decodingSubs{$protocol}{crcok}->( $rawData,$bitData );
    if (!$retcrc) {
      Log3 $iohash, 4, "$name: SD_WS_Parse $rawData protocolid $protocol ($SensorTyp) - ERROR CRC";
      return "";
    }

wird munter versucht, die Nachricht weiter zu verarbeiten.
Durch ein return "" wird nur die weitere Verarbeitung des einen Dispatchs abgebrochen.

Copy link
Contributor

Choose a reason for hiding this comment

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

@elektron-bbs
Ja das ist richtig. Dispatch von FHEM wird beendet (suchen eines weiteren Moduls). Wenn das 00_SIGNALduino einen weiteren Dispatch aufgrund eines weiteren Protokolls, auslöst dann wird dieser wieder an Dispatch von FHEM übergeben.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ich denke wir können das so belassen.
Wie ist eure finale Meinung?

Copy link
Contributor

Choose a reason for hiding this comment

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

@HomeAutoUser

Prinzipiell schon. Offen wäre noch die Frage nach einer Referenznachricht :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ich habe keine RAWMSG, aber vielleicht kannst du dir aus den Warnungen eine DMSG basteln:

2021.11.01 07:25:15 1: PERL WARNING: Use of uninitialized value $button in concatenation (.) or string at ./FHEM/14_SD_UT.pm line 1531.
2021.11.01 07:25:15 1: PERL WARNING: Use of uninitialized value $button in string ne at ./FHEM/14_SD_UT.pm line 1534.
2021.11.01 07:25:15 1: PERL WARNING: Use of uninitialized value $button in concatenation (.) or string at ./FHEM/14_SD_UT.pm line 1559.
2021.11.01 07:25:15 1: sduinoIP: SD_UT_Parse UNDEFINED sensor unknown detected, protocol 90, data FFCE32B4E, code FFCE

2021.12.15 15:50:01 1: PERL WARNING: Use of uninitialized value $button in concatenation (.) or string at ./FHEM/14_SD_UT.pm line 1531.
2021.12.15 15:50:01 1: PERL WARNING: Use of uninitialized value $button in string ne at ./FHEM/14_SD_UT.pm line 1534.
2021.12.15 15:50:01 1: PERL WARNING: Use of uninitialized value $button in concatenation (.) or string at ./FHEM/14_SD_UT.pm line 1559.
2021.12.15 15:50:01 1: sduinoIP: SD_UT_Parse UNDEFINED sensor unknown detected, protocol 90, data AC2E43C61, code AC2E

Copy link
Contributor

Choose a reason for hiding this comment

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

Das wäre dann P90#AC2E43C61 oder?

Copy link
Contributor

Choose a reason for hiding this comment

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

Müsste passen...

@sidey79 sidey79 added the SD_UT label Jan 16, 2022
@HomeAutoUser HomeAutoUser mentioned this pull request Jan 17, 2022
13 tasks
sidey79 and others added 3 commits January 17, 2022 22:46
Functions refs saved as coderefs
Umgestellt auf Nutzung Test2::SIGNALduino::RDmsg

Fehlerhafte Testdaten eingebunden
@sidey79
Copy link
Contributor

sidey79 commented Jan 17, 2022

@HomeAutoUser

Ich habe noch einen kleinen Test hinterlegt und die subs als als Coderefs hinterlegt

@HomeAutoUser HomeAutoUser merged commit 66a7145 into master Jan 20, 2022
@HomeAutoUser HomeAutoUser deleted the SD_UT_P90 branch January 20, 2022 18:54
sidey79 added a commit that referenced this pull request Jan 23, 2022
Release 3.5.3 
    00_SIGNALduino.pm:
          feature: Handling of missing modules changed
    
    SD_Protocols.pm
          feature: Graceful load JSON and Digest:CRC Modules (#1066)

    14_SD_UT.pm
          bugfix: fix buttons P90 (#1060)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants