Repository: home-assistant/core
Pull request: Fix invalid enocean unique_id
## Breaking change
This is not a breaking change

## Proposed change
Fix of Issue #73031 second channel is no longer displayed on enocean switches with 2 channels


## Type of change

- [ ] Dependency upgrade
- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New integration (thank you!)
- [ ] New feature (which adds functionality to an existing integration)
- [ ] Breaking change (fix/feature causing existing functionality to break)
- [ ] Code quality improvements to existing code or addition of tests

## Additional information
There are enocean switches available with two different channels. As they have one 4 Byte address as ID, and represent two different devices (channel 0 and channel 1), the same unique ID is created for both and HA complains. Now the channel is added as a suffix to the unique ID. 
- This PR fixes or closes issue: fixes #73031 

## Checklist
- [X] The code change is tested and works locally.
- [ ] Local tests pass. **Your PR cannot be merged unless tests pass**
- [X] There is no commented out code in this PR.
- [X] I have followed the [development checklist][dev-checklist]
- [X] The code has been formatted using Black (`black --fast homeassistant tests`)
- [ ] Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

- [ ] Documentation added/updated for [www.home-assistant.io][docs-repository]

If the code communicates with devices, web services, or third-party tools:

- [ ] The [manifest file][manifest-docs] has all fields filled out correctly.  
      Updated and included derived files by running: `python3 -m script.hassfest`.
- [ ] New or updated dependencies have been added to `requirements_all.txt`.  
      Updated by running `python3 -m script.gen_requirements_all`.
