Add Fritz services #50056

chemelli74 · 2021-05-04T07:06:36Z

Proposed change

Add reconnect and reboot services for Fritz devices.

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:

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.
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-05-04T07:06:43Z

Hey there @mammuth, @AaronDavidSchneider, mind taking a look at this pull request as its been labeled with an integration (fritz) you are listed as a codeowner for? Thanks!
_{^{(message by CodeOwnersMention)}}

AaronDavidSchneider

Works well! I think I would change the way how entities and devices are linked to fritzbox_tools.

Also: We need to handle those devices that have no way of reconnecting.

This is what I get if I reconnect a router that is connected as a repeater:

2021-05-04 09:19:34 ERROR (MainThread) [homeassistant] Error doing job: Future exception was never retrieved
Traceback (most recent call last):
  File "/usr/local/Cellar/python@3.9/3.9.4/Frameworks/Python.framework/Versions/3.9/lib/python3.9/concurrent/futures/thread.py", line 52, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/Users/schneider/codes/personal/hass/venv/lib/python3.9/site-packages/fritzconnection/core/fritzconnection.py", line 233, in reconnect
    self.call_action("WANIPConn1", "ForceTermination")
  File "/Users/schneider/codes/personal/hass/venv/lib/python3.9/site-packages/fritzconnection/core/fritzconnection.py", line 227, in call_action
    return self.soaper.execute(service, action_name, arguments)
  File "/Users/schneider/codes/personal/hass/venv/lib/python3.9/site-packages/fritzconnection/core/soaper.py", line 238, in execute
    return handle_response(response)
  File "/Users/schneider/codes/personal/hass/venv/lib/python3.9/site-packages/fritzconnection/core/soaper.py", line 222, in handle_response
    raise_fritzconnection_error(response)
  File "/Users/schneider/codes/personal/hass/venv/lib/python3.9/site-packages/fritzconnection/core/soaper.py", line 147, in raise_fritzconnection_error
    raise exception(message)
fritzconnection.core.exceptions.FritzConnectionException: UPnPError:
errorCode: 403
errorDescription: Not available Action

homeassistant/components/fritz/services.py

chemelli74 · 2021-05-04T07:47:00Z

fritzconnection.core.exceptions.FritzConnectionException: UPnPError:
errorCode: 403
errorDescription: Not available Action

I would expect a different error in such cases:
https://fritzconnection.readthedocs.io/en/1.0.1/sources/api.html#fritzconnection.core.exceptions.FritzActionError
or
https://fritzconnection.readthedocs.io/en/1.0.1/sources/api.html#fritzconnection.core.exceptions.FritzServiceError

chemelli74 · 2021-05-04T08:10:18Z

fritzconnection.core.exceptions.FritzConnectionException: UPnPError:
errorCode: 403
errorDescription: Not available Action

I would expect a different error in such cases:
https://fritzconnection.readthedocs.io/en/1.0.1/sources/api.html#fritzconnection.core.exceptions.FritzActionError
or
https://fritzconnection.readthedocs.io/en/1.0.1/sources/api.html#fritzconnection.core.exceptions.FritzServiceError

Updated documentation: https://fritzconnection.readthedocs.io/en/1.2.1/sources/introduction.html#module-usage

chemelli74 · 2021-05-04T22:33:55Z

File "/Users/schneider/codes/personal/hass/venv/lib/python3.9/site-packages/fritzconnection/core/soaper.py", line 147, in raise_fritzconnection_error
raise exception(message)
fritzconnection.core.exceptions.FritzConnectionException: UPnPError:
errorCode: 403
errorDescription: Not available Action

Should be fixed now, please retest.

Simone

bdraco · 2021-05-05T02:37:29Z

homeassistant/components/fritz/services.yaml

+  target:
+    entity:
+      integration: fritz
+      domain: binary_sensor


If the goal is to target the binary sensor entities, register these as entity services

https://developers.home-assistant.io/docs/dev_101_services/#entity-services

The idea was to target only a device, but didn't find a way to filter out all entities from GUI.
So I choice to filter by binary sensor and allow both to work ( device and entity).

AaronDavidSchneider

works as expected

homeassistant/components/fritz/services.py

bdraco · 2021-05-09T15:21:28Z

homeassistant/components/fritz/services.py

+                f"Failed to call service '{service_call.service}'. Config entry for target not found"
+            )
+
+        for entry in fritzbox_entry_ids:


Should we use asyncio.gather here?

Actually no since it's generating executor jobs.

We would save some context switches if we would wrap the iteration inside a function and make the service helper sync.

mib1185 · 2021-05-11T21:59:35Z

Since this merge mypy check fail -> https://github.com/home-assistant/core/runs/2559970649

Error: homeassistant/components/fritz/services.py:49: error: Item "None" of "Optional[ConfigEntry]" has no attribute "domain"  [union-attr]

KapJI · 2021-05-11T22:13:41Z

That's probably because of conflict between this and #50327

@chemelli74 @bdraco can you please fix it?

I wish github had merge-time checks against current master 😕

…onflict

frenck · 2021-05-11T22:45:59Z

This PR adds services, but is missing a documentation PR to document these. This PR should not have been merged without those.

@chemelli74 Please open up a documentation PR. Thanks 👍

