feat: unit tests on WebUsbHidTransport

packages/core/src/internal/usb/model/AccessibleUsbHidDevice.ts

@@ -7,6 +7,7 @@ export type AccessibleUsbHidDevice = {
   id: DeviceId, // UUID to map with the associated transport device 
   deviceModel: DeviceModel,
 
+  // TODO: actually not necessary
   // Informs if the device was selected (in the case of WebHID) during the discovery
-  isSelected: boolean,
+  // isSelected: boolean,
 };

packages/core/src/internal/usb/transport/WebUsbHidTransport.test.ts

@@ -0,0 +1,151 @@
+/* eslint-disable @typescript-eslint/no-explicit-any */
+/* eslint-disable @typescript-eslint/no-unsafe-assignment */
+import { NoAccessibleDevice, UsbHidTransportNotSupported } from "./UsbHidTransport";
+import { WebUsbHidTransport } from "./WebUsbHidTransport";
+
+describe("WebUsbHidTransport", () => {
+  describe("When WebHID API is not supported", () => {
+    test("isSupported should return false", () => {
+      const transport = new WebUsbHidTransport();
+      expect(transport.isSupported()).toBe(false);
+    });
+
+    test("startDiscovering should emit an error", (done) => {
+      const transport = new WebUsbHidTransport();
+      transport.startDiscovering().subscribe({
+        next: () => {
+          done("Should not emit any value");
+        },
+        error: (error) => {
+          expect(error).toBeInstanceOf(UsbHidTransportNotSupported);
+          done();
+        },
+      });
+    });
+  });
+
+  describe("When WebHID API is supported", () => {
+    const mockedGetDevices = jest.fn();
+    const mockedRequestDevice = jest.fn();
+
+    beforeAll(() => {
+      global.navigator = {
+        hid: {
+          getDevices: mockedGetDevices,
+          requestDevice: mockedRequestDevice,
+          addEventListener: jest.fn(),
+        }
+      } as any;
+    });
+
+    afterAll(() => {
+      global.navigator = undefined as any;
+    });
+
+    it("isSupported should return true", () => {
+      const transport = new WebUsbHidTransport();
+      expect(transport.isSupported()).toBe(true);
+    });
+
+    describe("startDiscovering", () => {
+      test("If the user grant us access to a device, we should emit it", (done) => {
+        mockedRequestDevice.mockResolvedValueOnce([{
+          opened: false,
+          productId: 0x0001,
+          vendorId: 0x2c97,
+          productName: "Ledger Nano X",
+          collections: [],  
+        }]);
+
+        const transport = new WebUsbHidTransport();
+        transport.startDiscovering().subscribe({
+          next: (discoveredDevice) => {
+            try {
+              expect(discoveredDevice).toEqual(expect.objectContaining({
+                deviceModel: "nanoX",
+              }));
+
+              done();
+            } catch (expectError) {
+              done(expectError);
+            }
+          },
+          error: (error) => {
+            done(error);
+          },
+        });
+      });
+
+      // It does not seem possible for a user to select several devices on the browser popup.
+      // But if it was possible, we should emit them
+      test("If the user grant us access to several devices, we should emit them", (done) => {
+        mockedRequestDevice.mockResolvedValueOnce([{
+          opened: false,
+          productId: 0x0004,
+          vendorId: 0x2c97,
+          productName: "Ledger Nano X",
+          collections: [],  
+        }, {
+          opened: false,
+          productId: 0x0005,
+          vendorId: 0x2c97,
+          productName: "Ledger Nano S Plus",
+          collections: [],  
+        }]);
+
+        const transport = new WebUsbHidTransport();
+
+        let count = 0;
+        transport.startDiscovering().subscribe({
+          next: (discoveredDevice) => {
+            console.log("🦄 discoveredDevice", discoveredDevice);
+            try {
+              switch(count) {
+                case 0:
+                  expect(discoveredDevice).toEqual(expect.objectContaining({
+                    deviceModel: "nanoX",
+                  }));
+                  break;
+                case 1:
+                  expect(discoveredDevice).toEqual(expect.objectContaining({
+                    deviceModel: "nanoSPlus",
+                  }));
+
+                  done();
+                  break;
+              }
+
+              count++;
+            } catch (expectError) {
+              done(expectError);
+            }
+          },
+          error: (error) => {
+            done(error);
+          },
+        });
+      });
+
+      // [ASK] Is this the behavior we want when the user does not select any device ?
+      test("If the user did not grant us access to a device (clicking on cancel on the browser popup for ex), we should emit an error", (done) => {
+        // When the user does not select any device, the `requestDevice` will return an empty array
+        mockedRequestDevice.mockResolvedValueOnce([]);
+
+        const transport = new WebUsbHidTransport();
+        transport.startDiscovering().subscribe({
+          next: (discoveredDevice) => {
+            done(`Should not emit any value, but emitted ${JSON.stringify(discoveredDevice)}`);
+          },
+          error: (error) => {
+            try {
+              expect(error).toBeInstanceOf(NoAccessibleDevice);
+              done();
+            } catch (expectError) {
+              done(expectError);
+            }
+          },
+        });
+      });
+    });
+  });
+});

packages/core/src/internal/usb/transport/WebUsbHidTransport.ts

@@ -4,7 +4,7 @@ import { v4 as uuid } from "uuid";
 import { ledgerVendorId } from '@internal/usb/data/UsbHidConfig';
 import { AccessibleUsbHidDevice } from '@internal/usb/model/AccessibleUsbHidDevice';
 import { NoAccessibleDevice, UsbHidTransportNotSupported, type GetAccessibleDevicesError, type PromptDeviceAccessError, type UsbHidTransport } from "./UsbHidTransport";
