Skip to content
This repository has been archived by the owner on Jan 27, 2023. It is now read-only.

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

Merged
merged 1 commit into from
Oct 27, 2019

Conversation

Supereg
Copy link
Contributor

@Supereg Supereg commented Oct 26, 2019

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

jngmrks commented Oct 26, 2019

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

andig commented Oct 27, 2019

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

andig commented Oct 27, 2019

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

Supereg commented Oct 27, 2019

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

Supereg commented Oct 27, 2019

Also ich beabsichtige aktuell #91 zuerst zu mergen.

@andig
Copy link
Owner

andig commented Oct 27, 2019

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

jngmrks commented Oct 27, 2019

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

…nges

* Thermostat behaviour for night and comfort temperature is now more explicit and updates correctly
* Some code cleanup
@jngmrks
Copy link
Contributor

jngmrks commented Oct 27, 2019

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.

@andig andig merged commit c38c774 into andig:master Oct 27, 2019
@Supereg Supereg deleted the update-value branch October 27, 2019 13:24
@jngmrks
Copy link
Contributor

jngmrks commented Oct 30, 2019

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

Supereg commented Oct 30, 2019

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

Supereg commented Oct 30, 2019

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

jngmrks commented Oct 31, 2019

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.

@andig andig mentioned this pull request Jan 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants