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

Allow DFU flashing devices by Device ID #566

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/cli/keys.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ module.exports = ({ commandProcessor, root }) => {
boolean: true,
default: false,
description: 'Force overwriting of <filename> if it exists',
},
'device': {
description: 'Device ID of the target device',
}
},
handler: (args) => {
Expand Down Expand Up @@ -73,6 +76,9 @@ module.exports = ({ commandProcessor, root }) => {
number: true,
description: 'Port number of the server to add to the key'
},
'device': {
description: 'Device ID of the target device',
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have a number of commands in this same namespace that accept deviceID as a parameter - e.g.

commandProcessor.createCommand(keys, 'doctor', 'Creates and assigns a new key to your device, and uploads it to the cloud', {
params: '<deviceID>',
options: protocolOption,
handler: (args) => {
const KeysCommand = require('../cmd/keys');
return new KeysCommand().keyDoctor(args);
}
});

i think we should either support that call signature (total pain, requires manual arg checking / juggling, etc) - OR - we rename the option (flag) to --deviceID. in both cases we want to be more explicit about the expected input (since device means id or name across the rest of the CLI commands)

'deviceType': {
description: 'Generate key file for the provided device type'
}
Expand All @@ -84,7 +90,11 @@ module.exports = ({ commandProcessor, root }) => {
});

commandProcessor.createCommand(keys, 'address', 'Read server configured in device server public key', {
options: protocolOption,
options: Object.assign({}, protocolOption, {
'device': {
description: 'Device ID of the target device',
}
}),
handler: (args) => {
const KeysCommand = require('../cmd/keys');
return new KeysCommand().readServerAddress(args);
Expand Down
5 changes: 4 additions & 1 deletion src/cli/keys.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ describe('Keys Command-Line Interface', () => {
'Usage: particle keys save [options] <filename>',
'',
'Options:',
' --force Force overwriting of <filename> if it exists [boolean] [default: false]',
' --force Force overwriting of <filename> if it exists [boolean] [default: false]',
' --device Device ID of the target device [string]',
''
].join(os.EOL));
});
Expand Down Expand Up @@ -286,6 +287,7 @@ describe('Keys Command-Line Interface', () => {
' --protocol Communication protocol for the device using the key. tcp or udp [string]',
' --host Hostname or IP address of the server to add to the key [string]',
' --port Port number of the server to add to the key [number]',
' --device Device ID of the target device [string]',
' --deviceType Generate key file for the provided device type [string]',
'',
'Defaults to the Particle public cloud or you can provide another key in DER format and the server hostname or IP and port',
Expand Down Expand Up @@ -320,6 +322,7 @@ describe('Keys Command-Line Interface', () => {
'',
'Options:',
' --protocol Communication protocol for the device using the key. tcp or udp [string]',
' --device Device ID of the target device [string]',
'',
].join(os.EOL));
});
Expand Down
49 changes: 41 additions & 8 deletions src/cmd/flash.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
const fs = require('fs');
const VError = require('verror');
const dfu = require('../lib/dfu');
const settings = require('../../settings');
const ParticleApi = require('./api');
const { getDevice, isDeviceId } = require('./device-util');
const ModuleParser = require('binary-version-reader').HalModuleParser;
const ModuleInfo = require('binary-version-reader').ModuleInfo;
const deviceSpecs = require('../lib/deviceSpecs');
Expand All @@ -15,7 +18,7 @@ const systemModuleIndexToString = {


module.exports = class FlashCommand {
flash(device, binary, files, { usb, serial, factory, force, target, port, yes }){
async flash(device, binary, files, { usb, serial, factory, force, target, port, yes }){
if (!device && !binary){
// if no device nor files are passed, show help
// TODO: Replace by UsageError
Expand All @@ -24,16 +27,42 @@ module.exports = class FlashCommand {

let result;
if (usb){
result = this.flashDfu({ binary, factory, force });
if (files.length > 0) {
// If both a device and a binary were provided, the binary will come in as the
// first element in the files array
binary = files[0];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is ambiguous at best and likely inaccurate. i've been adding tests like this to help clarify similar behavior.

} else {
// Otherwise, no device has been provided so unset the inaccurate value
device = undefined;
}

// Lookup the Device ID based on the provided Device Name
if (device && !isDeviceId(device)) {
const api = new ParticleApi(settings.apiUrl, { accessToken: settings.access_token }).api;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at some point i think we're going to have to insist on device IDs across the board as this type of thing leads to confusion - e.g. "why am i getting 4xx errors when flashing locally?!" etc


let deviceInfo;
try {
deviceInfo = await getDevice({ id: device, api, auth: settings.access_token });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getDevice() from device-util doesn't really do anything special - i'd love it if we could just use the ParticleApi bits directly so eventually i can just remove device-util entirely.

} catch (err) {
throw new VError(ensureError(err), 'Device ID lookup failed');
}

if (deviceInfo && deviceInfo.id) {
device = deviceInfo.id;
} else {
throw new VError('Device ID lookup failed');
}
}

result = this.flashDfu({ device, binary, factory, force });
} else if (serial){
result = this.flashYModem({ binary, port, yes });
} else {
result = this.flashCloud({ device, files, target });
}

return result.then(() => {
console.log ('\nFlash success!');
});
await result;
console.log ('\nFlash success!');
}

flashCloud({ device, files, target }){
Expand All @@ -47,12 +76,16 @@ module.exports = class FlashCommand {
return new SerialCommands().flashDevice(binary, { port, yes });
}

flashDfu({ binary, factory, force, requestLeave }){
flashDfu({ device, binary, factory, force, requestLeave }){
let specs, destSegment, destAddress;
let flashingKnownApp = false;
let targetDevice = {};
return Promise.resolve()
.then(() => dfu.isDfuUtilInstalled())
.then(() => dfu.findCompatibleDFU())
.then(() => dfu.findCompatibleDFU({ deviceId: device }))
.then((compatibleDevice) => {
targetDevice = compatibleDevice;
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: i've been holding off on async / await to avoid this king of wackiness. i'd prefer either keeping old-school promises or refactoring the entire method instead of mixing things like this.

.then(() => {
//only match against knownApp if file is not found
let stats;
Expand Down Expand Up @@ -148,7 +181,7 @@ module.exports = class FlashCommand {
const alt = 0;
// todo - leave on factory firmware write too?
const leave = requestLeave !== undefined ? requestLeave : (destSegment === 'userFirmware');
return dfu.writeDfu(alt, finalBinary, destAddress, leave);
return dfu.writeDfu(alt, finalBinary, destAddress, leave, targetDevice);
})
.catch((err) => {
throw new VError(ensureError(err), 'Error writing firmware');
Expand Down
42 changes: 29 additions & 13 deletions src/cmd/keys.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ module.exports = class KeysCommand {
.then(() => {
return Promise.resolve()
.then(() => this.dfu.isDfuUtilInstalled())
.then(() => this.dfu.findCompatibleDFU(showHelp))
.then(() => this.dfu.findCompatibleDFU({ showHelp }))
.catch((err) => {
if (protocol){
alg = this.keyAlgorithmForProtocol(protocol);
Expand Down Expand Up @@ -167,14 +167,15 @@ module.exports = class KeysCommand {
});
}

saveKeyFromDevice({ force, params: { filename } }){
saveKeyFromDevice({ force, device, params: { filename } }){
filename = utilities.filenameNoExt(filename) + '.der';
return this._saveKeyFromDevice({ filename, force });
return this._saveKeyFromDevice({ filename, force, deviceId: device });
}

_saveKeyFromDevice({ filename, force }){
_saveKeyFromDevice({ filename, force, deviceId }) {
const { tryDelete, filenameNoExt, deferredChildProcess } = utilities;
let protocol;
let targetDevice = {};

if (!force && fs.existsSync(filename)){
throw new VError('This file already exists, please specify a different file, or use the --force flag.');
Expand All @@ -187,12 +188,15 @@ module.exports = class KeysCommand {

return Promise.resolve()
.then(() => this.dfu.isDfuUtilInstalled())
.then(() => this.dfu.findCompatibleDFU())
.then(() => this.dfu.findCompatibleDFU({ deviceId }))
.then((compatibleDevice) => {
targetDevice = compatibleDevice;
})
.then(() => this.validateDeviceProtocol())
.then(_protocol => {
protocol = _protocol;
let segment = this._getPrivateKeySegmentName({ protocol });
return this.dfu.read(filename, segment, false);
return this.dfu.read(filename, segment, false, targetDevice);
})
.then(() => {
let pubPemFilename = filenameNoExt(filename) + '.pub.pem';
Expand Down Expand Up @@ -328,19 +332,20 @@ module.exports = class KeysCommand {
return addressBuf;
}

writeServerPublicKey({ protocol, host, port, deviceType, params: { filename, outputFilename } }){
writeServerPublicKey({ protocol, host, port, device, deviceType, params: { filename, outputFilename } }){
if (deviceType && !filename){
throw usageError(
'`filename` parameter is required when `--deviceType` is set'
);
}

if (filename && !fs.existsSync(filename)){
if (filename && !fs.existsSync(filename)) {
// TODO UsageError
throw new VError('Please specify a server key in DER format.');
}

let skipDFU = false;

if (deviceType){
skipDFU = true;

Expand All @@ -349,13 +354,18 @@ module.exports = class KeysCommand {
.filter(key => deviceSpecs[key].productName.toLowerCase() === deviceType.toLowerCase())[0];
}

let targetDevice = {};

return Promise.resolve()
.then(() => {
if (skipDFU){
return protocol;
}
return this.dfu.isDfuUtilInstalled()
.then(() => this.dfu.findCompatibleDFU())
.then(() => this.dfu.findCompatibleDFU({ deviceId: device }))
.then((compatibleDevice) => {
targetDevice = compatibleDevice;
})
.then(() => this.validateDeviceProtocol({ protocol }));
})
.then(_protocol => {
Expand All @@ -367,8 +377,9 @@ module.exports = class KeysCommand {
})
.then(bufferFile => {
let segment = this._getServerKeySegmentName({ protocol });

if (!skipDFU){
return this.dfu.write(bufferFile, segment, false);
return this.dfu.write(bufferFile, segment, false, targetDevice);
}
})
.then(() => {
Expand All @@ -383,25 +394,30 @@ module.exports = class KeysCommand {
});
}

readServerAddress({ protocol }){
readServerAddress({ protocol, device }) {
let keyBuf, serverKeySeg;
let targetDevice = {};

return Promise.resolve()
.then(() => this.dfu.isDfuUtilInstalled())
.then(() => this.dfu.findCompatibleDFU())
.then(() => this.dfu.findCompatibleDFU({ deviceId: device }))
.then((compatibleDevice) => {
targetDevice = compatibleDevice;
})
.then(() => this.validateDeviceProtocol({ protocol }))
.then(_protocol => {
protocol = _protocol;
serverKeySeg = this._getServerKeySegment({ protocol });
})
.then(() => {
let segment = this._getServerKeySegmentName({ protocol });
return this.dfu.readBuffer(segment, false)
return this.dfu.readBuffer(segment, false, targetDevice)
.then((buf) => {
keyBuf = buf;
});
})
.then(() => {
// TODO (mirande): extract this and unit test...
let offset = serverKeySeg.addressOffset || 384;
let portOffset = serverKeySeg.portOffset || 450;
let type = keyBuf[offset];
Expand Down
12 changes: 7 additions & 5 deletions src/cmd/update.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ const spin = new Spinner('Updating system firmware on the device...');

module.exports = class UpdateCommand {
updateDevice() {
return dfu.findCompatibleDFU().then((deviceId) => {
return doUpdate(deviceId);
}).catch((err) => {
return dfuError(err);
});
return dfu.findCompatibleDFU()
.then((targetDevice) => {
return doUpdate(targetDevice.dfuId);
})
.catch((err) => {
return dfuError(err);
});
}
};

Expand Down