Repository: home-assistant/core
Pull request: Make `unique_id` of the Shelly button entity immutable
<!--
  You are amazing! Thanks for contributing to our project!
  Please, DO NOT DELETE ANY TEXT from this template! (unless instructed).
-->
## Breaking change
<!--
  If your PR contains a breaking change for existing users, it is important
  to tell them what breaks, how to make it work again and why we did this.
  This piece of text is published with the release notes, so it helps if you
  write it towards our users, not us.
  Note: Remove this section if this PR is NOT a breaking change.
-->


## Proposed change
<!--
  Describe the big picture of your changes here to communicate to the
  maintainers why we should accept this pull request. If it fixes a bug
  or resolves a feature request, be sure to link to that issue in the
  additional information section.
-->
Currently, the button entity as `unique_id` uses the device name, which can be changed. This PR migrates the unique IDs for the button entity to the `<device MAC address>_<entity_description.key>` schema.

## Type of change
<!--
  What type of change does your PR introduce to Home Assistant?
  NOTE: Please, check only 1! box!
  If your PR requires multiple boxes to be checked, you'll most likely need to
  split it into multiple PRs. This makes things easier and faster to code review.
-->

- [ ] Dependency upgrade
- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New integration (thank you!)
- [ ] New feature (which adds functionality to an existing integration)
- [ ] Deprecation (breaking change to happen in the future)
- [ ] Breaking change (fix/feature causing existing functionality to break)
- [ ] Code quality improvements to existing code or addition of tests

## Additional information
<!--
  Details are important, and help maintainers processing your PR.
  Please be sure to fill out additional details, if applicable.
-->

- This PR fixes or closes issue: fixes #93049
- This PR is related to issue: 
- Link to documentation pull request: 

## Checklist
<!--
  Put an `x` in the boxes that apply. You can also fill these out after
  creating the PR. If you're unsure about any of them, don't hesitate to ask.
  We're here to help! This is simply a reminder of what we are going to look
  for before merging your code.
-->

- [x] The code change is tested and works locally.
- [x] 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] I have followed the [perfect PR recommendations][perfect-pr]
- [x] The code has been formatted using Black (`black --fast homeassistant tests`)
- [x] 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`.

<!--
  This project is very active and we have a high turnover of pull requests.

  Unfortunately, the number of incoming pull requests is higher than what our
  reviewers can review and merge so there is a long backlog of pull requests
  waiting for review. You can help here!
  
  By reviewing another pull request, you will help raise the code quality of
  that pull request and the final review will be faster. This way the general
  pace of pull request reviews will go up and your wait time will go down.
  
  When picking a pull request to review, try to choose one that hasn't yet
  been reviewed.

  Thanks for helping out!
-->

To help with the load of incoming pull requests:

- [ ] I have reviewed two other [open pull requests][prs] in this repository.

[prs]: https://github.com/home-assistant/core/pulls?q=is%3Aopen+is%3Apr+-author%3A%40me+-draft%3Atrue+-label%3Awaiting-for-upstream+sort%3Acreated-desc+review%3Anone+-status%3Afailure

<!--
  Thank you for contributing <3

  Below, some useful links you could explore:
-->
[dev-checklist]: https://developers.home-assistant.io/docs/development_checklist/
[manifest-docs]: https://developers.home-assistant.io/docs/creating_integration_manifest/
[quality-scale]: https://developers.home-assistant.io/docs/integration_quality_scale_index/
[docs-repository]: https://github.com/home-assistant/home-assistant.io
[perfect-pr]: https://developers.home-assistant.io/docs/review-process/#creating-the-perfect-pr


Problem statement:
Shelly EM - reboot button unavailable 
### The problem

Shelly Em reboot button .. that was always available.. now it's unavailable (maybe after it's recent Fw update?)
Anyone else has the same?

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

Home Assistant 2023.2.0 Supervisor 2023.01.1 Operating System 9.5 Frontend 20230201.0 - latest

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

_No response_

### What type of installation are you running?

Home Assistant OS

### Integration causing the issue

Shelly

### 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?

_No response_

### Additional information

_No response_

Hints from issue discussion:
Hey there @balloob, @bieniu, @thecode, @chemelli74, @bdraco, mind taking a look at this issue as it has been labeled with an integration (`shelly`) you are listed as a [code owner](https://github.com/home-assistant/core/blob/dev/CODEOWNERS#L1082) for? Thanks!

<details>
<summary>Code owner commands</summary>


Code owners of `shelly` can trigger bot actions by commenting:

- `@home-assistant close` Closes the issue.
- `@home-assistant rename Awesome new title` Renames the issue.
- `@home-assistant reopen` Reopen the issue.
- `@home-assistant unassign shelly` Removes the current integration label and assignees on the issue, add the integration domain after the command.

</details>


<sub><sup>(message by CodeOwnersMention)</sup></sub>

---

[shelly documentation](https://www.home-assistant.io/integrations/shelly)
[shelly source](https://github.com/home-assistant/core/tree/dev/homeassistant/components/shelly)
<sub><sup>(message by IssueLinks)</sup></sub>
Please download the [diagnostics ](https://www.home-assistant.io/integrations/diagnostics/) from the device card in Home Assistant and attach it in a comment.
Hello, 

I had the same issue with the newest firmware update from shelly (Plug S, 1PM).
With update to 2023.5.3 the shellys work normal and the switches shows the correct status and work.@spanzetta: you should test it with the actual version of HA
> Hello,
> 
> I had the same issue with the newest firmware update from shelly (Plug S, 1PM). With update to 2023.5.3 the shellys work normal and the switches shows the correct status and work.@spanzetta: you should test it with the actual version of HA

I already have 2023.5.3 and the reboot button (only that) is NOT available 
![Screenshot_20230516_111932_Chrome](https://github.com/home-assistant/core/assets/49818743/147826c2-f74d-4070-aad4-ba71bf1bd724)
![Screenshot_20230516_111943_Chrome](https://github.com/home-assistant/core/assets/49818743/e309a4e1-7ddb-43f8-a4cb-9ae9979f806a)

Anyone else has the same issue?
from few days also the "Shelly Channel 2 Power factory" is unavailable..
What's wrong?
Loosing those sensors without any apparent reason...
Any clue?
now "Shelly Channel 2 Power factory" came back..
But Reboot button is still unavailable..
Anyone can explain why it happens?
> Anyone can explain why it happens?

Without information from your system... nobody.

Add this to your `configuration.yaml` file:
```yaml
logger:
  default: error
  logs:
    aioshelly: debug
    homeassistant.components.shelly: debug
