Incorporate various improvements for the ws66i integration

[ssaenger]

Proposed change

This PR contains 3 commits:

1. General improvements to code as requested by MartinHjelmare in PR #56094
2. Remove custom services that can be replicated with scenes, as requested by frenck in PR #56094
3. Addresses an undesired behavior with setting the volume of a zone while that zone is muted. The WS66i amp ignores volume changes for a zone if that zone is muted. This means that turning on a scene while a zone is muted will cause the volume state to not be set. The fix is to follow the same behavior as the WS66i keypad controllers, which is to unmute before changing the volume.

If #3 doesn't belong to this PR, I can create a new PR just for it.

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 is related to issue: Add ws66i core integration #56094
• Link to documentation pull request: Add documentation for ws66i core integration home-assistant.io#19294

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.

[ssaenger]

I apologize for the delay. I was in Berlin last week and I am on vacation all this week. I will look at these on the weekend.

…

On Thu, May 19, 2022, 12:39 AM Martin Hjelmare ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In homeassistant/components/ws66i/config_flow.py <#71717 (comment)>: > - # Validation done. No issues. Close the connection - ws66i.close() + return bool(ret_val) + + is_valid: bool = await hass.async_add_executor_job(_verify_connection, ws66i) + if not is_valid: + raise ConnectionError("Not a valid WS66i connection") We prefer to raise a custom exception that inherits HomeAssistantError instead of using a built in exception. I'd suggest to use the exception CannotConnect for this purpose. If the device can raise ConnectionError we can catch that here and raise CannotConnect. ------------------------------ In tests/components/ws66i/test_init.py <#71717 (comment)>: > +CONFIG = {CONF_IP_ADDRESS: "1.1.1.1"} + +ZONE_1_ID = "media_player.zone_11" + + +async def test_cannot_connect(hass): + """Test connection error.""" + with patch( + "homeassistant.components.ws66i.get_ws66i", + new=lambda *a: MockWs66i(fail_open=True), + ): + config_entry = MockConfigEntry(domain=DOMAIN, data=MOCK_CONFIG) + config_entry.add_to_hass(hass) + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + assert hass.states.get(ZONE_1_ID) is None We should also assert the state of the config entry. https://github.com/home-assistant/core/blob/1d6e40451274cbe8ccc6be256146193bcfe97df5/homeassistant/config_entries.py#L78-L92 ------------------------------ In tests/components/ws66i/test_media_player.py <#71717 (comment)>: > @@ -635,58 +512,3 @@ async def test_unload_config_entry(hass): assert method_call.called assert not hass.data[DOMAIN] The test_unload_config_entry test should move to the test_init.py module. — Reply to this email directly, view it on GitHub <#71717 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGZYALH52DZ7FSYA53A2ZJTVKXV3PANCNFSM5VWZQYUA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[MartinHjelmare]

Thanks!

[MartinHjelmare]

Adding this to the milestone to avoid releasing the integration with the custom services which would make it a breaking change later when we remove the services.