Allow plugwise port number as IP address in config flow

[jeroen84]

Proposed change

Given that my Plugwise Smile is running on a different port than 80 due to my network setup, I am unable to add the integration to Home Assistant using the config flow. Currently there is no option to change this. Given that the Plugwise Smile API is able to handle different port numbers, I propose to add the possibility provide the port number via the "Smile IP address" config flow when manually adding the integration. The allowed format will be [HOST_IP]:[PORT], so for instance 1.1.1.1:7070. It remains possible to only provide the IP address, i.e. absence of a port number. Then, the default port of 80 will be assumed.

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

Example entry for configuration.yaml:

# Example configuration.yaml

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

[homeassistant]

Hi @jeroen84,

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!

[probot-home-assistant]

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

[bouwew]

@jeroen84 Thank you for your contribution!
@CoMPaTech and I will review your added code soon.

[CoMPaTech]

Hi @jeroen84 - out of curiosity, where do you change this setting? (We once had port support but dropped it since there weren't any obvious place to change the port number).

Thanks for also adding tests but as it stands we missed out on some tests in (already merged) #37289 we have to fix it in #37229 (and I'm still trying to get tests going on that part).

[jeroen84]

Hi @CoMPaTech - good question, which I should have explained. My Plugwise Smile is connected to a secondary router, which is used for all my IoT devices. Hence I use port forwarding to get access to Plugwise Smile through my primary router.

Will wait for #37229 to be merged and lets see if we need to extend the tests for this PR.

[CoMPaTech]

@jeroen84 I figured it would be something like that, but still hoped it was a configurable options somewhere hidden in the folding menu's :) Good and secure setup with a fully DMZ-ed IoT network, good call!

[CoMPaTech]

Thinking out loud, this might be a candidate for a secondary screen during setup. As the physical Smile port number can't change (the case is less likely for normal use) and we need communication with the Smile before being able to add it we can't just make it an option either.

So initial user step can remain host/password as it is now (with exception of discovery where only password is required) and the secondary user step can be showing the port number. Also see user steps.

[jeroen84]

That would be a sound solution, given the fact that this (= my setup) is an edge case. For my understanding, this secondary user step would only come up in case the connection fails using the default 80 port?

[CoMPaTech]

We'll have to dive in a little, but currently connection is only attempted after the create is called (hence, when user input is completed). But we could show the second step with 'these are the defaults, want to change anything') including port number and the scan_interval in #37229. Any thoughts on this, @bouwew ?

[bouwew]

Yeah, that could be a good alternative implementation for #37229.

[jeroen84]

Another alternative what I come up to and would include less complexity to the code and minor to no impact on UI, is the option to use [HOST_IP]:[PORT] as an entry. If no port is provided, the default value of 80 is used. In case a PORT is provided, the value is passed through the API.

This solution was by the way the first option I tried when I was not able to connect to the Plugwise Smile.

Any thoughts on this?

[CoMPaTech]

That would work as well with minimal changes, I'll have a go at that.

[bouwew]

@jeroen84 Feel free to update your PR towards the alternative solution :)

[jeroen84]

Alright, I'll update the PR later this week :)

[homeassistant]

Hi @dependabot[bot],

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!

[bouwew]

Looks like some things are going wrong? Many unrelated pull-request are being added?

[jeroen84]

Correct, excuse me, something went wrong on my side caused by my lack of knowledge of git. Have fixed this now. Will push the right version shortly.

[CoMPaTech]

No worries, we've all been there (rebasing and force push will fix it)

[jeroen84]

Somehow the test_config.py did not reset to the initial branch. Will look to into that later.

[jeroen84]

@CoMPaTech and @bouwew - please review the updated PR. I have also updated the description of the PR, given that we follow the route of allowing a port number to the IP address box in the dialog flow. Btw, I do not understand why the codecov/patch check failed; possibly due to an erroneous commit yesterday?

[CoMPaTech]

I think codecov.io is interpreting it wrong and indeed seems to diff on all the erronous rebases ... let's just make sure the code is good and deal with codecov after that?

[bouwew]

@jeroen84 We will incorporate and test your proposed changes in the Plugwise-beta custom_component during the weekend.

[bouwew]

@jeroen84 please test plugwise-beta version 0.6.0, this version allows entering a port number.
See also here: https://community.home-assistant.io/t/plugwise-smile-custom-component-beta/183560/107

[jeroen84]

@bouwew thanks! I am currently on holiday and will return in a week. I do not know if there is any rush for the release of this new version?

[bouwew]

@jeroen84 In that case, enjoy! Same here, enjoying the holidays from home :)

"any rush for release": plugwise-beta v0.6.0 is already released, also, it will probably take quite some time before the the beta-v0.6.0-changes end up in the Core plugwise component. That's why we changed the plugwise-beta code so that it (temporarily) can be used instead of the Core plugwise component.

Anyway, when you are able, please test using the non-80-port-number so that we get an extra confirmation that it works, or not.

[jeroen84]

@bouwew - I just tested version 0.6.0 of plugwise-beta and can confirm that the non-default-port works on my setup. Great work!

Do we need to close this PR?

[bouwew]

@jeroen84 Thanks for letting us know your result!

Yes, please close this PR as we will add this function via a new PR covering more updates like support for the Plugwise Stretch.

[jeroen84]

Closing this PR given that the changes are incorporated in v0.6.0 of plugwise-beta, see https://github.com/plugwise/plugwise-beta/releases/tag/0.6.0

[CoMPaTech]

Hi @jeroen84 as per request from Core we did add the port as a separate option going forward from 0.116 #40017

[jeroen84]

That is great news, @CoMPaTech . Looking forward to the 0.116 release. Thanks.