sidey79 commented 2 years ago

Please check if the PR fulfills these requirements
[x] 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, ...)
[x] Bugfix (please link issue)
[x] 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)

https://github.com/RFD-FHEM/RFFHEM/issues/855 https://github.com/RFD-FHEM/RFFHEM/issues/856

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

Timers are deleted correcty

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

codecov[bot] commented 2 years ago

Codecov Report

Merging #1075 (c31ed03) into master (8e29da6) will increase coverage by 0.21%. The diff coverage is 70.73%.

@@            Coverage Diff             @@
##           master    #1075      +/-   ##
==========================================
+ Coverage   63.42%   63.64%   +0.21%     
==========================================
  Files         128      132       +4     
  Lines        9479     9525      +46     
  Branches     1504     1508       +4     
==========================================
+ Hits         6012     6062      +50     
+ Misses       2308     2298      -10     
- Partials     1159     1165       +6

Flag	Coverage Δ
fhem	`55.39% <70.73%> (+0.24%)`	:arrow_up:
modules	`63.64% <70.73%> (+0.21%)`	:arrow_up:
perl	`90.59% <ø> (+0.21%)`	:arrow_up:
unittests	`63.64% <70.73%> (+0.21%)`	:arrow_up:

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

Impacted Files	Coverage Δ
FHEM/00_SIGNALduino.pm	`63.69% <55.55%> (+0.23%)`	:arrow_up:
..._SIGNALduino/01_SIGNALduino_CheckSendRawResponse.t	`100.00% <100.00%> (ø)`
t/FHEM/00_SIGNALduino/01_SIGNALduino_Undef.t	`100.00% <100.00%> (ø)`
t/FHEM/00_SIGNALduino/02_SIGNALduino_IdList.t	`100.00% <100.00%> (ø)`
t/FHEM/00_SIGNALduino/03_SIGNALduino_Set.t	`100.00% <100.00%> (ø)`
t/FHEM/14_SD_BELL/00_load.t	`100.00% <0.00%> (ø)`
t/FHEM/14_SD_BELL/09_parseDatat.t	`80.00% <0.00%> (ø)`
FHEM/lib/SD_Protocols.pm	`79.45% <0.00%> (+0.54%)`	:arrow_up:

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 8e29da6...c31ed03. Read the comment docs.

sidey79 commented 2 years ago

@HomeAutoUser @elektron-bbs

Ich habe auf die Timer Bibliothek umgestellt, damit keine Timer im Falle eines undef zurück bleiben.

Der Branch wäre aber mal intensiv zu testen, bevor wir der Anpassung trauen können.

elektron-bbs commented 2 years ago

Weit gekommen bin ich mit dem ersten Test nicht :-)

Ich habe drchgeführt:

http://fhem.de/fhemupdate/controls_fhem.txt
https://raw.githubusercontent.com/fhem/Timer/pre-release/controls_Timer.txt
https://raw.githubusercontent.com/RFD-FHEM/SIGNALduino_TOOL/pre-release/controls_SD_TOOL.txt
https://raw.githubusercontent.com/RFD-FHEM/RFFHEM/master/controls_signalduino.txt
https://raw.githubusercontent.com/fhem/SD_Keeloq/pre_release/controls_SD_Keeloq.txt

und

update all https://raw.githubusercontent.com/RFD-FHEM/RFFHEM/lib-timer/controls_signalduino.txt

FHEM hat sich verabschiedet mit folgendem im Log:

2022.02.22 17:20:59 3: sduinoEasyPico868: getAttrDevelopment, IdList ### Attribute development is in this version ignored ###
2022.02.22 17:20:59 3: sduinoEasyPico868: IdList, attr whitelist: 108,115,117
2022.02.22 17:20:59 3: sduinoEasyPico868: IdList, MS 
2022.02.22 17:20:59 3: sduinoEasyPico868: IdList, MU 
2022.02.22 17:20:59 3: sduinoEasyPico868: IdList, MC 
2022.02.22 17:20:59 3: sduinoEasyPico868: IdList, MN 108 115 117
2022.02.22 17:20:59 1: sduinoEasyPico434: DoInit, 192.168.178.44:23
2022.02.22 17:20:59 3: sduinoEasyPico434 device opened
2022.02.22 17:20:59 1: sduinoEasyPico868: DoInit, 192.168.178.44:2323
2022.02.22 17:20:59 3: sduinoEasyPico868 device opened
Can't use an undefined value as an ARRAY reference at ./FHEM/00_SIGNALduino.pm line 2994.
2022.02.22 17:29:53 1: Including fhem.cfg

Das dürfte aber nicht an den Timern liegen, sondern an Zeile 3347:

  return if (!defined $name || !IsDevice($name));

