[v0.4.9] Fix flush listen (#142)

* Fix flush listen - Prevent indefinite flush - Add missing cancellation handling - Dedup listen semaphore signals - Add missing semaphore signal for end listen - Use timeout enum * Bump version to 0.4.9 * Improve fix for flush listen
steamclock · Apr 9, 2018 · fc16350 · fc16350
1 parent d95e58f
commit fc16350
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 53 deletions.
diff --git a/Bluejay.podspec b/Bluejay.podspec
@@ -1,12 +1,12 @@
 Pod::Spec.new do |spec|
   spec.name = 'Bluejay'
-  spec.version = '0.4.8'
+  spec.version = '0.4.9'
   spec.license = { type: 'MIT', file: 'LICENSE' }
   spec.homepage = 'https://github.com/steamclock/bluejay'
   spec.authors = { 'Jeremy Chiang' => 'jeremy@steamclock.com' }
   spec.summary = 'Bluejay is a simple Swift framework for building reliable Bluetooth apps.'
   spec.homepage = 'https://github.com/steamclock/bluejay'
-  spec.source = { git: 'https://github.com/steamclock/bluejay.git', tag: 'v0.4.8' }
+  spec.source = { git: 'https://github.com/steamclock/bluejay.git', tag: 'v0.4.9' }
   spec.source_files = 'Bluejay/Bluejay/*.{h,swift}'
   spec.framework = 'SystemConfiguration'
   spec.platform = :ios, '9.3'

diff --git a/Bluejay/Bluejay/Error.swift b/Bluejay/Bluejay/Error.swift
@@ -54,6 +54,8 @@ public enum BluejayError {
     case listenCacheDecoding(Error)
     /// Bluejay has cancelled an expected end listen request.
     case endListenCancelled
+    /// Indefinite flush will not exit.
+    case indefiniteFlush
 }
 
 extension BluejayError: LocalizedError {
@@ -103,6 +105,8 @@ extension BluejayError: LocalizedError {
             return "Listen cache decoding failed with error: \(error.localizedDescription)"
         case .endListenCancelled:
             return "End listen cancelled."
+        case .indefiniteFlush:
+            return "Flush listen timeout cannot be none or zero."
         }
     }
 }
@@ -137,6 +141,7 @@ extension BluejayError: CustomNSError {
         case .listenCacheEncoding: return 20
         case .listenCacheDecoding: return 21
         case .endListenCancelled: return 22
+        case .indefiniteFlush: return 23
         }
     }
 

diff --git a/Bluejay/Bluejay/Info.plist b/Bluejay/Bluejay/Info.plist
@@ -15,7 +15,7 @@
 	<key>CFBundlePackageType</key>
 	<string>FMWK</string>
 	<key>CFBundleShortVersionString</key>
-	<string>0.4.8</string>
+	<string>0.4.9</string>
 	<key>CFBundleVersion</key>
 	<string>$(CURRENT_PROJECT_VERSION)</string>
 	<key>NSPrincipalClass</key>

