-
Notifications
You must be signed in to change notification settings - Fork 0
[#296] 연결이 끊기는 시점을 개선한다 #303
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
이번 Pull Request는 SwiftUI 뷰와 Observable Store 패턴을 도입하며 로직을 분리하는 리팩토링이 주를 이룹니다. 전반적으로 코드 구조를 개선하려는 노력이 엿보이나, 일부 핵심 컴포넌트의 생명주기 관리와 특정 로직에서 잠재적인 문제가 발견되었습니다. 특히 네트워킹 관련 객체의 초기화 방식 변경은 프로덕션 환경에서 불안정성을 야기할 수 있는 심각한 위험이 있습니다.
⚠️ Key Issues
-
AdvertisingView의Advertiser객체 생명주기 관리 문제:
AdvertisingView에서Advertiser객체를private let프로퍼티로 선언하여 사용하는 방식으로 변경되었습니다. SwiftUIstruct뷰의private let프로퍼티는 뷰가 업데이트될 때마다 새로 초기화될 수 있습니다.Advertiser는MCNearbyServiceAdvertiser및MCSession과 같은 네트워킹 관련 객체를 포함하고 있어, 이러한 반복적인 재초기화는 기존 연결의 손실, 불필요한 리소스 재할당, 그리고 심각한 경우 앱 충돌이나 예상치 못한 연결 동작을 유발할 수 있습니다.Advertiser와 같이 상태를 유지하고 지속적인 작업을 수행해야 하는 객체는@StateObject또는@EnvironmentObject와 같이 SwiftUI 생명주기에 맞춰 안정적으로 관리되어야 합니다. 이는 프로덕션 환경에서 치명적인 버그를 일으킬 수 있는 주요 문제입니다. -
ModeSelectionStore의remoteMode처리 로직 오류 가능성:
ModeSelectionStore의action(_ intent:)메서드 내.remoteMode케이스에서advertiser.sendCommand(.setRemoteMode)를 호출하는 조건이if let advertiser, !flag로 되어 있습니다. 여기서flag는RootView에서isRemoteEnable로 전달되며, 주석상 "리모트 가능 여부"를 나타냅니다. 이 조건은 "advertiser가 있고, 리모트가 가능하지 않을 때setRemoteMode명령을 보낸다"는 의미가 됩니다. 리모트 모드가 가능하지 않은 상태에서 리모트 모드로 설정하려는 시도는 논리적으로 맞지 않으며, 의도치 않은 동작을 유발할 수 있습니다.flag변수의 의미를 다시 확인하고,setRemoteMode명령이 올바른 조건에서 발송되는지 검토가 필요합니다.
🛠 Improvement Suggestions
Browser.disconnect(useType: onPurpose:)메서드의 명확성 강화:
disconnect메서드에onPurpose파라미터가 추가되어.remote사용 시remoteHeartBeater의nil할당 여부를 결정하도록 변경되었습니다. 현재.onSelectedTimerMode처리 시점에onPurpose: true가 잘 사용되고 있으나, 향후disconnect(useType: .remote)가 다른 곳에서onPurpose: false(기본값)로 호출될 때remoteHeartBeater가 명시적으로nil되지 않아 메모리 또는 상태 문제가 발생할 여지가 있습니다.onPurpose대신,shouldClearHeartBeater와 같이 특정 동작을 나타내는 더 명확한 파라미터 이름을 사용하거나,disconnectRemoteAndClearHeartBeater()와 같이 두 동작을 명시하는 별도의 메서드를 분리하는 것을 고려해볼 수 있습니다.
✅ Positive Observations
- Observable Store 패턴 도입:
ModeSelectionView와AdvertisingView에서 Observable Store 패턴을 적용하여 뷰 로직과 비즈니스 로직을 분리한 점이 좋습니다. 이는 코드의 가독성, 테스트 용이성, 유지보수성을 향상시키는 방향입니다. 특히ModeSelectionView에서showHomeAlert상태와noticeShootingMode로직을 Store로 옮긴 점은 잘된 리팩토링입니다. - 명령어 핸들링 및 상태 동기화 개선:
Browser와AdvertiserCommandManager에stopHeartBeat명령이 추가되고, 관련 로직(하트비트 중단, 리모트 연결 해제 등)이 반영되어 원격 장치와의 상태 동기화 및 자원 해제가 더욱 명확해졌습니다. 이는 리모트 장치와의 연결 종료 시 발생할 수 있는 잠재적 문제(예: 하트비터가 계속 동작하는 등)를 예방하는 데 도움이 됩니다. - 초기화 시 필수 조건 확인:
ModeSelectionStore의init에서type == .poseSuggestion && advertiser == nil일 경우 에러 로그를 남기는 로직은 필수 의존성 누락에 대한 런타임 검증을 제공하여 초기 개발 및 디버깅 시 유용합니다.
- Browser는 계산 프로퍼티로 스트림을 노출 - HeartBeeater는 스트림 매니저의 yield 메서드 사용 - 테스트 코드 모두 통과로 동등성 확인 완료
- MCNearbyServiceBrowserDelegate 분리 - discoveredPeers 상태 관리 분리 - Browser는 browsingManager 인스턴스를 사용하도록 개선 - delegate extenstion 제거
- executeCommand 메서드를 CommandManager의 execute(data:)로 분리 - BrowserCommandDelegate Protocol 정의 - Browser에서 delegate 채택
- 기존 Browser.MirroringDeviceCommand, Browser.RemoteDeviceCommand 사용처에서 Browser. 만 제거 - BrowserManager 폴더 생성 및 관련 파일 이동
- BrowserEvents 이름 변경
- Browser는 계산 프로퍼티로 스트림을 노출 - HeartBeeater는 스트림 매니저의 yield 메서드 사용 - 테스트 코드 모두 통과로 동등성 확인 완료
- BrowsingView에서 뒤로 가기(Navigation Pop) 시 연결을 해제하지 않아 세션이 유지되는 문제 해결 - isMovingToNextStep 상태를 도입하여 화면 이탈 사유(Push vs Pop) 구분 - 정상적인 다음 단계 이동(Push)에는 연결 유지, 뒤로 가기(Pop) 시에는 disconnect 호출
- 이게 왜 살아난거지
GRJeon
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.
고생하셨습니다!
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.
수고하셨습니다 👍
| noticeShootingMode() | ||
| router.push(to: MirroringRoute.streaming(isTimerMode: flag, isPoseSuggestionEnabled: true)) | ||
| store.send(.remoteMode) | ||
| router.push(to: MirroringRoute.streaming(isTimerMode: store.flag, isPoseSuggestionEnabled: true)) |
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.
firstCard랑 secondCard 모두 case .timerOrRemote: 에서 send 할까요?
포즈 버튼에서 모두 remoteMode라고 send 하고 있네요
포즈 버튼에서는 send 안 하고, 촬영 모드 선택 버튼에서만 하면 될 듯 합니다
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.
@sangYuLv
의도한 형태입니다. 포즈 선택 시 캡쳐 버튼이 노출된다면 미러링 기기에서 카메라 스트리밍이 나오지 않더라도 리모트 기기가 카메라 기기에 촬영 요청을 할 수 있기 때문에 막았습니다
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.
@sangYuLv
추가 반영 완료입니다
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.
👍 감사합니다 👍
mirroringBooth/mirroringBooth/Device/Mirroring/ModeSettings/Component/ModeSelectionStore.swift
Outdated
Show resolved
Hide resolved
mirroringBooth/mirroringBooth/Device/Mirroring/ModeSettings/Component/ModeSelectionStore.swift
Show resolved
Hide resolved
- 기존 Browser.MirroringDeviceCommand, Browser.RemoteDeviceCommand 사용처에서 Browser. 만 제거 - BrowserManager 폴더 생성 및 관련 파일 이동
…/boostcampwm2025/iOS03-dolAwang into refactor/#296-connections-timing
🔗 연관된 이슈
📝 작업 내용
📌 요약
🔍 상세
ModeSelectionView에서 타이머 모드를 선택했을 시 advertiser을 통해 카메라 기기로 리모트 기기에 내비게이션 하라는 명령을 보내어야 했습니다
이때 타이머 모드를 선택하는 ModeSelectionView는 advertiser를 넣어주지 않던 상황이라 추가하고
.sendCommand(.selectedTimerMode)를 호출하였습니다ModeSelectionView는 포즈 모드 선택 시에도 사용되기에, 호출하는 명령어 자체를 분리하고, flag 변수를 통해 앞 단계에서 타이머 모드를 선택했을 때 명령이 보내지는 현상을 방지했습니다
이때 뷰가 하는 일이 많아지기에, 뷰를 정리할 겸 Store를 새로 만들었습니다
리모트 세션 + 하트비트 정리
2-1. 카메라 기기
의도적인 끊김을 처리하기 위해
disconnect(useType:)메서드에 onPurpose 파라미터를 추가, remoteHeartBeater을 nil 처리 하였습니다disconnect호출은 촬영 기기에서의 얼럿에서 설명하겠습니다2-2. 리모트 기기
카메라 기기로부터 stopHeartBeat 명령을 받으면 본인의 heartBeater를 stop() 합니다
촬영 기기에서의 얼럿
disconnect() 메서드를 호출할 때 remoteHeartBeater가 nil이 되면서 얼럿이 뜨지 않습니다
리모트 기기에서의 얼럿
카메라 기기에서 .sendRemoteCommand(.stopHeartBeat) 명령어를 받으면 하트비트가 멈춰서 얼럿이 뜨지 않습니다
💬 리뷰 노트
WatchConnectionButton로 돌아갈 수 있는 방법이 없어, 작업한다면 사이즈가 클 것 같아 분리를 위해 작업하지 않았습니다📸 영상 / 이미지 (Optional)
2026-02-05.8.58.21.mov
2026-02-05.10.49.46.mov