```
restart HA, if the restart entity will be unavailable wait an hour and attach here log file. Additionally, please attach diagnostics file.
> > Anyone can explain why it happens?
> 
> Without information from your system... nobody.
> 
> Add this to your `configuration.yaml` file:
> 
> ```yaml
> logger:
>   default: error
>   logs:
>     aioshelly: debug
>     homeassistant.components.shelly: debug
> ```
> 
> restart HA, if the restart entity will be unavailable wait an hour and attach here log file. Additionally, please attach diagnostics file.

Thanks for the suggestions.. I will do it as soon as possible.

One question: when you say "attach here log file"  "and diagnostic file" to which files are you exactly referring to?
Can you pls specify both file names and positions?
Thanks again
> log file

https://www.home-assistant.io/docs/configuration/troubleshooting/#disable-debug-logging-and-download-logs


> diagnostic file

https://www.home-assistant.io/docs/configuration/troubleshooting/#download-diagnostics

Commit message: Fix tests

Patch to review:
diff --git a/homeassistant/components/shelly/button.py b/homeassistant/components/shelly/button.py
index 15a055736560ee..7dfeae8f4c316f 100644
--- a/homeassistant/components/shelly/button.py
+++ b/homeassistant/components/shelly/button.py
@@ -3,7 +3,7 @@
 
 from collections.abc import Callable, Coroutine
 from dataclasses import dataclass
-from typing import Any, Final, Generic, TypeVar
+from typing import TYPE_CHECKING, Any, Final, Generic, TypeVar
 
 from homeassistant.components.button import (
     ButtonDeviceClass,
@@ -12,14 +12,15 @@
 )
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import EntityCategory
-from homeassistant.core import HomeAssistant
+from homeassistant.core import HomeAssistant, callback
+from homeassistant.helpers import entity_registry as er
 from homeassistant.helpers.device_registry import CONNECTION_NETWORK_MAC
 from homeassistant.helpers.entity import DeviceInfo
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
 from homeassistant.helpers.update_coordinator import CoordinatorEntity
 from homeassistant.util import slugify
 
-from .const import SHELLY_GAS_MODELS
+from .const import LOGGER, SHELLY_GAS_MODELS
 from .coordinator import ShellyBlockCoordinator, ShellyRpcCoordinator, get_entry_data
 from .utils import get_device_entry_gen
 
@@ -79,12 +80,50 @@ class ShellyButtonDescription(
 ]
 
 
+@callback
+def async_migrate_unique_ids(
+    entity_entry: er.RegistryEntry,
+    coordinator: ShellyRpcCoordinator | ShellyBlockCoordinator,
+) -> dict[str, Any] | None:
+    """Migrate button unique IDs."""
+    if not entity_entry.entity_id.startswith("button"):
+        return None
+
+    for key in ("reboot", "self_test", "mute", "unmute"):
+        if entity_entry.unique_id.startswith(slugify(coordinator.device.name)):
+            old_unique_id = entity_entry.unique_id
+            new_unique_id = f"{coordinator.mac}_{key}"
+            LOGGER.debug(
+                "Migrating unique_id for %s entity from [%s] to [%s]",
+                entity_entry.entity_id,
+                old_unique_id,
+                new_unique_id,
+            )
+            return {
+                "new_unique_id": entity_entry.unique_id.replace(
+                    old_unique_id, new_unique_id
+                )
+            }
+
+    return None
+
+
 async def async_setup_entry(
     hass: HomeAssistant,
     config_entry: ConfigEntry,
     async_add_entities: AddEntitiesCallback,
 ) -> None:
     """Set buttons for device."""
+
+    @callback
+    def _async_migrate_unique_ids(
+        entity_entry: er.RegistryEntry,
+    ) -> dict[str, Any] | None:
+        """Migrate button unique IDs."""
+        if TYPE_CHECKING:
+            assert coordinator is not None
+        return async_migrate_unique_ids(entity_entry, coordinator)
+
     coordinator: ShellyRpcCoordinator | ShellyBlockCoordinator | None = None
     if get_device_entry_gen(config_entry) == 2:
         coordinator = get_entry_data(hass)[config_entry.entry_id].rpc
@@ -92,6 +131,10 @@ async def async_setup_entry(
         coordinator = get_entry_data(hass)[config_entry.entry_id].block
 
     if coordinator is not None:
+        await er.async_migrate_entries(
+            hass, config_entry.entry_id, _async_migrate_unique_ids
+        )
+
         entities: list[ShellyButton] = []
 
         for button in BUTTONS:
@@ -123,7 +166,7 @@ def __init__(
         self.entity_description = description
 
         self._attr_name = f"{coordinator.device.name} {description.name}"
-        self._attr_unique_id = slugify(self._attr_name)
+        self._attr_unique_id = f"{coordinator.mac}_{description.key}"
         self._attr_device_info = DeviceInfo(
             connections={(CONNECTION_NETWORK_MAC, coordinator.mac)}
         )
diff --git a/tests/components/shelly/test_button.py b/tests/components/shelly/test_button.py
index fdb2cd60450fc5..d3dccc3ef6abac 100644
--- a/tests/components/shelly/test_button.py
+++ b/tests/components/shelly/test_button.py
@@ -1,9 +1,13 @@
 """Tests for Shelly button platform."""
 from __future__ import annotations
 
+import pytest
+
 from homeassistant.components.button import DOMAIN as BUTTON_DOMAIN, SERVICE_PRESS
+from homeassistant.components.shelly.const import DOMAIN
 from homeassistant.const import ATTR_ENTITY_ID, STATE_UNKNOWN
 from homeassistant.core import HomeAssistant
+from homeassistant.helpers import entity_registry as er
 
 from . import init_integration
 
@@ -38,3 +42,41 @@ async def test_rpc_button(hass: HomeAssistant, mock_rpc_device) -> None:
         blocking=True,
     )
     assert mock_rpc_device.trigger_reboot.call_count == 1
+
+
+@pytest.mark.parametrize(
+    ("gen", "old_unique_id", "new_unique_id"),
+    [
+        (2, "test_name_reboot", "123456789ABC_reboot"),
+        (1, "test_name_reboot", "123456789ABC_reboot"),
+        (2, "123456789ABC_reboot", "123456789ABC_reboot"),
+    ],
+)
+async def test_migrate_unique_id(
+    hass: HomeAssistant,
+    mock_block_device,
+    mock_rpc_device,
+    gen,
+    old_unique_id,
+    new_unique_id,
+) -> None:
+    """Test migration of unique_id."""
+    entry = await init_integration(hass, gen, skip_setup=True)
+
+    entity_registry = er.async_get(hass)
+    entity: er.RegistryEntry = entity_registry.async_get_or_create(
+        suggested_object_id="test_name_reboot",
+        disabled_by=None,
+        domain=BUTTON_DOMAIN,
+        platform=DOMAIN,
+        unique_id=old_unique_id,
+        config_entry=entry,
+    )
+    assert entity.unique_id == old_unique_id
+
+    await hass.config_entries.async_setup(entry.entry_id)
+    await hass.async_block_till_done()
+
+    entity_entry = entity_registry.async_get("button.test_name_reboot")
+    assert entity_entry
+    assert entity_entry.unique_id == new_unique_id


Code review comments:
diff_hunk: @@ -79,19 +80,61 @@ class ShellyButtonDescription(
 ]
 
 
+@callback
+def async_migrate_unique_ids(
+    entity_entry: er.RegistryEntry,
+    coordinator: ShellyRpcCoordinator | ShellyBlockCoordinator,
+) -> dict[str, Any] | None:
+    """Migrate button unique IDs."""
+    if not entity_entry.entity_id.startswith("button"):
+        return None
+
+    for key in ("reboot", "self_test", "mute", "unmute"):
+        if entity_entry.unique_id.startswith(slugify(coordinator.device.name)):
path: homeassistant/components/shelly/button.py
text: @user1:
It doesn't matter much here, but slugify is expensive it won't change within the loop so its best to do outside the loop

@author:
Good point, thanks