diff --git a/Bluejay/Bluejay/SynchronizedPeripheral.swift b/Bluejay/Bluejay/SynchronizedPeripheral.swift
@@ -202,75 +202,77 @@ public class SynchronizedPeripheral {
     /**
      Flush a listen to a characteristic by receiving and discarding values for the specified duration.
 
+     **Warning** Timeout defaults to 3 seconds. Specifying no timeout or a timeout with zero second will result in a fatal error.
+
      - Parameters:
         - characteristicIdentifier: The characteristic to flush.
-        - idleWindow: How long to flush for in seconds.
+        - nonZeroTimeout: How long to wait for incoming data.
         - completion: Block to call when the flush is complete.
     */
-    public func flushListen(to characteristicIdentifier: CharacteristicIdentifier, idleWindow: Int = 3, completion: @escaping () -> Void) throws {
-        let flushSem = DispatchSemaphore(value: 0)
-        let cleanUpSem = DispatchSemaphore(value: 0)
-        let sem = DispatchSemaphore(value: 0)
+    public func flushListen(to characteristicIdentifier: CharacteristicIdentifier, nonZeroTimeout: Timeout = .seconds(3), completion: @escaping () -> Void) throws {
+        guard case let .seconds(timeoutInterval) = nonZeroTimeout, timeoutInterval > 0 else {
+            fatalError(BluejayError.indefiniteFlush.errorDescription!)
+        }
+
+        let listenSem = DispatchSemaphore(value: 0)
+        let endListenSem = DispatchSemaphore(value: 0)
         var error : Error?
 
         var shouldListenAgain = false
 
-        repeat {
-            DispatchQueue.main.async {
-                log("Flushing listen to \(characteristicIdentifier.uuid.uuidString)")
-
-                shouldListenAgain = false
+        DispatchQueue.main.async {
+            log("Flushing listen to \(characteristicIdentifier.uuid.uuidString)")
+
+            shouldListenAgain = false
+
+            self.parent.listen(to: characteristicIdentifier, completion: { (result : ReadResult<Data>) in
+                switch result {
+                case .success:
+                    log("Flushed some data.")
+
+                    shouldListenAgain = true
+                case .cancelled:
+                    log("Flush cancelled.")
+
+                    shouldListenAgain = false
+                    error = BluejayError.cancelled
+                case .failure(let e):
+                    log("Flush failed with error: \(e.localizedDescription)")
+
+                    shouldListenAgain = false
+                    error = e
+                }
 
-                self.parent.listen(to: characteristicIdentifier, completion: { (result : ReadResult<Data>) in
+                listenSem.signal()
+            })
+        }
+
+        repeat {
+            shouldListenAgain = false
+            _ = listenSem.wait(timeout: .now() + DispatchTimeInterval.seconds(Int(timeoutInterval)))
+            log("Flush to \(characteristicIdentifier.uuid.uuidString) finished, should flush again: \(shouldListenAgain).")
+        } while shouldListenAgain
+
+        DispatchQueue.main.async {
+            if self.parent.isListening(to: characteristicIdentifier) {
+                self.parent.endListen(to: characteristicIdentifier, error: nil, completion: { (result) in
                     switch result {
                     case .success:
-                        log("Flushed some data.")
-                        shouldListenAgain = true
-
-                        flushSem.signal()
+                        break
                     case .cancelled:
                         break
                     case .failure(let e):
-                        log("Flush failed with error: \(e.localizedDescription)")
-                        shouldListenAgain = false
                         error = e
-
-                        flushSem.signal()
                     }
+
+                    endListenSem.signal()
                 })
+            } else {
+                endListenSem.signal()
             }
-
-            _ = flushSem.wait(timeout: .now() + .seconds(idleWindow))
-
-            DispatchQueue.main.async {
-                if self.parent.isListening(to: characteristicIdentifier) {
-                    self.parent.endListen(to: characteristicIdentifier, error: nil, completion: { (result) in
-                        switch result {
-                        case .success:
-                            break
-                        case .cancelled:
-                            break
-                        case .failure(let e):
-                            error = e
-                        }
-
-                        cleanUpSem.signal()
-                    })
-                }
-            }
-
-            _ = cleanUpSem.wait(timeout: .distantFuture)
-
-            DispatchQueue.main.async {
-                log("Flush to \(characteristicIdentifier.uuid.uuidString) finished, should flush again: \(shouldListenAgain).")
-
-                if !shouldListenAgain {
-                    sem.signal()
-                }
-            }
-        } while shouldListenAgain
+        }
 
-        _ = sem.wait(timeout: .distantFuture)
+        _ = endListenSem.wait(timeout: .now() + DispatchTimeInterval.seconds(Int(timeoutInterval)))
 
         if let error = error {
             throw error