- [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
- [ ] Untested files have been added to `.coveragerc`.



Problem statement:
second channel is no longer displayed on enocean switches with 2 channels
### The problem

since the update to 22.06, the second channel is no longer displayed on enocean switches with 2 channels.
I think the channel no. must be integrated in the unique id.

### What version of Home Assistant Core has the issue?

2022.06

### What was the last working version of Home Assistant Core?

2022.5.5

### What type of installation are you running?

Home Assistant OS

### Integration causing the issue

Enocean

### Link to integration documentation on our website

_No response_

### Diagnostics information

_No response_

### Example YAML snippet

_No response_

### Anything in the logs that might be useful for us?

```txt
- platform: enocean
  id: [0x05, 0x0B, 0xB6, 0x99]
  channel: 1
  name: room0

- platform: enocean
  id: [0x05, 0x0B, 0xB6, 0x99]
  channel: 0
  name: room1

only first switch ist visible
```


### Additional information

_No response_

Hints from issue discussion:
[enocean documentation](https://www.home-assistant.io/integrations/enocean)
[enocean source](https://github.com/home-assistant/core/tree/dev/homeassistant/components/enocean)
<sub><sup>(message by IssueLinks)</sup></sub>
Hey there @bdurrer, mind taking a look at this issue as it has been labeled with an integration (`enocean`) you are listed as a [code owner](https://github.com/home-assistant/core/blob/dev/CODEOWNERS#L292) for? Thanks!
<sub><sup>(message by CodeOwnersMention)</sup></sub>
In case it helps. An example for a not working device is a NODON SIN2201.
Hi there, I can confirm the issue as well. It is still present in HA version 2022.6.6.
I had the same problem when I updated my version of home assistant core, and had to rollback to get everything working.
I believe this is caused by https://github.com/home-assistant/core/pull/71774 . (I had actually updated to get this feature) 
In this change, a unique id is now generated for EnOcean devices (which is great), however, it only takes the EURID into account. This is great, but it doesn't work for multi-channel devices where each channel is declared as a separate entity.

A fix would be to use the channel index, in the unique ID generation.
@rhadamantys , can you please have a look at this issue?
Ok, it looks like the introduction of the unique ID has definitely broken something here. Unfortunately, I don't have such a device, so I can't test it on my own...

By the way: do you get a warning, that there are two devices with identical unique IDs?


Edit:
I have looked into the code and it seems like in enocean/switch.py, [line 52](https://github.com/home-assistant/core/blob/148035c8ca8c6d706ec2ddcc9a52475b086ccb0d/homeassistant/components/enocean/switch.py#L52)

needs to be changed from
`self._attr_unique_id = f"{combine_hex(dev_id)}"`
to
`self._attr_unique_id = f"{combine_hex(dev_id)}{self.channel}"`

I will create a PR for this with the disclaimer, that I am not able to test it.
Many thanks for your prompt reply. 
I do not see any warnings on my side. Just the entity for the second channel is not available anymore. I can test on my side if it is possible.
I am running a version of enocean integration based on https://github.com/bakkerv/home-assistant/tree/add-enocean-covers/homeassistant/components/enocean, which exhibited the same problem. (It's now been running fine for two weeks)

While fixing a few bugs in it, I replaced unique id generation with this: `f"switch-{to_hex_string(self.dev_id)}-{hex(self.channel)}"`. I did not add the `switch-` prefix (see [here](https://github.com/bakkerv/home-assistant/blob/add-enocean-covers/homeassistant/components/enocean/switch.py#L164)) but if you're doing the fix, I'd greatly appreciate if you could migrate the entities to this pattern, so that I can later update without losing everything.
One reason for adding the `swicth-` prefix (and potentially others) is that a device could theoretically be mapped to two different components types. (at the same time, or if it was mapped to the wrong component type and later updated)

FWIW, I also tried fixing/improving the integration for `powersensor` (not the other kinds of sensors), and a correct integration also requires having extra parameters in the unique id for proper operation: I used `f"sensor-{to_hex_string(self.dev_id)}-{hex(self.channel)}-{self.data_type}"` (where `data_type` is either `0` (cumulated value) or `1` (current value))
The reason for this is that while the current integration considers sensors of type A5-12-** as having only one channel and only read current values (data type 1), they actually have up to 2*16, counting the cumulated & current values

I'll try publishing a version of what I'm running if you want to take inspiration on it.

Commit message: Migration of old unique id to new unique id

Patch to review:
diff --git a/homeassistant/components/enocean/switch.py b/homeassistant/components/enocean/switch.py
index a53f691df19847..3989b9d131c142 100644
--- a/homeassistant/components/enocean/switch.py
+++ b/homeassistant/components/enocean/switch.py
@@ -5,12 +5,14 @@
 import voluptuous as vol
 
 from homeassistant.components.switch import PLATFORM_SCHEMA, SwitchEntity
-from homeassistant.const import CONF_ID, CONF_NAME
+from homeassistant.const import CONF_ID, CONF_NAME, Platform
 from homeassistant.core import HomeAssistant
+from homeassistant.helpers import entity_registry
 import homeassistant.helpers.config_validation as cv
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
 from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType
 
+from .const import DOMAIN
 from .device import EnOceanEntity
 
 CONF_CHANNEL = "channel"
@@ -25,6 +27,26 @@
 )
 
 
+def generate_unique_id(dev_id: list[int], channel: int) -> str:
+    """Generate a valid unique id."""
+    return f"{combine_hex(dev_id)}{channel}"
+
+
+def _migrate_to_new_unique_id(hass: HomeAssistant, dev_id, channel) -> None:
+    """Migrate old unique ids to new unique ids."""
+    old_unique_id = f"{combine_hex(dev_id)}"
+
+    ent_reg = entity_registry.async_get(hass)
+    entity_id = ent_reg.async_get_entity_id(Platform.SWITCH, DOMAIN, old_unique_id)
+
+    if entity_id is not None:
+        new_unique_id = generate_unique_id(dev_id, channel)
+        try:
+            ent_reg.async_update_entity(entity_id, new_unique_id=new_unique_id)
+        except ValueError:
+            pass
+
+
 def setup_platform(
     hass: HomeAssistant,
     config: ConfigType,
@@ -36,6 +58,7 @@ def setup_platform(
     dev_id = config.get(CONF_ID)
     dev_name = config.get(CONF_NAME)
 
+    _migrate_to_new_unique_id(hass, dev_id, channel)
     add_entities([EnOceanSwitch(dev_id, dev_name, channel)])
 
 
@@ -49,7 +72,7 @@ def __init__(self, dev_id, dev_name, channel):
         self._on_state = False
         self._on_state2 = False
         self.channel = channel
-        self._attr_unique_id = f"{combine_hex(dev_id)}"
+        self._attr_unique_id = generate_unique_id(dev_id, channel)
 
     @property
     def is_on(self):


Code review comments:
diff_hunk: @@ -25,6 +27,26 @@
 )
 
 
+def generate_unique_id(dev_id: list[int], channel: int) -> str:
+    """Generate a valid unique id."""
+    return f"{combine_hex(dev_id)}{channel}"
+
+
+def _migrate_to_new_unique_id(hass: HomeAssistant, dev_id, channel) -> None:
+    """Migrate old unique ids to new unique ids."""
+    old_unique_id = f"{combine_hex(dev_id)}"
+
+    ent_reg = entity_registry.async_get(hass)
+    entity_id = ent_reg.async_get_entity_id(Platform.SWITCH, DOMAIN, old_unique_id)
+
+    if entity_id is not None:
+        new_unique_id = generate_unique_id(dev_id, channel)
+        try:
+            ent_reg.async_update_entity(entity_id, new_unique_id=new_unique_id)
+        except ValueError:
+            pass
path: homeassistant/components/enocean/switch.py
text: @user1:
Log a debug or warning log here probably.

@author:
Done ✅

diff_hunk: @@ -25,6 +27,26 @@
 )
 
 
+def generate_unique_id(dev_id: list[int], channel: int) -> str:
+    """Generate a valid unique id."""
+    return f"{combine_hex(dev_id)}{channel}"
+
+
+def _migrate_to_new_unique_id(hass: HomeAssistant, dev_id, channel) -> None:
+    """Migrate old unique ids to new unique ids."""
+    old_unique_id = f"{combine_hex(dev_id)}"
+
+    ent_reg = entity_registry.async_get(hass)
+    entity_id = ent_reg.async_get_entity_id(Platform.SWITCH, DOMAIN, old_unique_id)
+
+    if entity_id is not None:
+        new_unique_id = generate_unique_id(dev_id, channel)
+        try:
+            ent_reg.async_update_entity(entity_id, new_unique_id=new_unique_id)
+        except ValueError:
+            pass
+
+
 def setup_platform(
path: homeassistant/components/enocean/switch.py
text: @user1:
The async api needs to run inside the event loop.
```suggestion
async def async_setup_platform(
```

@author:
Done ✅