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 vera power meter. #7134

Merged
merged 2 commits into from
Apr 18, 2017
Merged

Add vera power meter. #7134

merged 2 commits into from
Apr 18, 2017

Conversation

pavoni
Copy link
Contributor

@pavoni pavoni commented Apr 16, 2017

Description:

Adds a vera power meter as a sensor - with watts as the state - and kwh as an additional attribute.

I don't have one of these - so @danodemano Could you test?

Related issue (if applicable): fixes #

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>

Example entry for configuration.yaml (if applicable):

Checklist:

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

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

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

@danodemano
Copy link

@pavoni - what do I need to do to test?

@pavoni
Copy link
Contributor Author

pavoni commented Apr 16, 2017

@danodemano Have you done any development on HA. If so you just need to checkout my branch (add_vera_power_meter) and run in the normal way.

If not you'll need to set up a dev version of HA as described here (https://home-assistant.io/developers/development_environment/). You can skip the initial stuff about forking the repo and creating a local repo (unless you plan to do some development yourself), and just git clone the master repo - and then git checkout add_vera_power_meter. You should then pick up from PREPARE YOUR ENVIRONMENT.

@danodemano
Copy link

@pavoni - I have not done any dev work, no. I will give that a shot and let you know.

@pavoni
Copy link
Contributor Author

pavoni commented Apr 16, 2017

Sounds like it may be a challenge for you to run this - but if you manage please test that your Power Meters appear - and update correctly.

@danodemano
Copy link

Nah, dev environment is nearly up and running. setup.py is just installing everything now. Should have some test results for you shortly.

@danodemano
Copy link

The watts look good:

image

However it looks like you are pulling watts for kwh as well?

image

@pavoni
Copy link
Contributor Author

pavoni commented Apr 16, 2017

Thanks for testing. Can you git pull and check again.

@pavoni
Copy link
Contributor Author

pavoni commented Apr 16, 2017

Those numbers all look pretty big. Should we display in kw?

@danodemano
Copy link

That's better!

image

I think watts is fine, it looks nice when you move it to a panel:

image

Though kw would be fine, I just worry that those with usage <1kw are going to see like .485 kw which isn't really nice either. I don't suppose there is any way to change it dynamically from w to kw if the usage is > 1000w?

@@ -67,6 +73,12 @@ def update(self):
self.current_value = self.vera_device.light
elif self.vera_device.category == "Humidity Sensor":
self.current_value = self.vera_device.humidity
elif self.vera_device.category == "Power meter":
power = self.vera_device.power;

Choose a reason for hiding this comment

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

statement ends with a semicolon

@@ -49,6 +49,12 @@ def unit_of_measurement(self):
return 'lux'
elif self.vera_device.category == "Humidity Sensor":
return '%'
elif self.vera_device.category == "Power meter":
power = self.vera_device.power;

Choose a reason for hiding this comment

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

statement ends with a semicolon

@pavoni
Copy link
Contributor Author

pavoni commented Apr 16, 2017

Good idea - let's try. Have also rounded the numbers for the main display. So if you can pull and try again.

@danodemano
Copy link

Hmmm, they don't show up at all now. I see this in the logs:

image

@@ -49,6 +49,12 @@ def unit_of_measurement(self):
return 'lux'
elif self.vera_device.category == "Humidity Sensor":
return '%'
elif self.vera_device.category == "Power meter":
power = convert(self.vera_device.power, float, 0)

Choose a reason for hiding this comment

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

undefined name 'convert'

@@ -49,6 +49,12 @@ def unit_of_measurement(self):
return 'lux'
elif self.vera_device.category == "Humidity Sensor":
return '%'
elif self.vera_device.category == "Power meter":
power = convert(self.vera_device.power, float, 0)

Choose a reason for hiding this comment

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

undefined name 'convert'

@pavoni
Copy link
Contributor Author

pavoni commented Apr 16, 2017

Can you try again @danodemano?

@danodemano
Copy link

NICE!

image

I'm headed to my folks for Easter but I'll leave it run and check the historical data when I get back in a few hours. Thanks!!

@pavoni
Copy link
Contributor Author

pavoni commented Apr 16, 2017

Thanks for your help.

@pavoni
Copy link
Contributor Author

pavoni commented Apr 16, 2017

I'm pretty sure switching units will mess up the historic data. So is probably a bad idea.

So we should settle on kW or W. What do you think?

My sense is probably kW - but with 3 rather than 2 digits.

@danodemano
Copy link

Ahh that's a good point. I would just stick with W then, people can adjust the display using templates if required. HA when directly connected to these meters uses W.

@dpembo
Copy link

dpembo commented Apr 16, 2017

quick test looks good, but I need to switch out a lot of config to be 100% sure it fixes the issues I had around the switch states/etc, especially in automation.

@danodemano
Copy link

Tried to update and test out the historical data but getting a merge conflict:

image

Copy link
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

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

🐬 ok to merge when it's all ok with you @pavoni

@pavoni
Copy link
Contributor Author

pavoni commented Apr 17, 2017

@danodemano Did you change anything locally? If not may be the rebase I did to combine the commits.

You could try the suggestions here to overwrite your local with the new remote http://stackoverflow.com/questions/1125968/how-do-i-force-git-pull-to-overwrite-local-files.

Worst case you can delete and re-clone the repo assuming you don't have anything valuable locally.

Will merge when you've tested.

@danodemano
Copy link

@pavoni - thanks, I was able to fetch and hard reset and am on the current copy of your branch. I have it running now, will let you know how the historical data looks in a few hours. I nuked the database before starting just to make sure it was fresh.

@pvizeli pvizeli merged commit 1925748 into dev Apr 18, 2017
@pavoni pavoni deleted the add_vera_power_meter branch April 18, 2017 11:14
@balloob balloob mentioned this pull request Apr 21, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Jul 17, 2017
@ghost ghost removed the platform: sensor.vera label Mar 21, 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.

9 participants