-
Notifications
You must be signed in to change notification settings - Fork 2
Separate Room Devices #52
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,8 +39,10 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: | |
| read_timeout = entry.options.get(CONF_READ_TIMEOUT, DEFAULT_READ_TIMEOUT) | ||
| timeout = aiohttp.ClientTimeout(sock_connect=connect_timeout, sock_read=read_timeout) | ||
|
|
||
| separate_room_devices = entry.options.get(CONF_SEPARATE_ROOM_DEVICES, DEFAULT_SEPARATE_ROOM_DEVICES) | ||
|
|
||
| coordinator = WundasmartDataUpdateCoordinator( | ||
| hass, wunda_ip, wunda_user, wunda_pass, update_interval, timeout | ||
| hass, wunda_ip, wunda_user, wunda_pass, update_interval, timeout, separate_room_devices | ||
| ) | ||
| await coordinator.async_config_entry_first_refresh() | ||
|
|
||
|
|
@@ -70,9 +72,39 @@ async def async_reload_entry(hass: HomeAssistant, entry: ConfigEntry) -> None: | |
|
|
||
| async def update_listener(hass: HomeAssistant, config_entry: ConfigEntry) -> None: | ||
| """Update listener.""" | ||
| # Check if we're switching from separate devices to legacy mode | ||
| separate_room_devices = config_entry.options.get(CONF_SEPARATE_ROOM_DEVICES, DEFAULT_SEPARATE_ROOM_DEVICES) | ||
|
|
||
| if not separate_room_devices: | ||
| # Clean up room devices when switching to legacy mode | ||
| await _cleanup_room_devices(hass, config_entry) | ||
|
|
||
| await hass.config_entries.async_reload(config_entry.entry_id) | ||
|
|
||
|
|
||
| async def _cleanup_room_devices(hass: HomeAssistant, config_entry: ConfigEntry) -> None: | ||
| """Remove room devices when switching to legacy mode.""" | ||
| from homeassistant.helpers import device_registry as dr | ||
|
|
||
| device_registry = dr.async_get(hass) | ||
|
|
||
| # Find all room devices for this integration | ||
| devices = dr.async_entries_for_config_entry(device_registry, config_entry.entry_id) | ||
|
|
||
| removed_count = 0 | ||
| for device in devices: | ||
| # Check if this is a room device (not the main hub) | ||
| for identifier in device.identifiers: | ||
| if identifier[0] == DOMAIN and "_room_" in str(identifier[1]): | ||
| _LOGGER.info(f"Removing orphaned room device: {device.name} ({identifier[1]})") | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be using the debug log level |
||
| device_registry.async_remove_device(device.id) | ||
| removed_count += 1 | ||
| break | ||
|
|
||
| if removed_count > 0: | ||
| _LOGGER.info(f"Removed {removed_count} orphaned room device(s) when switching to legacy mode") | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one too |
||
|
|
||
|
|
||
| class WundasmartDataUpdateCoordinator(DataUpdateCoordinator): | ||
| """Class to manage fetching data from WundaSmart API.""" | ||
|
|
||
|
|
@@ -82,7 +114,8 @@ def __init__(self, | |
| wunda_user: str, | ||
| wunda_pass: str, | ||
| update_interval: int, | ||
| timeout: aiohttp.ClientTimeout): | ||
| timeout: aiohttp.ClientTimeout, | ||
| separate_room_devices: bool = False): | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be a required argument since it's never called without it |
||
| """Initialize.""" | ||
| self._hass = hass | ||
| self._wunda_ip = wunda_ip | ||
|
|
@@ -95,6 +128,7 @@ def __init__(self, | |
| self._hw_version = None | ||
| self._timeout = timeout | ||
| self._keepalive_timeout = update_interval * 2 | ||
| self._separate_room_devices = separate_room_devices | ||
|
|
||
| super().__init__(hass, | ||
| _LOGGER, | ||
|
|
@@ -160,13 +194,34 @@ def device_sn(self): | |
|
|
||
| @property | ||
| def device_info(self) -> DeviceInfo | None: | ||
| """Return device info for the hub device.""" | ||
| if self._device_sn is None: | ||
| return None | ||
|
|
||
| return DeviceInfo( | ||
| identifiers={(DOMAIN, self._device_sn)}, | ||
| manufacturer="Wunda", | ||
| name=self._device_name or "Smart HubSwitch", | ||
| model="WundaSmart Hub", | ||
| hw_version=self._hw_version, | ||
| sw_version=self._sw_version | ||
| ) | ||
|
|
||
| def get_room_device_info(self, room_id: str, room_device: dict) -> DeviceInfo | None: | ||
| """Return device info for a room/zone.""" | ||
| if self._device_sn is None: | ||
| return None | ||
|
|
||
| # If separate_room_devices is disabled, return hub device info (legacy mode) | ||
| if not self._separate_room_devices: | ||
| return self.device_info | ||
|
|
||
| room_name = room_device.get("name", f"Room {room_id}") | ||
|
|
||
| return DeviceInfo( | ||
| identifiers={(DOMAIN, f"{self._device_sn}_room_{room_id}")}, | ||
| manufacturer="Wunda", | ||
| name=room_name, | ||
| model="WundaSmart Room", | ||
| via_device=(DOMAIN, self._device_sn), | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -124,10 +124,14 @@ def __init__( | |
| self._wunda_user = wunda_user | ||
| self._wunda_pass = wunda_pass | ||
| self._wunda_id = wunda_id | ||
| self._attr_name = device["name"] | ||
| self._attr_device_info = coordinator.get_room_device_info(wunda_id, device) | ||
| # Use simple name for separate devices, full name for legacy mode | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. full name is used in the default mode, so doesn't follow to call that legacy mode here. Maybe "Use simple name for separate devices, or full name otherwise"? |
||
| if coordinator._separate_room_devices: | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor point but this could be a single line, if it's not too long. Happy with leaving it this way if it looks better to you. |
||
| self._attr_name = "Climate" | ||
| else: | ||
| self._attr_name = device["name"] | ||
| self._attr_unique_id = device["id"] | ||
| self._attr_type = device["device_type"] | ||
| self._attr_device_info = coordinator.device_info | ||
| # This flag needs to be set until 2025.1 to prevent warnings about | ||
| # implicitly supporting the turn_off/turn_on methods. | ||
| # https://developers.home-assistant.io/blog/2024/01/24/climate-climateentityfeatures-expanded/ | ||
|
|
@@ -142,7 +146,7 @@ def __init__( | |
| self._attr_target_temperature = None | ||
| self._attr_current_humidity = None | ||
| self._attr_hvac_mode = HVACMode.AUTO | ||
| self._attr_preset_mode = PRESET_NONE | ||
| self._attr_preset_mode = PRESET_NONE | ||
| self._timeout = timeout | ||
|
|
||
| # Update with initial state | ||
|
|
@@ -229,7 +233,7 @@ def __set_preset_mode(self): | |
| except (ValueError, TypeError): | ||
| _LOGGER.warning(f"Unexpected {state_key} value '{state[state_key]}' for {self._attr_name}") | ||
| else: | ||
| self._attr_preset_mode = PRESET_NONE | ||
| self._attr_preset_mode = PRESET_NONE | ||
|
|
||
| def __set_hvac_state(self): | ||
| """Set the hvac action and hvac mode from the coordinator data.""" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -208,12 +208,17 @@ def _trv_get_room(coordinator: WundasmartDataUpdateCoordinator, device): | |
| return coordinator.data.get(room_id) | ||
|
|
||
|
|
||
| def _trv_get_sensor_name(room, trv, desc: WundaSensorDescription): | ||
| def _trv_get_sensor_name(room, trv, desc: WundaSensorDescription, separate_room_devices: bool = False): | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be a required argument to avoid accidentally calling it without it and getting the wrong result |
||
| """Return a human readable name for a TRV device""" | ||
| device_id = int(trv["device_id"]) | ||
| hw_version = float(trv["hw_version"]) | ||
| id_ranges = get_device_id_ranges(hw_version) | ||
| return room["name"] + f" TRV.{device_id - id_ranges.MIN_TRV_ID} {desc.name}" | ||
| # Simple name for separate devices, full name for legacy mode | ||
| trv_suffix = f"TRV.{device_id - id_ranges.MIN_TRV_ID} {desc.name}" | ||
| if separate_room_devices: | ||
| return trv_suffix | ||
| else: | ||
| return room["name"] + " " + trv_suffix | ||
|
|
||
|
|
||
| def _signal_pct_to_dbm(pct): | ||
|
|
@@ -259,27 +264,30 @@ async def async_setup_entry( | |
| sensors = itertools.chain( | ||
| ( | ||
| Sensor(wunda_id, | ||
| room["name"] + " " + desc.name, | ||
| desc.name if coordinator._separate_room_devices else room["name"] + " " + desc.name, | ||
| coordinator, | ||
| desc) | ||
| desc, | ||
| room_id=wunda_id) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand the wunda_id is the same as the room id for the type ROOM, but for consistency/clarity, it might be worth using get_room_id_from_device here too. The marginal overhead of that function call is worth it for clarity, IMHO |
||
| for wunda_id, device, room in devices_by_type["ROOM"] | ||
| for desc in descriptions_by_type["ROOM"] | ||
| ), | ||
|
|
||
| ( | ||
| Sensor(wunda_id, | ||
| room["name"] + " " + desc.name, | ||
| desc.name if coordinator._separate_room_devices else room["name"] + " " + desc.name, | ||
| coordinator, | ||
| desc) | ||
| desc, | ||
| room_id=get_room_id_from_device(device)) | ||
| for wunda_id, device, room in devices_by_type["SENSOR"] | ||
| for desc in descriptions_by_type["SENSOR"] | ||
| ), | ||
|
|
||
| ( | ||
| Sensor(wunda_id, | ||
| _trv_get_sensor_name(room, device, desc), | ||
| _trv_get_sensor_name(room, device, desc, coordinator._separate_room_devices), | ||
| coordinator, | ||
| desc) | ||
| desc, | ||
| room_id=get_room_id_from_device(device)) | ||
| for wunda_id, device, room in devices_by_type["TRV"] | ||
| for desc in descriptions_by_type["TRV"] | ||
| ) | ||
|
|
@@ -299,6 +307,7 @@ def __init__( | |
| name: str, | ||
| coordinator: WundasmartDataUpdateCoordinator, | ||
| description: WundaSensorDescription, | ||
| room_id: str = None, | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be required since the room_id is now always supplied |
||
| ) -> None: | ||
| """Initialize the sensor.""" | ||
| super().__init__(coordinator) | ||
|
|
@@ -308,7 +317,13 @@ def __init__( | |
|
|
||
| if (device_sn := coordinator.device_sn) is not None: | ||
| self._attr_unique_id = f"{device_sn}.{wunda_id}.{description.key}" | ||
| self._attr_device_info = coordinator.device_info | ||
|
|
||
| # Determine device info based on whether this sensor belongs to a room | ||
| if room_id is not None: | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be simplified since we will always have a room id and the logic for which device info to get is already in coordinator.get_room_device_info. Duplicating the logic here makes the code marginally harder to maintain. |
||
| room_device = coordinator.data.get(room_id, {}) | ||
| self._attr_device_info = coordinator.get_room_device_info(room_id, room_device) | ||
| else: | ||
| self._attr_device_info = coordinator.device_info | ||
|
|
||
| self.entity_description = self.__update_description_defaults(description) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -27,7 +27,11 @@ | |||||
| "scan_interval": "[%key:common::config_flow::data::scan_interval%]", | ||||||
| "connect_timeout": "Connect Timeout", | ||||||
| "read_timeout": "Read Timeout", | ||||||
| "ping_interval": "Heartbeat Interval" | ||||||
| "ping_interval": "Heartbeat Interval", | ||||||
| "separate_room_devices": "Separate Room Devices" | ||||||
| }, | ||||||
| "data_description": { | ||||||
| "separate_room_devices": "When enabled, each room becomes a separate device. When disabled, all entities are grouped under the hub device (legacy mode). WARNING: Switching from enabled to disabled may leave orphaned room devices that need to be manually removed from Settings > Devices & Services > Devices." | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Reworded and removed warning since it seems that the cleanup should now be done automatically. |
||||||
| }, | ||||||
| "title": "Wundasmart options" | ||||||
| } | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -27,7 +27,11 @@ | |||||
| "scan_interval": "Poll Interval", | ||||||
| "connect_timeout": "Connect Timeout", | ||||||
| "read_timeout": "Read Timeout", | ||||||
| "ping_interval": "Heartbeat Interval" | ||||||
| "ping_interval": "Heartbeat Interval", | ||||||
| "separate_room_devices": "Separate Room Devices" | ||||||
| }, | ||||||
| "data_description": { | ||||||
| "separate_room_devices": "When enabled, each room becomes a separate device. When disabled, all entities are grouped under the hub device (legacy mode). WARNING: Switching from enabled to disabled may leave orphaned room devices that need to be manually removed from Settings > Devices & Services > Devices." | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Reworded and removed warning since it seems that the cleanup should now be done automatically. |
||||||
| }, | ||||||
| "title": "Wundasmart options" | ||||||
| } | ||||||
|
|
||||||
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.
Can this import be moved to the top of the module?