Replace Synology DSM services with buttons #57352

mib1185 · 2021-10-08T21:07:04Z

Breaking change

The services reboot and shutdown are deprecated and will be removed in future release. Please use the new button entities.

Proposed change

services are deprecated
buttons are added as successor

Type of change

Dependency upgrade
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

This PR fixes or closes issue: fixes #
This PR is related to issue:
Link to documentation pull request: Replace Synology DSM services with buttons home-assistant.io#19677

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
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

If the code communicates with devices, web services, or third-party tools:

The manifest file 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.

The integration reached or maintains the following Integration Quality Scale:

No score or internal
🥈 Silver
🥇 Gold
🏆 Platinum

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

probot-home-assistant · 2021-10-08T21:07:15Z

Hey there @hacf-fr, @Quentame, mind taking a look at this pull request as it has been labeled with an integration (synology_dsm) you are listed as a code owner for? Thanks!
_{^{(message by CodeOwnersMention)}}

homeassistant/components/synology_dsm/service.py

homeassistant/components/synology_dsm/services.yaml

homeassistant/components/synology_dsm/common.py

homeassistant/components/synology_dsm/service.py

frenck · 2022-01-03T14:38:53Z

@mib1185 Could you resolve the CI issue in this PR?

mib1185 · 2022-01-23T13:08:36Z

rebased to solve conflicts

mib1185 · 2022-01-23T22:24:07Z

rebased to get #64792 in

bdraco · 2022-01-23T22:25:37Z

Should we still accept the serial for a few versions but warn so they have time to change any automations?

mib1185 · 2022-01-23T23:25:37Z

Should we still accept the serial for a few versions but warn so they have time to change any automations?

With this, the new paramater device_id cannot be required: true, which will need a second breaking change in the future, when completely removing serial parameter and set device_id as required: true 🤔

bdraco · 2022-01-24T02:31:22Z

Should we still accept the serial for a few versions but warn so they have time to change any automations?

With this, the new paramater device_id cannot be required: true, which will need a second breaking change in the future, when completely removing serial parameter and set device_id as required: true 🤔

Its likely not worth putting users through two breaking changes.

bdraco · 2022-01-24T02:32:22Z

Some of these services could actually be buttons

mib1185 · 2022-01-24T19:34:28Z

Some of these services could actually be buttons

What do you think about to keep the services as is - just deprecate them - and add buttons for shutdown and reboot?
With this the same goals are achieved:

the execution of reboot and shutdown is targeted by the device, but not the serial
unnecessary services will slightly be deprecated and removed
user get his time to migrate from service to button

btw. I will remove the poweron service, since it needs a bit more thinking to get it work, since when device is off, the entry is not loaded - so the service is not loaded, too.

bdraco · 2022-01-24T20:48:49Z

What do you think about to keep the services as is - just deprecate them - and add buttons for shutdown and reboot?

Sounds good to me.

homeassistant/components/synology_dsm/button.py

bdraco · 2022-01-24T21:20:27Z

homeassistant/components/synology_dsm/button.py

+        self.entity_description = description
+        self.syno_api = api
+
+        self._attr_name = f"{api.network.hostname} {description.name}"
+        self._attr_unique_id = f"{api.information.serial}_{description.key}"
+        self._attr_device_info = DeviceInfo(
+            identifiers={(DOMAIN, api.information.serial)}
+        )


It would be good to make a base class that sets the device info that is a subset of SynologyDSMBaseEntity so we don't have the code duplicated here.

I thought about inheriting from SynologyDSMBaseEntity but there is to much in it, which is not needed by a simple button entity. So I decided to add this minimum of device_info to get the buttons connected to the device created by the integration.

It would be cleaner to split up SynologyDSMBaseEntity to avoid the code duplication.

This PR is already large enough though so it can wait for the future.

I will do this in a follow-up PR soon 👍

bdraco

Tested and working as expected

mib1185 requested a review from Quentame as a code owner October 8, 2021 21:07

homeassistant added the cla-signed label Oct 8, 2021

probot-home-assistant bot added Hacktoberfest breaking-change code-quality integration: synology_dsm new-feature by-code-owner labels Oct 8, 2021

mib1185 mentioned this pull request Oct 8, 2021

Replace Synology DSM services with buttons home-assistant/home-assistant.io#19677

Merged

8 tasks

balloob reviewed Oct 8, 2021

View reviewed changes

homeassistant/components/synology_dsm/service.py Outdated Show resolved Hide resolved

balloob reviewed Oct 8, 2021

View reviewed changes

homeassistant/components/synology_dsm/services.yaml Outdated Show resolved Hide resolved

mib1185 requested a review from balloob October 9, 2021 20:38

MartinHjelmare added this to By Code Owner in Dev Oct 15, 2021

mib1185 force-pushed the synology_dsm/use-target-and-add-poweron-service branch from a9e6510 to 6a7a574 Compare October 15, 2021 19:22

mib1185 changed the title ~~Switch to target selector for services and add new service for power on of Synology DSM~~ Switch to device selector for services and add new service for power on of Synology DSM Oct 16, 2021

