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

feat: support separate control & current temperature & notification sound #38

Merged
merged 6 commits into from
Feb 3, 2025

Conversation

junbongwe
Copy link
Contributor

@junbongwe junbongwe commented Jan 16, 2025

구현사항

좌우 분리난방 기능을 지원합니다.

  • 좌우 분리난방 기능 지원 (퀸/킹 사이즈) #25 를 구현하였습니다.
  • 관련된 설정으로 separateControldisplayName 이 있으며 각각을 통해 좌우를 분리하여 조작할지 여부와, 분리하여 조작하는 경우 각 컴포넌트(Service) 들의 개별 이름을 설정할 수 있습니다. separateControl 을 설정하지 않은 경우 기존과 동일하게 분리난방이 지원되는 기기여도 통합 제어가 가능하도록 하였습니다.
  • 분리 조작하는 경우도 기존의 통합 조작과 동일하게 HeaterCooler, Thermostat 모두 지원하도록 구현하였습니다.

현재 온도 표시를 지원합니다.

모든 기기가 지원하지는 않는 것 같지만 제가 가진 기기(EMW720)의 경우 현재 온도를 표시해줄 수 있어서, 기기에서 temperature.current 필드를 내려주는 경우에 한해서 현재 온도를 그 온도로 표시하도록 하였습니다.
만약 기기에서 해당 필드를 내려주지 않으면, _temperatureCurrent 는 항상 undefined 이며, 이 경우 현재 온도로 설정 온도를 사용합니다(기존 구현과 동일).

조작시 알람 소리 재생 설정을 지원합니다.

이 또한 모든 기기가 지원하지는 않는 것 같지만, 제가 가진 기기의 경우 앱을 통해 설정시마다 소리를 통해 알려주는 기능이 있습니다. /device/${deviceSeq}/control 로 보내는 payload.state.desired 속에 beep: true 를 설정해주면 매 설정이 기기에 반영될 때마다 소리를 통해 알 수 있습니다. 조작 시 소리 재생을 할지 여부는 soundEnabled config 를 통해 제어 가능하며, default 값은 false(재생하지 않음)입니다.

UI 및 동작

아래는 separateControl: true, accessaryType: HeaterCooler 설정일 때의 애플 홈 UI 입니다.

분리 조작을 하는 경우 전체 전원 스위치, 좌측 컨트롤러우측 컨트롤러 총 세가지의 컴포넌트가 생성됩니다.
현재 구현에서 전원 조작시의 동작은 아래와 같습니다.

  • 전체 전원 스위치를 끄는 경우, 좌우측 컨트롤러도 꺼짐
  • 한쪽의 컨트롤러만 끄는 상황에서, 다른쪽의 컨트롤러가 켜져있다면 전체 전원 스위치 및 다른쪽 컨트롤러에는 영향을 주지 않음. 이 경우 끄는 쪽의 온도를 heatRange.min - heatRange.step 으로 설정
  • 한쪽의 컨트롤러를 끄는 상황에서, 다른쪽의 컨트롤러가 이미 꺼져있는 경우 온도 설정 없이 전체 전원만 꺼짐

위에 설명한 상황 외에도 다양한 상황들을 고려하여 구현하였습니다. 혹시 직관과는 다른 동작이 있다면 말씀해주시면 수정하겠습니다.

테스트

제가 가진 기기(EMW720)으로 모든 config 들에 대해(separateControl: true|false x accessaryType: "HeaterCooler"|"Thermostat", 총 4가지 경우) 테스트해보았고, 잘 동작하는 것을 확인했습니다.
다만 현재 온도 표시를 지원하지 않는 기기, 조작시 알림 소리 재생을 지원하지 않는 기기, 그리고 좌우 분리난방이 지원되지 않는 기기(싱글매트)의 경우는 그러한 기기들을 고려하여 구현하기는 하였으나 직접 테스트해보지는 못하였습니다.
혹시 테스트 가능하시다면 한번 테스트해봐주시면 좋을 것 같습니다.

commit 1a23a23
Author: Junbong We <[email protected]>
Date:   Thu Jan 16 19:43:18 2025 +0000

    feat: implement separate control & current temperature

commit b225c88
Author: Junbong We <[email protected]>
Date:   Tue Jan 14 18:01:01 2025 +0900

    feat: add separateControl config