Nachdem ich diese auskommentiert habe, läuft FHEM erstmal.

Ich habe jetzt aber keinen Plan, wie ich die Timer testen könnte.

elektron-bbs commented 2 years ago

Die Aussage, das FHEM nach der Auskommentierung, läuft, ziehe ich zurück - ich hatte aus Versehen, die vorhergehende 00_SIGNALduino.pm verwendet.

elektron-bbs commented 2 years ago

Wenige Minuten später der Test auf dem zweiten System:

2022.02.22 17:56:23 3: sduinoD1: getAttrDevelopment, IdList ### Attribute development is in this version ignored ###
2022.02.22 17:56:23 3: sduinoD1: IdList, attr whitelist: 108,115,117
2022.02.22 17:56:23 3: sduinoD1: IdList, MS 
2022.02.22 17:56:23 3: sduinoD1: IdList, MU 
2022.02.22 17:56:23 3: sduinoD1: IdList, MC 
2022.02.22 17:56:23 3: sduinoD1: IdList, MN 108 115 117
2022.02.22 17:56:23 3: sduinoEasyESP32_868: getAttrDevelopment, IdList ### Attribute development is in this version ignored ###
2022.02.22 17:56:23 3: sduinoEasyESP32_868: IdList, attr whitelist: 0,0.1,0.2,0.3,0.4,0.5,1,3,3.1,4,6,7,8,9,10,11,12,13,13.1,13.2,14,15,16,17,17.1,18,20,21,22,23,24,25,26,27,28,29,30,31,32,33,33.1,33.2,34,35,36,37,38,39,41,42,43,44,44.1,45,46,47,48,49,49.1,49.2,50,51,53,54,54.1,55,56,57,58,59,60,61,62,64,65,66,67,68,69,70,71,72,73,74,74.1,76,78,79,80,81,83,84,85,86,87,88,89,90,91,91.1,92,93,94,95,96,97,98,99,100,101,102,103,104,105,106,107,107.1,108,109,110,111,113,114,115,116,116.1,117
2022.02.22 17:56:23 3: sduinoEasyESP32_868: IdList, MS 0 0.1 0.2 0.3 0.4 0.5 1 3 3.1 4 6 7 13 13.2 14 15 17 20 23 25 33 33.1 33.2 35 41 49 51 53 54.1 55 65 68 74.1 87 88 90 91.1 93 106 113
2022.02.22 17:56:23 3: sduinoEasyESP32_868: IdList, MU 8 9 13.1 16 17.1 21 22 24 26 27 28 29 30 31 32 34 36 37 38 39 42 44 44.1 45 46 48 49.1 49.2 50 54 56 59 60 61 62 64 66 67 69 70 71 72 73 74 76 78 79 80 81 83 84 85 86 89 91 92 94 95 97 98 99 104 105 110 111 114
2022.02.22 17:56:23 3: sduinoEasyESP32_868: IdList, MC 10 11 12 18 43 47 57 58 96
2022.02.22 17:56:23 3: sduinoEasyESP32_868: IdList, MN 100 101 102 103 107 107.1 108 109 115 116 116.1 117
2022.02.22 17:56:23 2: DbLog myDbLog - Last database write cycle due to shutdown ...
2022.02.22 17:56:23 2: DbLog myDbLog - no data for last database write cycle
2022.02.22 17:56:23 1: Server shutdown delayed due to myDbLog for max 10 sec
2022.02.22 17:56:23 1: sduinoEasyESP32_868: DoInit, 192.168.178.49:2323
2022.02.22 17:56:23 3: sduinoEasyESP32_868 device opened
2022.02.22 17:56:23 1: sduinoIP: DoInit, 192.168.178.46:23
2022.02.22 17:56:23 3: sduinoIP device opened
2022.02.22 17:56:23 3: LaCrosseGateway device opened
2022.02.22 17:56:23 2: sduinoACM: CheckVersionResp, initialized 3.5.4+20220220
2022.02.22 17:56:23 3: sduinoACM: CheckVersionResp, enable receiver (XE) 
2022.02.22 17:56:23 1: sduinoD1: DoInit, 192.168.178.42:23
2022.02.22 17:56:23 3: sduinoD1 device opened
Can't use an undefined value as an ARRAY reference at ./FHEM/00_SIGNALduino.pm line 2994.
2022.02.22 17:56:24 1: Including fhem.cfg

FHEM schmiert wahrscheinlich immer ab, wenn eine der IdList leer ist.

sidey79 commented 2 years ago

FHEM schmiert wahrscheinlich immer ab, wenn eine der IdList leer ist.

Das ist aber so gewollt dass es bei einem leer ist oder? An der Stelle habe ich nichts geändert, seltsam dass es diese Auswirkungen hat.

