diff --git a/Example/nRF Connect Device Manager.xcodeproj/project.pbxproj b/Example/nRF Connect Device Manager.xcodeproj/project.pbxproj index 4af4d8c..eb964d6 100644 --- a/Example/nRF Connect Device Manager.xcodeproj/project.pbxproj +++ b/Example/nRF Connect Device Manager.xcodeproj/project.pbxproj @@ -540,7 +540,7 @@ "$(inherited)", "@executable_path/Frameworks", ); - MARKETING_VERSION = 1.2.3; + MARKETING_VERSION = 1.2.4; PRODUCT_BUNDLE_IDENTIFIER = com.nordicsemi.nrfconnect.devicemanager; PRODUCT_NAME = "Device Manager"; SWIFT_VERSION = 5.0; @@ -563,7 +563,7 @@ "$(inherited)", "@executable_path/Frameworks", ); - MARKETING_VERSION = 1.2.3; + MARKETING_VERSION = 1.2.4; PRODUCT_BUNDLE_IDENTIFIER = com.nordicsemi.nrfconnect.devicemanager; PRODUCT_NAME = "Device Manager"; SWIFT_VERSION = 5.0; diff --git a/Source/Bluetooth/McuMgrBleTransport.swift b/Source/Bluetooth/McuMgrBleTransport.swift index a458f1a..b02f56b 100644 --- a/Source/Bluetooth/McuMgrBleTransport.swift +++ b/Source/Bluetooth/McuMgrBleTransport.swift @@ -169,7 +169,7 @@ extension McuMgrBleTransport: McuMgrTransport { switch self._send(data: data, timeoutInSeconds: timeout) { case .failure(McuMgrTransportError.waitAndRetry): sleep(UInt32(McuMgrBleTransportConstant.WAIT_AND_RETRY_INTERVAL)) - self.log(msg: "Retry \(i)", atLevel: .info) + self.log(msg: "Retry \(i + 1)", atLevel: .info) case .failure(let error): self.log(msg: error.localizedDescription, atLevel: .error) DispatchQueue.main.async { diff --git a/Source/Managers/DFU/FirmwareUpgradeManager.swift b/Source/Managers/DFU/FirmwareUpgradeManager.swift index a4ccdab..b5d3c4b 100644 --- a/Source/Managers/DFU/FirmwareUpgradeManager.swift +++ b/Source/Managers/DFU/FirmwareUpgradeManager.swift @@ -786,6 +786,11 @@ public struct FirmwareUpgradeConfiguration: Codable { /// Can be used in conjunction with SMP Pipelining. public var reassemblyBufferSize: UInt64 + /// SMP Pipelining is considered Enabled for `pipelineDepth` values larger than `1`. + public var pipeliningEnabled: Bool { + return pipelineDepth > 1 + } + public init(eraseAppSettings: Bool = true, pipelineDepth: Int = 1, byteAlignment: ImageUploadAlignment = .disabled, reassemblyBufferSize: UInt64 = 0) { self.eraseAppSettings = eraseAppSettings diff --git a/Source/Managers/ImageManager.swift b/Source/Managers/ImageManager.swift index 23af4bd..bb7f216 100644 --- a/Source/Managers/ImageManager.swift +++ b/Source/Managers/ImageManager.swift @@ -13,6 +13,7 @@ public class ImageManager: McuManager { override class var TAG: McuMgrLogCategory { .image } + private static let PIPELINED_WRITES_TIMEOUT_SECONDS = 10 private static let truncatedHashLen = 3 // MARK: - IDs @@ -73,7 +74,16 @@ public class ImageManager: McuManager { payload.updateValue(CBOR.byteString([UInt8](data.sha256()[0.. 1 { - if let uploadIndex = self.uploadExpectedOffsets.firstIndex(of: UInt64(offset)) { - self.uploadExpectedOffsets.remove(at: uploadIndex) - } else { - // We've missed an offset, so let's compensate or else the upload will stall. - self.log(msg: "Missed ACK for offset \(offset), image \(self.uploadIndex). Clearing first offset (\(self.uploadExpectedOffsets.first ?? 0)) to compensate.", atLevel: .warning) - self.uploadExpectedOffsets.removeFirst() + var packetReceivedOutOfOrder = false + + // Note that pipelining requires the use of byte-alignment, otherwise we + // can't predict how many bytes the firmware will accept in each chunk. + if self.uploadConfiguration.pipeliningEnabled { + guard let i = self.uploadExpectedOffsets.firstIndex(where: { $0.sequenceNumber == response.header.sequenceNumber }) else { + self.cancelUpload(error: ImageUploadError.invalidUploadSequenceNumber(response.header.sequenceNumber)) + return } - } else { - // So if we're not pipelining, we usually don't apply byte alignment. - // And even if we did, we don't need to 'predict' offsets, so we don't care. - self.uploadExpectedOffsets.removeAll() + + packetReceivedOutOfOrder = i != 0 + if packetReceivedOutOfOrder { + self.log(msg: "OOD Packet: Received Seq No. \(response.header.sequenceNumber) instead of expected Seq No. \(self.uploadExpectedOffsets[0].sequenceNumber)", atLevel: .debug) + } + self.uploadExpectedOffsets.remove(at: i) } self.uploadLastOffset = max(self.uploadLastOffset, UInt64(offset)) @@ -413,23 +428,24 @@ public class ImageManager: McuManager { // Don't trigger writes to another image unless all write(s) have returned for // the current one. guard self.uploadExpectedOffsets.isEmpty else { return } - let firstPacketOffset = self.maxDataPacketLengthFor(data: images[self.uploadIndex].data, image: self.uploadIndex, offset: 0) - self.uploadExpectedOffsets.append(firstPacketOffset) self.uploadLastOffset = 0 self.sendNext(from: UInt64(0)) } return } + guard !packetReceivedOutOfOrder || self.uploadExpectedOffsets.isEmpty else { + // If packet was received OOD, we must throttle to allow device to catch-up. + // If there's no pipelining, `uploadExpectedOffsets` will always be empty here. + return + } + for i in 0..<(self.uploadConfiguration.pipelineDepth - self.uploadExpectedOffsets.count) { - guard let chunkOffset = self.uploadExpectedOffsets.last ?? self.uploadLastOffset, + guard let chunkOffset = self.uploadExpectedOffsets.last?.offset ?? self.uploadLastOffset, chunkOffset < self.imageData?.count ?? 0 else { // No remaining chunks to be sent. return } - - let chunkSize = self.maxDataPacketLengthFor(data: images[self.uploadIndex].data, image: self.uploadIndex, offset: chunkOffset) - self.uploadExpectedOffsets.append(chunkOffset + chunkSize) self.sendNext(from: chunkOffset) } } else { @@ -538,6 +554,8 @@ public enum ImageUploadError: Error { case invalidPayload /// Image Data is nil. case invalidData + + case invalidUploadSequenceNumber(UInt8) /// McuMgrResponse contains a error return code. case mcuMgrErrorCode(McuMgrReturnCode) } @@ -550,6 +568,8 @@ extension ImageUploadError: LocalizedError { return "Response payload values do not exist." case .invalidData: return "Image data is nil." + case .invalidUploadSequenceNumber(let sequenceNumber): + return "Received Response for Unknown Sequence Number \(sequenceNumber)." case .mcuMgrErrorCode(let code): return "Remote error: \(code)." } diff --git a/Source/McuManager.swift b/Source/McuManager.swift index 56b2877..3cc169e 100644 --- a/Source/McuManager.swift +++ b/Source/McuManager.swift @@ -45,9 +45,6 @@ open class McuManager { /// Logger delegate will receive logs. public weak var logDelegate: McuMgrLogDelegate? - /// Each Packet gets its own Sequence Number, which we will rotate for every - /// packet sent within the bounds of an unsigned UInt8, so 0...255 (inclusive). - private var sequenceNumber: UInt8 //************************************************************************** // MARK: Initializers @@ -56,39 +53,36 @@ open class McuManager { public init(group: McuMgrGroup, transporter: McuMgrTransport) { self.group = group self.transporter = transporter - self.sequenceNumber = 0 self.mtu = McuManager.getDefaultMtu(scheme: transporter.getScheme()) } // MARK: - Send - public func send(op: McuMgrOperation, + public func send(op: McuMgrOperation, sequenceNumber: UInt8 = 0, commandId: R, payload: [String:CBOR]?, timeout: Int = DEFAULT_SEND_TIMEOUT_SECONDS, callback: @escaping McuMgrCallback) where R.RawValue == UInt8 { - send(op: op, flags: 0, commandId: commandId, payload: payload, callback: callback) + send(op: op, sequenceNumber: sequenceNumber, flags: 0, commandId: commandId, payload: payload, timeout: timeout, + callback: callback) } - public func send(op: McuMgrOperation, flags: UInt8, - commandId: R, payload: [String:CBOR]?, + public func send(op: McuMgrOperation, sequenceNumber: UInt8, + flags: UInt8, commandId: R, payload: [String:CBOR]?, timeout: Int = DEFAULT_SEND_TIMEOUT_SECONDS, callback: @escaping McuMgrCallback) where R.RawValue == UInt8 { - let packetPacketSequenceNumber = sequenceNumber - sequenceNumber = sequenceNumber.next() - - log(msg: "Sending \(op) command (Group: \(group), seq: \(packetPacketSequenceNumber), ID: \(commandId)): \(payload?.debugDescription ?? "nil")", + log(msg: "Sending \(op) command (Group: \(group), seq: \(sequenceNumber), ID: \(commandId)): \(payload?.debugDescription ?? "nil")", atLevel: .verbose) let mcuPacketData = McuManager.buildPacket(scheme: transporter.getScheme(), op: op, flags: flags, group: group.uInt16Value, - sequenceNumber: packetPacketSequenceNumber, + sequenceNumber: sequenceNumber, commandId: commandId, payload: payload) let _callback: McuMgrCallback = logDelegate == nil ? callback : { [weak self] (response, error) in if let self = self { if let response = response { - self.log(msg: "Response (Group: \(self.group), seq: \(packetPacketSequenceNumber), ID: \(response.header!.commandId!)): \(response)", + self.log(msg: "Response (Group: \(self.group), seq: \(sequenceNumber), ID: \(response.header!.commandId!)): \(response)", atLevel: .verbose) } else if let error = error { - self.log(msg: "Request (Group: \(self.group), seq: \(packetPacketSequenceNumber)) failed: \(error.localizedDescription))", + self.log(msg: "Request (Group: \(self.group), seq: \(sequenceNumber)) failed: \(error.localizedDescription))", atLevel: .error) } } @@ -336,13 +330,3 @@ extension McuMgrReturnCode: CustomStringConvertible { } } } - -// MARK: - UInt8 - -extension UInt8 { - - func next() -> UInt8 { - guard self != .max else { return 0} - return self + 1 - } -} diff --git a/iOSMcuManagerLibrary.podspec b/iOSMcuManagerLibrary.podspec index 566061c..0a93bbc 100644 --- a/iOSMcuManagerLibrary.podspec +++ b/iOSMcuManagerLibrary.podspec @@ -1,6 +1,6 @@ Pod::Spec.new do |s| s.name = "iOSMcuManagerLibrary" - s.version = "1.2.3" + s.version = "1.2.4" s.license = { :type => "Apache 2.0", :file => "LICENSE" } s.summary = "A mobile management library for devices running Apache Mynewt or Zephyr." s.homepage = "https://github.com/NordicSemiconductor/IOS-nRF-Connect-Device-Manager"