Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IOS-5674 Fix reader #338

Merged
merged 3 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions TangemSdk/TangemSdk/Common/Core/CardSession.swift
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ public class CardSession {
/// - Parameter error: The error to show
public func stop(error: Error, completion: (() -> Void)?) {
Log.session("Stop session")
reader.stopSession(with: error.localizedDescription)
reader.stopSession(with: error)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Добавил интерфейс, чтобы доносить до калльбэков правильную ошибку. Сейчас тип теряется и постится userCancelled во всех случаях, например, если происходит остановка по тайм-ауту тэга

sessionDidStop(completion: completion)
}

Expand Down Expand Up @@ -321,7 +321,6 @@ public class CardSession {
.sink(receiveCompletion: {[weak self] readerCompletion in
self?.sendSubscription = []
if case let .failure(error) = readerCompletion {
Log.error(error)
completion(.failure(error))
}
}, receiveValue: {[weak self] responseApdu in
Expand Down
1 change: 1 addition & 0 deletions TangemSdk/TangemSdk/Common/NFC/CardReader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public protocol CardReader: AnyObject {
func resumeSession()
func stopSession(with errorMessage: String?)
func pauseSession(with errorMessage: String?)
func stopSession(with error: Error)
func sendPublisher(apdu: CommandApdu) -> AnyPublisher<ResponseApdu, TangemSdkError>
func restartPolling(silent: Bool)
}
Expand Down
97 changes: 71 additions & 26 deletions TangemSdk/TangemSdk/Common/NFC/NFCReader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,12 @@ final class NFCReader: NSObject {
private var sendRetryCount = Constants.retryCount
private var startRetryCount = Constants.startRetryCount
private let pollingOption: NFCTagReaderSession.PollingOption
private var sessionDidBecomeActiveTimestamp: Date = .init()

private var sessionDidBecomeActiveTS: Date = .init()
private var firstConnectionTS: Date? = nil
private var tagConnectionTS: Date? = nil
private var isBeingStopped = false
private var stoppedError: TangemSdkError? = nil

/// Starting from iOS 17 is no longer possible to invoke restart polling after 20 seconds from first connection on some devices
private lazy var shouldReduceRestartPolling: Bool = {
if #available(iOS 17, *), NFCUtils.isBrokenRestartPollingDevice {
Expand All @@ -87,8 +91,6 @@ final class NFCReader: NSObject {
return false
}()

private var firstConnectionTimestamp: Date? = nil

private lazy var nfcUtils: NFCUtils = .init()

init(pollingOption: NFCTagReaderSession.PollingOption = [.iso14443]) {
Expand All @@ -108,6 +110,11 @@ extension NFCReader: CardReader {
var alertMessage: String {
get { return _alertMessage ?? "" }
set {
if isBeingStopped {
Log.nfc("Session is being stopped. Skip alert message.")
return
}

readerSession?.alertMessage = newValue
_alertMessage = newValue
}
Expand All @@ -122,6 +129,11 @@ extension NFCReader: CardReader {
invalidatedWithError = nil
cancelled = false
connectedTag = nil
isBeingStopped = false
stoppedError = nil
tagConnectionTS = nil
firstConnectionTS = nil
sessionDidBecomeActiveTS = Date()

let alertMessage = message ?? "view_delegate_scan_description".localized
_alertMessage = alertMessage
Expand All @@ -140,7 +152,7 @@ extension NFCReader: CardReader {
.filter{[weak self] _ in
guard let self else { return false }

let distanceToSessionActive = self.sessionDidBecomeActiveTimestamp.distance(to: Date())
let distanceToSessionActive = self.sessionDidBecomeActiveTS.distance(to: Date())
if !self.isSessionReady || distanceToSessionActive < 1 {
Log.nfc("Filter out cancelled event")
return false
Expand Down Expand Up @@ -207,16 +219,17 @@ extension NFCReader: CardReader {
.dropFirst()
.sink {[weak self] isSilent in
guard let self, let session = self.readerSession,
session.isReady else {
session.isReady,
!self.isBeingStopped else {
return
}

if self.shouldReduceRestartPolling, let firstConnectionTimestamp = self.firstConnectionTimestamp {
let interval = Date().timeIntervalSince(firstConnectionTimestamp)
if self.shouldReduceRestartPolling, let firstConnectionTS = self.firstConnectionTS {
let interval = Date().timeIntervalSince(firstConnectionTS)
Log.nfc("Restart polling interval is: \(interval)")

// 20 is too much because of time inaccuracy
if interval >= 18 {
if interval >= 16 {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

пришлось сократить до минимума, иначе session invalidated unexpectedly

Log.nfc("Ignore restart polling")
return
}
Expand Down Expand Up @@ -247,6 +260,15 @@ extension NFCReader: CardReader {
}

func stopSession(with errorMessage: String? = nil) {
guard (readerSession?.isReady == true) else {
return
}

if isBeingStopped {
return
}

isBeingStopped = true
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Суть фикса номер 2. Вводится эта переменная, которая помогает отфильтровать ненужные сообщения.

Log.nfc("Stop reader session invoked")
stopTimers()
if let errorMessage = errorMessage {
Expand All @@ -256,6 +278,11 @@ extension NFCReader: CardReader {
}
}

func stopSession(with error: Error) {
stoppedError = error.toTangemSdkError()
stopSession(with: error.localizedDescription)
}

func restartPolling(silent: Bool) {
restartPollingPublisher.send(silent)
}
Expand All @@ -264,6 +291,13 @@ extension NFCReader: CardReader {
/// - Parameter apdu: serialized apdu
/// - Parameter completion: result with ResponseApdu or NFCError otherwise
func sendPublisher(apdu: CommandApdu) -> AnyPublisher<ResponseApdu, TangemSdkError> {
if isBeingStopped {
Log.nfc("Session is being stopped. Skip sending.")
return Empty(completeImmediately: false)
.setFailureType(to: TangemSdkError.self)
.eraseToAnyPublisher()
}

Log.nfc("Send publisher invoked")

return Just(())
Expand All @@ -289,7 +323,7 @@ extension NFCReader: CardReader {
return Fail(error: TangemSdkError.unsupportedCommand).eraseToAnyPublisher()
} //todo: handle tag lost

let requestTimestamp = Date()
let requestTS = Date()
Log.apdu("SEND --> \(apdu)")
return iso7816tag
.sendCommandPublisher(cApdu: apdu)
Expand All @@ -307,8 +341,8 @@ extension NFCReader: CardReader {
.eraseToAnyPublisher()
}

let distance = requestTimestamp.distance(to: Date())
let isDistanceTooLong = distance > Constants.timestampTolerance
let distance = requestTS.distance(to: Date())
let isDistanceTooLong = distance > Constants.requestToleranceTS
if isDistanceTooLong || self.sendRetryCount <= 0 { //retry to fix old device issues
Log.nfc("Invoke restart polling on error")
self.sendRetryCount = Constants.retryCount
Expand Down Expand Up @@ -337,7 +371,7 @@ extension NFCReader: CardReader {
}

private func start() {
firstConnectionTimestamp = nil
firstConnectionTS = nil
readerSession?.invalidate() //Important! We must keep invalidate/begin in balance after start retries
readerSession = NFCTagReaderSession(pollingOption: self.pollingOption, delegate: self, queue: queue)!
readerSession!.alertMessage = _alertMessage!
Expand Down Expand Up @@ -371,12 +405,11 @@ extension NFCReader: CardReader {
.TimerPublisher(interval: Constants.tagTimeout, tolerance: 0, runLoop: RunLoop.main, mode: .common)
.autoconnect()
.receive(on: queue)
.filter {[weak self] _ in self?.idleTimerCancellable != nil }
Copy link
Collaborator Author

@tureck1y tureck1y Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

из-за этой строки, этот таймер вообще не работал и не останавливал сессию. Это суть фикса номер 1

.sink {[weak self] _ in
guard let self else { return }

Log.nfc("Stop by tag timer")
self.stopSession(with: TangemSdkError.nfcTimeout.localizedDescription)
self.stopSession(with: TangemSdkError.nfcTimeout)
self.tagTimerCancellable = nil
}
}
Expand All @@ -390,7 +423,7 @@ extension NFCReader: CardReader {
guard let self else { return }

Log.nfc("Stop by session timer")
self.stopSession(with: TangemSdkError.nfcTimeout.localizedDescription)
self.stopSession(with: TangemSdkError.nfcTimeout)
self.sessionTimerCancellable = nil
}
}
Expand All @@ -400,6 +433,7 @@ extension NFCReader: CardReader {
.TimerPublisher(interval: Constants.idleTimeout, runLoop: RunLoop.main, mode: .common)
.autoconnect()
.receive(on: queue)
.filter { [weak self] _ in !(self?.isBeingStopped ?? true) }
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

При анализе кейсов и логов добавил фильтрацию и тут

.sink {[weak self] _ in
guard let self else { return }

Expand Down Expand Up @@ -470,36 +504,47 @@ extension NFCReader: CardReader {
@available(iOS 13.0, *)
extension NFCReader: NFCTagReaderSessionDelegate {
func tagReaderSessionDidBecomeActive(_ session: NFCTagReaderSession) {
sessionDidBecomeActiveTimestamp = Date()
sessionDidBecomeActiveTS = Date()
isSessionReady = true
}

func tagReaderSession(_ session: NFCTagReaderSession, didInvalidateWithError error: Error) {
Log.nfc("NFC Session did invalidate with: \(error.localizedDescription)")
Log.nfc("NFC Session did invalidate with NFC error: \(error.localizedDescription)")
if nfcStuckTimerCancellable == nil { //handle stuck retries ios14
invalidatedWithError = TangemSdkError.parse(error as! NFCReaderError)
invalidatedWithError = stoppedError ?? TangemSdkError.parse(error as! NFCReaderError)
}

if let tagConnectionTS {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

анврапится только одно значение, второе не optional, но все равно загнал под один if для единообразия

let currentTS = Date()
Log.nfc("Session time is: \(currentTS.timeIntervalSince(sessionDidBecomeActiveTS))")
Log.nfc("Tag time is: \(currentTS.timeIntervalSince(tagConnectionTS))")
}
}

func tagReaderSession(_ session: NFCTagReaderSession, didDetect tags: [NFCTag]) {
Log.nfc("NFC tag detected: \(tags)")

if firstConnectionTimestamp == nil {
firstConnectionTimestamp = Date()
}

let nfcTag = tags.first!

sessionConnectCancellable = session.connectPublisher(tag: nfcTag)
.receive(on: queue)
.sink {[weak self] completion in
guard let self else { return }

switch completion {
case .failure:
self?.restartPolling(silent: false)
self.restartPolling(silent: false)
case .finished:
break
}
self?.sessionConnectCancellable = nil
self.sessionConnectCancellable = nil

self.tagConnectionTS = Date()

if self.firstConnectionTS == nil {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

судя по экспериментам, тут правильнее это держать

self.firstConnectionTS = self.tagConnectionTS
}

} receiveValue: {[weak self] _ in
self?.tagDidConnect(nfcTag)
}
Expand All @@ -516,7 +561,7 @@ extension NFCReader {
static let nfcStuckTimeout = 1.0
static let retryCount = 20
static let startRetryCount = 5
static let timestampTolerance = 1.0
static let requestToleranceTS = 1.0
static let searchTagTimeout = 1.0
}
}
Loading