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

Home Assistant Driver #3089

Merged
merged 78 commits into from
Oct 18, 2023
Merged

Home Assistant Driver #3089

merged 78 commits into from
Oct 18, 2023

Conversation

riley206
Copy link
Contributor

@riley206 riley206 commented Jun 6, 2023

Description

This is a pull request to add the Home Assistant Driver to the VOLTTRON repository.

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How Has This Been Tested?

This has been tested by running the driver on the same network as our Home Assistant instance with real devices such as thermostats and lights connected to Home Assistant.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@craig8
Copy link
Contributor

craig8 commented Jun 6, 2023

This should be against the develop branch not main. @riley206 you can modify the branch by changing the branch at the top of the pr and clicking edit.

@riley206 riley206 changed the base branch from main to develop June 6, 2023 17:32
Copy link
Contributor

@craig8 craig8 left a comment

Choose a reason for hiding this comment

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

Really good job starting out @riley206. I have taken to add some additions/modifications that should help it be better

HomeAssistant_Driver/home_assistant.config Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@craig8 craig8 left a comment

Choose a reason for hiding this comment

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

Please document all of your functions in rst format. Especially the public ones (the ones the platform driver will call).

@schandrika
Copy link
Contributor

@riley206 Update documentation to rst and move it to docs under docs/source/agent-framework/driver-framework/home_assistant directory and also make sure there is a link to this document from docs/source/agent-framework/driver-framework/drivers-overview.rst

Also other documentation updates that would be useful:

Mention how to see the list of devices - i.e. specify device = entity and home assistant prefixes thermostats with climate. and light with light.
Mention any limitations - i.e. this driver can scrape data from all devices but can only control light and thermostat

@schandrika
Copy link
Contributor

@riley206 Could you make errors to raise exceptions instead of suppressing it with just _log.error. Warnings are okay to just log, but when there are actual errors such as writing to a read-only point or writing a wrong value the error should get propagated to the calling agent/user. So raise the exception or custom IOError or ValueError in addition to _log.error

@craig8 craig8 merged commit 089c53a into VOLTTRON:develop Oct 18, 2023
13 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants