Skip to content

Commit

Permalink
1. Fix accessory state not updating after a rare Nest-side service er…
Browse files Browse the repository at this point in the history 
…ror.

2. Fix race condition affecting scenes/automations changing thermostat mode and temperature at the same time.
  • Loading branch information
@adriancable
adriancable committed Jan 28, 2022
1 parent bc724b7 commit 3284197
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 72 deletions.
1 change: 1 addition & 0 deletions index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class NestPlatform {
loadDevices(TempSensorAccessory);
loadDevices(ProtectAccessory);
loadDevices(LockAccessory);
this.conn.accessories = this.accessoryLookup;

return foundAccessories;
}.bind(this);
Expand Down
157 changes: 89 additions & 68 deletions lib/nest-connection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -513,18 +513,63 @@ class Connection {
}

return new Promise((res, rej) => {
let promiseCompleted = false;

const cleanAndResolve = val => {
clearTimeout(this.timeoutTimer);
clearInterval(this.cancelObserveTimer);
clearInterval(this.pingTimer);
this.timeoutTimer = null;
this.cancelObserveTimer = null;
this.pingTimer = null;

try {
req.destroy();
} catch(error) {
// Ignore
}

try {
client.destroy();
} catch(error) {
// Ignore
}

if (!promiseCompleted) {
promiseCompleted = true;
res(val);
}
};

const cleanAndReject = val => {
clearTimeout(this.timeoutTimer);
clearInterval(this.cancelObserveTimer);
clearInterval(this.pingTimer);
this.timeoutTimer = null;
this.cancelObserveTimer = null;
this.pingTimer = null;

try {
req.destroy();
} catch(error) {
// Ignore
}

try {
client.destroy();
} catch(error) {
// Ignore
}

if (!promiseCompleted) {
promiseCompleted = true;
rej(val);
}
};

const timeoutFunction = () => {
this.verbose('API observe POST: session/request timed out');
if (!client.forceClosed) {
client.forceClosed = true;
clearInterval(this.timeoutTimer);
clearInterval(this.cancelObserveTimer);
clearInterval(this.pingTimer);
this.cancelObserve = null;
this.timeoutTimer = null;
this.pingTimer = null;
res();
}
cleanAndReject({ code: 'ETIMEDOUT' });
};

if (!this.token || !this.connected) {
Expand All @@ -538,63 +583,31 @@ class Connection {

this.timeoutTimer = setTimeout(timeoutFunction, API_OBSERVE_TIMEOUT_SECONDS * 1000);

if (this.cancelObserve) {
this.verbose('API observe cancelled as initiating new call.');
try {
this.cancelObserve();
} catch (error) {
// Ignore
}
}

this.cancelObserve = null;
this.cancelObserveTimer = setInterval(() => {
if ((!this.token || !this.connected) && this.cancelObserve) {
this.verbose('API observe cancelled as not connected to Nest.');
try {
this.cancelObserve();
} catch (error) {
// Ignore
}
}
}, 1000);

let client = http2.connect(NestEndpoints.URL_PROTOBUF, { maxOutstandingPings: 2 });
client.on('error', error => {
this.verbose('API observe POST: client error', error);
rej(error);
cleanAndReject(error);
});

client.on('stream', () => {
this.verbose('API observe POST: new stream');
});

client.on('frameError', () => {
this.verbose('API observe POST: frame error');
});

client.on('goaway', () => {
this.verbose('API observe POST: go away');
});

client.on('ping', payload => {
this.verbose('API observe POST: incoming ping', payload.toString('base64'));
});

/* this.observePingTimer = setInterval(() => {
client.ping((err, duration, payload) => {
console.log('API observe PING:', duration, payload, err);
if (!client.connecting && !client.closed && err) {
clearInterval(this.observePingTimer);
client.destroy();
}
});
}, 20000); */

client.on('close', () => {
this.verbose('API observe POST: stream ended');
if (!client.forceClosed) {
client.forceClosed = true;
clearInterval(this.timeoutTimer);
clearInterval(this.cancelObserveTimer);
clearInterval(this.pingTimer);
this.cancelObserve = null;
this.timeoutTimer = null;
this.pingTimer = null;
res();
}
this.verbose('API observe POST: session ended');
cleanAndResolve();
});

// API_OBSERVE_TIMEOUT_SECONDS
Expand Down Expand Up @@ -635,8 +648,12 @@ class Connection {
}
}, API_HTTP2_PING_INTERVAL_SECONDS * 1000);

req.write(protodata);
req.end();
this.cancelObserveTimer = setInterval(() => {
if (!this.token || !this.connected) {
this.verbose('API observe cancelled as not connected to Nest.');
cleanAndReject('not_connected');
}
}, 1000);

// this.cancelObserve = () => req.close(http2.constants.NGHTTP2_CANCEL);
this.cancelObserve = () => {
Expand Down Expand Up @@ -675,14 +692,7 @@ class Connection {
if (this.lastProtobufCode) {
try {
client.forceClosed = true;
clearInterval(this.timeoutTimer);
clearInterval(this.cancelObserveTimer);
clearInterval(this.pingTimer);
this.cancelObserve = null;
this.timeoutTimer = null;
this.pingTimer = null;
res();
client.close();
cleanAndResolve();
} catch(error) {
// Ignore
}
Expand All @@ -700,8 +710,11 @@ class Connection {
});
req.on('error', (error) => {
this.verbose('API observe POST: stream error', error);
rej(error);
cleanAndReject(error);
});

req.write(protodata);
req.end();
});
}

Expand Down Expand Up @@ -1028,7 +1041,7 @@ class Connection {

translateProperty(object, 'backplate_temperature', null, (backplateTemperature, id) => {
if (checkDeviceExists(body, id)) {
body.device[id].backplate_temperature = backplateTemperature.data.property.value.temperature.value.value;
body.device[id].backplate_temperature = backplateTemperature.data.property.value.temperature ? backplateTemperature.data.property.value.temperature.value.value : null;
}
});

Expand Down Expand Up @@ -1230,6 +1243,7 @@ class Connection {
// some point, and then removed. This thermostat was missing all its properties, so we
// need to catch error conditions in general and not prevent other devices from mounting
this.log('Warning: unable to use REST API device ' + unit[1] + ' (' + unit[0] + ') due to missing required properties.');
this.log(error);
}
});
}
Expand Down Expand Up @@ -1305,6 +1319,7 @@ class Connection {
}
}).catch(error => {
this.error('API observe: error', error);
this.error('^^^^^ this message is for information only, it does not mean there is a problem, please do not file a ticket unless you actually have a problem with the function of the plug-in');
this.error('Retrying in ' + API_RETRY_DELAY_SECONDS + ' seconds.');
return Promise.delay(API_RETRY_DELAY_SECONDS * 1000);
}).finally(() => {
Expand Down Expand Up @@ -1645,7 +1660,11 @@ class Connection {
};

if (protoType == 'type.nestlabs.com/nest.trait.hvac.TargetTemperatureSettingsTrait') {
let targetTemperatureType = currentState.shared[device].target_temperature_type.toUpperCase();
// Use this.accessories[device].device instead of this.currentState, because if an automation/scene does a mode change, then a temperature change, if Nest hasn't acknowledged
// the mode change before the temperature change goes in, the temperature change will set the mode back (since currentState
// will be out of date)
// let targetTemperatureType = currentState.shared[device].target_temperature_type.toUpperCase();
let targetTemperatureType = this.accessories[device].device.target_temperature_type.toUpperCase();
let deviceActive = true;
if (targetTemperatureType == 'OFF') {
deviceActive = false;
Expand All @@ -1666,8 +1685,10 @@ class Connection {
newEl.property.value = {
settings: {
hvacMode: targetTemperatureType,
targetTemperatureHeat: { value: currentState.shared[device].target_temperature_low },
targetTemperatureCool: { value: currentState.shared[device].target_temperature_high },
// targetTemperatureHeat: { value: currentState.shared[device].target_temperature_low },
// targetTemperatureCool: { value: currentState.shared[device].target_temperature_high },
targetTemperatureHeat: { value: this.accessories[device].device.target_temperature_low },
targetTemperatureCool: { value: this.accessories[device].device.target_temperature_high },
// targetTemperatureHeat: { value: targetTemperatureHeat },
// targetTemperatureCool: { value: targetTemperatureCool },
updateInfo: updateInfo,
Expand Down
7 changes: 6 additions & 1 deletion lib/nest-thermostat-accessory.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,12 @@ class NestThermostatAccessory extends NestDeviceAccessory {

if (this.device.has_temperature_sensors || this.platform.optionSet('Thermostat.SeparateBuiltInTemperatureSensor.Enable', this.device.serial_number, this.device.device_id)) {
const tempService = this.addService(Service.TemperatureSensor, this.homeKitSanitize(this.device.where_name + ' Temperature'), 'temp-sensor.' + this.device.serial_number);
this.bindCharacteristic(tempService, Characteristic.CurrentTemperature, 'Temperature', this.getBackplateTemperature, null, this.formatAsDisplayTemperature);

if (this.platform.optionSet('Thermostat.UseActiveTempSensorAsThermostatTemperature.Enable', this.device.serial_number, this.device.device_id)) {
this.bindCharacteristic(tempService, Characteristic.CurrentTemperature, 'Temperature', this.getCurrentTemperature, null, this.formatAsDisplayTemperature);
} else {
this.bindCharacteristic(tempService, Characteristic.CurrentTemperature, 'Temperature', this.getBackplateTemperature, null, this.formatAsDisplayTemperature);
}
tempService.getCharacteristic(Characteristic.CurrentTemperature).setProps({ minStep: this.tempStep, minValue: minGetTemp, maxValue: maxGetTemp });
}

Expand Down
6 changes: 3 additions & 3 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"url": "git://github.com/chrisjshull/homebridge-nest.git"
},
"deprecated": false,
"description": "Nest Thermostat and Protect plug-in for homebridge",
"description": "Nest Thermostat, Protect and Lock plug-in for homebridge",
"engines": {
"homebridge": ">=0.2.5",
"node": ">=7.0.0"
Expand All @@ -37,15 +37,15 @@
"prepublishOnly": "npm run lint",
"preversion": "npm run lint"
},
"version": "4.6.3",
"version": "4.6.4",
"warnings": [
{
"code": "ENOTSUP",
"required": {
"node": ">=7.0.0",
"homebridge": ">=0.2.5"
},
"pkgid": "[email protected].3"
"pkgid": "[email protected].4"
}
]
}

0 comments on commit 3284197

Please sign in to comment.