Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Abode water valve support #30635

Merged
merged 1 commit into from Feb 8, 2020
Merged

Conversation

@shred86
Copy link
Contributor

shred86 commented Jan 10, 2020

Description:

Add support for water valves in Abode. Unfortunately, I don't have a water valve to test so I'll need someone else to test this.

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][dev-checklist]
@project-bot project-bot bot added this to Needs review in Dev Jan 10, 2020
@project-bot project-bot bot moved this from Needs review to By Code Owner in Dev Jan 10, 2020
@jbbarnes77

This comment has been minimized.

Copy link

jbbarnes77 commented Jan 10, 2020

I can assist with that. I have a Home Assistant setup and a Dome water valve and am glad to test.

@shred86

This comment has been minimized.

Copy link
Contributor Author

shred86 commented Jan 11, 2020

Awesome! Are you just able to run the Abode integration as a custom component with this changed file? The water valve should show up and just appear as a switch.

Dev automation moved this from By Code Owner to Reviewer approved Jan 18, 2020
@springstan

This comment has been minimized.

Copy link
Member

springstan commented Jan 20, 2020

@jbbarnes77 any updates on testing this addition? :)

@jbbarnes77

This comment has been minimized.

Copy link

jbbarnes77 commented Jan 20, 2020

Hi. Just got my Dome valve installed the other day. Needed a plumber to remove my rotating shutoff valve with a quarter-turn valve. It is now operational from within Abode.

I have installed a HA Docker container on my Synology NAS and connected to my Abode system. It shows all the devices that were present BEFORE I added the Dome shutoff valve and I haven't been able to refresh it successfully. Could you instruct me on how to do that?

Below is a screenshot of my current devices.

2020-01-19 22_47_19

@jbbarnes77 any updates on testing this addition? :)

@shred86

This comment has been minimized.

Copy link
Contributor Author

shred86 commented Jan 20, 2020

I’m not quite sure what you mean. Are you saying you added or removed other Abode devices besides the Dome shutoff valve?

You won’t see the Dome shutoff valve unless you run the Abode integration as a custom component in Home Assistant with the changes in this pull request.

@jbbarnes77

This comment has been minimized.

Copy link

jbbarnes77 commented Jan 20, 2020

Yes, that's what I meant. That i didn't see the Dome valve at all within HA once adding it. I first connected it with my SmartThings hub, but Abode didn't see it. I reset the Dome valve and then added it as a new device within the Abode web interface. At that point I was able to open it, close it, and create Cue automations successfully.

So at this point I'm just looking for your guidance in what I need to do to test and give you the feedback you need. I'm afraid I have no experience with HA at all besides installing the Docker container and integrating it with a few accounts such as IFTTT, LIFX, Kasa and Abode.

@shred86

This comment has been minimized.

Copy link
Contributor Author

shred86 commented Jan 20, 2020

Do you have access to the Home Assistant config folder? If so, it's fairly straight forward to run custom component. From the config folder, create a folder called custom_components and unzip this file such that the abode folder is inside custom_components directory. The directory path should look like config/custom_components/abode.

https://drive.google.com/open?id=1cTCLSLsVzqME3EmYKB3Y0lDw_jnl8-qg

That file is the abode component but with the changes I made to the switch.py file. Once that's done, restart Home Assistant and now it should load the Abode component from that custom components directory. If everything worked correctly, you should now see the Dome water valve show up as a switch.

@springstan

This comment has been minimized.

Copy link
Member

springstan commented Jan 24, 2020

@jbbarnes77 have you successfully tested this addition? :)

@springstan

This comment has been minimized.

Copy link
Member

springstan commented Feb 2, 2020

@shred86 the documentation will need to be updated as well, therefore I have added the docs-missing label. Could you please open a PR for that? Thanks 👍

@jbbarnes77 do you need any help testing this new addition? We can assist you if you want to :)

@shred86

This comment has been minimized.

Copy link
Contributor Author

shred86 commented Feb 2, 2020

Will do. I also sent a message to @jbbarnes77 on this Reddit thread.

@shred86 shred86 mentioned this pull request Feb 2, 2020
3 of 3 tasks complete
@shred86

This comment has been minimized.

Copy link
Contributor Author

shred86 commented Feb 2, 2020

PR made for the documentation: home-assistant/home-assistant.io#11954

@springstan springstan removed the docs-missing label Feb 2, 2020
@springstan

This comment has been minimized.

Copy link
Member

springstan commented Feb 2, 2020

@shred86 thanks for updating the docs! 👍

@springstan

This comment has been minimized.

Copy link
Member

springstan commented Feb 8, 2020

@shred86 do you want to wait for user validation before we merge this?

@shred86

This comment has been minimized.

Copy link
Contributor Author

shred86 commented Feb 8, 2020

I don’t think it’s required. I’m about 99.9% sure this fixes the issue. However, I made another larger PR #31514 that will conflict with this one. Would it be easier to close this PR and I just make the changes in the other PR, or wait until after the other one is merged and rebase this one?

@springstan

This comment has been minimized.

Copy link
Member

springstan commented Feb 8, 2020

Why not merge this one and rebase #31514 to include these changes?

@shred86

This comment has been minimized.

Copy link
Contributor Author

shred86 commented Feb 8, 2020

Honestly, I’m just a Git newb and rebasing scares me lol. That should work, I’m sure I’ll figure it out. ;)

@springstan

This comment has been minimized.

Copy link
Member

springstan commented Feb 8, 2020

I am sure too! 👍

@springstan springstan merged commit 14e0dde into home-assistant:dev Feb 8, 2020
10 of 11 checks passed
10 of 11 checks passed
docs-missing Please open a documentation PR.
CI Build #20200110.11 had test failures
Details
CI (FullCheck Mypy) FullCheck Mypy succeeded
Details
CI (FullCheck Pylint) FullCheck Pylint succeeded
Details
CI (Overview CheckFormat) Overview CheckFormat succeeded
Details
CI (Overview Lint) Overview Lint succeeded
Details
CI (Overview Validate) Overview Validate succeeded
Details
CI (Tests PyTest Python37) Tests PyTest Python37 succeeded
Details
cla-bot Everyone involved has signed the CLA
codecov/patch Coverage not affected when comparing d25aa1f...2582729
Details
codecov/project 94.59% (target 90%)
Details
Dev automation moved this from Reviewer approved to Done Feb 8, 2020
@shred86 shred86 deleted the shred86:abode_valve branch Feb 8, 2020
@jbbarnes77

This comment has been minimized.

Copy link

jbbarnes77 commented Feb 8, 2020

@shred86

This comment has been minimized.

Copy link
Contributor Author

shred86 commented Feb 8, 2020

@jbbarnes77 If you're in the Home Assistant discord, I can walk you through setting up a custom component to test these changes plus some others. My username in there is shred.

@lock lock bot locked and limited conversation to collaborators Feb 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Dev
  
Done
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.