-
Notifications
You must be signed in to change notification settings - Fork 4
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
[Fix] #371 전화 수신, 발신 시 소리 재생 오류 해결 #374
Conversation
Macro/Service/SoundManager.swift
Outdated
self.setupNotifications() | ||
} | ||
|
||
@objc func handleInterruption(notification: Notification) { |
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.
이 메서드는 private으로 설정하지 않으신 이유가 궁금합니다.
하단에서 selector로 등록할때 문제가 생길까요?
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.
아뇽,, 마지막에 private 체크했는데 objc가 앞에 있어서 당연히 뒤에 달려있다고 생각했나봅니다~ 수정해서 올리겠습니다!
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.
0723f0b 해당 커밋을 통해서 수정해두었습니다.
|
||
self.soundManager.callInterruptPublisher.sink {[weak self] callInterrupt in | ||
guard let self else { return } | ||
callInterrupt == true ? self.stop() : nil |
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.
Bool값을 == true 인지 검증하는게 조금 어색해 보입니다
isCalling 같이 자체적으로 Bool값을 암시하는 변수명으로 대체하는게 어떨가요
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.
받는 값이 false일때 (전화가 끊겼을때) 아무 일도 하지 않는다고 하면
Bool값 대신 Void 타입으로 이벤트 발생만 전달하는 방안도 있을것 같습니다.
전화가 올때 재생이 멈추고 유저가 전화를 끊고 다시 재생을 눌러야 하는 방식으로 구현될것 같아요
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.
임시변수를 만들어도 해당 코드 내부에서 사용처가 없기 때문에 안만드는 것이 더 적합하다고 생각합니다. 방식은 말씀하신 것처럼 현재 전화 오면 재생이 멈추고 전화를 끊고 유저가 다시 재생을 누르는 식으로 구현되어 있습니다.
처리 방식을 변수로 처리하기보다 callInterrupt ? self.stop : nil 로 수정하는게 더 나을 것 같고,
이벤트 발생 여부만 전달하는 것이 가장 좋을 것 같습니다. 이벤트 여부 전달 방식으로 바꿔도 괜찮을까요?
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.
MetronomeOnOffUseCase의 tickPublisher가 Void 타입 Publisher로 구현되어 있습니다
참고가 되면 좋겠네여
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.
@YunKi-H 수정했습니다~ 확인 부탁드리며, 추가적으로 피드백 주실 부분이 있을지 확인 후 머지하겠습니다!
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.
좋아요!
Macro/Service/SoundManager.swift
Outdated
|
||
case .ended: | ||
self.audioEngineStart() | ||
self.publisher.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.
전화 받는 이벤트만 처리한다면 .ended 케이스를 처리할 필요가 없을거같습니다
혹시 모르니 남겨두셨나욤?
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.
publisher 처리는 삭제하겠습니다.
다만 self.audioEngineStart()의 경우에는 물론 항상 작동하지는 않지만,,, 시스템 상황적으로 끝나고 정상적으로 Engine이 start 되는게 가장 안정적이라고 생각합니다. 어떻게 생각하시나욥...?
개요
전화 수신, 발신 시 소리 재생 오류 해결
작업 내용
1. 전화 수신/발신 시 메트로놈 재생 중단
2. 전화 종료 후 재실행 시 오디오 재실행
비고
작업 스크린샷
리뷰어에게 남길 말
인터럽트 종료시 실행되는 .ended 부분에서 Engine.Start()가 정상 처리(isRunning) 되는 것으로 찍히는데, 실제 Beep() 실행 시점에 isRunning하지 않는 것으로 나오는지 모르겠어요.
위 사항을 해결하면 코드가 조금 더 많이 깔끔해질 것 같은데,,,
CheckList <!-- 리뷰어가 체크할 항목입니다. >