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

SD_WS07 fix bug autocreate #531

Merged
merged 11 commits into from
Mar 6, 2019
Merged

SD_WS07 fix bug autocreate #531

merged 11 commits into from
Mar 6, 2019

Conversation

elektron-bbs
Copy link
Contributor

@elektron-bbs elektron-bbs commented Mar 2, 2019

  • 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)

bugfix, update and remove

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    fix bug autocreate define
    remove rssi
    remove slider for offsets
    remove reading battery

  • What is the current behavior? (You can also link to an open issue here)
    SD_WS07 - Autocreate loop  #527

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

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

  • Other information:

fix bug autocreate define
remove rssi
remove slider for offsets
remove reading battery
update CHANGED
@elektron-bbs elektron-bbs changed the title Dev r34 sd ws07 SD_WS07 fix bug autocreate Mar 2, 2019
@coveralls
Copy link

Pull Request Test Coverage Report for Build 1261

  • 0 of 8 (0.0%) changed or added relevant lines in 1 file are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-1.9%) to 6.667%

Changes Missing Coverage Covered Lines Changed/Added Lines %
FHEM/14_SD_WS07.pm 0 8 0.0%
Files with Coverage Reduction New Missed Lines %
FHEM/14_SD_WS07.pm 2 6.57%
Totals Coverage Status
Change from base Build 1260: -1.9%
Covered Lines: 9
Relevant Lines: 135

💛 - Coveralls

1 similar comment
@coveralls
Copy link

Pull Request Test Coverage Report for Build 1261

  • 0 of 8 (0.0%) changed or added relevant lines in 1 file are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-1.9%) to 6.667%

Changes Missing Coverage Covered Lines Changed/Added Lines %
FHEM/14_SD_WS07.pm 0 8 0.0%
Files with Coverage Reduction New Missed Lines %
FHEM/14_SD_WS07.pm 2 6.57%
Totals Coverage Status
Change from base Build 1260: -1.9%
Covered Lines: 9
Relevant Lines: 135

💛 - Coveralls

@coveralls
Copy link

coveralls commented Mar 2, 2019

Pull Request Test Coverage Report for Build 1303

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

Totals Coverage Status
Change from base Build 1297: 58.5%
Covered Lines: 95
Relevant Lines: 138

💛 - Coveralls

FHEM/14_SD_WS07.pm Outdated Show resolved Hide resolved
FHEM/14_SD_WS07.pm Outdated Show resolved Hide resolved
FHEM/14_SD_WS07.pm Show resolved Hide resolved
FHEM/14_SD_WS07.pm Outdated Show resolved Hide resolved
@Ralf9
Copy link
Contributor

Ralf9 commented Mar 3, 2019

@sidey79
wo wir gerade dabei sind, kannst Du Dich noch daran errinnern, warum Du bei $def auch die Abfrage des namens eingebaut hast?
my $def = $modules{SIGNALduino_ID7}{defptr}{$hash->{NAME} . "." . $deviceCode};

c9e79bf#diff-1de9e37ef5f5a44a032fd42b485bf891

@sidey79
Copy link
Contributor

sidey79 commented Mar 3, 2019

@sidey79
wo wir gerade dabei sind, kannst Du Dich noch daran errinnern, warum Du bei $def auch die Abfrage des namens eingebaut hast?
my $def = $modules{SIGNALduino_ID7}{defptr}{$hash->{NAME} . "." . $deviceCode};

Um ehrlich zu sein. Nein.
Das sieht nach einer Programmiersünde aus der Jugend aus :)

Ich habe damals ja von dem Fhem Verhalten auch noch nicht so die Ahnung gehabt und in den von dir Referenzierten Commit ist auch zu sehen, dass ich das TCM_97001 Modul als Vorlage verwendet habe.
Vermutlich war das da so eingebaut und ich habe es einfach übernommen.

Eine derartige Definition (Name IOHash _Code) hat es in unserem Modul aber schon damals nicht gegeben.

FHEM/14_SD_WS07.pm Outdated Show resolved Hide resolved
@sidey79 sidey79 added the bug label Mar 4, 2019
sidey79 added a commit that referenced this pull request Mar 4, 2019
Added some basic tets for SD_WS07 Module, checks autocreate and pre defined
Added Test which checks for bug #527

This test must be added to #531 to verify if the PR is complete
@sidey79
Copy link
Contributor

sidey79 commented Mar 4, 2019

Ich habe den Test, den ich geschrieben habe mal schnell bei mir lokal mit deinem Branch zusammengeführt.

Fast wie erwartet.

Der Bug, der noch in dev-r34 enthalten ist, der ist weg
2019.03.04 23:28:00 3: ok 3 - Check if definition is found by dispatch() # TODO This bug must be fixed

Allerdings haben wir einen neuen, wenn eine Definition nur mit dem channel angelegt wurde:
2019.03.04 23:28:00 3: not ok 3 - Check if definition is found by dispatch()

