Skip to content

Address comments from copilot on PR 48#50

Merged
HaudinFlorence merged 10 commits intoimprove_bluetooth_manager_classfrom
address-comments-from-copilot-PR48
Apr 29, 2026
Merged

Address comments from copilot on PR 48#50
HaudinFlorence merged 10 commits intoimprove_bluetooth_manager_classfrom
address-comments-from-copilot-PR48

Conversation

@HaudinFlorence
Copy link
Copy Markdown
Member

@HaudinFlorence HaudinFlorence commented Apr 21, 2026

  1. Try and Catch in connect and disconnect methods
  2. Rewrite checkWebBluetoothSupport
  3. Make arguments consistent between declaration and implementation of removeAllDevices
  4. Rewrite removeAllDevices and emit signal only once, not at every device removal => it is still not okay after that see point 11.
  5. Prevent Double Disconnection in _removeDeviceFromList
  6. Add Null Check for native in connect Method
  7. remove async in:
device.disconnected.connect( async() => {
          this._removeDeviceFromList(device);
        });
  1. Return type in IBluetoothManager interface (is currently set to any)
  2. In the disconnect method of BluetoothManager.Device class, consider edge case where the GATT server is already disconnected,
  3. Remove device.dispose from:
  async disconnect(device: BluetoothManager.Device) {
    await device.disconnect();
    device.dispose();
  }

Indeed, in the current code, when a user calls disconnect(device), this device is disposed: this triggers the gattserverdisconnected event, which emits the disconnected signal, which calls _removeDeviceFromList and then tries to dispose again.
11. Fix multiple signal emissions in removeAllDevices: In the current code, on each loop iteration, _removeDeviceFromList() is called and a signal emitted (deviceListChanged).
12. Return undefined in connect() in BluetoothManager class for cases where device.connected is false.
13. Fix signal Emission Logic in addDeviceToList: in the current code, the signal is emitted regardless of whether the device was actually added. The signal should only been emitted when the device is newly added.
14. Prevent from double dispose() in removeDeviceFromList: In the current code, in removeDeviceFromList, you call device.dispose() after removal. It is better to first check it is not already disposed.
15. Improve requestDevice return logic to:

async requestDevice(
    registryItem: IDeviceTypeRegistryItem
  ): Promise<BluetoothDevice | undefined> {
    const isSupported = await this.checkWebBluetoothSupport();
    if (!isSupported) {
      return undefined;
    }
    return await navigator.bluetooth.requestDevice(registryItem.options);
  1. Replace index lookup by using a Map
    In removeDeviceFromList, the identifier index lookup is fragile. Consider using a Map lookups instead of array indexOf:

@HaudinFlorence HaudinFlorence force-pushed the address-comments-from-copilot-PR48 branch from b76659d to b4d7731 Compare April 21, 2026 16:16
@HaudinFlorence HaudinFlorence self-assigned this Apr 22, 2026
@HaudinFlorence HaudinFlorence added the enhancement New feature or request label Apr 22, 2026
@HaudinFlorence HaudinFlorence force-pushed the address-comments-from-copilot-PR48 branch from 7e9143e to 0878601 Compare April 22, 2026 15:35
@HaudinFlorence HaudinFlorence requested a review from afshin April 23, 2026 15:01
@HaudinFlorence HaudinFlorence changed the title Address comments from copilot pr48 Address comments from copilot on PR 48 Apr 23, 2026
@HaudinFlorence HaudinFlorence merged commit bdc1a5e into improve_bluetooth_manager_class Apr 29, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant