Skip to content

Added support to HTTPS URLs on SynologyDSM#15270

Merged
pvizeli merged 6 commits into
home-assistant:devfrom
tchellomello:synology_https
Jul 4, 2018
Merged

Added support to HTTPS URLs on SynologyDSM#15270
pvizeli merged 6 commits into
home-assistant:devfrom
tchellomello:synology_https

Conversation

@tchellomello
Copy link
Copy Markdown
Contributor

@tchellomello tchellomello commented Jul 2, 2018

Description:

Added support to HTTPS URLs on Synology DSM

[x] 3rd party PR approved ProtoThis/python-synology#6

Related issue (if applicable): fixes #15269

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

Example entry for configuration.yaml (if applicable):

  - platform: synologydsm
    host: 192.168.0.1
    username: admin
    ssl: true
    password: 'secret#qq'
    port: 5001
    monitored_conditions:
      - cpu_total_load

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

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

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

  • New dependencies have been added to the REQUIREMENTS variable ([example][ex-requir]).
  • New dependencies are only imported inside functions that use them ([example][ex-import]).
  • New or updated 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:

  • Tests have been added to verify that the new code works.


try:
self._api = SynologyDSM(host, port, username, password)
self._api = SynologyDSM(host, port, username, password, use_https=use_ssl)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (86 > 79 characters)

@tchellomello tchellomello changed the title WIP Added support to HTTPS URLs on SynologyDSM Added support to HTTPS URLs on SynologyDSM Jul 2, 2018
PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
vol.Required(CONF_HOST): cv.string,
vol.Optional(CONF_PORT, default=DEFAULT_PORT): cv.port,
vol.Optional(CONF_SSL, default=False): cv.boolean,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default True

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pvizeli the default port listed on the component is 5000 which is a not HTTPS. If you set the default to True, then we will need to change the default port to 5001 and that will be a breaking change. In this case, do you still want that set to True? I can make the changes if that is the case, but we will need to release a small note due to the breaking change.

Please let me know your thoughts.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we do default the best possible solution and if a user don't want be save, he need that downgrade in config. In that case, set default port to 5001.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. I'll make sure to update the docs and update the PR soon.

@tchellomello
Copy link
Copy Markdown
Contributor Author

@pvizeli code and docs updated. Thanks!! 👍

@pvizeli pvizeli merged commit a6e9dc8 into home-assistant:dev Jul 4, 2018
@ghost ghost removed the in progress label Jul 4, 2018
@tchellomello tchellomello deleted the synology_https branch July 4, 2018 05:49
awarecan pushed a commit to awarecan/home-assistant that referenced this pull request Jul 16, 2018
* Added support to HTTPS URLs on SynologyDSM

* Bumped python-synology to 0.1.1

* Makes lint happy

* Added attribution to Synology and fixed 3rd library version

* Fixed requirements_all.txt

* Makes SynologyDSM defaults to 5001 using SSL
@balloob balloob mentioned this pull request Jul 20, 2018
girlpunk pushed a commit to girlpunk/home-assistant that referenced this pull request Sep 4, 2018
* Added support to HTTPS URLs on SynologyDSM

* Bumped python-synology to 0.1.1

* Makes lint happy

* Added attribution to Synology and fixed 3rd library version

* Fixed requirements_all.txt

* Makes SynologyDSM defaults to 5001 using SSL
@home-assistant home-assistant locked and limited conversation to collaborators Dec 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SynologyDSM does not work when using HTTPS or when the user's password has a special character

4 participants