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 initial support for Sony SDCP projector control #20269

Merged
merged 10 commits into from Feb 23, 2019

Conversation

Projects
None yet
8 participants
@alistairg
Copy link
Contributor

alistairg commented Jan 20, 2019

Description:

Add basic support as a Switch platform for turning on and off Sony projectors via network SDCP control.

Related issue (if applicable): fixes #

Pull request in home-assistant/home-assistant.io#8390

(To be added)

Example entry for configuration.yaml (if applicable):

switch:
  - platform: sony_projector
    host: "192.168.1.43"

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.

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).
  • New dependencies are only imported inside functions that use them (example).
  • 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.
@rohankapoorcom
Copy link
Member

rohankapoorcom left a comment

Don't forget to update requirements_all.txt

Show resolved Hide resolved homeassistant/components/switch/sony_projector.py Outdated
Show resolved Hide resolved homeassistant/components/switch/sony_projector.py Outdated
Show resolved Hide resolved homeassistant/components/switch/sony_projector.py Outdated
Show resolved Hide resolved homeassistant/components/switch/sony_projector.py Outdated
@frenck

This comment has been minimized.

Copy link
Member

frenck commented Jan 21, 2019

⚠️ I was unable to find a documentation PR (matching this code change) in our documentation repository.

📚Please, update the documentation and open a PR for it (be sure to create a documentation PR against the next branch) and link 🔗 this PR and the new documentation PR by mentioning the link to the PR's in both openings posts/descriptions.

🏷 I am adding the docs-missing label until this has been resolved.

👍 Thanks (times a million!)

@frenck frenck added the docs-missing label Jan 21, 2019

alistairg added some commits Jan 23, 2019

Added code per change requests
- Validation of connection during setup_platform
- Docs pending
@alistairg

This comment has been minimized.

Copy link
Contributor Author

alistairg commented Feb 2, 2019

alistairg added some commits Feb 2, 2019

@rohankapoorcom
Copy link
Member

rohankapoorcom left a comment

Don't forget to update .coveragerc :).

Show resolved Hide resolved homeassistant/components/switch/sony_projector.py Outdated
Show resolved Hide resolved homeassistant/components/switch/sony_projector.py Outdated
Show resolved Hide resolved homeassistant/components/switch/sony_projector.py Outdated
@rohankapoorcom

This comment has been minimized.

Copy link
Member

rohankapoorcom commented Feb 3, 2019

@alistairg can you update the description with the docs PR in the correct place per the template?

rohankapoorcom and others added some commits Feb 3, 2019

Update homeassistant/components/switch/sony_projector.py
Co-Authored-By: alistairg <alistair@alistairs.net>
@alistairg

This comment has been minimized.

Copy link
Contributor Author

alistairg commented Feb 5, 2019

@rohankapoorcom - Forgive me... is there something I need to do to close this out? As far as I can see it's all good to go, but Git still shows as "changes requested"...

@rohankapoorcom

This comment has been minimized.

Copy link
Member

rohankapoorcom commented Feb 17, 2019

@rohankapoorcom - Forgive me... is there something I need to do to close this out? As far as I can see it's all good to go, but Git still shows as "changes requested"...

Sorry - I hadn't had a chance to review this again. LGTM.

@rohankapoorcom rohankapoorcom removed their assignment Feb 19, 2019

@rohankapoorcom

This comment has been minimized.

Copy link
Member

rohankapoorcom commented Feb 19, 2019

Can be merged when build passes.

@andrewsayre andrewsayre merged commit 1eba90d into home-assistant:dev Feb 23, 2019

3 checks passed

Hound No violations found. Woof!
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@wafflebot wafflebot bot removed the in progress label Feb 23, 2019

@MartinHjelmare
Copy link
Member

MartinHjelmare left a comment

Please open a new PR where we can address the comments.

return self._state

@property
def state_attributes(self):

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Feb 23, 2019

Member

Use device_state_attributes instead. state_attributes should only be used by the base entity in the base component (switch).

_LOGGER.debug("Powering on projector '%s'...", self.name)
if self._sdcp.set_power(True):
_LOGGER.debug("Powered on successfully.")
self._state = STATE_ON

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Feb 23, 2019

Member

We can remove the change of state holding instance attributes. update will be called at the end of the service call, ie after this method is called.

_LOGGER.debug("Powering off projector '%s'...", self.name)
if self._sdcp.set_power(False):
_LOGGER.debug("Powered off successfully.")
self._state = STATE_OFF

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Feb 23, 2019

Member

See above.

sdcp_connection.get_power()
except ConnectionError:
_LOGGER.error("Failed to connect to projector '%s'", host)
return False

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Feb 23, 2019

Member

Nothing is checking this return value. Just return.

return False
_LOGGER.debug("Validated projector '%s' OK", host)
add_entities([SonyProjector(sdcp_connection, name)], True)
return True

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Feb 23, 2019

Member

See above. We can remove this statement.

"""Turn the projector on."""
_LOGGER.debug("Powering on projector '%s'...", self.name)
if self._sdcp.set_power(True):
_LOGGER.debug("Powered on successfully.")

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Feb 23, 2019

Member

We don't end logging messages with period.

@alistairg alistairg deleted the alistairg:sony-sdcp branch Feb 23, 2019

@balloob balloob referenced this pull request Mar 6, 2019

Merged

0.89.0 #21712

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.