bdraco · 2021-05-11T23:53:13Z

mypy fix: #50497

dhcp conflict fix (not related but other reason the CI is failing) #50498

MartinHjelmare · 2021-05-12T09:59:34Z

homeassistant/components/fritz/services.py

+
+async def async_setup_services(hass: HomeAssistant):
+    """Set up services for Fritz integration."""
+    if hass.data.get(FRITZ_SERVICES, False):


We have hass.services.has_service that could be used instead.

MartinHjelmare · 2021-05-12T10:00:54Z

homeassistant/components/fritz/services.py

+
+        for entry in fritzbox_entry_ids:
+            _LOGGER.debug("Executing service %s", service_call.service)
+            fritz_tools = hass.data[DOMAIN].get(entry)


We don't check for None below. If that's not needed we should use dict[key] instead of dict.get(key).

probot-home-assistant bot added integration: fritz new-feature labels May 4, 2021

homeassistant added the cla-signed label May 4, 2021

probot-home-assistant bot added the by-code-owner label May 4, 2021

AaronDavidSchneider suggested changes May 4, 2021

View reviewed changes

homeassistant/components/fritz/services.py Outdated Show resolved Hide resolved

bdraco reviewed May 5, 2021

View reviewed changes

AaronDavidSchneider approved these changes May 5, 2021

View reviewed changes

bdraco reviewed May 7, 2021

View reviewed changes

homeassistant/components/fritz/services.py Outdated Show resolved Hide resolved

chemelli74 added 4 commits May 7, 2021 21:43

Add Fritz services

5f3f66f

pylint

5fdc7f0

Fix missing service handling

5fe1ba9

Add one more exception handling

a4e59cb

chemelli74 force-pushed the chemelli74-fritz-services branch from 2305cdc to a4e59cb Compare May 7, 2021 21:43

Switch to helper function

2e4470a

bdraco approved these changes May 9, 2021

View reviewed changes

bdraco merged commit d877c0c into home-assistant:dev May 11, 2021

chemelli74 deleted the chemelli74-fritz-services branch May 11, 2021 21:01

mib1185 mentioned this pull request May 11, 2021

Add strict typing to fritzbox #50486

Merged

21 tasks

chemelli74 added a commit to chemelli74/core that referenced this pull request May 11, 2021

Fix mypy for Fritz after home-assistant#50056, home-assistant#50327 c…

9e35e2b

…onflict

chemelli74 mentioned this pull request May 11, 2021

Fix mypy for Fritz after #50056, #50327 conflict #50497

Merged

21 tasks

bdraco pushed a commit that referenced this pull request May 11, 2021

Fix mypy for Fritz after #50056, #50327 conflict (#50497)

8d73184

chemelli74 mentioned this pull request May 12, 2021

Add services and platforms to Fritz home-assistant/home-assistant.io#17802

Merged

8 tasks

MartinHjelmare reviewed May 12, 2021

View reviewed changes

chemelli74 added a commit to chemelli74/core that referenced this pull request May 12, 2021

Quality improvements from home-assistant#50056

5c331f1

github-actions bot locked and limited conversation to collaborators May 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Fritz services #50056

Add Fritz services #50056

chemelli74 commented May 4, 2021

probot-home-assistant bot commented May 4, 2021

AaronDavidSchneider left a comment

chemelli74 commented May 4, 2021

chemelli74 commented May 4, 2021

chemelli74 commented May 4, 2021

bdraco May 5, 2021 •

edited

Loading

chemelli74 May 5, 2021

AaronDavidSchneider left a comment

bdraco May 9, 2021

bdraco May 9, 2021

MartinHjelmare May 12, 2021

mib1185 commented May 11, 2021 •

edited

Loading

KapJI commented May 11, 2021 •

edited

Loading

frenck commented May 11, 2021

bdraco commented May 11, 2021

MartinHjelmare May 12, 2021

MartinHjelmare May 12, 2021

Add Fritz services #50056

Add Fritz services #50056

Conversation

chemelli74 commented May 4, 2021

Proposed change

Type of change

Additional information

Checklist

probot-home-assistant bot commented May 4, 2021

AaronDavidSchneider left a comment

Choose a reason for hiding this comment

chemelli74 commented May 4, 2021

chemelli74 commented May 4, 2021

chemelli74 commented May 4, 2021

bdraco May 5, 2021 • edited Loading

Choose a reason for hiding this comment

chemelli74 May 5, 2021

Choose a reason for hiding this comment

AaronDavidSchneider left a comment

Choose a reason for hiding this comment

bdraco May 9, 2021

Choose a reason for hiding this comment

bdraco May 9, 2021

Choose a reason for hiding this comment

MartinHjelmare May 12, 2021

Choose a reason for hiding this comment

mib1185 commented May 11, 2021 • edited Loading

KapJI commented May 11, 2021 • edited Loading

frenck commented May 11, 2021

bdraco commented May 11, 2021

MartinHjelmare May 12, 2021

Choose a reason for hiding this comment

MartinHjelmare May 12, 2021

Choose a reason for hiding this comment

bdraco May 5, 2021 •

edited

Loading

mib1185 commented May 11, 2021 •

edited

Loading

KapJI commented May 11, 2021 •

edited

Loading