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

IOS-5674 Fix reader #338

merged 3 commits into from
Feb 12, 2024

Conversation

tureck1y
Copy link
Collaborator

@tureck1y tureck1y commented Feb 6, 2024

Репорт в том, что плашка NFC либо закрывалась сама без каких-либо сообщений, либо с ошибкой, но слишко быстро. https://tangem.atlassian.net/browse/IOS-5674

Непосредственно баг из-за изменения поведение каллбэков от NFC-слоя, пришлось расставить больше костылей.

Тестировал на разных девайсах и осях, нигде не оторвало, поэтому не стал расставлять ифы по версиям
TODO: Взять в работу рефакторинг ридера, он очень сложный, события поступают с разных источников, может нужна архитектура в стиле редукса.

@@ -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 во всех случаях, например, если происходит остановка по тайм-ауту тэга

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

@@ -371,12 +401,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

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 для единообразия


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.

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

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. Вводится эта переменная, которая помогает отфильтровать ненужные сообщения.

@@ -400,6 +429,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.

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

@tureck1y tureck1y marked this pull request as ready for review February 6, 2024 18:26
@tureck1y tureck1y requested a review from a team as a code owner February 6, 2024 18:26
Andoran90
Andoran90 previously approved these changes Feb 7, 2024
Copy link
Contributor

@Andoran90 Andoran90 left a comment

Choose a reason for hiding this comment

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

так по мелочи комменты

ba1ashov
ba1ashov previously approved these changes Feb 8, 2024
@tureck1y tureck1y dismissed stale reviews from ba1ashov and Andoran90 via 1517748 February 9, 2024 17:52
@tureck1y
Copy link
Collaborator Author

tureck1y commented Feb 9, 2024

слегка подправил, QA потестировали, все ок

@tureck1y tureck1y enabled auto-merge February 9, 2024 17:55
@tureck1y tureck1y merged commit 7ddce67 into develop Feb 12, 2024
3 checks passed
@tureck1y tureck1y deleted the IOS-5674_fix_reader branch February 12, 2024 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants