Fix wake_on_lan ping with None as host

[flamechair]

With the new update, wake_on_lan requires a host key in the configuration

Description:

Related issue (if applicable): fixes #

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#2235

Example entry for configuration.yaml (if applicable):

Checklist:

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated in home-assistant.github.io

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

• Local tests with tox run successfully. Your PR cannot be merged unless tests pass
• New dependencies have been added to the REQUIREMENTS variable (example).
• New dependencies are only imported inside functions that use them (example).
• New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
• New files were added to .coveragerc.

If the code does not interact with devices:

• Local tests with tox run successfully. Your PR cannot be merged unless tests pass
• Tests have been added to verify that the new code works.

[mention-bot]

@iamtpage, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @fabaff and @Chris-V to be potential reviewers.

[homeassistant]

Hi @iamtpage,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

[MartinHjelmare]

I'm marking this as a breaking change, even though someone else made it break. We need to remember to update the docs, and mention this when released.

[MartinHjelmare]

Actually, I think we should instead convert self._host to a string on lines 89 and 92.

str(self._host)

[MartinHjelmare]

Now the question is if we should retain the old behavior where host is optional. I'm thinking we don't need to have it required now since the code will work passing a stringified None to the subprocess call.

[flamechair]

Yeah that would make sense, making it required would be considered a breaking change, and casting it to a string wouldn't break anything if it wasn't there.

[MartinHjelmare]

Thanks! Updated the PR title to reflect latest changes.

[balloob]

Cherry-picked for 0.40.1

[brianjking]

I'm confused @balloob. I don't see any updates here or in the referenced PR here.

[flamechair]

@brianjking having a host is still optional after this merge, we just changed how it is handled on the backend. So updating documentation is not necessary, and I closed the documentation PR.

[siebert]

Why is the external ping command executed at all if host is None?

[flamechair]

Because WOL only needs a Mac address to work, the host entry is to be able to verify if the host is woken up (by pinging it, and with ping you need a host IP to ping)

[siebert]

Yes, WoL needs only a Mac to wake the host, but as you said, for pinging the host you also need the IP. My point is, if host is None (no IP), there is no need to ping at all, so the code should IMHO be changed to skip the execution of the ping command in this case.