Skip to content

Commit 3666732

Browse files
haslinghuishaslinghuis
committed
Fix BT reboot and CRC issues
1 parent 501b95b commit 3666732

File tree

6 files changed

+256
-92
lines changed

6 files changed

+256
-92
lines changed

‎src/js/WebBluetooth.js

Whitespace-only changes.

‎src/js/protocols/WebBluetooth.js

+93-62
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ class WebBluetooth extends EventTarget {
4646
this.bluetooth.addEventListener("gatserverdisconnected", (e) => this.handleRemovedDevice(e.target));
4747

4848
this.loadDevices();
49+
50+
// Properly bind all event handlers ONCE
51+
this.boundHandleDisconnect = this.handleDisconnect.bind(this);
52+
this.boundHandleNotification = this.handleNotification.bind(this);
53+
this.boundHandleReceiveBytes = this.handleReceiveBytes.bind(this);
4954
}
5055

5156
handleNewDevice(device) {
@@ -135,7 +140,8 @@ class WebBluetooth extends EventTarget {
135140

136141
console.log(`${this.logHead} Opening connection with ID: ${path}, Baud: ${options.baudRate}`);
137142

138-
this.device.addEventListener("gattserverdisconnected", this.handleDisconnect.bind(this));
143+
// Use bound method references
144+
this.device.addEventListener("gattserverdisconnected", this.boundHandleDisconnect);
139145

140146
try {
141147
console.log(`${this.logHead} Connecting to GATT Server`);
@@ -163,8 +169,9 @@ class WebBluetooth extends EventTarget {
163169
this.failed = 0;
164170
this.openRequested = false;
165171

166-
this.device.addEventListener("disconnect", this.handleDisconnect.bind(this));
167-
this.addEventListener("receive", this.handleReceiveBytes);
172+
// Use bound references here too
173+
this.device.addEventListener("disconnect", this.boundHandleDisconnect);
174+
this.addEventListener("receive", this.boundHandleReceiveBytes);
168175

169176
console.log(`${this.logHead} Connection opened with ID: ${this.connectionId}, Baud: ${options.baudRate}`);
170177

@@ -221,47 +228,69 @@ class WebBluetooth extends EventTarget {
221228
}
222229

223230
async getCharacteristics() {
224-
const characteristics = await this.service.getCharacteristics();
231+
try {
232+
const characteristics = await this.service.getCharacteristics();
225233

226-
characteristics.forEach((characteristic) => {
227-
// console.log("Characteristic: ", characteristic);
228-
if (characteristic.uuid == this.deviceDescription.writeCharacteristic) {
229-
this.writeCharacteristic = characteristic;
234+
if (!characteristics || characteristics.length === 0) {
235+
throw new Error("No characteristics found");
230236
}
231237

232-
if (characteristic.uuid == this.deviceDescription.readCharacteristic) {
233-
this.readCharacteristic = characteristic;
238+
// Reset characteristics
239+
this.writeCharacteristic = null;
240+
this.readCharacteristic = null;
241+
242+
for (const characteristic of characteristics) {
243+
if (characteristic.uuid === this.deviceDescription.writeCharacteristic) {
244+
this.writeCharacteristic = characteristic;
245+
}
246+
247+
if (characteristic.uuid === this.deviceDescription.readCharacteristic) {
248+
this.readCharacteristic = characteristic;
249+
}
250+
251+
if (this.writeCharacteristic && this.readCharacteristic) {
252+
break;
253+
}
234254
}
235-
return this.writeCharacteristic && this.readCharacteristic;
236-
});
237255

238-
if (!this.writeCharacteristic) {
239-
throw new Error(
240-
"Unexpected write characteristic found - should be",
241-
this.deviceDescription.writeCharacteristic,
242-
);
243-
}
256+
if (!this.writeCharacteristic) {
257+
throw new Error(`Write characteristic not found: ${this.deviceDescription.writeCharacteristic}`);
258+
}
244259

245-
if (!this.readCharacteristic) {
246-
throw new Error(
247-
"Unexpected read characteristic found - should be",
248-
this.deviceDescription.readCharacteristic,
249-
);
250-
}
260+
if (!this.readCharacteristic) {
261+
throw new Error(`Read characteristic not found: ${this.deviceDescription.readCharacteristic}`);
262+
}
251263

252-
this.readCharacteristic.addEventListener("characteristicvaluechanged", this.handleNotification.bind(this));
264+
// Use the bound method for the event listener
265+
this.readCharacteristic.addEventListener("characteristicvaluechanged", this.boundHandleNotification);
253266

254-
return await this.readCharacteristic.readValue();
267+
return await this.readCharacteristic.readValue();
268+
} catch (error) {
269+
console.error(`${this.logHead} Error getting characteristics:`, error);
270+
throw error;
271+
}
255272
}
256273

257274
handleNotification(event) {
258-
const buffer = new Uint8Array(event.target.value.byteLength);
275+
try {
276+
if (!event.target.value) {
277+
console.warn(`${this.logHead} Empty notification received`);
278+
return;
279+
}
259280

260-
for (let i = 0; i < event.target.value.byteLength; i++) {
261-
buffer[i] = event.target.value.getUint8(i);
262-
}
281+
const buffer = new Uint8Array(event.target.value.byteLength);
282+
283+
// Copy data with validation
284+
for (let i = 0; i < event.target.value.byteLength; i++) {
285+
buffer[i] = event.target.value.getUint8(i);
286+
}
263287

264-
this.dispatchEvent(new CustomEvent("receive", { detail: buffer }));
288+
if (buffer.length > 0) {
289+
this.dispatchEvent(new CustomEvent("receive", { detail: buffer }));
290+
}
291+
} catch (error) {
292+
console.error(`${this.logHead} Error handling notification:`, error);
293+
}
265294
}
266295

267296
startNotifications() {
@@ -287,48 +316,50 @@ class WebBluetooth extends EventTarget {
287316
return;
288317
}
289318

290-
const doCleanup = async () => {
291-
this.removeEventListener("receive", this.handleReceiveBytes);
319+
this.closeRequested = true; // Set this to prevent reentry
320+
321+
try {
322+
this.removeEventListener("receive", this.boundHandleReceiveBytes);
292323

293324
if (this.device) {
294-
this.device.removeEventListener("disconnect", this.handleDisconnect.bind(this));
295-
this.device.removeEventListener("gattserverdisconnected", this.handleDisconnect);
296-
this.readCharacteristic.removeEventListener(
297-
"characteristicvaluechanged",
298-
this.handleNotification.bind(this),
299-
);
300-
301-
if (this.device.gatt.connected) {
302-
this.device.gatt.disconnect();
325+
// Use the properly bound references
326+
this.device.removeEventListener("disconnect", this.boundHandleDisconnect);
327+
this.device.removeEventListener("gattserverdisconnected", this.boundHandleDisconnect);
328+
329+
if (this.readCharacteristic) {
330+
try {
331+
// Stop notifications first to avoid errors
332+
await this.readCharacteristic.stopNotifications();
333+
this.readCharacteristic.removeEventListener(
334+
"characteristicvaluechanged",
335+
this.boundHandleNotification,
336+
);
337+
} catch (err) {
338+
console.warn(`${this.logHead} Error stopping notifications:`, err);
339+
}
303340
}
304341

305-
this.writeCharacteristic = false;
306-
this.readCharacteristic = false;
307-
this.deviceDescription = false;
308-
this.device = null;
309-
}
310-
};
311-
312-
try {
313-
await doCleanup();
342+
// Safely disconnect GATT
343+
if (this.device.gatt?.connected) {
344+
await this.device.gatt.disconnect();
345+
}
314346

315-
console.log(
316-
`${this.logHead} Connection with ID: ${this.connectionId} closed, Sent: ${this.bytesSent} bytes, Received: ${this.bytesReceived} bytes`,
317-
);
347+
// Clear references
348+
this.writeCharacteristic = null;
349+
this.readCharacteristic = null;
350+
this.deviceDescription = null;
351+
}
318352

353+
console.log(`${this.logHead} Connection closed successfully`);
319354
this.connectionId = false;
320-
this.bitrate = 0;
355+
this.device = null;
321356
this.dispatchEvent(new CustomEvent("disconnect", { detail: true }));
322357
} catch (error) {
323-
console.error(error);
324-
console.error(
325-
`${this.logHead} Failed to close connection with ID: ${this.connectionId} closed, Sent: ${this.bytesSent} bytes, Received: ${this.bytesReceived} bytes`,
326-
);
358+
console.error(`${this.logHead} Error during disconnect:`, error);
327359
this.dispatchEvent(new CustomEvent("disconnect", { detail: false }));
328360
} finally {
329-
if (this.openCanceled) {
330-
this.openCanceled = false;
331-
}
361+
this.closeRequested = false;
362+
this.openCanceled = false;
332363
}
333364
}
334365

‎src/js/protocols/__tests__/WebBluetooth.test.js

+114
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
import WebBluetooth from "../WebBluetooth";
2+
import { bluetoothDevices } from "../devices";
3+
4+
// Mock navigator.bluetooth API
5+
global.navigator = {
6+
bluetooth: {
7+
getAvailability: jest.fn().mockResolvedValue(true),
8+
requestDevice: jest.fn(),
9+
},
10+
};
11+
12+
// Create mock event listeners
13+
global.navigator.bluetooth.addEventListener = jest.fn();
14+
15+
describe("WebBluetooth", () => {
16+
let webBluetooth;
17+
let mockServer;
18+
let mockService;
19+
let mockGatt;
20+
21+
beforeEach(() => {
22+
// Reset mocks
23+
jest.clearAllMocks();
24+
25+
// Setup mock device
26+
mockGatt = {
27+
connected: true,
28+
connect: jest.fn().mockResolvedValue({}),
29+
};
30+
31+
mockServer = {
32+
getPrimaryServices: jest.fn(),
33+
};
34+
35+
mockService = {
36+
getCharacteristics: jest.fn(),
37+
uuid: bluetoothDevices[0].serviceUuid,
38+
};
39+
40+
// Create WebBluetooth instance
41+
webBluetooth = new WebBluetooth();
42+
webBluetooth.device = {
43+
gatt: mockGatt,
44+
addEventListener: jest.fn(),
45+
name: "TestDevice",
46+
};
47+
webBluetooth.server = mockServer;
48+
webBluetooth.service = mockService;
49+
webBluetooth.deviceDescription = bluetoothDevices[0];
50+
});
51+
52+
describe("getCharacteristics", () => {
53+
test("should throw error when no characteristics are found", async () => {
54+
// Return empty array from getCharacteristics
55+
mockService.getCharacteristics.mockResolvedValue([]);
56+
57+
// Expect the method to throw with specific message
58+
await expect(webBluetooth.getCharacteristics()).rejects.toThrow("No characteristics found");
59+
});
60+
61+
test("should throw error when write characteristic is not found", async () => {
62+
// Return only read characteristic
63+
const mockCharacteristics = [
64+
{
65+
uuid: webBluetooth.deviceDescription.readCharacteristic,
66+
addEventListener: jest.fn(),
67+
readValue: jest.fn().mockResolvedValue({}),
68+
properties: { notify: true },
69+
},
70+
];
71+
mockService.getCharacteristics.mockResolvedValue(mockCharacteristics);
72+
73+
await expect(webBluetooth.getCharacteristics()).rejects.toThrow(/Write characteristic not found/);
74+
});
75+
76+
test("should throw error when read characteristic is not found", async () => {
77+
// Return only write characteristic
78+
const mockCharacteristics = [
79+
{
80+
uuid: webBluetooth.deviceDescription.writeCharacteristic,
81+
},
82+
];
83+
mockService.getCharacteristics.mockResolvedValue(mockCharacteristics);
84+
85+
await expect(webBluetooth.getCharacteristics()).rejects.toThrow(/Read characteristic not found/);
86+
});
87+
88+
test("should handle service.getCharacteristics throwing an error", async () => {
89+
// Make the API call throw
90+
const testError = new Error("API Error");
91+
mockService.getCharacteristics.mockRejectedValue(testError);
92+
93+
await expect(webBluetooth.getCharacteristics()).rejects.toThrow("API Error");
94+
});
95+
96+
test("should succeed when both characteristics are found", async () => {
97+
// Return both characteristics
98+
const mockCharacteristics = [
99+
{
100+
uuid: webBluetooth.deviceDescription.writeCharacteristic,
101+
},
102+
{
103+
uuid: webBluetooth.deviceDescription.readCharacteristic,
104+
addEventListener: jest.fn(),
105+
readValue: jest.fn().mockResolvedValue({}),
106+
properties: { notify: true },
107+
},
108+
];
109+
mockService.getCharacteristics.mockResolvedValue(mockCharacteristics);
110+
111+
await expect(webBluetooth.getCharacteristics()).resolves.not.toThrow();
112+
});
113+
});
114+
});

‎src/js/serial.js

+22-13
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@ class Serial extends EventTarget {
231231
this._protocol.connect(path, options);
232232
} else {
233233
console.error(`${this.logHead} Failed to disconnect before reconnecting`);
234+
return false;
234235
}
235236
});
236237

@@ -243,40 +244,48 @@ class Serial extends EventTarget {
243244

244245
/**
245246
* Disconnect from the current connection
247+
* @param {function} [callback] - Optional callback for backward compatibility
248+
* @returns {boolean|Promise<boolean>} - Returns boolean for sync calls, Promise for async calls
246249
*/
247250
disconnect(callback) {
251+
// Return immediately if no protocol is selected
248252
if (!this._protocol) {
249253
console.warn(`${this.logHead} No protocol selected, nothing to disconnect`);
250254
if (callback) callback(false);
251255
return false;
252256
}
253257

254-
if (!this._protocol.connected) {
255-
console.warn(`${this.logHead} Protocol not connected, nothing to disconnect`);
256-
if (callback) callback(false);
257-
return false;
258-
}
259-
260-
console.log(`${this.logHead} Disconnecting from current protocol`);
258+
console.log(`${this.logHead} Disconnecting from current protocol`, this._protocol);
261259

262260
try {
263-
// Disconnect from the protocol
261+
// Handle case where we're already disconnected
262+
if (!this._protocol.connected) {
263+
console.log(`${this.logHead} Already disconnected, performing cleanup`);
264+
if (callback) {
265+
callback(true);
266+
}
267+
return true;
268+
}
269+
270+
// Perform the actual disconnect
264271
const result = this._protocol.disconnect((success) => {
265272
if (success) {
266-
// Ensure our connection state is updated
267273
console.log(`${this.logHead} Disconnection successful`);
268274
} else {
269275
console.error(`${this.logHead} Disconnection failed`);
270276
}
271-
272-
// Call the callback with the result
273-
if (callback) callback(success);
277+
// Call callback with disconnect result
278+
if (callback) {
279+
callback(success);
280+
}
274281
});
275282

276283
return result;
277284
} catch (error) {
278285
console.error(`${this.logHead} Error during disconnect:`, error);
279-
if (callback) callback(false);
286+
if (callback) {
287+
callback(false);
288+
}
280289
return false;
281290
}
282291
}

0 commit comments

Comments
 (0)