node-usb-detection icon indicating copy to clipboard operation
node-usb-detection copied to clipboard

Events no longer fired after stopping and restarting monitoring

Open timfish opened this issue 6 years ago • 4 comments

We're using this library rather successfully in our Electron app but I'm trying to fix an issue on both Windows and macOS.

We use Electrons powerMonitor API to stopMonitoring and startMonitoring from the suspend and resume events respectively. After stopping and restarting monitoring, we no longer get USB change events. Is this the expected behaviour?

timfish avatar Jan 19 '18 13:01 timfish

Ok, so my initial thoughts were why are we even disabling usb detection when the device is in sleep. Testing on Windows 10 and macOS High Sierra seems to suggest that usb detection works perfectly well after coming out of suspend even if monitoring was left enabled throughout sleep.

This then begs the question whether it's possible to restart monitoring after its been stopped. If the stopMonitoring function is only for cleanup before exiting the process, it might be cleaner to use the node.js AtExit hooks and then consumers of this library don't have to worry about it anymore?

timfish avatar Jan 19 '18 14:01 timfish

Hi @timfish, Using AtExit would be the smart thing to do, but it looks like Electron 1.8 breaks that functionality currently: https://github.com/electron/electron/issues/11299

todbot avatar Jan 19 '18 18:01 todbot

I actually only found out that AtExit exists from that bug 🙂

timfish avatar Jan 20 '18 17:01 timfish

@timfish Sorry for the delay. In the 2.x release, I tried removing the side-effects and require startMonitoring to be called before things kick off but I didn't sufficiently test things and there are still side-effects.

~~I need to get https://github.com/MadLittleMods/node-usb-detection/pull/46 fixed up and merged and also look into ensuring start/stop work regardless of how many times called (help welcome).~~ I got https://github.com/MadLittleMods/node-usb-detection/pull/46 in working order and will aim for a merge tomorrow. But that is just the first step to making startMonitoring/stopMonitoring work after multiple calls.

stopMonitoring was more meant to be called once in order to remove all the blocking threads so your app can exit gracefully.

On macOS, I suspect after stopping, you can't start again because we stop those threads and don't start them back up in startMonitoring, see https://github.com/MadLittleMods/node-usb-detection/blob/9164178f40c28b0d5e2407be42f5aca9c8768e78/src/detection_mac.cpp#L390

On Windows, startMonitoring may work after stopMonitoring but I haven't tested it yet, https://github.com/MadLittleMods/node-usb-detection/blob/9164178f40c28b0d5e2407be42f5aca9c8768e78/src/detection_win.cpp#L167


For reference, here is a test I wrote to cover this basis but it doesn't succeed 100% of the time.

describe('multiple start/stop', () => {
	it('should only listen when monitoring', function(done) {
		usbDetect.startMonitoring();

		console.log(chalk.black.bgCyan('Add/Insert or Remove a USB device (again)'));
		once('change')
			.then(function(device) {
				testDeviceShape(device);
			})
			.then(() => {
				usbDetect.stopMonitoring();

				// Make sure we don't detect anything while not monitoring
				console.log(chalk.black.bgCyan(`Add/Insert or Remove a USB device (again). We don't want to detect anything for ${MANUAL_INTERACTION_TIMEOUT / 1000} seconds!`));
				var shouldDetect = false;
				once('change')
					.then(() => {
						if(!shouldDetect) {
							done.fail('We detected an `add` event when we were NOT monitoring anything');
						}
					});

				return getSetTimeoutPromise(MANUAL_INTERACTION_TIMEOUT)
					.then(() => {
						shouldDetect = true;
					});
			})
			.then(() => {
				usbDetect.startMonitoring();

				console.log(chalk.black.bgCyan('Add/Insert or Remove a USB device (again)'));
				return once('change');
			})
			.then(() => {
				console.log('gfjewfakefakljfewajlkfewalkjafewjlkafewjlkafewjlk');
				usbDetect.stopMonitoring();
			})
			.then(done)
			.catch(done.fail);
	}, 3 * MANUAL_INTERACTION_TIMEOUT);
});

MadLittleMods avatar Feb 04 '18 20:02 MadLittleMods