balloob reviewed Oct 16, 2021

View reviewed changes

homeassistant/components/synology_dsm/common.py Outdated Show resolved Hide resolved

balloob reviewed Oct 16, 2021

View reviewed changes

homeassistant/components/synology_dsm/service.py Outdated Show resolved Hide resolved

balloob reviewed Oct 16, 2021

View reviewed changes

homeassistant/components/synology_dsm/service.py Outdated Show resolved Hide resolved

balloob reviewed Oct 16, 2021

View reviewed changes

homeassistant/components/synology_dsm/service.py Show resolved Hide resolved

mib1185 requested a review from a team as a code owner October 18, 2021 20:53

mib1185 requested a review from balloob October 23, 2021 14:34

emontnemery added the smash Indicator this PR is close to finish for merging or closing label Oct 25, 2021

MartinHjelmare added the hacktoberfest-accepted label Oct 31, 2021

mib1185 force-pushed the synology_dsm/use-target-and-add-poweron-service branch from aff7000 to 8acfb5f Compare November 5, 2021 22:56

frenck removed the smash Indicator this PR is close to finish for merging or closing label Jan 3, 2022

mib1185 force-pushed the synology_dsm/use-target-and-add-poweron-service branch from df6b0c2 to 4baa81a Compare January 23, 2022 13:08

mib1185 force-pushed the synology_dsm/use-target-and-add-poweron-service branch from 4baa81a to e111ed8 Compare January 23, 2022 13:24

mib1185 added 3 commits January 23, 2022 22:21

remove not needed check

10bb684

remove unused import

6d5163a

fix isort

011383a

mib1185 force-pushed the synology_dsm/use-target-and-add-poweron-service branch from e111ed8 to 011383a Compare January 23, 2022 22:23

wrap api calls

5a5a8d8

bdraco self-requested a review January 24, 2022 02:31

mib1185 marked this pull request as draft January 24, 2022 11:18

mib1185 added 2 commits January 24, 2022 19:59

revert config_validation changes

81b5852

revert services to be targeted by serial number

1162476

add buttons for reboot and shutdown

0310360

mib1185 changed the title ~~Switch to device selector for services and add new service for power on of Synology DSM~~ Replace Synology DSM services with buttons Jan 24, 2022

mib1185 marked this pull request as ready for review January 24, 2022 21:13

mib1185 added the new-platform label Jan 24, 2022

bdraco reviewed Jan 24, 2022

View reviewed changes

homeassistant/components/synology_dsm/button.py Outdated Show resolved Hide resolved

bdraco reviewed Jan 24, 2022

View reviewed changes

mib1185 added 2 commits January 24, 2022 21:35

add button to .coveragerc

4917ada

use a generator expression as suggested

0221459

bdraco approved these changes Jan 25, 2022

View reviewed changes

Dev automation moved this from By Code Owner to Reviewer approved Jan 25, 2022

bdraco merged commit 5d7d652 into home-assistant:dev Jan 25, 2022

Dev automation moved this from Reviewer approved to Done Jan 25, 2022

mib1185 deleted the synology_dsm/use-target-and-add-poweron-service branch January 25, 2022 08:52

github-actions bot locked and limited conversation to collaborators Jan 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace Synology DSM services with buttons #57352

Replace Synology DSM services with buttons #57352

mib1185 commented Oct 8, 2021 •

edited

probot-home-assistant bot commented Oct 8, 2021

frenck commented Jan 3, 2022

mib1185 commented Jan 23, 2022

mib1185 commented Jan 23, 2022

bdraco commented Jan 23, 2022

mib1185 commented Jan 23, 2022

bdraco commented Jan 24, 2022

bdraco commented Jan 24, 2022

mib1185 commented Jan 24, 2022 •

edited

bdraco commented Jan 24, 2022

bdraco Jan 24, 2022

mib1185 Jan 24, 2022 •

edited

bdraco Jan 25, 2022

mib1185 Jan 25, 2022

bdraco left a comment

Replace Synology DSM services with buttons #57352

Replace Synology DSM services with buttons #57352

Conversation

mib1185 commented Oct 8, 2021 • edited

Breaking change

Proposed change

Type of change

Additional information

Checklist

probot-home-assistant bot commented Oct 8, 2021

frenck commented Jan 3, 2022

mib1185 commented Jan 23, 2022

mib1185 commented Jan 23, 2022

bdraco commented Jan 23, 2022

mib1185 commented Jan 23, 2022

bdraco commented Jan 24, 2022

bdraco commented Jan 24, 2022

mib1185 commented Jan 24, 2022 • edited

bdraco commented Jan 24, 2022

bdraco Jan 24, 2022

Choose a reason for hiding this comment

mib1185 Jan 24, 2022 • edited

Choose a reason for hiding this comment

bdraco Jan 25, 2022

Choose a reason for hiding this comment

mib1185 Jan 25, 2022

Choose a reason for hiding this comment

bdraco left a comment

Choose a reason for hiding this comment

mib1185 commented Oct 8, 2021 •

edited

mib1185 commented Jan 24, 2022 •

edited

mib1185 Jan 24, 2022 •

edited