Fix race condition during initial RileyLink configuration (#487)

* Fixes race condition during initial RileyLink configuration

* Avoid deadlock in logging

MinimedKit/PumpManager/MinimedPumpManager.swift

@@ -203,7 +203,7 @@ public class MinimedPumpManager: RileyLinkPumpManager, PumpManager {
  pumpOps.runSession(withName: "Tune pump", using: device) { (session) in
  do {
  let scanResult = try session.tuneRadio()
- self.log.error("Device %{public}@ auto-tuned to %{public}@ MHz", device.name ?? "", String(describing: scanResult.bestFrequency))
+ self.log.default("Device %{public}@ auto-tuned to %{public}@ MHz", device.name ?? "", String(describing: scanResult.bestFrequency))
  } catch let error {
  self.log.error("Device %{public}@ auto-tune failed with error: %{public}@", device.name ?? "", String(describing: error))
  self.rileyLinkDeviceProvider.deprioritize(device, completion: nil)
@@ -230,6 +230,8 @@ public class MinimedPumpManager: RileyLinkPumpManager, PumpManager {
  private func updatePumpStatus(_ status: MySentryPumpStatusMessageBody, from device: RileyLinkDevice) {
  dispatchPrecondition(condition: .onQueue(queue))
 
+ log.default("MySentry message received")
+
  var pumpDateComponents = status.pumpDateComponents
  var glucoseDateComponents = status.glucoseDateComponents
 
@@ -407,7 +409,7 @@ public class MinimedPumpManager: RileyLinkPumpManager, PumpManager {
  return
  }
 
- self.log.debug("Pump data is stale, fetching.")
+ self.log.default("Pump data is stale, fetching.")
 
  rileyLinkDeviceProvider.getDevices { (devices) in
  guard let device = devices.firstConnected else {

MinimedKit/PumpManager/PumpOps.swift

@@ -9,6 +9,7 @@
 import Foundation
 import RileyLinkKit
 import RileyLinkBLEKit
+import os.log
 
 
 public protocol PumpOpsDelegate: class {
@@ -18,6 +19,8 @@ public protocol PumpOpsDelegate: class {
 
 public class PumpOps {
 
+ private let log = OSLog(category: "PumpOps")
+
  public let pumpSettings: PumpSettings
 
  private var pumpState: PumpState {
@@ -69,10 +72,15 @@ public class PumpOps {
  sessionQueue.async {
  let semaphore = DispatchSemaphore(value: 0)
 
- device.runSession(withName: name) { (session) in
- let session = PumpOpsSession(settings: self.pumpSettings, pumpState: self.pumpState, session: session, delegate: self)
+ device.runSession(withName: name) { (commandSession) in
+ let session = PumpOpsSession(settings: self.pumpSettings, pumpState: self.pumpState, session: commandSession, delegate: self)
  self.sessionDevice = device
- self.configureDevice(device, with: session)
+ if !commandSession.firmwareVersion.isUnknown {
+ self.configureDevice(device, with: session)
+ } else {
+ self.log.error("Skipping device configuration due to unknown firmware version")
+ }
+
  block(session)
  self.sessionDevice = nil
  semaphore.signal()
@@ -88,10 +96,13 @@ public class PumpOps {
  return
  }
 
+ log.default("Configuring RileyLinkDevice: %{public}@", String(describing: device.deviceURI))
+
  do {
  _ = try session.configureRadio(for: pumpSettings.pumpRegion, frequency: pumpState.lastValidFrequency)
- } catch {
+ } catch let error {
  // Ignore the error and let the block run anyway
+ log.error("Error configuring device: %{public}@", String(describing: error))
  return
  }
 
@@ -108,7 +119,9 @@ public class PumpOps {
 
  NotificationCenter.default.removeObserver(self, name: .DeviceRadioConfigDidChange, object: device)
  NotificationCenter.default.removeObserver(self, name: .DeviceConnectionStateDidChange, object: device)
- configuredDevices.remove(device)
+
+ // TODO: Unsafe access
+ self.configuredDevices.remove(device)
  }
 
  public func getPumpState(_ completion: @escaping (_ state: PumpState) -> Void) {

RileyLink.xcodeproj/project.pbxproj

@@ -51,6 +51,7 @@
  431CE7A51F9D78F500255374 /* RFPacket.swift in Sources */ = {isa = PBXBuildFile; fileRef = 431CE7A41F9D78F500255374 /* RFPacket.swift */; };
  431CE7A71F9D98F700255374 /* CommandSession.swift in Sources */ = {isa = PBXBuildFile; fileRef = 431CE7A61F9D98F700255374 /* CommandSession.swift */; };
  4322B75620282DA60002837D /* ResponseBufferTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4322B75520282DA60002837D /* ResponseBufferTests.swift */; };
+ 43260F6F21C9B21000DD6837 /* Locked.swift in Sources */ = {isa = PBXBuildFile; fileRef = 435D26B720DC83E400891C17 /* Locked.swift */; };
  432847C11FA1737400CDE69C /* RileyLinkBLEKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 431CE76F1F98564100255374 /* RileyLinkBLEKit.framework */; };
  432847C31FA57C0F00CDE69C /* RadioFirmwareVersion.swift in Sources */ = {isa = PBXBuildFile; fileRef = 432847C21FA57C0F00CDE69C /* RadioFirmwareVersion.swift */; };
  432CF9061FF74CCB003AB446 /* RileyLinkKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 43722FAE1CB9F7630038B7F2 /* RileyLinkKit.framework */; };
@@ -3118,6 +3119,7 @@
  431CE7A11F9D195600255374 /* CBCentralManager.swift in Sources */,
  431CE7911F985D8D00255374 /* RileyLinkDeviceManager.swift in Sources */,
  431CE78D1F985B5400255374 /* PeripheralManager.swift in Sources */,
+ 43260F6F21C9B21000DD6837 /* Locked.swift in Sources */,
  431CE79E1F9BE73900255374 /* BLEFirmwareVersion.swift in Sources */,
  432847C31FA57C0F00CDE69C /* RadioFirmwareVersion.swift in Sources */,
  431CE7931F985DE700255374 /* PeripheralManager+RileyLink.swift in Sources */,

RileyLinkBLEKit/BLEFirmwareVersion.swift

@@ -20,8 +20,26 @@ public struct BLEFirmwareVersion {
  return nil
  }
 
+ self.init(
+ components: versionString[versionIndex...].split(separator: ".").compactMap({ Int($0) }),
+ versionString: versionString
+ )
+ }
+
+ init(components: [Int], versionString: String) {
+ self.components = components
  self.versionString = versionString
- components = versionString[versionIndex...].split(separator: ".").compactMap({ Int($0) })
+ }
+}
+
+
+extension BLEFirmwareVersion {
+ static var unknown: BLEFirmwareVersion {
+ return self.init(components: [0], versionString: "Unknown")
+ }
+
+ public var isUnknown: Bool {
+ return self == BLEFirmwareVersion.unknown
  }
 }
 
@@ -33,6 +51,13 @@ extension BLEFirmwareVersion: CustomStringConvertible {
 }
 
 
+extension BLEFirmwareVersion: Equatable {
+ public static func ==(lhs: BLEFirmwareVersion, rhs: BLEFirmwareVersion) -> Bool {
+ return lhs.components == rhs.components
+ }
+}
+
+
 extension BLEFirmwareVersion {
  var responseType: PeripheralManager.ResponseType {
  guard let major = components.first, major >= 2 else {

RileyLinkBLEKit/CommandSession.swift

@@ -75,7 +75,7 @@ public struct RileyLinkStatistics {
 public struct CommandSession {
  let manager: PeripheralManager
  let responseType: PeripheralManager.ResponseType
- let firmwareVersion: RadioFirmwareVersion
+ public let firmwareVersion: RadioFirmwareVersion
 
  /// Invokes a command expecting a response
  ///

RileyLinkBLEKit/PeripheralManager.swift

@@ -27,7 +27,7 @@ class PeripheralManager: NSObject {
  oldValue.delegate = nil
  peripheral.delegate = self
 
- queue.async {
+ queue.sync {
  self.needsConfiguration = true
  }
  }
@@ -52,8 +52,15 @@ class PeripheralManager: NSObject {
  // Confined to `queue`
  private var needsConfiguration = true
 
- weak var delegate: PeripheralManagerDelegate?
+ weak var delegate: PeripheralManagerDelegate? {
+ didSet {
+ queue.sync {
+ needsConfiguration = true
+ }
+ }
+ }
 
+ // Called from RileyLinkDeviceManager.managerQueue
  init(peripheral: CBPeripheral, configuration: Configuration, centralManager: CBCentralManager) {
  self.peripheral = peripheral
  self.central = centralManager
@@ -100,21 +107,28 @@ protocol PeripheralManagerDelegate: class {
 extension PeripheralManager {
  func configureAndRun(_ block: @escaping (_ manager: PeripheralManager) -> Void) -> (() -> Void) {
  return {
+ // TODO: Accessing self might be a race on initialization
  if !self.needsConfiguration && self.peripheral.services == nil {
  self.log.error("Configured peripheral has no services. Reconfiguring…")
  }
 
  if self.needsConfiguration || self.peripheral.services == nil {
  do {
  try self.applyConfiguration()
+ self.log.default("Peripheral configuration completed")
  } catch let error {
  self.log.error("Error applying peripheral configuration: %@", String(describing: error))
  // Will retry
  }
 
  do {
- try self.delegate?.completeConfiguration(for: self)
- self.needsConfiguration = false
+ if let delegate = self.delegate {
+ try delegate.completeConfiguration(for: self)
+ self.log.default("Delegate configuration completed")
+ self.needsConfiguration = false
+ } else {
+ self.log.error("No delegate set for configuration")
+ }
  } catch let error {
  self.log.error("Error applying delegate configuration: %@", String(describing: error))
  // Will retry
@@ -439,3 +453,17 @@ extension PeripheralManager: CBCentralManagerDelegate {
  }
  }
 }
+
+
+extension PeripheralManager {
+ public override var debugDescription: String {
+ var items = [
+ "## PeripheralManager",
+ "peripheral: \(peripheral)",
+ ]
+ queue.sync {
+ items.append("needsConfiguration: \(needsConfiguration)")
+ }
+ return items.joined(separator: "\n")
+ }
+}

RileyLinkBLEKit/RadioFirmwareVersion.swift

@@ -19,17 +19,26 @@ public struct RadioFirmwareVersion {
  return nil
  }
 
- self.versionString = versionString
- components = versionString[versionIndex...].split(separator: ".").compactMap({ Int($0) })
+ self.init(
+ components: versionString[versionIndex...].split(separator: ".").compactMap({ Int($0) }),
+ versionString: versionString
+ )
  }
 
- private init(components: [Int]) {
- versionString = "Unknown"
+ private init(components: [Int], versionString: String) {
+ self.versionString = versionString
  self.components = components
  }
+}
 
+
+extension RadioFirmwareVersion {
  static var unknown: RadioFirmwareVersion {
- return self.init(components: [1])
+ return self.init(components: [0], versionString: "Unknown")
+ }
+
+ public var isUnknown: Bool {
+ return self == RadioFirmwareVersion.unknown
  }
 }
 
@@ -40,6 +49,12 @@ extension RadioFirmwareVersion: CustomStringConvertible {
  }
 }
 
+extension RadioFirmwareVersion: Equatable {
+ public static func ==(lhs: RadioFirmwareVersion, rhs: RadioFirmwareVersion) -> Bool {
+ return lhs.components == rhs.components
+ }
+}
+
 // Version 2 changes
 extension RadioFirmwareVersion {
 

RileyLinkBLEKit/RileyLinkDevice.swift

@@ -155,9 +155,16 @@ extension RileyLinkDevice {
 extension RileyLinkDevice {
  public func runSession(withName name: String, _ block: @escaping (_ session: CommandSession) -> Void) {
  sessionQueue.addOperation(manager.configureAndRun({ [weak self] (manager) in
- self?.log.debug("======================== %{public}@ ===========================", name)
- block(CommandSession(manager: manager, responseType: self?.bleFirmwareVersion?.responseType ?? .buffered, firmwareVersion: self?.radioFirmwareVersion ?? .unknown))
- self?.log.debug("------------------------ %{public}@ ---------------------------", name)
+ self?.log.default("======================== %{public}@ ===========================", name)
+ let bleFirmwareVersion = self?.bleFirmwareVersion
+ let radioFirmwareVersion = self?.radioFirmwareVersion
+
+ if bleFirmwareVersion == nil || radioFirmwareVersion == nil {
+ self?.log.error("Running session with incomplete configuration: bleFirmwareVersion %{public}@, radioFirmwareVersion: %{public}@", String(describing: bleFirmwareVersion), String(describing: radioFirmwareVersion))
+ }
+
+ block(CommandSession(manager: manager, responseType: bleFirmwareVersion?.responseType ?? .buffered, firmwareVersion: radioFirmwareVersion ?? .unknown))
+ self?.log.default("------------------------ %{public}@ ---------------------------", name)
  }))
  }
 }
@@ -307,6 +314,8 @@ extension RileyLinkDevice: PeripheralManagerDelegate {
  } else {
  self.log.error("Unknown idle response: %@", value.hexadecimalString)
  }
+ } else {
+ self.log.error("Skipping parsing characteristic value update due to missing BLE firmware version")
  }
 
  self.assertIdleListening(forceRestart: true)
@@ -341,6 +350,7 @@ extension RileyLinkDevice: PeripheralManagerDelegate {
 
  func completeConfiguration(for manager: PeripheralManager) throws {
  // Read bluetooth version to determine compatibility
+ log.default("Reading firmware versions for PeripheralManager configuration")
  let bleVersionString = try manager.readBluetoothFirmwareVersion(timeout: 1)
  bleFirmwareVersion = BLEFirmwareVersion(versionString: bleVersionString)
 
@@ -361,7 +371,7 @@ extension RileyLinkDevice: CustomDebugStringConvertible {
  "isTimerTickNotifying: \(manager.timerTickEnabled)",
  "radioFirmware: \(String(describing: radioFirmwareVersion))",
  "bleFirmware: \(String(describing: bleFirmwareVersion))",
- "peripheral: \(manager.peripheral)",
+ "peripheralManager: \(String(reflecting: manager))",
  "sessionQueue.operationCount: \(sessionQueue.operationCount)"
  ].joined(separator: "\n")
  }