From a1421928af13d270b8f8e212796f6243787fdd13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Pantalea=CC=83o?= Date: Mon, 18 Mar 2024 10:56:37 +0100 Subject: [PATCH] PR improvements --- Extensions/Mocks/HAConnection+Mock.swift | 7 ---- Source/HAConnection.swift | 5 --- Source/Internal/HAConnectionImpl.swift | 16 +++------ .../HARequestController.swift | 2 +- .../HAResponseController.swift | 5 +++ Source/Requests/HARequestType.swift | 2 +- Source/Requests/HASttData.swift | 8 +++++ Tests/FakeEngine.swift | 1 + Tests/HAConnectionImpl.test.swift | 34 +++++++++++++++---- Tests/HARequestController.test.swift | 2 +- Tests/HAResponseController.test.swift | 12 +++++++ 11 files changed, 62 insertions(+), 32 deletions(-) create mode 100644 Source/Requests/HASttData.swift diff --git a/Extensions/Mocks/HAConnection+Mock.swift b/Extensions/Mocks/HAConnection+Mock.swift index ecceb3b..7fff5a9 100644 --- a/Extensions/Mocks/HAConnection+Mock.swift +++ b/Extensions/Mocks/HAConnection+Mock.swift @@ -77,9 +77,6 @@ public class HAMockConnection: HAConnection { /// Whether to turn to 'connecting' when a 'send' or 'subscribe' occurs when 'disconnected' public var automaticallyTransitionToConnecting = true - /// Data request received to be written - public var sttDataRequestReceived: HARequest? - // MARK: - Mock Implementation public weak var delegate: HAConnectionDelegate? @@ -229,8 +226,4 @@ public class HAMockConnection: HAConnection { CFRunLoopRunInMode(runLoopMode, 1.0, false) } } - - public func sendSttAudio(_ request: HARequest) { - sttDataRequestReceived = request - } } diff --git a/Source/HAConnection.swift b/Source/HAConnection.swift index 9fef108..dc5e152 100644 --- a/Source/HAConnection.swift +++ b/Source/HAConnection.swift @@ -180,9 +180,4 @@ public protocol HAConnection: AnyObject { initiated: @escaping SubscriptionInitiatedHandler, handler: @escaping (HACancellable, T) -> Void ) -> HACancellable - - /// Send audio to Assist - /// - Parameters: - /// - request: The data request containing sttBinaryHandlerId and data (as base64 string) to be written - func sendSttAudio(_ request: HARequest) } diff --git a/Source/Internal/HAConnectionImpl.swift b/Source/Internal/HAConnectionImpl.swift index fc35e6b..73e2374 100644 --- a/Source/Internal/HAConnectionImpl.swift +++ b/Source/Internal/HAConnectionImpl.swift @@ -302,14 +302,6 @@ internal class HAConnectionImpl: HAConnection { ) -> HACancellable { commonSubscribe(to: request, initiated: initiated, handler: handler) } - - // MARK: - Write - - public func sendSttAudio(_ request: HARequest) { - defer { connectAutomaticallyIfNeeded() } - let invocation = HARequestInvocationSingle(request: request) { _ in } - requestController.add(invocation) - } } // MARK: - @@ -407,7 +399,9 @@ extension HAConnectionImpl { // Prefix audioData with handler ID so the API can map the binary data audioData.insert(sttBinaryHandlerId, at: 0) workQueue.async { [connection] in - connection?.write(data: audioData) + connection?.write(data: audioData) { [weak self] in + self?.responseController.didWrite() + } } } @@ -420,8 +414,8 @@ extension HAConnectionImpl { sendWebSocket(identifier: identifier, request: request, command: command) case let .rest(method, command): sendRest(identifier: identifier!, request: request, method: method, command: command) - case let .sttData(sttBinaryHandlerId): - sendWrite(sttBinaryHandlerId, audioDataString: request.data["audioData"] as? String) + case let .sttData(data): + sendWrite(data.sttBinaryHandlerId, audioDataString: request.data["audioData"] as? String) } } } diff --git a/Source/Internal/RequestController/HARequestController.swift b/Source/Internal/RequestController/HARequestController.swift index 05c7421..cd544d3 100644 --- a/Source/Internal/RequestController/HARequestController.swift +++ b/Source/Internal/RequestController/HARequestController.swift @@ -5,7 +5,7 @@ internal struct HARequestControllerAllowedSendKind: OptionSet { static let webSocket: Self = .init(rawValue: 0b1) static let rest: Self = .init(rawValue: 0b10) - static let sttData: Self = .init(rawValue: 0b10) + static let sttData: Self = .init(rawValue: 0b11) static let all: Self = [.webSocket, .rest, .sttData] func allows(requestType: HARequestType) -> Bool { diff --git a/Source/Internal/ResponseController/HAResponseController.swift b/Source/Internal/ResponseController/HAResponseController.swift index 7cd30da..32fa50c 100644 --- a/Source/Internal/ResponseController/HAResponseController.swift +++ b/Source/Internal/ResponseController/HAResponseController.swift @@ -36,6 +36,7 @@ internal protocol HAResponseController: AnyObject { var phase: HAResponseControllerPhase { get } func reset() + func didWrite() func didReceive(event: Starscream.WebSocketEvent) func didReceive( for identifier: HARequestIdentifier, @@ -187,4 +188,8 @@ internal class HAResponseControllerImpl: HAResponseController { } } } + + func didWrite() { + HAGlobal.log(.info, "Data written") + } } diff --git a/Source/Requests/HARequestType.swift b/Source/Requests/HARequestType.swift index 1207b2a..df0e204 100644 --- a/Source/Requests/HARequestType.swift +++ b/Source/Requests/HARequestType.swift @@ -7,7 +7,7 @@ public enum HARequestType: Hashable, Comparable, ExpressibleByStringLiteral { /// Sent over REST, the HTTP method to use and the post-`api/` path case rest(HAHTTPMethod, String) /// Sent over WebSocket, the stt binary handler id - case sttData(UInt8) + case sttData(HASttData) /// Create a WebSocket request type by string literal /// - Parameter value: The name of the WebSocket command diff --git a/Source/Requests/HASttData.swift b/Source/Requests/HASttData.swift new file mode 100644 index 0000000..e082e1d --- /dev/null +++ b/Source/Requests/HASttData.swift @@ -0,0 +1,8 @@ +/// Write audio data to websocket, sttBinaryHandlerId is provided by run-start in Assist pipeline +public struct HASttData: Hashable { + var sttBinaryHandlerId: UInt8 + + public init(sttBinaryHandlerId: UInt8) { + self.sttBinaryHandlerId = sttBinaryHandlerId + } +} diff --git a/Tests/FakeEngine.swift b/Tests/FakeEngine.swift index fc93f5e..2599912 100644 --- a/Tests/FakeEngine.swift +++ b/Tests/FakeEngine.swift @@ -31,6 +31,7 @@ internal class FakeEngine: Engine { func write(data: Data, opcode: FrameOpCode, completion: (() -> Void)?) { events.append(.writeData(data, opcode: opcode)) + completion?() } func write(string: String, completion: (() -> Void)?) { diff --git a/Tests/HAConnectionImpl.test.swift b/Tests/HAConnectionImpl.test.swift index 4250469..5a16232 100644 --- a/Tests/HAConnectionImpl.test.swift +++ b/Tests/HAConnectionImpl.test.swift @@ -1463,29 +1463,47 @@ internal class HAConnectionImplTests: XCTestCase { } func testWriteDataRequestAddsToRequestController() { + connection.connectAutomatically = true let expectedData = "Fake data".data(using: .utf8)! let request = HARequest( - type: .sttData(1), + type: .sttData(.init(sttBinaryHandlerId: 1)), data: [ "audioData": expectedData.base64EncodedString(), ] ) connection.connect() - connection.sendSttAudio(request) + connection.send(request) { _ in } XCTAssertNotNil(requestController.added.first(where: { invocation in if case let .sttData(sttBinaryHandlerId) = invocation.request.type { - return sttBinaryHandlerId == 1 && invocation.request.data["audioData"] as? String == expectedData + return sttBinaryHandlerId == .init(sttBinaryHandlerId: 1) && invocation.request + .data["audioData"] as? String == expectedData .base64EncodedString() } return false })) } + func testWriteDataRequestsCallCompletion() { + let expectation = expectation(description: "Waiting for completion") + let expectedData = "Fake data".data(using: .utf8)! + let request = HARequest( + type: .sttData(.init(sttBinaryHandlerId: 1)), + data: [ + "audioData": expectedData.base64EncodedString(), + ] + ) + connection.connect() + responseController.receivedWaitExpectation = expectation + connection.requestController(requestController, didPrepareRequest: request, with: .init(integerLiteral: 1)) + + wait(for: [expectation], timeout: 5.0) + } + func testSendRawWithSttRequest() { connection.connect() let expectedData = "Fake data".data(using: .utf8)! let request = HARequest( - type: .sttData(1), + type: .sttData(.init(sttBinaryHandlerId: 1)), data: [ "audioData": expectedData.base64EncodedString(), ] @@ -1503,13 +1521,13 @@ internal class HAConnectionImplTests: XCTestCase { func testWriteSttRequestCommand() { let expectedData = "Fake data".data(using: .utf8)! let request = HARequest( - type: .sttData(1), + type: .sttData(.init(sttBinaryHandlerId: 1)), data: [ "audioData": expectedData.base64EncodedString(), ] ) let request2 = HARequest( - type: .sttData(2), + type: .sttData(.init(sttBinaryHandlerId: 1)), data: [ "audioData": expectedData.base64EncodedString(), ] @@ -1671,6 +1689,10 @@ private class FakeHAResponseController: HAResponseController { receivedWaitExpectation?.fulfill() } + func didWrite() { + receivedWaitExpectation?.fulfill() + } + var receivedRestWaitExpectation: XCTestExpectation? var receivedRest: [Swift.Result<(HTTPURLResponse, Data?), Error>] = [] func didReceive(for identifier: HARequestIdentifier, response: Swift.Result<(HTTPURLResponse, Data?), Error>) { diff --git a/Tests/HARequestController.test.swift b/Tests/HARequestController.test.swift index c713150..aa64e34 100644 --- a/Tests/HARequestController.test.swift +++ b/Tests/HARequestController.test.swift @@ -190,7 +190,7 @@ internal class HARequestControllerTests: XCTestCase { func testAddingWriteRequestAllowed() { delegate.allowedSendKinds = .all - controller.add(.init(request: .init(type: .sttData(1)))) + controller.add(.init(request: .init(type: .sttData(.init(sttBinaryHandlerId: 1))))) XCTAssertEqual(delegate.didPrepare.count, 1) } diff --git a/Tests/HAResponseController.test.swift b/Tests/HAResponseController.test.swift index e9a2f48..fd98a4c 100644 --- a/Tests/HAResponseController.test.swift +++ b/Tests/HAResponseController.test.swift @@ -241,6 +241,18 @@ internal class HAResponseControllerTests: XCTestCase { waitForCallback() XCTAssertEqual(delegate.lastReceived, .result(identifier: 2, result: .success(.dictionary(resultDictionary)))) } + + func testDidWriteEventLogs() { + let expectation = expectation(description: "Receive log") + HAGlobal.log = { level, message in + XCTAssertEqual(level, .info) + XCTAssertEqual(message, "Data written") + HAGlobal.log = { _, _ in } + expectation.fulfill() + } + controller.didWrite() + wait(for: [expectation], timeout: 2) + } } private extension HAResponseControllerTests {