elektron-bbs commented 2 years ago

Ja, klar, ich aktiviere eigentlich immer nur die Protokolle, die benötigt werden.

Ich habe jetzt noch etwas weiter probiert. Wenn ich hier, den alten Timeraufruf wieder verwende, kommt FHEM etwas weiter:

  # FHEM::Core::Timer::Helper::addTimer($name, time(), \&SIGNALduino_IdList,"sduino_IdList:$name",0 );
  InternalTimer(gettimeofday(), \&SIGNALduino_IdList,"sduino_IdList:$name",0);       # verzoegern bis alle Attribute eingelesen sind

... scheitert dann aber wieder:

2022.02.22 20:48:01 3: sduinoEasyPico868: getAttrDevelopment, IdList ### Attribute development is in this version ignored ###
2022.02.22 20:48:01 3: sduinoEasyPico868: IdList, attr whitelist: 108,115,117
2022.02.22 20:48:01 3: sduinoEasyPico868: IdList, MS 
2022.02.22 20:48:01 3: sduinoEasyPico868: IdList, MU 
2022.02.22 20:48:01 3: sduinoEasyPico868: IdList, MC 
2022.02.22 20:48:01 3: sduinoEasyPico868: IdList, MN 108 115 117
2022.02.22 20:48:01 3: Timer: time difference too large! interval=59, Sekunde=01
2022.02.22 20:48:02 2: sduino868: CheckVersionResp, initialized 3.5.4+20220220
2022.02.22 20:48:02 3: sduino868: CheckVersionResp, enable receiver (XE) 
2022.02.22 20:48:02 1: sduinoEasyPico868: DoInit, 192.168.178.44:2323
2022.02.22 20:48:02 3: sduinoEasyPico868 device opened
2022.02.22 20:48:02 1: sduinoEasyPico434: DoInit, 192.168.178.44:23
2022.02.22 20:48:02 3: sduinoEasyPico434 device opened
2022.02.22 20:48:02 3: FB7590_Call device opened
Undefined subroutine &main::582820 called at fhem.pl line 3458.
2022.02.22 20:54:03 1: Including fhem.cfg

sidey79 commented 2 years ago

Ich denke, der Timer versucht eine Funktion aufzurufen die es nicht gibt:

Undefined subroutine &main::582820 called at fhem.pl line 3458.

Das FHEM da hängen bleibt, ist aber eher darauf zurückzuführen, dass es hier nicht robust ist. Naja ich suche jetzt den fehlerhaften Timer.

sidey79 commented 2 years ago

Ich denke, der Timer versucht eine Funktion aufzurufen die es nicht gibt:

Vermutung war korrekt. Ich werde die Timer::Helper Bibliothek anpassen, damit sie das nicht erlaubt.

Das FHEM da hängen bleibt, ist aber eher darauf zurückzuführen, dass es hier nicht robust ist. Naja ich suche jetzt den fehlerhaften Timer.

Timer wurde gefunden. gettimeofday() liefert einen array zurück, wenn es nicht auf scalar context angewiesen ist. Das kann vorher allerdings auch nicht funktioniert haben denke ich.

elektron-bbs commented 2 years ago

Ist das jetzt schon soweit, das ein neuer Test auf einem Live-System sinnvoll ist, oder arbeitest du noch daran?

sidey79 commented 2 years ago

Ist das jetzt schon soweit, das ein neuer Test auf einem Live-System sinnvoll ist, oder arbeitest du noch daran?

Aktuell arbeite ich nicht daran. Es scheint alles zu funktionieren. Freue mich über weitere Tests.

elektron-bbs commented 2 years ago

Tja, hier schmiert FHEM weiterhin an der gleichen Stelle ab:

2022.02.28 17:22:25 3: sduinoEasyPico868: getAttrDevelopment, IdList ### Attribute development is in this version ignored ###
2022.02.28 17:22:25 3: sduinoEasyPico868: IdList, attr whitelist: 12,33,60,108,115,117
2022.02.28 17:22:25 3: sduinoEasyPico868: IdList, MS 33
2022.02.28 17:22:25 3: sduinoEasyPico868: IdList, MU 60
2022.02.28 17:22:25 3: sduinoEasyPico868: IdList, MC 12
2022.02.28 17:22:25 3: sduinoEasyPico868: IdList, MN 108 115 117
2022.02.28 17:22:25 2: sduino868: CheckVersionResp, initialized 3.5.4+20220223
2022.02.28 17:22:25 3: sduino868: CheckVersionResp, enable receiver (XE) 
Can't use an undefined value as an ARRAY reference at ./FHEM/00_SIGNALduino.pm line 2997.
2022.02.28 17:23:27 1: Including fhem.cfg
2022.02.28 17:23:28 3: WEB: port 8083 opened

