Skip to content

Commit

Permalink
Wifi: eliminate use of timeout in wifi connection verification
Browse files Browse the repository at this point in the history 
Eliminated the time out and replace it with a recursively called "polling" mechanism, I could increase the likelihood of successfully verifying the wifi connection.

Arbitrarily selected 10 as the limit of tries that will be made; this could actually be increased to ~33, which is the approximate number of tries that could potentially be made in 12 seconds (the delay period (2) + timeout period (10)). I determined this by counting the number of complete tries made against a known invalid ssid, within a 12 second period. I ran the measurement code 10 times and the rounded average count was 33 tries.
  • Loading branch information
@rwaldron
rwaldron committed Mar 2, 2016
1 parent 4119b79 commit 60e04ee
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 94 deletions.
110 changes: 60 additions & 50 deletions lib/tessel/wifi.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,17 +92,18 @@ Tessel.prototype.connectToNetwork = function(opts) {
var ssid = opts.ssid;
var password = opts.password;
var security = opts.security;
var status = 'Wifi connection successful.';
var status = 'Wifi Connected.';

if (password && !security) {
security = 'psk2';
}

status += ` SSID: ${ssid}`;

if (password && security) {
status += ' SSID: ' + ssid + ', password ' + password + ', security mode: ' + security;
status += `, password: ${password}, security: ${security}`;
} else if (!password && (!security || security === 'none')) {
security = 'none';
status += ' SSID: ' + ssid;
}

var setSSID = () => this.simpleExec(commands.setNetworkSSID(ssid));
Expand Down Expand Up @@ -137,61 +138,70 @@ Tessel.prototype.resetMDNS = function() {
};

Tessel.prototype.setWiFiState = function(enable) {
var logState = () => logs.info('Wifi', enable ? 'Enabled.' : 'Disabled.');

return new Promise((resolve, reject) => {
return this.simpleExec(commands.turnOnWifi(enable))
.then(() => this.simpleExec(commands.commitWirelessCredentials()))
.then(() => this.simpleExec(commands.reconnectWifi()))
.then(() => new Promise((resolve) => setTimeout(resolve, 2000))) // delay to let wifi reconnection settle before fetching info
.then(() => this.connection.exec(commands.getWifiInfo(), (err, remoteProcess) => {
if (err) {
return reject(err);
}
if (enable) {

var result = {
type: 'reject',
message: 'Unable to verify connection. Please ensure you have entered the correct network credentials.'
};

// A timeout in case the process never closes
var timeout = setTimeout(function() {
result = {
type: 'reject',
message: 'Timed out waiting to verify connection. Run `t2 wifi` to manually verify connection. If not connected, ensure you have entered the correct network credentials.'
};
return returnResult();
}, Tessel.__wifiConnectionTimeout);

remoteProcess.stdout.on('data', function(data) {
if (data.indexOf('signal') > -1) {
logs.info('Successfully connected!');
clearTimeout(timeout);
result = {
type: 'resolve',
message: '' // resolve doesn't report messages
};
.then(() => {
var settle = (rejection) => {
if (rejection) {
reject(rejection);
} else {
logs.info('Wifi', enable ? 'Enabled.' : 'Disabled.');
resolve();
}
};
/*
To explain the following "magic number"...
The `tries` limit is set to 10 as an arbitrarily chosen minimum and maximum restriction.
This could actually be increased to ~33, which is the approximate number of tries
that could potentially be made in 12 seconds (the delay period (2) + timeout period (10)).
This was determined by counting the number of complete tries made against a
known invalid ssid, within a 12 second period. I ran the measurement code 10 times
and the rounded average count was 33 tries.
`tries` shouldn't be set lower than 10, as each wifi info request takes approximately 340ms.
This was determined by measuring the time between each attempt against a known invalid
ssid (with no tries limit). 340 is the rounded average of 10 complete operations, including
only the lowest number of measurements; ie. if one operation made 35 tries, and nine
operatons made 30 tries, then the only the first 30 were included in the result set.
The result is a maximum of 3.4s before verification is treated as a failure.
This is slightly more than the previous delay + one operation, but substantially less than
the full 10 second timeout, with a higher likelihood of success (that is, zero failures were
observed when using a valid ssid + password + security).
*/
var tries = 10;
var pollForWifiSignal = () => {
this.connection.exec(commands.getWifiInfo(), (err, remoteProcess) => {
if (err) {
return reject(err);
}
this.receive(remoteProcess, (err, result) => {
if (err) {
return reject(err);
}
if (result.toString().includes('signal')) {
settle();
} else {
tries--;

if (tries) {
pollForWifiSignal();
} else {
settle('Unable to verify connection. Please ensure you have entered the correct network credentials.');
}
}
});
});
};

var returnResult = () => {
if (result.type === 'reject') {
reject(result.message);
} else {
logState();
resolve();
}
};

remoteProcess.on('close', returnResult);
if (enable) {
pollForWifiSignal();
} else {
logState();
resolve();
settle();
}

}));
});
});
};

Tessel.__wifiConnectionTimeout = 10000;
79 changes: 35 additions & 44 deletions test/unit/wifi.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,15 +197,13 @@ module.exports['Tessel.prototype.connectToNetwork'] = {
// Write to stdout so it completes as expected
// Wrap in setImmediate to make sure listener is set up before emitting
setImmediate(() => {
this.tessel._rps.stdout.emit('data', 'signal');
setImmediate(() => this.tessel._rps.emit('close'));
this.tessel._rps.stdout.emit('data', new Buffer('signal'));
this.tessel._rps.emit('close');
});
} else {
setImmediate(() => {
// Remove any listeners on stdout so we don't break anything when we write to it
this.tessel._rps.stdout.removeAllListeners();

// Continue
this.tessel._rps.emit('close');
});
}
Expand Down Expand Up @@ -241,15 +239,13 @@ module.exports['Tessel.prototype.connectToNetwork'] = {
// Write to stdout so it completes as expected
// Wrap in setImmediate to make sure listener is set up before emitting
setImmediate(() => {
this.tessel._rps.stdout.emit('data', 'signal');
setImmediate(() => this.tessel._rps.emit('close'));
this.tessel._rps.stdout.emit('data', new Buffer('signal'));
this.tessel._rps.emit('close');
});
} else {
setImmediate(() => {
// Remove any listeners on stdout so we don't break anything when we write to it
this.tessel._rps.stdout.removeAllListeners();

// Continue
this.tessel._rps.emit('close');
});
}
Expand Down Expand Up @@ -288,15 +284,13 @@ module.exports['Tessel.prototype.connectToNetwork'] = {
// Write to stdout so it completes as expected
// Wrap in setImmediate to make sure listener is set up before emitting
setImmediate(() => {
this.tessel._rps.stdout.emit('data', 'signal');
setImmediate(() => this.tessel._rps.emit('close'));
this.tessel._rps.stdout.emit('data', new Buffer('signal'));
this.tessel._rps.emit('close');
});
} else {
setImmediate(() => {
// Remove any listeners on stdout so we don't break anything when we write to it
this.tessel._rps.stdout.removeAllListeners();

// Continue
this.tessel._rps.emit('close');
});
}
Expand Down Expand Up @@ -327,24 +321,20 @@ module.exports['Tessel.prototype.connectToNetwork'] = {
ssid: 'tank',
password: 'not_gonna_work'
};
var errMessage = 'Unable to connect to the network.';
// Set the timeout to be small so we don't wait forever for the test to complete
Tessel.__wifiConnectionTimeout = 10;
// Test is expecting several closes...;
this.tessel._rps.on('control', (command) => {
if (command.toString() === 'ubus call iwinfo info {"device":"wlan0"}') {
// Write to stdout so it completes as expected
// Wrap in setImmediate to make sure listener is set up before emitting
setImmediate(() => {
this.tessel._rps.stdout.emit('data', errMessage);
setImmediate(() => this.tessel._rps.emit('close'));
// Is missing the "signal" status key
this.tessel._rps.stderr.emit('data', new Buffer('Unable to connect to the network.'));
this.tessel._rps.emit('close');
});
} else {
setImmediate(() => {
// Remove any listeners on stderr so we don't break anything when we write to it
this.tessel._rps.stdout.removeAllListeners();

// Continue
this.tessel._rps.emit('close');
});
}
Expand All @@ -355,7 +345,8 @@ module.exports['Tessel.prototype.connectToNetwork'] = {
test.ok(false, 'Test should have rejected with an error.');
test.done();
})
.catch(() => {
.catch((error) => {
console.log(error);
test.equal(this.setNetworkSSID.callCount, 1);
test.equal(this.setNetworkPassword.callCount, 1);
test.equal(this.setNetworkEncryption.callCount, 1);
Expand All @@ -369,24 +360,30 @@ module.exports['Tessel.prototype.connectToNetwork'] = {
});
},

connectionTimeout: function(test) {
connectionUnverifiable: function(test) {
test.expect(10);
var creds = {
ssid: 'tank',
password: 'not_gonna_work'
};

// Make it timeout super fast so this test doesn't take forever
Tessel.__wifiConnectionTimeout = 10;

// Test is expecting several closes...;
this.tessel._rps.on('control', (command) => {
if (command.toString() !== 'ubus call iwinfo info {"device":"wlan0"}') {
if (command.toString() === 'ubus call iwinfo info {"device":"wlan0"}') {
// console.log(this.tessel._rps);
// Write to stdout so it completes as expected
// Wrap in setImmediate to make sure listener is set up before emitting
setImmediate(() => {
// Is missing the "signal" status key
this.tessel._rps.stdout.emit('data', new Buffer('{"frequency": 2422,"txpower": 20}'));
this.tessel._rps.stdout.removeAllListeners();
this.tessel._rps.stderr.removeAllListeners();
this.tessel._rps.emit('close');
});
} else {
setImmediate(() => {
// Remove any listeners on stderr so we don't break anything when we write to it
this.tessel._rps.stdout.removeAllListeners();
this.tessel._rps.stderr.removeAllListeners();

// Continue
this.tessel._rps.emit('close');
});
}
Expand All @@ -398,7 +395,7 @@ module.exports['Tessel.prototype.connectToNetwork'] = {
test.done();
})
.catch((error) => {
test.ok(error.toLowerCase().indexOf('timed out') !== -1);
test.equal(error.includes('Unable to verify connection'), true);
test.equal(this.setNetworkSSID.callCount, 1);
test.equal(this.setNetworkPassword.callCount, 1);
test.equal(this.setNetworkEncryption.callCount, 1);
Expand Down Expand Up @@ -426,17 +423,15 @@ module.exports['Tessel.prototype.connectToNetwork'] = {
// Wrap in setImmediate to make sure listener is set up before emitting
setImmediate(() => {
// First write some random data
this.tessel._rps.stdout.emit('data', 'do not have success yet');
this.tessel._rps.stdout.emit('data', new Buffer('do not have success yet'));
// then write the keyword we're looking for for success
this.tessel._rps.stdout.emit('data', 'signal');
setImmediate(() => this.tessel._rps.emit('close'));
this.tessel._rps.stdout.emit('data', new Buffer('signal'));
this.tessel._rps.emit('close');
});
} else {
setImmediate(() => {
// Remove any listeners on stderr so we don't break anything when we write to it
this.tessel._rps.stdout.removeAllListeners();

// Continue
this.tessel._rps.emit('close');
});
}
Expand Down Expand Up @@ -492,15 +487,13 @@ module.exports['Tessel.setWifiState'] = {
// Write to stdout so it completes as expected
// Wrap in setImmediate to make sure listener is set up before emitting
setImmediate(() => {
this.tessel._rps.stdout.emit('data', 'signal');
setImmediate(() => this.tessel._rps.emit('close'));
this.tessel._rps.stdout.emit('data', new Buffer('signal'));
this.tessel._rps.emit('close');
});
} else {
setImmediate(() => {
// Remove any listeners on stdout so we don't break anything when we write to it
this.tessel._rps.stdout.removeAllListeners();

// Continue
this.tessel._rps.emit('close');
});
}
Expand All @@ -513,7 +506,7 @@ module.exports['Tessel.setWifiState'] = {
test.deepEqual(this.turnOnWifi.lastCall.returnValue, ['uci', 'set', 'wireless.@wifi-iface[0].disabled=0']);
test.equal(this.commitWirelessCredentials.callCount, 1);
test.equal(this.reconnectWifi.callCount, 1);
test.equal(this.logsInfo.calledTwice, true);
test.equal(this.logsInfo.calledOnce, true);
test.equal(this.logsInfo.lastCall.args[1].indexOf('Enabled.') !== -1, true);
test.ok(this.getWifiInfo.callCount, 1);
test.done();
Expand All @@ -533,15 +526,13 @@ module.exports['Tessel.setWifiState'] = {
// Write to stdout so it completes as expected
// Wrap in setImmediate to make sure listener is set up before emitting
setImmediate(() => {
this.tessel._rps.stdout.emit('data', 'signal');
setImmediate(() => this.tessel._rps.emit('close'));
this.tessel._rps.stdout.emit('data', new Buffer('signal'));
this.tessel._rps.emit('close');
});
} else {
setImmediate(() => {
// Remove any listeners on stdout so we don't break anything when we write to it
this.tessel._rps.stdout.removeAllListeners();

// Continue
this.tessel._rps.emit('close');
});
}
Expand All @@ -556,7 +547,7 @@ module.exports['Tessel.setWifiState'] = {
test.equal(this.reconnectWifi.callCount, 1);
test.equal(this.logsInfo.calledOnce, true);
test.equal(this.logsInfo.lastCall.args[1].indexOf('Disabled.') !== -1, true);
test.ok(this.getWifiInfo.callCount, 1);
test.equal(this.getWifiInfo.callCount, 0);
test.done();
})
.catch(error => {
Expand Down

0 comments on commit 60e04ee

Please sign in to comment.