-import { Observable, from, map } from "rxjs";
+import { Observable, from, switchMap } from "rxjs";
 
 @injectable()
 export class WebUsbHidTransport implements UsbHidTransport {
@@ -31,7 +31,14 @@ export class WebUsbHidTransport implements UsbHidTransport {
   };
 
   isSupported(): boolean {
-    return !!(navigator?.hid);
+    try {
+      const result = !!(navigator?.hid);
+      console.log(`isSupported: ${result}`);
+      return result;
+    } catch (error) {
+      console.error(`isSupported: ${error}`, error);
+      return false;
+    }
   }
 
   /**
@@ -63,7 +70,6 @@ export class WebUsbHidTransport implements UsbHidTransport {
           id: deviceId,
           // TODO
           deviceModel: "nanoX",
-          isSelected: false,
         };
 
         this.accessibleDevicesToHidDevices.set(accessibleUsbHidDevice, hidDevice);
@@ -75,7 +81,7 @@ export class WebUsbHidTransport implements UsbHidTransport {
     }).run();
   }
 
-  async _promptDeviceAccess(): Promise<Either<PromptDeviceAccessError, AccessibleUsbHidDevice>> {
+  async promptDeviceAccess(): Promise<Either<PromptDeviceAccessError, AccessibleUsbHidDevice[]>> {
 
     return EitherAsync.liftEither(this.hidApi()).map(async (hidApi) => {
       // `requestDevice` returns an array. but normally the user can select only one device at a time.
@@ -90,35 +96,46 @@ export class WebUsbHidTransport implements UsbHidTransport {
 
       console.log("promptDeviceAccess: hidDevices len", hidDevices.length);
 
-      // Currently, only 1 device can be selected at a time
+      // Granted access to 0 device (by clicking on cancel for ex) results in an error
       if (hidDevices.length === 0) {
         console.warn("No device was selected");
         throw new NoAccessibleDevice(new Error("No selected device"));
       }
 
-      // [ASK] typescript should see that i have at least 1 element in the array, i should not need the casting
-      const hidDevice = hidDevices[0] as HIDDevice;
-      const id = uuid();
-      const accessibleUsbHidDevice: AccessibleUsbHidDevice = {
+      const accessibleDevices: AccessibleUsbHidDevice[] = [];
+      // There is no unique identifier for the device from the USB/HID connection,
+      // so the previously known accessible devices list cannot be trusted. 
+      this.accessibleDevicesToHidDevices.clear();
+
+      for (const hidDevice of hidDevices) {
+        const id = uuid();
+        const accessibleUsbHidDevice: AccessibleUsbHidDevice = {
           id,
           // TODO
           deviceModel: "nanoX",
-          // The user granted access to this device by selecting it          
-          isSelected: true,
-      };
+        };
 
-      this.accessibleDevicesToHidDevices.set(accessibleUsbHidDevice, hidDevice);
+        this.accessibleDevicesToHidDevices.set(accessibleUsbHidDevice, hidDevice);
+        accessibleDevices.push(accessibleUsbHidDevice);
 
-      console.log("promptDeviceAccess: selected device", hidDevice);
-      console.log("promptDeviceAccess: AccessibleUsbHidDevice", id);
+        console.log("promptDeviceAccess: selected device", hidDevice);
+        console.log("promptDeviceAccess: AccessibleUsbHidDevice", id);
+      }
 
-      return accessibleUsbHidDevice;
+      return accessibleDevices;
     }).run();
   }
 
   /**
    * For WebHID, the client can only discover devices for which the user granted access to.
    * 
+   * The issue is that once a user grant access to a device of a model/productId A, any other model/productId A device will be accessible.
+   * Even if plugged on another USB port.
+   * So we cannot rely on the `hid.getDevices` to get the list of accessible devices, because it is not possible to differentiate
+   * between 2 devices of the same model.
+   * Neither on `connect` and `disconnect` events.
+   * We can only rely on the `hid.requestDevice` because it is the user who will select the device that we can access.
+   * 
    * 2 possible implementations:
    * - only `hid.requestDevice` and return the one selected device
    * - `hid.getDevices` first to get the previously accessible devices, then a `hid.requestDevice` to get any new one
@@ -129,18 +146,18 @@ export class WebUsbHidTransport implements UsbHidTransport {
    * So the consumer can directly select this device.
    */
   startDiscovering(): Observable<AccessibleUsbHidDevice> {
+    // Logs the connection and disconnection events
     this.startListeningToConnectionEvents();
 
-    // TODO: add hid.getDevices()
-    return from(this._promptDeviceAccess()).pipe(map((either) => {
+    return from(this.promptDeviceAccess()).pipe(switchMap((either) => {
       return either.caseOf({
         Left: (error) => {
           console.error("Error while getting accessible device", error);
           throw error;
         },
-        Right: (device) => {
-          console.log("Got accessible device", device);
-          return device;
+        Right: (devices) => {
+          console.log("Got accessible devices:", devices);
+          return from(devices);
         },
       })
     }));
@@ -150,6 +167,9 @@ export class WebUsbHidTransport implements UsbHidTransport {
     this.stopListeningToConnectionEvents();
   }
 
+  /**
+   * Logs `connect` and `disconnect` events for already accessible devices
+   */
   private startListeningToConnectionEvents(): void {
     this.hidApi().map((hidApi) => {
       hidApi.addEventListener("connect", (event) => {