search

andig / homebridge-fritz

Homebridge platform for Fritz!Box router and supported DECT devices
MIT License
74 stars 22 forks source link

Migrating code to use updateValue instead of setValue for state changes #91

Closed Supereg closed 4 years ago

Supereg commented 4 years ago

Wie besprochen wird nun ausschließlich updateValue benutzt.

Außerdem habe ich, wie ich glaube, ein paar Inkonsistenzen innerhalb des Thermostat accessories behoben. So wie ich das aktuelle Verhalten verstanden habe, setzt das Thermostat bei target=COOL die target temperature auf die night temperature und bei target=HEAT die target temperature auf die comfort temperature(?).

Weiterhin hab ich bisschen code cleanup betrieben und den ein oder anderen kleineren Fehler ausgemerzt, denn ich zufällig entdeckt habe (z.B. würde dass wifi accessory, wenn es im fallback modus ist, nicht die korrekte update Methode aufrufen).

@jngmrks wäre cool, wenn du das mal ausführlich testen und dein feedback da lassen könntest. Hab leider nicht so ein großes Setup und die meisten der Gerät überhaupt nicht.

jngmrks commented 4 years ago

Was genau sollte sich denn von setValue() auf updateValue() geändert haben? Das COOL und HEAT mit Komfort- und Spartemperatur haben wir doch nicht mehr? Gibt es jetzt 2 verschiedene Versionen von dir?

Die #update-value funktioniert nicht. Bin wieder bei der #restirct-valid-values mit dem Tippfehler 😅

[2019-10-26 22:47:13] [Fritz!Box] Fritz!Box platform login successful
[2019-10-26 22:47:13] [Fritz!Box] Discovering accessories
[2019-10-26 22:47:13] [Fritz!Box] Using tr64 api for guest Wifi
[2019-10-26 22:47:13] [Fritz!Box] Updating guest WLAN
[2019-10-26 22:47:13] [Fritz!Box] TypeError: Cannot read property 'actions' of undefined
    at FritzWifiAccessory.queryOn (/usr/local/lib/node_modules/homebridge-fritz/lib/accessories/wifi.js:114:22)
    at FritzWifiAccessory.update (/usr/local/lib/node_modules/homebridge-fritz/lib/accessories/wifi.js:153:14)
    at new FritzWifiAccessory (/usr/local/lib/node_modules/homebridge-fritz/lib/accessories/wifi.js:84:10)
    at /usr/local/lib/node_modules/homebridge-fritz/lib/platform.js:93:34
    at tryCatcher (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise.js:547:31)
    at Promise._settlePromise (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise.js:604:18)
    at Promise._settlePromise0 (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise.js:649:10)
    at Promise._settlePromises (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise.js:729:18)
    at _drainQueueStep (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/async.js:93:12)
    at _drainQueue (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/async.js:86:9)
    at Async._drainQueues (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/async.js:102:5)
    at Immediate.Async.drainQueues [as _onImmediate] (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/async.js:15:14)
    at runCallback (timers.js:763:18)
    at tryOnImmediate (timers.js:734:5)
    at processImmediate (timers.js:716:5)
[2019-10-26 22:47:13] [Fritz!Box] Initializing Fritz!Box platform accessories failed - wrong user credentials?
[2019-10-26 22:47:13] [Fritz!Box] > getOSVersion ("dcd420540bbbd47b")
[2019-10-26 22:47:20] [Fritz!Box] < getOSVersion "07.12"
andig commented 4 years ago

@jngmrks der wifi Fehler taucht schon länger mal sporadisch auf, ich konnte ihn aber nie isolieren. Hast Du eine Idee wie der sich reproduzieren lässt?

andig commented 4 years ago

Das COOL und HEAT mit Komfort- und Spartemperatur haben wir doch nicht mehr? Gibt es jetzt 2 verschiedene Versionen von dir?

@jngmrks Das sind 2 unterschiedliche PRs mit unterschiedlichem Inhalt. Ich bin allerdings auch verwirrt welchen ich jetzt zuerst mergen müsste.

So wie ich das aktuelle Verhalten verstanden habe, setzt das Thermostat bei target=COOL die target temperature auf die night temperature und bei target=HEAT die target temperature auf die comfort temperature(?).

Das war die Idee, wäre dann aber mit dem anderen PR überflüssig. Hast Du einen Vorschlag in welcher Reihenfolge wir das machen wollen? @jngmrks hätte wohl gerne erst den #82 drin, dann den hier.

Supereg commented 4 years ago

Was genau sollte sich denn von setValue() auf updateValue() geändert haben?

Nichts, genau das solltest du mal testen 😅

Das COOL und HEAT mit Komfort- und Spartemperatur haben wir doch nicht mehr?

Naja schon noch. Darüber handelt ja der andere PR. Dieser PR stellt erstmal die code base auf die updateValue Methode um. Um #82 würde ich mich dann im Nachhinein kümmern.

@jngmrks Der crash sollte gefixt sein. Hab da nicht genau gelesen, welche services am wann verfügbar werden. Sollte jetzt auch sichergestellt werden, dass der tr64service auch wirklich erst gequeried wird, wenn er initialisiert wurde. Bis zu diesem Zeitpunkt wird das Wifi Netzwerk als OFF dargestellt.

Supereg commented 4 years ago

Also ich beabsichtige aktuell #91 zuerst zu mergen.

andig commented 4 years ago

@jngmrks könntest Du diesen nochmal kurz testen? Damit wäre das Thermostatverhalten dann noch das "alte", hier gehts nur um eine technische Änderung.

jngmrks commented 4 years ago 
[2019-10-27 12:32:07] [Fritz!Box] Thermostats found: 117950350088,117950212336,117950210296,117950252408,109711038560
Unhandled rejection TypeError: Cannot set property 'fritzCurrentTemperature' of undefined
    at new FritzThermostatAccessory (/usr/local/lib/node_modules/homebridge-fritz/lib/accessories/thermostat.js:68:61)
    at /usr/local/lib/node_modules/homebridge-fritz/lib/platform.js:118:46
    at Array.forEach (<anonymous>)
    at /usr/local/lib/node_modules/homebridge-fritz/lib/platform.js:116:26
    at tryCatcher (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise.js:547:31)
    at Promise._settlePromise (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise.js:604:18)
    at Promise._fulfillPromises (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise.js:704:14)
    at Promise._settlePromises (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise.js:730:18)
    at Promise._fulfill (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise.js:673:18)
    at Promise._settlePromise (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise.js:617:21)
    at Promise._settlePromise0 (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise.js:649:10)
    at Promise._settlePromises (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise.js:729:18)
    at Promise._fulfill (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise.js:673:18)
    at Promise._settlePromise (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise.js:617:21)
    at Promise._settlePromise0 (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise.js:649:10)
    at Promise._settlePromises (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise.js:729:18)
    at Promise._fulfill (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise.js:673:18)
    at MappingPromiseArray.PromiseArray._resolve (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise_array.js:127:19)
    at MappingPromiseArray._promiseFulfilled (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/map.js:108:18)
    at MappingPromiseArray.PromiseArray._iterate (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise_array.js:115:31)
    at MappingPromiseArray.init (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise_array.js:79:10)
    at Promise._settlePromise (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise.js:601:21)
    at Promise._settlePromise0 (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise.js:649:10)
    at Promise._settlePromises (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/promise.js:729:18)
    at _drainQueueStep (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/async.js:93:12)
    at _drainQueue (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/async.js:86:9)
    at Async._drainQueues (/usr/local/lib/node_modules/homebridge-fritz/node_modules/bluebird/js/release/async.js:102:5)
jngmrks commented 4 years ago

Das ging ja Fix. Sieht alles gut aus, kann keine Probleme erkennen. Auch die Spar- und Komforttemperatur alles funktioniert. Kommt mir sogar minimal schneller vor, aber das kann täuschen.

jngmrks commented 4 years ago

Das Update anstatt Set hat einiges verändert, das ich so nicht erwartet hätte. Die Home-App fragt nun nicht mehr alle Geräte jedes mal einzeln ab, sondern speichert die Werte in einem "Cache". Aktualisierungen der Werte kommen jetzt wohl nur noch über das Interval (oder auch nicht). Jedenfalls kommt mir alles so vor, kann das jemand bestätigen?

Supereg commented 4 years ago

Ich hab folgende Änderungen im Endeffekt gemacht: Batterie Status wird nicht mehr extra jedes Mal abgefragt sondern nur noch über den Intervall. Und da dann auch nur mit einer Anfrage. Und anstatt für die TargetHearingCoolingState, CurrentHeatingCoolingState und die TargetTemperature Characteristics jedes mal drei mal dieselbe „getTargetTemp“ Anfrage zu schicken, wird nur noch beim abfragen der temperatur die Fritz Box angefunkt, weil jene drei characteristics sowieso immer gleichzeitig abgerufen werden. Von dieser einen Abfrage ausgehend werden dann immer die beiden anderen characteristics gesetzt und per Event geupdated.

Und der interval funktioniert weiterhin wie immer.

Supereg commented 4 years ago

Das Update anstatt Set hat einiges verändert, das ich so nicht erwartet hätte. Die Home-App fragt nun nicht mehr alle Geräte jedes mal einzeln ab, sondern speichert die Werte in einem "Cache“

Und wie genau meinst du das 🤔

jngmrks commented 4 years ago

Und wie genau meinst du das 🤔

Jetzt verstehe ich, dadurch das die Anfragen so massiv reduziert wurden, dachte ich eben die werden gecached. Aber durch deine Erklärung macht nun alles einen Sinn.