Skip to content

Commit

Permalink
🎨 (core): BleConnection use sendapdu promise instead of rxjs subject
Browse files Browse the repository at this point in the history
  • Loading branch information
jdabbech-ledger committed Oct 4, 2024
1 parent 95dacb9 commit 0ceb52d
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,7 @@ describe("BleDeviceConnection", () => {
// @ts-expect-error private function call to mock web ble response
connection.onNotifyCharacteristicValueChanged({
target: {
value: {
buffer,
},
value: new DataView(buffer.buffer),
},
} as DataViewEvent);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Either, Left, Maybe, Nothing, Right } from "purify-ts";
import { Subject } from "rxjs";

import { CommandUtils } from "@api/command/utils/CommandUtils";
import { ApduResponse } from "@api/device-session/ApduResponse";
Expand Down Expand Up @@ -39,11 +38,13 @@ export class BleDeviceConnection implements DeviceConnection {
) => ApduSenderService;
private readonly _apduReceiver: ApduReceiverService;
private _isDeviceReady: boolean;
private _sendApduSubject: Subject<ApduResponse>;
private _settleReconnectionPromise: Maybe<{
private _sendApduPromiseResolver: Maybe<{
resolve(value: Either<SdkError, ApduResponse>): void;
}>;
private _settleReconnectionPromiseResolvers: Maybe<{
resolve(): void;
reject(err: SdkError): void;
}> = Maybe.zero();
}>;

constructor(
{
Expand All @@ -65,9 +66,16 @@ export class BleDeviceConnection implements DeviceConnection {
this.onNotifyCharacteristicValueChanged,
);
this._isDeviceReady = false;
this._sendApduSubject = new Subject();
this._sendApduPromiseResolver = Maybe.zero();
this._settleReconnectionPromiseResolvers = Maybe.zero();
}

/**
* NotifyCharacteristic setter
* Register a listener on characteristic value change
* @param notifyCharacteristic
* @private
*/
private set notifyCharacteristic(
notifyCharacteristic: BluetoothRemoteGATTCharacteristic,
) {
Expand All @@ -78,6 +86,11 @@ export class BleDeviceConnection implements DeviceConnection {
);
}

/**
* WriteCharacteristic setter
* @param writeCharacteristic
* @private
*/
private set writeCharacteristic(
writeCharacteristic: BluetoothRemoteGATTCharacteristic,
) {
Expand All @@ -95,9 +108,9 @@ export class BleDeviceConnection implements DeviceConnection {
const [frameSize] = mtuResponse.slice(5);
if (frameSize) {
this._apduSender = Maybe.of(this._apduSenderFactory({ frameSize }));
this._settleReconnectionPromise.ifJust((promise) => {
this._settleReconnectionPromiseResolvers.ifJust((promise) => {
promise.resolve();
this._settleReconnectionPromise = Maybe.zero();
this._settleReconnectionPromiseResolvers = Maybe.zero();
});
this._isDeviceReady = true;
}
Expand Down Expand Up @@ -132,47 +145,50 @@ export class BleDeviceConnection implements DeviceConnection {
* APDU 0x0800000000 is used to get this mtu size
*/
public async setup() {
const apdu = Uint8Array.from([0x08, 0x00, 0x00, 0x00, 0x00]);
const requestMtuApdu = Uint8Array.from([0x08, 0x00, 0x00, 0x00, 0x00]);

await this._notifyCharacteristic.startNotifications();
await this._writeCharacteristic.writeValueWithResponse(apdu);
await this._writeCharacteristic.writeValueWithResponse(requestMtuApdu);
}

/**
* Receive APDU response
* Complete sendApdu subject once the framer receives all the frames of the response
* Resolve sendApdu promise once the framer receives all the frames of the response
* @param data
*/
receiveApdu(data: ArrayBuffer) {
const response = this._apduReceiver.handleFrame(new Uint8Array(data));

response.caseOf({
Right: (maybeApduResponse) => {
response
.map((maybeApduResponse) => {
maybeApduResponse.map((apduResponse) => {
this._sendApduSubject.next(apduResponse);
this._logger.debug("Received APDU Response", {
data: { response: apduResponse },
});
this._sendApduSubject.complete();
this._sendApduPromiseResolver.map(({ resolve }) =>
resolve(Right(apduResponse)),
);
});
},
Left: (error) => this._sendApduSubject.error(error),
});
})
.mapLeft((error) => {
this._sendApduPromiseResolver.map(({ resolve }) =>
resolve(Left(error)),
);
});
}

/**
* Send apdu if the mtu had been set
*
* Get all frames for a given APDU
* Subscribe to a Subject that would be complete once the response had been received
* Save a promise that would be completed once the response had been received
* @param apdu
* @param triggersDisconnection
*/
async sendApdu(
apdu: Uint8Array,
triggersDisconnection?: boolean,
): Promise<Either<SdkError, ApduResponse>> {
this._sendApduSubject = new Subject();

if (!this._isDeviceReady) {
return Promise.resolve(
Left(new DeviceNotInitializedError("Unknown MTU")),
Expand All @@ -181,22 +197,8 @@ export class BleDeviceConnection implements DeviceConnection {
// Create a promise that would be resolved once the response had been received
const resultPromise = new Promise<Either<SdkError, ApduResponse>>(
(resolve) => {
this._sendApduSubject.subscribe({
next: async (response) => {
if (
triggersDisconnection &&
CommandUtils.isSuccessResponse(response)
) {
const reconnectionRes = await this.setupWaitForReconnection();
reconnectionRes.caseOf({
Left: (err) => resolve(Left(err)),
Right: () => resolve(Right(response)),
});
} else {
resolve(Right(response));
}
},
error: (err) => resolve(Left(err)),
this._sendApduPromiseResolver = Maybe.of({
resolve,
});
},
);
Expand All @@ -216,7 +218,22 @@ export class BleDeviceConnection implements DeviceConnection {
this._logger.error("Error sending frame", { data: { error } });
}
}
return resultPromise;
const response = await resultPromise;
this._sendApduPromiseResolver = Maybe.zero();
return response.caseOf({
Right: async (apduResponse) => {
if (
triggersDisconnection &&
CommandUtils.isSuccessResponse(apduResponse)
) {
const reconnectionRes = await this.setupWaitForReconnection();
return reconnectionRes.map(() => apduResponse);
} else {
return Right(apduResponse);
}
},
Left: async (error) => Promise.resolve(Left(error)),
});
}

/**
Expand All @@ -227,16 +244,9 @@ export class BleDeviceConnection implements DeviceConnection {
*/
private isDataViewEvent(event: Event): event is DataViewEvent {
return (
typeof event.target === "object" &&
event.target !== null &&
"value" in event.target &&
typeof event.target.value === "object" &&
event.target.value !== null &&
"buffer" in event.target.value &&
typeof event.target.value.buffer === "object" &&
event.target.value.buffer !== null &&
"byteLength" in event.target.value.buffer &&
typeof event.target.value.buffer.byteLength === "number"
event.target.value instanceof DataView
);
}

Expand All @@ -247,7 +257,7 @@ export class BleDeviceConnection implements DeviceConnection {
*/
private setupWaitForReconnection(): Promise<Either<SdkError, void>> {
return new Promise<Either<SdkError, void>>((resolve) => {
this._settleReconnectionPromise = Maybe.of({
this._settleReconnectionPromiseResolvers = Maybe.of({
resolve: () => resolve(Right(undefined)),
reject: (error: SdkError) => resolve(Left(error)),
});
Expand Down Expand Up @@ -278,9 +288,9 @@ export class BleDeviceConnection implements DeviceConnection {
*/
public async disconnect() {
// if a reconnection promise is pending, reject it
this._settleReconnectionPromise.ifJust((promise) => {
this._settleReconnectionPromiseResolvers.ifJust((promise) => {
promise.reject(new ReconnectionFailedError());
this._settleReconnectionPromise = Maybe.zero();
this._settleReconnectionPromiseResolvers = Maybe.zero();
});
this._notifyCharacteristic.removeEventListener(
"characteristicvaluechanged",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ describe("WebBleTransport", () => {
});

it("should emit an error if the user did not grant us access to a device (clicking on cancel on the browser popup for ex)", (done) => {
mockedRequestDevice.mockResolvedValueOnce({});
mockedRequestDevice.mockResolvedValueOnce({ forget: jest.fn() });

discoverDevice(
(discoveredDevice) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export class WebBleTransport implements Transport {
return Left(new BleTransportNotSupportedError("WebBle not supported"));
}

isSupported() {
isSupported(): boolean {
try {
const result = !!navigator?.bluetooth;
return result;
Expand Down Expand Up @@ -254,13 +254,13 @@ export class WebBleTransport implements Transport {
},

Left: (error) => {
bleDevice.gatt?.disconnect();
bleDevice.forget();
throw error;
},
});
},
Left: (error) => {
bleDevice.gatt?.disconnect();
bleDevice.forget();
throw error;
},
});
Expand Down Expand Up @@ -302,6 +302,7 @@ export class WebBleTransport implements Transport {
});
return Left(new UnknownDeviceError(`Unknown device ${deviceId}`));
}
// if device already connected, remove device id from internal state and remove error
if (this._connectedDevices.includes(internalDevice.bleDevice)) {
this._internalDevicesById.delete(deviceId);
return Left(new DeviceAlreadyConnectedError("Device already connected"));
Expand Down Expand Up @@ -342,6 +343,7 @@ export class WebBleTransport implements Transport {
this._connectedDevices.push(internalDevice.bleDevice);
return Right(connectedDevice);
} catch (error) {
await internalDevice.bleDevice.forget();
this._internalDevicesById.delete(deviceId);
this._logger.error("Error while getting characteristics", {
data: { error },
Expand Down Expand Up @@ -424,12 +426,18 @@ export class WebBleTransport implements Transport {
);
maybeDeviceConnection.map((dConnection) => dConnection.disconnect());
// disconnect device gatt server
bleDevice.gatt?.disconnect();
if (bleDevice.gatt?.connected) {
bleDevice.gatt.disconnect();
}
// clean up objects
this._internalDevicesById.delete(device.id);
this._deviceConnectionById.delete(device.id);
this._disconnectionHandlersById.delete(device.id);
delete this._connectedDevices[this._connectedDevices.indexOf(bleDevice)];
if (this._connectedDevices.includes(bleDevice)) {
delete this._connectedDevices[
this._connectedDevices.indexOf(bleDevice)
];
}
});

return Right(void 0);
Expand Down

0 comments on commit 0ceb52d

Please sign in to comment.