Ich habe diesmal extra noch bei dem sduinoEasyPico868 jeweils noch ein Protokoll eingetragen. Daran liegt es aber auch nicht. Wenn ich wieder auf die 00_SIGNALduino.pm Version 3.5.4+20220201 zurück gehe, läuft FHEM wieder. Ich habe diesmal nur die 00_SIGNALduino.pm ausgetauscht, alles andere blieb unverändert.

sidey79 commented 2 years ago

Tja, hier schmiert FHEM weiterhin an der gleichen Stelle ab:

2022.02.28 17:22:25 3: sduinoEasyPico868: getAttrDevelopment, IdList ### Attribute development is in this version ignored ###
2022.02.28 17:22:25 3: sduinoEasyPico868: IdList, attr whitelist: 12,33,60,108,115,117
2022.02.28 17:22:25 3: sduinoEasyPico868: IdList, MS 33
2022.02.28 17:22:25 3: sduinoEasyPico868: IdList, MU 60
2022.02.28 17:22:25 3: sduinoEasyPico868: IdList, MC 12
2022.02.28 17:22:25 3: sduinoEasyPico868: IdList, MN 108 115 117
2022.02.28 17:22:25 2: sduino868: CheckVersionResp, initialized 3.5.4+20220223
2022.02.28 17:22:25 3: sduino868: CheckVersionResp, enable receiver (XE) 
Can't use an undefined value as an ARRAY reference at ./FHEM/00_SIGNALduino.pm line 2997.
2022.02.28 17:23:27 1: Including fhem.cfg
2022.02.28 17:23:28 3: WEB: port 8083 opened

Sehr seltsam. Meine Anpassung ist nicht eingecheckt, das wundert mich doch sehr.

elektron-bbs commented 2 years ago

Was meinst du mit "ist nicht eingecheckt"?

sidey79 commented 2 years ago

Was meinst du mit "ist nicht eingecheckt"?

Ich dachte ich hätte den commit nicht gepusht, aber es ist doch enthalten.

Die Sache ist seltsam. Diesen Fehler habe ich nicht mehr:

Can't use an undefined value as an ARRAY reference at ./FHEM/00_SIGNALduino.pm line 2997.

Tatsächlich hatte ich aber das Problem, das FHEM nach einem neustart einmal abgestürzt ist. Jetzt läuft es aber wieder. Es findet sich ja auch kein Fehler mehr in deinem Log, dass eine nicht vorhandene coderef angesteuert wurde. Mir fehlt gerade eine Idee, wie ich das Problem finden könnte.

sidey79 commented 2 years ago

Tatsächlich hatte ich aber das Problem, das FHEM nach einem neustart einmal abgestürzt ist. Jetzt läuft es aber wieder. Es findet sich ja auch kein Fehler mehr in deinem Log, dass eine nicht vorhandene coderef angesteuert wurde. Mir fehlt gerade eine Idee, wie ich das Problem finden könnte.

Fehler in Zeile 1388 gefunden, hier wurden zu viele Timer gelöscht:

    FHEM::Core::Timer::Helper::removeTimer($name);

Fraglich ist, welche solle hier wirklich gelöscht werden.

    FHEM::Core::Timer::Helper::removeTimer($name,undef,$hash); # What timer should be removed here is not clear

elektron-bbs commented 2 years ago

Ich habe die aktuelle 00_SIGNALduino.pm aus diesem Branch jetzt auf 3 Systemen laufen. Bis jetzt habe ich noch keine Auffälligkeiten bemerkt. Ich habe aber keine Ahnung, wo, wann und wie die Timer verwendet werden. Was müsste gezielt probiert werden?

sidey79 commented 2 years ago

Ich habe die aktuelle 00_SIGNALduino.pm aus diesem Branch jetzt auf 3 Systemen laufen. Bis jetzt habe ich noch keine Auffälligkeiten bemerkt. Ich habe aber keine Ahnung, wo, wann und wie die Timer verwendet werden. Was müsste gezielt probiert werden?

Das sind doch gute Nachrichten. Die Timer werden zu unterschiedlich Aktionen verwenden. Mindestens beim Senden mehrerer Kommandos, aber auch beim Abfragen von Konfigurationen des cc1101.

sidey79 commented 2 years ago

@HomeAutoUser @elektron-bbs

wollen wir hier mergen oder ist euch noch was aufgefallen?

elektron-bbs commented 2 years ago

Ich denke mal, ich kann grünes Licht geben :-)

RFD-FHEM / RFFHEM

Use FHEM::Core::Timer::Helper #1075

Codecov Report