-
Notifications
You must be signed in to change notification settings - Fork 0
[Fix/#305] 와치 session이 열렸을 때 상태 업데이트 로직 개선 #310
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
Conversation
YunDaeHyeon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📄 PR Code Review Report
🔍 Summary
제출된 변경 사항은 WatchConnectivity 세션 활성화 및 상태 전달 로직을 개선하려는 시도로 보입니다. iOS 측에서는 코드 중복을 줄이고 일관성을 높이는 방향으로 리팩토링이 진행되었으며, 이는 긍정적입니다. 그러나 watchOS 측의 변경 사항은 Watch 앱의 활성 상태 전달 시점에 미묘한 변화를 주어, iPhone 측에서 Watch 앱의 상태를 인지하는 데 지연이 발생하거나 특정 상황에서 상태가 정확히 전달되지 않을 잠재적 위험이 있습니다.
⚠️ Key Issues
- watchOS: Watch 앱 활성 상태 전달 지연 또는 누락 가능성:
activateSessionIfAvailable메서드 내에서pushWatchAppState(.active)호출이if session.activationState == .activated조건문 안으로 이동되었습니다. 이전에는 세션 활성화 시도 시점에 무조건active상태를 전달했지만, 이제는 세션이 이미activated상태일 때만 즉시 전달됩니다. 만약activateSessionIfAvailable호출 시점에 세션이 아직.notActivated또는.inactive상태라면,pushWatchAppState(.active)호출은session(_:activationDidCompleteWith:error:)델리게이트 메서드가 성공적으로 호출될 때까지 지연됩니다. 이로 인해 iPhone 앱이 Watch 앱이 활성화되었음을 인지하는 데 불필요한 지연이 발생하거나, 특정 에러 시나리오에서는 상태가 전혀 전달되지 않을 수 있습니다. Watch 앱이 활성화를 시도한다는 의도를 iPhone에 최대한 빨리 알리는 것이 중요할 수 있습니다.
🛠 Improvement Suggestions
- iOS:
handleWatchReachability메서드의 Main Actor 디스패치 보장 확인: 변경 사항에서handleWatchReachability(session: session)메서드로 로직이 추출되었습니다. 이전 코드에서Task { @MainActor in ... }블록을 사용했으므로, 이 새 메서드도onReachableChanged클로저 호출이 반드시 메인 스레드에서 이루어지도록 보장하는지 확인하는 것이 좋습니다. 만약handleWatchReachability내부에서 명시적인 메인 액터 디스패치가 없다면, UI 관련 콜백이 백그라운드 스레드에서 실행될 위험이 있습니다.
✅ Positive Observations
- iOS: 코드 일관성 및 중복 제거:
handleWatchReachability메서드를 도입하여activateSessionIfAvailable및session(_:activationDidCompleteWith:error:)에서 Watch의 reachable 상태를 처리하는 로직을 통합한 점은 코드의 가독성과 유지보수성을 높이는 좋은 리팩토링입니다. - watchOS: 활성화 성공 후 명시적인 상태 전달:
session(_:activationDidCompleteWith:error:)델리게이트 메서드 내에서 세션 활성화 성공 시pushWatchAppState(.active)를 명시적으로 호출하고MainActor로 디스패치하도록 추가한 점은 Watch 앱의 정확한 상태를 iPhone에 전달하려는 의도를 명확히 하고, 잠재적인 스레딩 문제를 방지합니다.
sangYuLv
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이렇게 간단했다니...
감사합니다 귀로님 😭
YunDaeHyeon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다 🥲🙇
| Task { @MainActor in | ||
| self.onReachableChanged?(session.isReachable) | ||
| } | ||
| handleWatchReachability(session: session) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handleWatchReachability을 호출하는 방법이 있었네요..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
의외네요.. 흠.. (너무 간단했다는 의미) 👍
9377fa7 to
de26f56
Compare
🔗 연관된 이슈
📝 작업 내용
📌 요약
📸 영상 / 이미지 (Optional)
2026-02-05.6.34.59.mov