Skip to content

Prevent the projector to toogle on/off#22985

Merged
andrewsayre merged 1 commit into
home-assistant:devfrom
stbkde:patch-2
Apr 11, 2019
Merged

Prevent the projector to toogle on/off#22985
andrewsayre merged 1 commit into
home-assistant:devfrom
stbkde:patch-2

Conversation

@stbkde
Copy link
Copy Markdown
Contributor

@stbkde stbkde commented Apr 11, 2019

Prevents the projector to toogle on/off.

Description:

The Epson Projector EH-TW7300 turns on and off with the same command. This minor change fix it.

Checklist:

  • [ x] The code change is tested and works locally.
  • [ x] Local tests pass with tox.
  • [ x] There is no commented out code in this PR.

@homeassistant
Copy link
Copy Markdown
Contributor

Hi @stbkde,

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!

@pszafer
Copy link
Copy Markdown
Contributor

pszafer commented Apr 11, 2019

According to this #22878 I think it is better to manipulate SUPPORT_EPSON in state attributes.

@andrewsayre
Copy link
Copy Markdown
Member

According to this #22878 I think it is better to manipulate SUPPORT_EPSON in state attributes.

I think submitted approach correct. The referenced PR is to guard against calls that aren't implemented in the device -- not that the current state doesn't allow it. Media player is the exception to the rule that supported_features is to be static.

Copy link
Copy Markdown
Member

@andrewsayre andrewsayre left a comment

Choose a reason for hiding this comment

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

So if I'm understanding this PR correctly, the constants TURN_ON and TURN_OFF are the same underlying control command hence why we need check the current state of the device before issuing the command to prevent a toggle. If that's correct, this is good to go once build passes.

@pszafer
Copy link
Copy Markdown
Contributor

pszafer commented Apr 11, 2019

@andrewsayre no, TURN_ON and TURN_OFF are not exactly the same.
TURN_OFF is 2 x TURN_OFF in case of projector and have different timeout for waiting for reply.

If you send TURN_ON if it is turned ot projector would probably show info that you need click TURN_ON second time.
If you click TURN_OFF during OFF it will TURN ON projector.

@stbkde
Copy link
Copy Markdown
Contributor Author

stbkde commented Apr 11, 2019

So if I'm understanding this PR correctly, the constants TURN_ON and TURN_OFF are the same underlying control command hence why we need check the current state of the device before issuing the command to prevent a toggle. If that's correct, this is good to go once build passes.

Correct. TURN_ON and TURN_OFF toggles the power state. This is the easiest way to prevent that for any kind of epson projectors.

@andrewsayre
Copy link
Copy Markdown
Member

I think you're both saying the same thing, just at different levels of abstraction. Are we aligned on this fix?

@pszafer
Copy link
Copy Markdown
Contributor

pszafer commented Apr 11, 2019

It doesn't break anything, so I think it's good to merge.

@andrewsayre andrewsayre merged commit 02347df into home-assistant:dev Apr 11, 2019
@ghost ghost removed the in progress label Apr 11, 2019
@stbkde stbkde deleted the patch-2 branch April 11, 2019 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants