update 00_SIGNALduino.pm - fix break & WARNING #592

HomeAutoUser · 2019-05-20T18:47:21Z

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, ...)

added check, method defined on protocolDef
check defined method exist -> not exists, ERROR Message | no method defined, used system general method SIGNALduino_MCRAW

--> no SIGNALduino break an PEARL WARNING

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

PERL WARNING if method not defined in SD_ProtocolData.pm file

2019.05.20 19:45:51 1: PERL WARNING: Use of uninitialized value in exists at ./FHEM/00_SIGNALduino.pm line 2538.
2019.05.20 19:45:51 1: PERL WARNING: Use of uninitialized value $method in concatenation (.) or string at ./FHEM/00_SIGNALduino.pm line 2540.

system break if defined method in SD_ProtocolData.pm file not exists
Undefined subroutine &main::SIGNALduino_MCRAW1 called at ./FHEM/00_SIGNALduino.pm line 2546.

to TEST:
a) in file SD_ProtocolData.pm file delete method
b) in file SD_ProtocolData.pm file rename method to a unknown name

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

no PERL WARNING
break
User gets information in logfile or system used standard SIGNALduino_MCRAW sub

- added check method defined on protocolDef - check defined method exist -> not exists, ERROR Message | no method defined, used system general method SIGNALduino_MCRAW --> no SIGNALduino break an PEARL WARNING

sidey79 · 2019-05-20T19:50:18Z

FHEM/00_SIGNALduino.pm

-			{
-				SIGNALduino_Log3 $name, 5, "$name: Error: Unknown function=$method. Please define it in file $0";
+	   	my $method;
+	    if (!exists $ProtocolListSIGNALduino{$id}{method}) {


Die Intention verstehe ich, allerdings würde ich es etwas anders lösen.
Ich glaube wir hatten einen vergleichbaren Fall auch schon mal. Ich hoffe ich finde die codestelle.

Die Defaults würde ich 1x beim Einlesen der Daten setzen und nicht bei jeder Verarbeitung.

Die Defaults würde ich 1x beim Einlesen der Daten setzen und nicht bei jeder Verarbeitung.

korrigierst du den code wenn du die Stelle fandest?

Ja, ich hoffe es eilt nicht zu sehr :)

Die Stelle habe ich Gefunden.... allerdings ist das noch in 00_SIGNALduino.pm

Das passt da aber nicht gut hin...

Ihh bin jetzt etwas hin und her gerissen.

Die Protokolldaten werden über lib::SD_Protocols verwaltet.

Ich könnte dort eine Funktion einbauen die nach LoadHash aufgerufen wird.
In etwa sowas wie setDefaut. Dann werden die Standardwerte im Hash hinterlegt.
Das Abrufen der Daten würde ich auf getProperty($id,"method"); ändern.

Die Werte müssen nur einmal gesetzt werden und sind dann immer vorhanden

00_SIGNALduino.pm hat keine Kontrolle mehr über die Daten.

In 00_SIGNALduino.pm wird der Wert von Method via $method= checkProperty($id,"method",\&main::SIGNALduino_MCRAW);
Gesetzt. Dann wird zur Laufzeit verifiziert ob es den method Wert gibt und dann dieser oder der default zurückgegeben.

Die Zugriffe auf die Protokolldaten würde ich ohnehin umstellen wollen

Das prüfen eines Inhaltes zur Laufzeit kostet ein paar CPU Zyklen.

Was du vorhast, habe ich verstanden. Was durchaus besser oder CPU sparender ist, kann ich schwer einschätzen.

Das ein Standard gesetzt wird, wenn Eintrag noch vorhanden ist, finde ich gut.

Ich denke, den Standard kann man beim Laden der Protokolldaten setzen. Das Programm funktioniert ja ohne die Methode nicht.

Deinen PR müsste ich dafür aber ein bisschen erweitern :)

elektron-bbs · 2019-05-24T15:22:57Z

Wie ist es denn bei MU und MS geregelt? Ich finde, es sollte wie postDemodulation behandelt werden, um es zu vereinheitlichen und damit auch vereinfachen. Unter Umständen könnte es ja sogar eine Routine für alle werden.

sidey79 · 2019-05-24T21:35:40Z

Wie ist es denn bei MU und MS geregelt? Ich finde, es sollte wie postDemodulation behandelt werden, um es zu vereinheitlichen und damit auch vereinfachen. Unter Umständen könnte es ja sogar eine Routine für alle werden.

Naja, postDemodulation ist optional und muss nicht vorhanden sein.
Die "method" Angabe bei der MC Verarbeitung ist nicht optional

elektron-bbs · 2019-05-25T09:49:36Z

Warum ist die Angabe eigentlich nicht optional? Wenn sowieso an den Daten keine Veränderung erforderlich ist, braucht man die "method" auch nicht.
Die Längenprüfung könnte sicher auch direkt in SIGNALduino_Parse_MC erfolgen.

sidey79 · 2019-05-25T16:28:43Z

Ohne die methode gibt es nichts zum Ausgeben. Daher ist eine Protokolldefinition ohne diese methode für nichts zu gebrauchen.

elektron-bbs · 2019-05-25T18:46:06Z

Mhmm, das verstehe ich jetzt nicht.
Wenn ich z.B. Protokoll 57 dispatche:
MC;LL=-653;LH=665;SL=-317;SH=348;D=D55B58;C=330;L=21;
kommt im Modul dieses an:
SD_BELL_Parse Check P57 - 2AA4A7 alone
Das ist die ursprüngliche Nachricht, nur halt invertiert (weil polarity => 'invert',)
Es müsste halt nur hier etwas angepasst werden:

	   	my $method = $ProtocolListSIGNALduino{$id}{method};
	    if (!exists &$method)
		{
			SIGNALduino_Log3 $name, 5, "$name: Error: Unknown function=$method. Please define it in file $0";
		} else {

HomeAutoUser · 2019-05-26T09:09:30Z

Deinen PR müsste ich dafür aber ein bisschen erweitern :)

Wenn du dies musst, kein Problem ;)

sidey79 · 2019-05-28T20:45:48Z

Mhmm, das verstehe ich jetzt nicht.
Wenn ich z.B. Protokoll 57 dispatche:
MC;LL=-653;LH=665;SL=-317;SH=348;D=D55B58;C=330;L=21;
kommt im Modul dieses an:
SD_BELL_Parse Check P57 - 2AA4A7 alone
Das ist die ursprüngliche Nachricht, nur halt invertiert (weil polarity => 'invert',)
Es müsste halt nur hier etwas angepasst werden:
	   	my $method = $ProtocolListSIGNALduino{$id}{method};
	    if (!exists &$method)
		{
			SIGNALduino_Log3 $name, 5, "$name: Error: Unknown function=$method. Please define it in file $0";
		} else {

Ich habe es damals so implementiert, da alle MC Protokolle eine indivduelle Verarbeitung benötigt haben. Ich finde die Variante, verschiedene Verarbeitungsoptionen je Protokoll anwenden zu können auch flexibel.
Wenns hier einen Standard gibt, der die Nachricht 1:1 weitergibt, dann haben wir doch hier keine Baustelle mehr,

sidey79 · 2019-05-28T20:47:39Z

Deinen PR müsste ich dafür aber ein bisschen erweitern :)

Wenn du dies musst, kein Problem ;)

Gut ich mach das. Hatte jetzt nur ein paar andere Aktivitäten um die Ohren.
Ich werde einen Default in der SD_Protocols.pm beim Ladevorgang setzen und \&main::SIGNALduino_MCRAW auch in diese lib verschieben.

elektron-bbs · 2019-05-29T12:17:51Z

Ich finde es wenig effizient, eine Routine als Standard zu definieren, die eigentlich nichts tut, außer einmal hin und wieder zurück zu wandeln.
Warum nicht die method nur anwenden, wenn sie im Protokoll auch definiert ist, so wie es auch bei postdemodulation realisiert wurde?

Added sub to set defaults moved MCRAW Output sub to SD_Protocols.pm

This reverts commit ac30131, reversing changes made to a87e2b1.

removed SIGNALduino_MCRAW

sidey79 · 2019-05-31T23:20:02Z

@HomeAutoUser

Ich habe das nun mal umgeändert.
Beim Implementieren bin ich allerdings unsicher geworden, ob das wirklich eine gute Idee war einen Standard zu setzen. Man hätte das Protokoll auch ignorieren können wenn die methode fehlt.

HomeAutoUser · 2019-06-01T06:21:59Z

Ich habe das nun mal umgeändert.
Beim Implementieren bin ich allerdings unsicher geworden, ob das wirklich eine gute Idee war einen Standard zu setzen. Man hätte das Protokoll auch ignorieren können wenn die methode fehlt.

Du kannst es auch so machen, das das Protokoll ignoriert wird ABER dann bitte einen Hinweis für den User mit maximal Verbose 3.

Man kann es so oder so auslegen wie man solch einen Fehler abfängt. Wir sollten nur keine Pear Warning generieren.

sidey79 · 2019-06-01T08:09:29Z

Da stimme ich dir zu.
Nicht angefangen wird aktuell, ob es die referenzierte sub überhaupt gibt.

HomeAutoUser · 2019-06-01T08:41:41Z

Ich habe hier den Konlfikt beseitigt.

Nicht angefangen wird aktuell, ob es die referenzierte sub überhaupt gibt.

Du meinst, wenn es die Sub nicht gibt, so wird nichts angestellt?
Erhält der User eine Ausgabe?

sidey79 · 2019-06-01T18:38:33Z

Ich habe hier den Konlfikt beseitigt.

Nicht angefangen wird aktuell, ob es die referenzierte sub überhaupt gibt.

Du meinst, wenn es die Sub nicht gibt, so wird nichts angestellt?
Erhält der User eine Ausgabe?

Manchmal macht das Handy einfach was es will und das ist nicht was ich gerne hätte.
Oder hab ich das am PC geschrieben? Egal, ich meinte so ein Fehler wird nicht abgefangen und das verursacht eine PERL Warnung.

Auf die Schnelle ist mir nicht eingefallen, wie man das überprüft außer via eval.

Added check for existing sub SD_Protocols.pm fixed sub syntax for args test_SD_Protocols-definition.txt Fixed check for protocol with id 0. Any protocol with a 0 was threated as 0 which is wrong added test if setDefaults does it job to add method if it is not present added test for sub MCRAW. not finished for now test_loadprotohash-definition.txt moved tests to subtest

…r/RFFHEM into dev-r34_method_check

fixed test sub MCRAW() - test output with max length equal current length - test output with current length higher max length

sidey79 · 2019-06-03T20:35:30Z

@HomeAutoUser

So ich habe den Fehler mit einer falschen sub abgefangen und auch Tests erstellt.

HomeAutoUser · 2019-06-03T21:13:46Z

@HomeAutoUser

So ich habe den Fehler mit einer falschen sub abgefangen und auch Tests erstellt.

Sind hier von mir noch Handlungen notwendig weil "At least 1 approving review is required by reviewers" steht aber ich kein approve geben kann?

sidey79 · 2019-06-04T18:35:09Z

@HomeAutoUser

Schau doch bitte noch mal drüber.
Den Approve button kann ich dann ja drücken, wenn wir beide der Meinung sind, dass so alles "okay" ist.

elektron-bbs · 2019-06-04T19:46:50Z

FHEM/00_SIGNALduino.pm

@@ -2535,7 +2535,7 @@ SIGNALduino_Parse_MC($$$$@)
 		   	SIGNALduino_Log3 $name, 5, "$name: extracted data $bitData (bin)";

 		   	my $method = lib::SD_Protocols::getProperty($id,"method");
