nathankellenicki / node-poweredup

A Javascript module to interface with LEGO Powered Up components.
https://nathankellenicki.github.io/node-poweredup/
MIT License
478 stars 59 forks source link

multiple discover event listeners on repeated scans calls #10

Closed firien closed 5 years ago

firien commented 5 years ago

I am tinkering with a web app to control some Duplo trains. I want it to handle multiple trains and be robust enough to deal with them powering on and off. (My thought was to have this running on a Pi Zero 24/7)

The scan function for PoweredUP stops scanning after a discovery. Calling scan again seems to add duplicate event listeners for the Noble discover event. Was it not designed for the scan to start and stop?


thanks for making this project

nathankellenicki commented 5 years ago

I'm afraid I experience different behaviour to you; in fact, I've specifically included a workaround to get around common reasons why it stops scanning after a discovery - see here: https://github.com/nathankellenicki/node-poweredup/blob/master/src/poweredup.ts#L87-L89

I myself connect to multiple hubs in multiple Lego trains with multiple remotes. I also connect boost hubs with PUP remotes.

So, to answer your questions:

  1. I don't. :)
  2. Yes of course, assuming we discover why it stops scanning for you in the first place.

So, can you provide a little more information to help debug? What OS/Node version/BLE adapter/etc? Any additional information?

Regarding attaching multiple event listeners to Noble's discover event - good find, this is indeed a bug. It should better handling calling scan multiple times (even if it does nothing when already scanning). Will add it to the todo list. Thanks!

firien commented 5 years ago

I don't actually have multiple hubs to connect to yet - so I didn't experience it - just thought I noticed a bug while reading the code. But I definitely missed the bang! when i read it - thought it only started again for browser contexts.

nathankellenicki commented 5 years ago

Ah I see. Yes, for browser contexts it explicitly doesn't call scan again. The reason for this is that doing so with the web-bluetooth spec displays a modal dialog box asking the user to select the Bluetooth device they want. Therefore repeated calls to scan is the most desirable behaviour as they would have to choose their devices individually.

However the browser context functionality isn't fully implemented in this library at the moment for various reasons.

Let me know if you experience discovery issues when you have multiple hubs to test with. In the meantime, I'll work on improving the scan event listener attachments.

nathankellenicki commented 5 years ago

Fixed with 363eb5f4d0f4e04f623c6c6f22a1322fa1e54dde