Address review comments

src/dfu-device.js

@@ -8,7 +8,7 @@ const DfuDevice = (base) => class extends base {
 	 * @param {Object} options Optional options for the flashing process.
 	 * @returns {Promise<void>} A Promise that resolves when the firmware is successfully flashed.
 	 */
-	async flashWithDfu(altSetting, buffer, addr, options) {
+	async writeOverDfu(altSetting, buffer, addr, options) {
 		await this._dfu.setAltSetting(altSetting);
 		await this._dfu.doDownload(addr, buffer, options);
 	}

src/dfu.js

@@ -18,7 +18,7 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-const { DeviceError } = require('./error');
+const { DeviceError, UsbStallError } = require('./error');
 
 /**
  * A generic DFU error.
@@ -136,8 +136,7 @@ const DfuseCommand = {
 
 const DfuBmRequestType = {
 	HOST_TO_DEVICE: 0x21,
-	DEVICE_TO_HOST: 0xA1,
-	DEVICE_TO_HOST_STANDARD: 0x80 // Direction: device-to-host; type: standard; recipient: device
+	DEVICE_TO_HOST: 0xA1
 };
 
 const DFU_STATUS_SIZE = 6;
@@ -192,7 +191,7 @@ class Dfu {
 	async leave() {
 		await this._goIntoDfuIdleOrDfuDnloadIdle();
 
-		await this._sendDnloadRequest(0, 2);
+		await this._sendDnloadRequest(Buffer.alloc(0), 2);
 
 		await this._pollUntil(
 			// Wait for dfuDNLOAD_IDLE in case of Gen2 and for dfuMANIFEST in case of Gen3 and above.
@@ -253,16 +252,13 @@ class Dfu {
 	 * @param {object} options - Options for the download process.
 	 * @return {Promise}
 	 */
-	async doDownload(startAddr, data, options) {
+	async doDownload(startAddr, data, options = { noErase: false, leave: false }) {
 		if (!this._memoryInfo || !this._memoryInfo.segments) {
 			throw new Error('No memory map available');
 		}
 
-		let startAddress = startAddr;
-		if (isNaN(startAddress)) {
-			startAddress = this._memoryInfo.segments[0].start;
-			this._log.warn('Using inferred start address 0x' + startAddress.toString(16));
-		} else if (this._getSegment(startAddress) === null) {
+		const startAddress = startAddr;
+		if (this._getSegment(startAddress) === null) {
 			this._log.error(`Start address 0x${startAddress.toString(16)} outside of memory map bounds`);
 		}
 		const expectedSize = data.byteLength;
@@ -282,12 +278,11 @@ class Dfu {
 
 			let dfuStatus;
 			try {
-				await this._dfuseCommand(DfuseCommand.DFUSE_COMMAND_SET_ADDRESS_POINTER, address, 4);
+				await this._dfuseCommand(DfuseCommand.DFUSE_COMMAND_SET_ADDRESS_POINTER, address);
 				this._log.trace(`Set address to 0x${address.toString(16)}`);
 				await this._sendDnloadRequest(data.slice(bytesSent, bytesSent + chunkSize), 2);
-				dfuStatus = await this._pollUntilIdle(DfuDeviceState.dfuDNLOAD_IDLE);
+				dfuStatus = await this._pollUntil(state => (state === DfuDeviceState.dfuDNLOAD_IDLE));
 				this._log.trace('Sent ' + chunkSize + ' bytes');
-				dfuStatus = await this._goIntoDfuIdleOrDfuDnloadIdle();
 				address += chunkSize;
 			} catch (error) {
 				throw new Error('Error during DfuSe download: ' + error);
@@ -346,14 +341,14 @@ class Dfu {
 	 * @param {Buffer} req The request data buffer to be sent to the device.
 	 * @param {number} wValue The value to be sent as part of the request.
 	 */
-	async _sendDnloadRequest(req, wValue) {
+	async _sendDnloadRequest(data, wValue) {
 		const setup = {
 			bmRequestType: DfuBmRequestType.HOST_TO_DEVICE,
 			bRequest: DfuRequestType.DFU_DNLOAD,
 			wIndex: this._interface,
 			wValue: wValue
 		};
-		return this._dev.transferOut(setup, req);
+		return this._dev.transferOut(setup, data);
 	}
 
 	/**
@@ -380,10 +375,6 @@ class Dfu {
 		bStatusWithPollTimeout &= ~(0xff);
 		const bState = data.readUInt8(4);
 
-		if (!bState) {
-			throw new DfuError('Could not parse DFU result or state');
-		}
-
 		return {
 			status: bStatus,
 			pollTimeout: bStatusWithPollTimeout,
@@ -415,13 +406,6 @@ class Dfu {
 		return dfuStatus;
 	}
 
-	/**
-	 * Polls until the dfu state is dfuDNLOAD_IDLE
-	 */
-	_pollUntilIdle(idleState) {
-		return this._pollUntil(state => (state === idleState));
-	}
-
 	/**
 	 * Sends the DFU_CLRSTATUS request to the DFU device to clear any error status.
 	 */
@@ -493,10 +477,9 @@ class Dfu {
 	 * @param {number} len - Optional. The length of the command payload.
 	 * @return {Promise}
 	 */
-	async _dfuseCommand(command, param, len) {
-		if (typeof param === 'undefined' && typeof len === 'undefined') {
+	async _dfuseCommand(command, param) {
+		if (typeof param === 'undefined') {
 			param = 0x00;
-			len = 1;
 		}
 
 		const commandNames = {
@@ -506,18 +489,15 @@ class Dfu {
 		};
 
 		const payload = Buffer.alloc(5);
-		payload[0] = command;
-		payload[1] = param & 0xff;
-		payload[2] = (param >> 8) & 0xff;
-		payload[3] = (param >> 16) & 0xff;
-		payload[4] = (param >> 24) & 0xff;
+		payload.writeUInt8(command, 0);
+		payload.writeUInt32LE(param, 1);
 
 		for (let triesLeft = 4; triesLeft >= 0; triesLeft--) {
 			try {
 				await this._sendDnloadRequest(payload, 0);
 				break;
 			} catch (error) {
-				if (triesLeft === 0 || error.message !== 'LIBUSB_TRANSFER_STALL') {
+				if (triesLeft === 0 || !(error instanceof UsbStallError)) {
 					throw new Error('Error during special DfuSe command ' + commandNames[command] + ':' + error);
 				}
 				this._log.trace('dfuse error, retrying', error);
@@ -622,7 +602,7 @@ class Dfu {
 			const sectorIndex = Math.floor((addr - segment.start) / segment.sectorSize);
 			const sectorAddr = segment.start + sectorIndex * segment.sectorSize;
 			this._log.trace(`Erasing ${segment.sectorSize}B at 0x${sectorAddr.toString(16)}`);
-			await this._dfuseCommand(DfuseCommand.DFUSE_COMMAND_ERASE, sectorAddr, 4);
+			await this._dfuseCommand(DfuseCommand.DFUSE_COMMAND_ERASE, sectorAddr);
 			addr = sectorAddr + segment.sectorSize;
 			bytesErased += segment.sectorSize;
 		}
@@ -631,7 +611,7 @@ class Dfu {
 	async _getStringDescriptor(index) {
 		// Read the size of the descriptor
 		let d = await this._dev.transferIn({
-			bmRequestType: DfuBmRequestType.DEVICE_TO_HOST_STANDARD,
+			bmRequestType: 0x80, // Direction: device-to-host; type: standard; recipient: device
 			bRequest: 0x06, // GET_DESCRIPTOR
 			wValue: (0x03 /* STRING */ << 8) | (index & 0xff),
 			wIndex: 0x0409, // English (US)
@@ -640,7 +620,7 @@ class Dfu {
 		const len = d.readUInt8(0); // bLength
 		// Read the descriptor
 		d = await this._dev.transferIn({
-			bmRequestType: DfuBmRequestType.DEVICE_TO_HOST_STANDARD,
+			bmRequestType: 0x80, // Direction: device-to-host; type: standard; recipient: device
 			bRequest: 0x06,
 			wValue: (0x03 << 8) | (index & 0xff),
 			wIndex: 0x0409,
@@ -659,7 +639,7 @@ class Dfu {
 	async _getConfigDescriptor(index) {
 		// Read the total size of the descriptors
 		const d = await this._dev.transferIn({
-			bmRequestType: DfuBmRequestType.DEVICE_TO_HOST_STANDARD,
+			bmRequestType: 0x80, // Direction: device-to-host; type: standard; recipient: device
 			bRequest: 0x06, // GET_DESCRIPTOR
 			wValue: (0x02 /* CONFIGURATION */ << 8) | (index & 0xff),
 			wIndex: 0,
@@ -668,7 +648,7 @@ class Dfu {
 		const len = d.readUInt16LE(2); // wTotalLength
 		// Read the descriptors
 		return await this._dev.transferIn({
-			bmRequestType: DfuBmRequestType.DEVICE_TO_HOST_STANDARD,
+			bmRequestType: 0x80, // Direction: device-to-host; type: standard; recipient: device
 			bRequest: 0x06,
 			wValue: (0x02 << 8) | (index & 0xff),
 			wIndex: 0,

src/error.js

@@ -99,6 +99,16 @@ class RequestError extends DeviceError {
 	}
 }
 
+/**
+ * USB stall error.
+ */
+class UsbStallError extends UsbError {
+	constructor(...args) {
+		super(...args);
+		this.name = this.constructor.name;
+	}
+}
+
 function assert(val, msg = null) {
 	if (!val) {
 		throw new InternalError(msg ? msg : 'Assertion failed');
@@ -116,5 +126,6 @@ module.exports = {
 	UsbError,
 	InternalError,
 	RequestError,
+	UsbStallError,
 	assert
 };

src/usb-device-node.js

@@ -1,4 +1,4 @@
-const { UsbError, NotAllowedError } = require('./error');
+const { UsbError, NotAllowedError, UsbStallError } = require('./error');
 const { globalOptions } = require('./config');
 
 let usb = null;
@@ -19,6 +19,9 @@ function wrapUsbError(err, message) {
 	if (err.message === 'LIBUSB_ERROR_ACCESS') {
 		return new NotAllowedError(message, { cause: err });
 	}
+	if (err.message === 'LIBUSB_TRANSFER_STALL') {
+		return new UsbStallError(message, { cause: err });
+	}
 	return new UsbError(message, { cause: err });
 }
 

src/usb-device-webusb.js

@@ -1,4 +1,4 @@
-const { UsbError } = require('./error');
+const { UsbError, UsbStallError } = require('./error');
 
 // Maximum size of a control transfer's data stage
 const MAX_CONTROL_TRANSFER_DATA_SIZE = 4096;
@@ -73,6 +73,9 @@ class UsbDevice {
 				index: setup.wIndex
 			}, setup.wLength);
 			if (res.status !== 'ok') {
+				if (res.status === 'stall') {
+					throw new UsbStallError(`Status: ${res.status}`);
+				}
 				throw new Error(`Status: ${res.status}`);
 			}
 			return Buffer.from(res.data.buffer);
@@ -94,6 +97,9 @@ class UsbDevice {
 				index: setup.wIndex
 			}, data); // data is optional
 			if (res.status !== 'ok') {
+				if (res.status === 'stall') {
+					throw new UsbStallError(`Status: ${res.status}`);
+				}
 				throw new Error(`Status: ${res.status}`);
 			}
 		} catch (err) {

test/dfu-device.js

@@ -94,7 +94,7 @@ describe('dfu device', () => {	// actually tests src/dfu.js which is the dfu dri
 				let error;
 				try {
 					await argonDev._dfu._goIntoDfuIdleOrDfuDnloadIdle();
-					await argonDev._dfu._dfuseCommand(0x21, 0x08060000, 5);
+					await argonDev._dfu._dfuseCommand(0x21, 0x08060000);
 				} catch (_error) {
 					error = _error;
 				}
@@ -131,7 +131,7 @@ describe('dfu device', () => {	// actually tests src/dfu.js which is the dfu dri
 				await argonDev._dfu._erase(startAddr, length);
 
 				expect(dfuseCommandStub.calledOnce).to.be.true;
-				expect(dfuseCommandStub.calledWithExactly(DfuseCommand.DFUSE_COMMAND_ERASE ,sectorAddr, 4)).to.be.true;
+				expect(dfuseCommandStub.calledWithExactly(DfuseCommand.DFUSE_COMMAND_ERASE ,sectorAddr)).to.be.true;
 			});
 
 			it('erases the memory on a p2', async () => {

test/support/fake-usb.js

@@ -51,7 +51,7 @@ class Protocol {
 
 	deviceToHostRequest(setup) {
 		if (setup.bmRequestType !== proto.BmRequestType.DEVICE_TO_HOST &&
-			setup.bmRequestType !== dfu.DfuBmRequestType.DEVICE_TO_HOST_STANDARD) {
+			setup.bmRequestType !== 0x80) {
 			throw new ProtocolError(`Unsupported device-to-host request: bmRequestType: ${setup.bmRequestType}`);
 		}
 		let data = null;
@@ -351,7 +351,7 @@ class DfuClass {
 	deviceToHostRequest(setup) {
 		// Implements DFU_GETSTATUS only
 		if (setup.bmRequestType !== dfu.DfuBmRequestType.DEVICE_TO_HOST &&
-			setup.bmRequestType !== dfu.DfuBmRequestType.DEVICE_TO_HOST_STANDARD) {
+			setup.bmRequestType !== 0x80) {
 			throw new UsbError('Unknown bmRequestType');
 		}