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

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

Merged
merged 29 commits into from
Mar 12, 2022
Merged

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

merged 29 commits into from
Mar 12, 2022

Conversation

sidey79
Copy link
Contributor

@sidey79 sidey79 commented Feb 6, 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)

#855 #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:

sidey79 and others added 9 commits January 23, 2022 22:40
Name der Lib angepasst
- make SIGNALduino_IdList more robust
- updated tests
- Added plan
- Removed duplicarte subtest from 02_SIGNALduino_IdList.t
- Updated test
- verifys the undef function and if timers are removed
@codecov
Copy link

codecov bot commented Feb 6, 2022

Codecov Report

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

Impacted file tree graph

@@            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%) ⬆️
modules 63.64% <70.73%> (+0.21%) ⬆️
perl 90.59% <ø> (+0.21%) ⬆️
unittests 63.64% <70.73%> (+0.21%) ⬆️

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%) ⬆️
..._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%) ⬆️

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 sidey79 marked this pull request as ready for review February 21, 2022 20:41
@sidey79
Copy link
Contributor Author

sidey79 commented Feb 21, 2022

@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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

sidey79 commented Feb 22, 2022

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
Copy link
Contributor

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
Copy link
Contributor Author

sidey79 commented Feb 22, 2022

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
Copy link
Contributor Author

sidey79 commented Feb 23, 2022

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
Copy link
Contributor

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

@sidey79
Copy link
Contributor Author

sidey79 commented Feb 27, 2022

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
Copy link
Contributor

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
Copy link
Contributor Author

sidey79 commented Feb 28, 2022

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
Copy link
Contributor

Was meinst du mit "ist nicht eingecheckt"?

@sidey79
Copy link
Contributor Author

sidey79 commented Feb 28, 2022

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 and others added 2 commits March 1, 2022 23:59
remove less timer if device is connected
@sidey79
Copy link
Contributor Author

sidey79 commented Mar 1, 2022

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
Copy link
Contributor

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
Copy link
Contributor Author

sidey79 commented Mar 2, 2022

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
Copy link
Contributor Author

sidey79 commented Mar 10, 2022

@HomeAutoUser
@elektron-bbs

wollen wir hier mergen oder ist euch noch was aufgefallen?

@elektron-bbs
Copy link
Contributor

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

@sidey79 sidey79 merged commit 316876f into master Mar 12, 2022
@sidey79 sidey79 deleted the lib-timer branch March 12, 2022 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants