Skip to content

Add hive trv support#27033

Merged
MartinHjelmare merged 18 commits into
home-assistant:devfrom
MagicalTrev89:TRV-Support
Oct 5, 2019
Merged

Add hive trv support#27033
MartinHjelmare merged 18 commits into
home-assistant:devfrom
MagicalTrev89:TRV-Support

Conversation

@MagicalTrev89
Copy link
Copy Markdown
Contributor

@MagicalTrev89 MagicalTrev89 commented Sep 28, 2019

Description:

This PR brings TRV support to the Hive Plugin

Docs PR:

home-assistant/home-assistant.io#10607

Related issue (if applicable):
fixes Rendili/hive-for-home-assistant#3

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

@homeassistant
Copy link
Copy Markdown
Contributor

Hi @MagicalTrev89,

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
Copy link
Copy Markdown
Member

Please solve the merge conflicts.

Copy link
Copy Markdown
Contributor

@KJonline KJonline left a comment

Choose a reason for hiding this comment

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

Hi @MagicalTrev89, I have fixed the pylint issues from the merge conflict. as I cannot checkout your branch, I cannot fix the formatting issue that the latest commit is failing on. Please could you take a look at this.

@MagicalTrev89
Copy link
Copy Markdown
Contributor Author

Hi @KJonline, New to this... please could you point me to what its failing on ?

@MagicalTrev89
Copy link
Copy Markdown
Contributor Author

Hi @KJonline, worked it out ... i've updated the formatting issues, those tests that failed have passed now

Comment thread homeassistant/components/hive/__init__.py
Comment thread homeassistant/components/hive/climate.py Outdated
Comment thread homeassistant/components/hive/climate.py
Comment thread homeassistant/components/hive/climate.py Outdated
@MartinHjelmare MartinHjelmare changed the title Trv support Add hive trv support Sep 29, 2019
Comment thread homeassistant/components/hive/climate.py Outdated
Comment thread homeassistant/components/hive/climate.py
@KJonline
Copy link
Copy Markdown
Contributor

@MagicalTrev89, please can you also include the manifest.json file updating the pyhiveapi library version to 0.2.19.3 and running the below before committing it in.

python3 -m script.gen_requirements_all

Comment thread homeassistant/components/hive/climate.py Outdated
@MartinHjelmare
Copy link
Copy Markdown
Member

Please solve the merge conflict.

@MagicalTrev89
Copy link
Copy Markdown
Contributor Author

@KJonline can you sort out the conflict please?

@KJonline
Copy link
Copy Markdown
Contributor

@KJonline can you sort out the conflict please?

That’s sorted now

@MartinHjelmare
Copy link
Copy Markdown
Member

Please run black from the project root.

black --fast homeassistant

@MagicalTrev89
Copy link
Copy Markdown
Contributor Author

Hi @KJonline Are you sure about pyhiveapi version: 0.2.19.3, build is failing saying it doesn't exist, are you in the process of releasing that version ?

@KJonline
Copy link
Copy Markdown
Contributor

Hi @KJonline Are you sure about pyhiveapi version: 0.2.19.3, build is failing saying it doesn't exist, are you in the process of releasing that version ?

Apologies I seem to have created the version but not uploaded it to PyPi. I’ll upload it this evening.

@KJonline
Copy link
Copy Markdown
Contributor

Hi @MagicalTrev89, I have uploaded ver 0.2.19.3 of pyhiveapi, it should now be available to download.

@MagicalTrev89
Copy link
Copy Markdown
Contributor Author

@MartinHjelmare @KJonline is there any way to restart the build checks without having to do another commit ?

@MartinHjelmare
Copy link
Copy Markdown
Member

You can close and reopen the PR.

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Good!

@MartinHjelmare
Copy link
Copy Markdown
Member

We should probably update the docs and mention TRV here:
https://www.home-assistant.io/components/hive/#climate

@MartinHjelmare
Copy link
Copy Markdown
Member

Can be merged when a docs PR is linked in the PR description.

@KJonline
Copy link
Copy Markdown
Contributor

KJonline commented Oct 4, 2019

Hi @MagicalTrev89, I have updated the docs in PR home-assistant/home-assistant.io#10607, Please can you edit this PR linking them together in the description of this PR

@MagicalTrev89
Copy link
Copy Markdown
Contributor Author

@KJonline looks like its already done! 👍 @MartinHjelmare can you approve now ?

@MartinHjelmare MartinHjelmare merged commit a907345 into home-assistant:dev Oct 5, 2019
@lock lock Bot locked and limited conversation to collaborators Oct 6, 2019
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.

Hive TRV Support

6 participants