From 1373cc5e7111c60381d267d681fce9db11bb0f68 Mon Sep 17 00:00:00 2001 From: jdabbech-ledger Date: Thu, 16 May 2024 09:48:23 +0200 Subject: [PATCH 1/3] :sparkles: (core): Unplug connected device remove device session --- .../device-session/model/DeviceSession.ts | 1 + .../service/DefaultDeviceSessionService.ts | 8 +++ .../service/DeviceSessionService.ts | 1 + .../discovery/use-case/ConnectUseCase.ts | 14 ++++- .../internal/usb/transport/UsbHidTransport.ts | 2 + .../usb/transport/WebUsbHidTransport.test.ts | 57 ++++++++++++++++--- .../usb/transport/WebUsbHidTransport.ts | 21 +++++++ 7 files changed, 96 insertions(+), 8 deletions(-) diff --git a/packages/core/src/internal/device-session/model/DeviceSession.ts b/packages/core/src/internal/device-session/model/DeviceSession.ts index 47d0937e0..b49275a45 100644 --- a/packages/core/src/internal/device-session/model/DeviceSession.ts +++ b/packages/core/src/internal/device-session/model/DeviceSession.ts @@ -85,6 +85,7 @@ export class DeviceSession { } close() { + this.updateDeviceStatus(DeviceStatus.NOT_CONNECTED); this._deviceState.complete(); } } diff --git a/packages/core/src/internal/device-session/service/DefaultDeviceSessionService.ts b/packages/core/src/internal/device-session/service/DefaultDeviceSessionService.ts index 0b2d07353..1d2eca8fa 100644 --- a/packages/core/src/internal/device-session/service/DefaultDeviceSessionService.ts +++ b/packages/core/src/internal/device-session/service/DefaultDeviceSessionService.ts @@ -55,6 +55,14 @@ export class DefaultDeviceSessionService implements DeviceSessionService { return deviceSession.toEither(new DeviceSessionNotFound()); } + getDeviceSessionByDeviceId(deviceId: string) { + const deviceSession = Maybe.fromNullable( + this._sessions.find((s) => s.connectedDevice.id === deviceId), + ); + + return deviceSession.toEither(new DeviceSessionNotFound()); + } + getDeviceSessions() { return this._sessions; } diff --git a/packages/core/src/internal/device-session/service/DeviceSessionService.ts b/packages/core/src/internal/device-session/service/DeviceSessionService.ts index e0abd3bc4..ca486fa5d 100644 --- a/packages/core/src/internal/device-session/service/DeviceSessionService.ts +++ b/packages/core/src/internal/device-session/service/DeviceSessionService.ts @@ -6,6 +6,7 @@ import { DeviceSession } from "@internal/device-session/model/DeviceSession"; export interface DeviceSessionService { addDeviceSession(deviceSession: DeviceSession): DeviceSessionService; getDeviceSessionById(sessionId: string): Either; + getDeviceSessionByDeviceId(deviceId: string): Either; removeDeviceSession(sessionId: string): DeviceSessionService; getDeviceSessions(): DeviceSession[]; } diff --git a/packages/core/src/internal/discovery/use-case/ConnectUseCase.ts b/packages/core/src/internal/discovery/use-case/ConnectUseCase.ts index 646fb37b6..21bb70438 100644 --- a/packages/core/src/internal/discovery/use-case/ConnectUseCase.ts +++ b/packages/core/src/internal/discovery/use-case/ConnectUseCase.ts @@ -9,6 +9,7 @@ import { loggerTypes } from "@internal/logger-publisher/di/loggerTypes"; import { LoggerPublisherService } from "@internal/logger-publisher/service/LoggerPublisherService"; import { usbDiTypes } from "@internal/usb/di/usbDiTypes"; import type { UsbHidTransport } from "@internal/usb/transport/UsbHidTransport"; +import type { DisconnectHandler } from "@internal/usb/transport/WebUsbHidTransport"; export type ConnectUseCaseArgs = { /** @@ -39,8 +40,19 @@ export class ConnectUseCase { this._logger = loggerFactory("ConnectUseCase"); } + private handleHardwareDisconnect: DisconnectHandler = (deviceId) => { + const deviceSessionOrError = + this._sessionService.getDeviceSessionByDeviceId(deviceId); + deviceSessionOrError.map((deviceSession) => { + this._sessionService.removeDeviceSession(deviceSession.id); + }); + }; + async execute({ deviceId }: ConnectUseCaseArgs): Promise { - const either = await this._usbHidTransport.connect({ deviceId }); + const either = await this._usbHidTransport.connect({ + deviceId, + onDisconnect: this.handleHardwareDisconnect, + }); return either.caseOf({ Left: (error) => { diff --git a/packages/core/src/internal/usb/transport/UsbHidTransport.ts b/packages/core/src/internal/usb/transport/UsbHidTransport.ts index cdebe9a4a..5e62aceae 100644 --- a/packages/core/src/internal/usb/transport/UsbHidTransport.ts +++ b/packages/core/src/internal/usb/transport/UsbHidTransport.ts @@ -6,6 +6,7 @@ import { SdkError } from "@api/Error"; import { DiscoveredDevice } from "@internal/usb/model/DiscoveredDevice"; import { ConnectError } from "@internal/usb/model/Errors"; import { InternalConnectedDevice } from "@internal/usb/model/InternalConnectedDevice"; +import type { DisconnectHandler } from "@internal/usb/transport/WebUsbHidTransport"; /** * Transport interface representing a USB HID communication @@ -25,6 +26,7 @@ export interface UsbHidTransport { */ connect(params: { deviceId: DeviceId; + onDisconnect: DisconnectHandler; }): Promise>; disconnect(params: { diff --git a/packages/core/src/internal/usb/transport/WebUsbHidTransport.test.ts b/packages/core/src/internal/usb/transport/WebUsbHidTransport.test.ts index 3795fc37d..4bf91357b 100644 --- a/packages/core/src/internal/usb/transport/WebUsbHidTransport.test.ts +++ b/packages/core/src/internal/usb/transport/WebUsbHidTransport.test.ts @@ -67,6 +67,7 @@ describe("WebUsbHidTransport", () => { getDevices: mockedGetDevices, requestDevice: mockedRequestDevice, addEventListener: jest.fn(), + ondisconnect: jest.fn(), }, } as unknown as Navigator; }); @@ -238,9 +239,9 @@ describe("WebUsbHidTransport", () => { describe("connect", () => { it("should throw UnknownDeviceError if no internal device", async () => { - const device = { deviceId: "fake" }; + const connectParams = { deviceId: "fake", onDisconnect: jest.fn() }; - const connect = await transport.connect(device); + const connect = await transport.connect(connectParams); expect(connect).toStrictEqual( Left(new UnknownDeviceError(new Error("Unknown device fake"))), @@ -248,7 +249,7 @@ describe("WebUsbHidTransport", () => { }); it("should throw OpeningConnectionError if the device is already opened", async () => { - const device = { deviceId: "fake" }; + const device = { deviceId: "fake", onDisconnect: jest.fn() }; const connect = await transport.connect(device); @@ -271,7 +272,10 @@ describe("WebUsbHidTransport", () => { transport.startDiscovering().subscribe({ next: (discoveredDevice) => { transport - .connect({ deviceId: discoveredDevice.id }) + .connect({ + deviceId: discoveredDevice.id, + onDisconnect: jest.fn(), + }) .then((value) => { expect(value).toStrictEqual( Left(new OpeningConnectionError(new Error(message))), @@ -302,7 +306,10 @@ describe("WebUsbHidTransport", () => { transport.startDiscovering().subscribe({ next: (discoveredDevice) => { transport - .connect({ deviceId: discoveredDevice.id }) + .connect({ + deviceId: discoveredDevice.id, + onDisconnect: jest.fn(), + }) .then((connectedDevice) => { connectedDevice .ifRight((device) => { @@ -331,7 +338,10 @@ describe("WebUsbHidTransport", () => { transport.startDiscovering().subscribe({ next: (discoveredDevice) => { transport - .connect({ deviceId: discoveredDevice.id }) + .connect({ + deviceId: discoveredDevice.id, + onDisconnect: jest.fn(), + }) .then((connectedDevice) => { connectedDevice .ifRight((device) => { @@ -380,7 +390,10 @@ describe("WebUsbHidTransport", () => { transport.startDiscovering().subscribe({ next: (discoveredDevice) => { transport - .connect({ deviceId: discoveredDevice.id }) + .connect({ + deviceId: discoveredDevice.id, + onDisconnect: jest.fn(), + }) .then((connectedDevice) => { connectedDevice .ifRight((device) => { @@ -407,6 +420,36 @@ describe("WebUsbHidTransport", () => { }, }); }); + + it("should call disconnect handler if a connected device is unplugged", (done) => { + // given + const onDisconnect = jest.fn(); + mockedRequestDevice.mockResolvedValueOnce([stubDevice]); + + // when + transport.startDiscovering().subscribe({ + next: (discoveredDevice) => { + transport + .connect({ + deviceId: discoveredDevice.id, + onDisconnect, + }) + .then(() => { + // @ts-expect-error + transport.handleDeviceDisconnectionEvent({ + device: { productId: stubDevice.productId }, + } as Event); + + // then + expect(onDisconnect).toHaveBeenCalled(); + done(); + }) + .catch((error) => { + done(error); + }); + }, + }); + }); }); }); }); diff --git a/packages/core/src/internal/usb/transport/WebUsbHidTransport.ts b/packages/core/src/internal/usb/transport/WebUsbHidTransport.ts index 020466ec3..3304e0da5 100644 --- a/packages/core/src/internal/usb/transport/WebUsbHidTransport.ts +++ b/packages/core/src/internal/usb/transport/WebUsbHidTransport.ts @@ -35,10 +35,13 @@ type WebHidInternalDevice = { discoveredDevice: DiscoveredDevice; }; +export type DisconnectHandler = (deviceId: DeviceId) => void; + @injectable() export class WebUsbHidTransport implements UsbHidTransport { // Maps uncoupled DiscoveredDevice and WebHID's HIDDevice WebHID private _internalDevicesById: Map; + private _internalDisconnectionHandlersByHidId: Map void>; private _connectionListenersAbortController: AbortController; private _logger: LoggerPublisherService; private _usbHidDeviceConnectionFactory: UsbHidDeviceConnectionFactory; @@ -52,6 +55,7 @@ export class WebUsbHidTransport implements UsbHidTransport { usbHidDeviceConnectionFactory: UsbHidDeviceConnectionFactory, ) { this._internalDevicesById = new Map(); + this._internalDisconnectionHandlersByHidId = new Map(); this._connectionListenersAbortController = new AbortController(); this._logger = loggerServiceFactory("WebUsbHidTransport"); this._usbHidDeviceConnectionFactory = usbHidDeviceConnectionFactory; @@ -267,8 +271,10 @@ export class WebUsbHidTransport implements UsbHidTransport { */ async connect({ deviceId, + onDisconnect, }: { deviceId: DeviceId; + onDisconnect: DisconnectHandler; }): Promise> { this._logger.debug("connect", { data: { deviceId } }); @@ -309,6 +315,10 @@ export class WebUsbHidTransport implements UsbHidTransport { id: deviceId, type: "USB", }); + this._internalDisconnectionHandlersByHidId.set( + internalDevice.hidDevice.productId.toString(), + () => onDisconnect(deviceId), + ); return Right(connectedDevice); } @@ -365,5 +375,16 @@ export class WebUsbHidTransport implements UsbHidTransport { }); } }); + const maybeDisconnectHandler = Maybe.fromNullable( + this._internalDisconnectionHandlersByHidId.get( + hidDevice.productId.toString(), + ), + ); + maybeDisconnectHandler.map((handler) => { + handler(); + this._internalDisconnectionHandlersByHidId.delete( + hidDevice.productId.toString(), + ); + }); }; } From ce87be01c9c501f5e737a9178600db3664c1b886 Mon Sep 17 00:00:00 2001 From: jdabbech-ledger Date: Thu, 16 May 2024 09:49:13 +0200 Subject: [PATCH 2/3] :lipstick: (smpl): Display not connected device status --- apps/sample/src/components/Device/StatusText.tsx | 2 ++ packages/core/src/api/device/DeviceStatus.ts | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/sample/src/components/Device/StatusText.tsx b/apps/sample/src/components/Device/StatusText.tsx index 4e983cf30..d95222375 100644 --- a/apps/sample/src/components/Device/StatusText.tsx +++ b/apps/sample/src/components/Device/StatusText.tsx @@ -15,6 +15,8 @@ const getColorFromState = ({ case DeviceStatus.BUSY: case DeviceStatus.LOCKED: return theme.colors.warning.c60; + case DeviceStatus.NOT_CONNECTED: + return theme.colors.error.c60; } }; diff --git a/packages/core/src/api/device/DeviceStatus.ts b/packages/core/src/api/device/DeviceStatus.ts index 99a7eb2fb..556739785 100644 --- a/packages/core/src/api/device/DeviceStatus.ts +++ b/packages/core/src/api/device/DeviceStatus.ts @@ -2,5 +2,5 @@ export enum DeviceStatus { LOCKED = "LOCKED", BUSY = "BUSY", CONNECTED = "CONNECTED", - NOT_CONNECTED = "NOT_CONNECTED", + NOT_CONNECTED = "NOT CONNECTED", } From 43e3372efa2f264677837c6bf1d045ea808b3bcf Mon Sep 17 00:00:00 2001 From: jdabbech-ledger Date: Thu, 16 May 2024 09:53:14 +0200 Subject: [PATCH 3/3] :bookmark: (core): Changeset --- .changeset/serious-clouds-provide.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/serious-clouds-provide.md diff --git a/.changeset/serious-clouds-provide.md b/.changeset/serious-clouds-provide.md new file mode 100644 index 000000000..95e6c5767 --- /dev/null +++ b/.changeset/serious-clouds-provide.md @@ -0,0 +1,5 @@ +--- +"@ledgerhq/device-sdk-core": minor +--- + +Handle unwanted transport disconnection