Fix nfandroidtv service notify disappears when restarting home assistant

[sVnsation]

sVnsation:sVnsation-nfandroidtv_issue_108296

Breaking change

No

Proposed change

Fixes Issue #108296

These changes should ensure that the notify action ‘notify.android_tv’ is always available, even if the Android Device (TV) is switched off. The connection is only established when a message is actually to be sent.

1. in __init__.py:

   • We store the host information in hass.data[DOMAIN][entry.entry_id] to be able to use it later in notify.py.

2. in notify.py:

   • The NFAndroidTVNotificationService class has been customised to store the host information and establish the connection when needed.
   • The send_message method now tries to establish a connection if it does not yet exist and catches ConnectError exceptions.

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)
• 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

• This PR fixes or closes issue: fixes service notify.my_android_tv disappears when restarting home assistant #108296

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
• I have followed the perfect PR recommendations
• The code has been formatted using Ruff (ruff format 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.

To help with the load of incoming pull requests:

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

[home-assistant]

Hey there @tkdrob, mind taking a look at this pull request as it has been labeled with an integration (nfandroidtv) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of nfandroidtv can trigger bot actions by commenting:

• @home-assistant close Closes the pull request.
• @home-assistant rename Awesome new title Renames the pull request.
• @home-assistant reopen Reopen the pull request.
• @home-assistant unassign nfandroidtv Removes the current integration label and assignees on the pull request, add the integration domain after the command.
• @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
• @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

[joostlek]

This PR is incorrect.

1. Previously we tried connecting when setting up the integration, and if we could not connect we would raise ConfigEntryNotReady, causing the setup to be tried again a minute later.
2. I am not sure how error handling fixes the notify service dissapearing from Home Assistant. Can you elaborate on that? Because if the integration is unable to setup, it would have a reason to

[home-assistant]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

[sVnsation]

1. I understand the reason of the current implementation of the setup. However, when Home Assistant is restarted, setting up the integration is repeated every time. If the TV is switched off, it fails and the notify service is not created.
   It certainly depends on how you look at it. But, it is not absolutely necessary to establish a connection when setting up the integration. When setting up for the first time, it is of course helpful to receive direct feedback as to whether the connection is working or not. The TV is certainly switched on at this point of time.
   However, if Home Assistant is restarted later in time, the TV is probably switched off and the setup cannot establish a connection to the TV and therefore fails to setup. This leads to a ‘disappearing’ notification service, but in reality it was not created after the restart.

2. If the establishment of the connection is removed from the async_setup_entry of the integration, then the notify service can be created even if the TV is switched off.

[sVnsation]

@joostlek Thank you for reviewing this pull request.

I did some further testing.
When you setup the integration for the first time you still get a error message when the TV is switched off or something else is wrong. Because of the config_flow.py implementation. This was tested with the changes from the pull request.
I think connecting when setting up the integration in async_setup_entry was used in the past while using yaml configuration.

So in my opinion it's perfectly fine to remove ConfigEntryNotReady from __init__.py.
Because ConfigFlow shows an error if something is wrong when first time setting up the integration.

[joostlek]

Right, let me ask around to see what we want with this situation

[erkr]

My two cents:

• TV needs to be on for first time discovery
• once added, the notify service should always be created at startup (tv on or off)
• add an extra state entity that reflects the TV being detected On or Off. Use case: don't call the notify when the TV is off to avoid errors in my log

[sVnsation]

Any news on this? I would also be happy to write tests if the changes are accepted.

[joostlek]

Okay let me dive into this again.

• We do want to check if we are able to set up. For example when the device got a new IP via DHCP, we don't want to pretend like nothing happened and just give the idea like it would work. Technical people might figure out why, but my parents would not.
• I do understand that a TV is a different kind of device, you don't change them every week and they are still there, even though they can be turned off and not respond to requests. I recently had the discussion about LG tv's if they should go unavailable if they are not present. And I think this falls into the same pit. One thing that helps LG case, is that they have autodiscovery and are able to update the device's IP address.

So in theory, if the config entry setup is retried, it will retry indefinetly until it works. And when it works, it should just add the service. I am fairly certain that that works.

I do understand that that leaves you without service from restart until the tv is turned on. We also don't like flapping services. Hence we have introduced the notify entity, which would be called via a generic action, rather than have 1 action for every notify target.

So this is how I see the situation, please let me know if this is correct or if I am missing something, after that I will take this up with some other people to hopefully unblock this PR.

[sVnsation]

Everything is correct.
The crucial part is indeed that it leaves you without the service from restart until the TV is turned on.
I also agree with you about the part with the changing IP address.

Do I understand that correctly: The generic notification service is still under development and has not been implemented yet?

So the service call will change like this?

Old
action:
  - service: notify.tv
    data:
      message: "Test"
  - service: notify.phone
    data:
      message: "Test"

New
action:
  - service: notify.notify
    data:
      target: ["tv", "phone"]  
      message: "Test"

[joostlek]

It has been implemented, but very few have it implemented yet, mostly because it's not usable by most integrations. In the coming months that will be improved