-		    if (!exists &$method)
+		    if (!exists &$method || !defined &{ $method })
 			{
 				SIGNALduino_Log3 $name, 5, "$name: Error: Unknown function=$method. Please define it in file $0";
 			} else {


Ich muss hier nochmal einhaken:
Wenn ich das richtig verstehe, wird bei falscher method oder bei nicht existierender method eine Fehlermeldung nur bei verbose 5 ausgegeben und es erfolgt keine weitere Auswertung. Der User bekommt im Normalfall (verbsoe 3) davon nichts mit.

Warum drehst du das Ganze nicht einfach rum:
Bei existierender method diese anwenden, sonst halt keine anwenden und einfach die Auswertung fortsetzen.

Existieren wird der Eintrag method immer, solange nicht jemand "rumbastelt".

Die Fehlermeldung, ob die definierte Methode auch existiert ist eher für Entwickler gedacht, welche die Angabe der Methode ja vornehmen.

Man könnte den Eintrag aber auch einfach weglassen, wenn man keine Vorverarbeitung braucht, da es sowieso in einem selbst entwickelten Modul weiter verarbeitet wird.

Die "Vorverarbeitung" soll prüfen,

ob die maximale Länge überschritten wurde

Die Daten von einer binären darstellung in eine hexadezimale wandeln

Ich verstehe nicht, wieso dieses flexiblen Auswerten geändert werden soll.

Ich dachte ja nur, das es ein guter Zeitpunkt wäre, das gleich zu optimieren...

HomeAutoUser · 2019-06-06T17:40:10Z

@HomeAutoUser

Schau doch bitte noch mal drüber.
Den Approve button kann ich dann ja drücken, wenn wir beide der Meinung sind, dass so alles "okay" ist.

Mir ist nichts aufgefallen.

Ich verstehe nicht, wieso dieses flexiblen Auswerten geändert werden soll.

Wenn ich @elektron-bbs richtig verstanden habe, so meint er das so, das wenn keine Methode angegeben ist, das dann die Bits unverändert herauskommen. Es geht nicht ums ändern, es geht darum, das wenn nichts angegeben ist und auch die Methode nicht angegeben wurde, das als Standard benutzt wird. (ich vermute als Endresultat in der Verarbeitungsroutine) Die flexible Verarbeitung ist dadurch bleibend. Sobald du eine separate Verarbeitung benötigst, so kann du diese flexible angeben.

sidey79 · 2019-06-06T21:00:31Z

Ohne Verarbeitungsmethode fehlt die Längenprüfung und es gibt auch keine Daten zum Dispatchen.
Dann müsste man das, was es schon in der Standard Methode passiert wieder nachentwickeln. Ich sehe da keinen Vortei,l.

coveralls · 2019-06-07T14:18:25Z

Pull Request Test Coverage Report for Build 1519

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at ?%

Totals
Change from base Build 1518:	0.0%
Covered Lines:
Relevant Lines:	0

💛 - Coveralls

coveralls · 2019-06-07T14:18:25Z

Pull Request Test Coverage Report for Build 1519

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at ?%

Totals
Change from base Build 1518:	0.0%
Covered Lines:
Relevant Lines:	0

💛 - Coveralls

update 00_SIGNALduino.pm - fix break & WARNING

a87e2b1

- added check method defined on protocolDef - check defined method exist -> not exists, ERROR Message | no method defined, used system general method SIGNALduino_MCRAW --> no SIGNALduino break an PEARL WARNING

sidey79 reviewed May 20, 2019

View reviewed changes

HomeAutoUser added 2 commits May 20, 2019 22:09

Merge branch 'dev-r34' into dev-r34_method_check

ac30131

Merge branch 'dev-r34' into dev-r34_method_check

ce992c5

sidey79 added 3 commits June 1, 2019 00:47

SD_Protocols.pm

f0851b1

Added sub to set defaults moved MCRAW Output sub to SD_Protocols.pm

00_SIGNALduino.pm

f5fddeb

This reverts commit ac30131, reversing changes made to a87e2b1.

00_SIGNALduino.pm

2280ed6

removed SIGNALduino_MCRAW

Merge branch 'dev-r34' into dev-r34_method_check

4348479

sidey79 added 3 commits June 2, 2019 23:45

Merge branch 'dev-r34_method_check' of https://github.com/HomeAutoUse…

852a200

…r/RFFHEM into dev-r34_method_check

test_SD_Protocols-definition.txt

c9e9e6e

fixed test sub MCRAW() - test output with max length equal current length - test output with current length higher max length

HomeAutoUser requested a review from sidey79 June 3, 2019 21:12

elektron-bbs reviewed Jun 4, 2019

View reviewed changes

sidey79 previously approved these changes Jun 6, 2019

View reviewed changes

Merge branch 'dev-r34' into dev-r34_method_check

1364e71

HomeAutoUser dismissed sidey79’s stale review via 1364e71 June 7, 2019 14:13

sidey79 approved these changes Jun 7, 2019

View reviewed changes

HomeAutoUser merged commit 2a928bc into RFD-FHEM:dev-r34 Jun 8, 2019

HomeAutoUser deleted the dev-r34_method_check branch June 14, 2019 17:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update 00_SIGNALduino.pm - fix break & WARNING #592

update 00_SIGNALduino.pm - fix break & WARNING #592

HomeAutoUser commented May 20, 2019 •

edited by sidey79

Loading

sidey79 May 20, 2019

HomeAutoUser May 20, 2019

sidey79 May 21, 2019

sidey79 May 23, 2019

HomeAutoUser May 24, 2019

sidey79 May 24, 2019

elektron-bbs commented May 24, 2019

sidey79 commented May 24, 2019

elektron-bbs commented May 25, 2019

sidey79 commented May 25, 2019

elektron-bbs commented May 25, 2019

HomeAutoUser commented May 26, 2019

sidey79 commented May 28, 2019

sidey79 commented May 28, 2019

elektron-bbs commented May 29, 2019

sidey79 commented May 31, 2019

HomeAutoUser commented Jun 1, 2019

sidey79 commented Jun 1, 2019

HomeAutoUser commented Jun 1, 2019

sidey79 commented Jun 1, 2019

sidey79 commented Jun 3, 2019

HomeAutoUser commented Jun 3, 2019

sidey79 commented Jun 4, 2019

elektron-bbs Jun 4, 2019

sidey79 Jun 4, 2019

elektron-bbs Jun 4, 2019

sidey79 Jun 4, 2019

elektron-bbs Jun 5, 2019

HomeAutoUser commented Jun 6, 2019 •

edited

Loading

sidey79 commented Jun 6, 2019

coveralls commented Jun 7, 2019

coveralls commented Jun 7, 2019 •

edited

Loading

update 00_SIGNALduino.pm - fix break & WARNING #592

update 00_SIGNALduino.pm - fix break & WARNING #592

Conversation

HomeAutoUser commented May 20, 2019 • edited by sidey79 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elektron-bbs commented May 24, 2019

sidey79 commented May 24, 2019

elektron-bbs commented May 25, 2019

sidey79 commented May 25, 2019

elektron-bbs commented May 25, 2019

HomeAutoUser commented May 26, 2019

sidey79 commented May 28, 2019

sidey79 commented May 28, 2019

elektron-bbs commented May 29, 2019

sidey79 commented May 31, 2019

HomeAutoUser commented Jun 1, 2019

sidey79 commented Jun 1, 2019

HomeAutoUser commented Jun 1, 2019

sidey79 commented Jun 1, 2019

sidey79 commented Jun 3, 2019

HomeAutoUser commented Jun 3, 2019

sidey79 commented Jun 4, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HomeAutoUser commented Jun 6, 2019 • edited Loading

sidey79 commented Jun 6, 2019

coveralls commented Jun 7, 2019

Pull Request Test Coverage Report for Build 1519

💛 - Coveralls

coveralls commented Jun 7, 2019 • edited Loading

Pull Request Test Coverage Report for Build 1519

💛 - Coveralls

HomeAutoUser commented May 20, 2019 •

edited by sidey79

Loading

HomeAutoUser commented Jun 6, 2019 •

edited

Loading

coveralls commented Jun 7, 2019 •

edited

Loading