2019.03.04 23:28:00 3: # Subtest: Check autocreate SD_WS07_TH_1
2019.03.04 23:28:00 3:     1..3
2019.03.04 23:28:00 3:     ok 1 - check sensor not created with single dispatch
2019.03.04 23:28:00 3:     ok 2 - check sensor created with second dispatch
2019.03.04 23:28:00 3:     ok 3 - check sensor deleted correctly
2019.03.04 23:28:00 3: ok 1 - Check autocreate SD_WS07_TH_1
2019.03.04 23:28:00 3: # Subtest: Check Dispatch() with existing definition only channel SD_WS07_TH_1
2019.03.04 23:28:00 3:     1..4
2019.03.04 23:28:00 3:     ok 1 - check sensor deleted correctly
2019.03.04 23:28:00 3:     ok 2 - check sensor not created with single dispatch
2019.03.04 23:28:00 3:     not ok 3 - Check if definition is found by dispatch()
2019.03.04 23:28:00 3:     ok 4 - check sensor deleted correctly
2019.03.04 23:28:00 3: not ok 2 - Check Dispatch() with existing definition only channel SD_WS07_TH_1
2019.03.04 23:28:00 3: # Subtest: Check Dispatch() with existing definition model_channel SD_WS07_TH_1
2019.03.04 23:28:00 3:     1..4
2019.03.04 23:28:00 3:     ok 1 - check sensor deleted correctly
2019.03.04 23:28:00 3:     ok 2 - check sensor not created with single dispatch
2019.03.04 23:28:00 3:     ok 3 - Check if definition is found by dispatch() # TODO This bug must be fixed
2019.03.04 23:28:00 3:     ok 4 - check sensor deleted correctly
2019.03.04 23:28:00 3: ok 3 - Check Dispatch() with existing definition model_channel SD_WS07_TH_1
2019.03.04 23:28:00 3: # Subtest: Check autocreate with longid SD_WS07_TH_631
2019.03.04 23:28:00 3:     1..4
2019.03.04 23:28:00 3:     ok 1 - check sensor not created with single dispatch
2019.03.04 23:28:00 3:     ok 2 - check sensor created with second dispatch
2019.03.04 23:28:00 3:     ok 3 - Check if definition is found by dispatch()
2019.03.04 23:28:00 3:     ok 4 - check sensor deleted correctly
2019.03.04 23:28:00 3: ok 4 - Check autocreate with longid SD_WS07_TH_631
2019.03.04 23:28:00 3:     #   Failed test 'Check if definition is found by dispatch()'
2019.03.04 23:28:00 3:     #   at (eval 76) line 1.
2019.03.04 23:28:00 3:     #          got: undef
2019.03.04 23:28:00 3:     #     expected: 'SD_WS07_TH_1'
2019.03.04 23:28:00 3:     # Looks like you failed 1 test of 4.
2019.03.04 23:28:00 3: #   Failed test 'Check Dispatch() with existing definition only channel SD_WS07_TH_1'
2019.03.04 23:28:00 3: #   at (eval 76) line 1.
2019.03.04 23:28:00 3: <---- Test test_SDWS07 ends here ----

@elektron-bbs
Copy link
Contributor Author

elektron-bbs commented Mar 5, 2019

Ist klar, dafür war ja eine Zeile schon im ursprünglichen PR. Habe ich dann auf dein Anraten wieder heraus genommen. So müsste es dann wieder komplett sein:

my $def = $modules{SD_WS07}{defptr}{$deviceCode};	# test for already defined devices use wrong naming convention (only channel or longid)
$deviceCode = $model . "_" . $deviceCode;
$def = $modules{SD_WS07}{defptr}{$deviceCode} if(!$def);	# test for already defined devices use normal naming convention (model_channel or model_lonid)

elektron-bbs and others added 4 commits March 5, 2019 18:04
test for already defined devices use wrong naming convention (only
channel or longid),
test for already defined devices use normal naming convention
(model_channel or model_lonid)
Migrate wrong DEF to correct ones
@sidey79
Copy link
Contributor

sidey79 commented Mar 5, 2019

Ich habe den Zweig aktualisiert, damit der Test hier läuft.

Ich habe mir auch erlaubt, eine Migration der "fehlerhaften Definitionen" einzubauen. Den kann man nach ein paar Wochen wieder ausbauen :)

Ich hoffe die unit tests laufen durch, dann haben wir den Fix in 3.4.0

Eigentlich brauchen wir ihn aber auch noch für 3.3.4. (cherrypick) :) Die Version 3.4.0 sollten wir vielleicht noch mal eine Woche oder so testen :)

sidey79
sidey79 previously approved these changes Mar 5, 2019
removed todo block because this branch fixes #527
sidey79
sidey79 previously approved these changes Mar 5, 2019
@sidey79
Copy link
Contributor

sidey79 commented Mar 5, 2019

@elektron-bbs

Schau es dir noch mal an, aus meiner Sicht können wir in dev-r34 mergen und ich pimpe dann 3.3.4 mit einem cherrypick :)

@elektron-bbs
Copy link
Contributor Author

Ich habe das nochmal durchgetestet und keine Fehler mehr feststellen können.

@elektron-bbs elektron-bbs merged commit 2ebdcd7 into RFD-FHEM:dev-r34 Mar 6, 2019
sidey79 pushed a commit that referenced this pull request Mar 6, 2019
fix bug autocreate define
remove rssi
remove slider for offsets
remove reading battery

Migrate wrong DEF to correct ones

(cherry picked from commit 2ebdcd7)
sidey79 pushed a commit that referenced this pull request Mar 9, 2019
* fix bug autocreate define

fix bug autocreate define
remove rssi
remove slider for offsets
remove reading battery

* update CHANGED

update CHANGED

* small changes

* update help

* test for already defined devices old and new naming convention

test for already defined devices use wrong naming convention (only
channel or longid),
test for already defined devices use normal naming convention
(model_channel or model_lonid)

* 14_SD_WS07.pm

Migrate wrong DEF to correct ones

* test/test_SDWS07-definition.txt
removed todo block because this branch fixes #527

* Update Changed and authors in 14_SD_WS07.pm

(cherry picked from commit 2ebdcd7)

# Conflicts:
#	CHANGED
#	test/test_SDWS07-definition.txt
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.

4 participants