@junbongwe junbongwe marked this pull request as ready for review January 17, 2025 07:40
@junbongwe junbongwe changed the title feat: [WIP] support separate control & current temperature & notification sound feat: support separate control & current temperature & notification sound Jan 17, 2025
@junbongwe
Copy link
Contributor Author

@kyle-seongwoo-jun 안녕하세요 :)
제가 필요한 기능들이 몇가지 있어서 구현해보았습니다.
PR description 에 구현 사항들 관련해서 자세히 적어두었고, 구현상 디테일들은 코드 내의 주석에 적어두었습니다.
저는 이미 제가 구현한 버전으로 사용중이고 구현 양이 꽤 많다보니, 시간 되실 때 천천히 테스트 및 리뷰해주시면 감사하겠습니다 🙏

Copy link
Owner

@kyle-seongwoo-jun kyle-seongwoo-jun left a comment

Choose a reason for hiding this comment

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

@junbongwe PR 감사합니다! 아직 제 환경에선 테스트를 해보진 않았는데, 우선 먼저 코드 리뷰 드립니다. 감사합니다 😄

src/navien/navien.api.ts Outdated Show resolved Hide resolved
src/navien/navien.device.ts Outdated Show resolved Hide resolved
src/navien/navien.service.ts Outdated Show resolved Hide resolved
src/navien/navien.service.ts Outdated Show resolved Hide resolved
src/homebridge/electric-mat.device.ts Outdated Show resolved Hide resolved
@junbongwe
Copy link
Contributor Author

@kyle-seongwoo-jun 안녕하세요 조금 늦었지만 말씀해주셨던 리뷰사항들 모두 수정해두었습니다(https://github.com/junbongwe/homebridge-navien-smart/commit/11079243d3723d5c561f5085c1fd01c67256852c).
HeatingMat 클래스들 별도 소스파일로 분리하는 것 해두었는데, 혹시 더 나은 방식이나 구조 있다면 말씀해주세요.

그리고 이는 별개로, 테스트하던 중 이상한 동작 하나를 발견해서 이를 수정하는 커밋을 하나 더 쌓아두었습니다(https://github.com/junbongwe/homebridge-navien-smart/commit/23424f4c70a2d9b006bf110ee956ed4aa8c82ca4).
코드 보시면 이해가 되실 것 같지만 간단히 설명드리자면 기존에 생성된 accessory 가 cache 에 있는 상태에서 accessoryType 이나 separateControl 설정을 바꾼 뒤 HB 를 재시작하게되면 cache 내 accessory 의 Service 들과 새로 바뀐 config 에서 사용되는 Service 가 달라서 Service 들이 중복으로 생성되게 됩니다.
이를 수정하기 위해 cache 에서 들고와서 restore 하기 전에 그 accessory 가 들고있는 서비스들을 확인해서 현재 config 에서 생성되는 Service 와 존재하는 Service 가 다른 경우 accessory 를 다시 만들도록 했습니다(혹시 더 좋은 방법이 떠오르신다면 말씀해주세요).

미리 감사드립니다 :)

Copy link
Owner

@kyle-seongwoo-jun kyle-seongwoo-jun left a comment

Choose a reason for hiding this comment

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

늦어져서 죄송합니다!

구 버전의 config 에서 시작하면 죽는 문제가 있어서 제가 수정했습니다.

테스트를 해봤는데 통합 제어에서 온도 제어를 http 요청 두 번 나눠 보내도록 수정된 부분때문인지, 가끔 하나가 변경 안 되고 싱크가 틀어지는 부분이 있는데 이 부분은 머지 후에 제가 수정하도록 하겠습니다.

이렇게 소중한 시간 써주시며 PR 열어주셔서 감사합니다!

@kyle-seongwoo-jun kyle-seongwoo-jun merged commit b246f16 into kyle-seongwoo-jun:latest Feb 3, 2025
3 checks passed
@kyle-seongwoo-jun
Copy link
Owner

1.8.0 버전으로 릴리즈했습니다. 감사합니다! 👏

@junbongwe
Copy link
Contributor Author

네 릴리즈된 버전으로 잘 작동하는 것 확인했습니다.
긴 PR 리뷰해주셔서 감사합니다 🙇 🎉

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.

2 participants