增加了HD设备支持#2
Conversation
增加滤网清洁提示的信息，但没有重写do方法。只接收并处理，但没有后续动作，主要是为了在log中少一点未知消息。
增加了HD实体支持
|
/gemini review |
|
@codex review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces support for HD (Heat Exchanger) devices, adding WATER_HEATER and SWITCH platforms. It includes new data access objects (HD, HDStatus), command types, and decoding logic for HD devices. The service layer has been updated to manage HD devices, including initialization, status polling, and control. New Home Assistant entities (DsWaterHeater, DsHdMuteSwitch) are implemented to expose HD functionality. The changes appear to be well-structured and integrate HD device support effectively into the existing system. The author has also added helpful comments in Chinese to explain certain parts of the code, which is a good practice for maintainability.
| # 发送指令 | ||
| CONTROL = 1 | ||
| STATUS_CHANGED = 2 | ||
| STATUS_CHANGED = 2 #对于HD设备，是开关状态改变会出现本指令 |
There was a problem hiding this comment.
The comment 对于HD设备，是开关状态改变会出现本指令 is helpful, but it would be better to place it on a separate line or use a more standard inline comment format if it's meant to be a permanent explanation. If it's a temporary note, it should be removed before merging.
| STATUS_CHANGED = 2 #对于HD设备，是开关状态改变会出现本指令 | |
| STATUS_CHANGED = 2 # 对于HD设备，是开关状态改变会出现本指令 |
| HD_INFO_QUERY = 12 | ||
| HD_INFO_CHANGE = 13 | ||
| HD_CONTROL_BASE = 14 | ||
| HD_CONTROL_OTHER = 15 #这个当前只有获取夜间节能模式这一个功能 |
There was a problem hiding this comment.
Similar to the previous comment, consider placing this comment on a separate line for better readability and to avoid potential issues with line length limits or code formatters. If it's a temporary note, it should be removed before merging.
| HD_CONTROL_OTHER = 15 #这个当前只有获取夜间节能模式这一个功能 | |
| HD_CONTROL_OTHER = 15 # 这个当前只有获取夜间节能模式这一个功能 |
| preheat: int | None = None, | ||
| switch_enable: EnumControl.Switch | None = None, | ||
| temperature_set: int | None = None, | ||
| warm_cold: int | None = None, #1为制热状态 |
There was a problem hiding this comment.
The comment 1为制热状态 is helpful, but it would be better to place it on a separate line or use a more standard inline comment format if it's meant to be a permanent explanation. If it's a temporary note, it should be removed before merging.
| warm_cold: int | None = None, #1为制热状态 | |
| warm_cold: int | None = None, # 1为制热状态 |
| #HDQueryInfoParam在我的DTA117C611上没有返回，注释掉了 | ||
| #p = HDQueryInfoParam() | ||
| #p.device = hd | ||
| #service.send_msg(p) |
There was a problem hiding this comment.
These commented-out lines suggest that HDQueryInfoParam might not be fully functional or necessary. It's generally better to remove dead code or code that is not actively used, especially if it's causing issues or not returning responses. If there's a plan to re-enable this, consider adding a TODO comment with a clear explanation.
| #def do(self, service: Service) -> None: | ||
| # _LOGGER.debug(f"[HDStatusChangeResult] 执行HD状态查询结果: room={self._room}, unit={self._unit}") | ||
| # try: | ||
| # service.set_hd_status(self._room, self._unit, self._status) | ||
| # except Exception as e: | ||
| # _LOGGER.error(f"[HDStatusChangeResult] 设置HD状态时出错: {e}") |
There was a problem hiding this comment.
These commented-out lines suggest that the do method for HDStatusChangeResult is intentionally disabled. If this functionality is not needed, the method should be removed entirely. If it's temporarily disabled, add a TODO comment explaining why and when it should be re-enabled.
|
|
||
| _log(display(self._device_info.status)) | ||
| self.hass.loop.call_soon_threadsafe(self.async_write_ha_state) | ||
| _log(f"[HD Entity] 状态更新完成") |
There was a problem hiding this comment.
The _log call here is for debugging and should ideally be removed or converted to _LOGGER.debug calls before merging into production code. Excessive logging can impact performance and disk space.
| _log(f"[HD Entity] 模式未知: {self._device_info.status.switch}") | ||
| return None |
There was a problem hiding this comment.
The _log call here is for debugging and should ideally be removed or converted to _LOGGER.debug calls before merging into production code. Excessive logging can impact performance and disk space.
| async def async_set_temperature(self, **kwargs) -> None: | ||
| """Set new target temperature.""" | ||
| temperature = kwargs.get(ATTR_TEMPERATURE) | ||
| if temperature is not None: |
There was a problem hiding this comment.
The _log call here is for debugging and should ideally be removed or converted to _LOGGER.debug calls before merging into production code. Excessive logging can impact performance and disk space.
|
|
||
| async def async_set_operation_mode(self, operation_mode: str) -> None: | ||
| """Set new operation mode.""" | ||
| _log(f"[HD Entity] 设置操作模式: {operation_mode}") |
There was a problem hiding this comment.
The _log call here is for debugging and should ideally be removed or converted to _LOGGER.debug calls before merging into production code. Excessive logging can impact performance and disk space.
| # 同时更新本地状态以提供即时UI反馈 | ||
| self._device_info.status.switch = EnumControl.Switch.OFF | ||
| else: | ||
| _log(f"[HD Entity] 不支持的操作模式: {operation_mode}") |
There was a problem hiding this comment.
The _log call here is for debugging and should ideally be removed or converted to _LOGGER.debug calls before merging into production code. Excessive logging can impact performance and disk space.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b552283ca0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #def do(self, service: Service) -> None: | ||
| # _LOGGER.debug(f"[HDStatusChangeResult] 执行HD状态查询结果: room={self._room}, unit={self._unit}") | ||
| # try: |
There was a problem hiding this comment.
result_factory routes HD STATUS_CHANGED packets to HDStatusChangeResult (decoder.py:149-150), and load_bytes parses the switch field, but the do(...) handler is commented out here. As a result, when gateways emit this packet type (notably for switch changes), the update is decoded then dropped, so Home Assistant state can remain stale until a later poll/other message arrives.
Useful? React with 👍 / 👎.
|
我有点犹豫要不要合哈，因为我也没有HD设备可以测试，我觉得有需要的直接去拉你的仓库用也可以，这个PR可以留在这里给别人指路。 我这个仓库只对原版做了一些代码重构，核心逻辑是没有动的，所以协议这块其实我也没细看过。 |
|
另外你看看上面这条评论 #2 (comment) 这里的逻辑似乎和原本有些区别，之前是等4个都初始化完成，现在你用了 room and (.. or .. or ..) 的形式，而且还没包含你新增的 hd_initialized |
感谢回复，看了下好像确实逻辑被改的有点问题。这个初始化因为流程很快，所以我本地倒没出问题。我再去改一下逻辑 |
修复初始化的逻辑问题
|
上游已恢复活跃，这边先关闭了 |
2 participants
趁着春节放假花了好几天反解了大金的包，把HD的支持做上去了。和原来的主线代码差别太大，我从你这里拉的基础包，给你推回来。我python是三脚猫水平，代码是AI生成+自己审核并测试上来的，在合并的时候注意了尽可能减少对已有代码的扰动。目前简单用下来没有问题，不过可能需要更多人来进一步测试。