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

volume_change check for supported_feature volume_set #84

Closed
Sennevds opened this issue Jun 2, 2020 · 14 comments
Closed

volume_change check for supported_feature volume_set #84

Sennevds opened this issue Jun 2, 2020 · 14 comments
Assignees
Labels
enhancement New feature or request

Comments

@Sennevds
Copy link

Sennevds commented Jun 2, 2020

Feature Request

Is your feature request related to a problem?

I'm trying to use your app with a custom media player I made (https://github.com/Sennevds/media_player.template) where I use IR commands to control my receiver. Your app uses the service volume_set but that isn't supported in my case. Instead it should use the volume_up and volume_down service

Describe the solution / feature you'd like

I've checked your code very fast(so probably overlooked quiet few things but maybe if you create a check in the volume_change to check for the supported features volume_set and if it's not supported use the volume_up or down like this:

if not bool(supported_features & 4): #SUPPORT_VOLUME_SET = 4 but maybe you can import the constant
            if direction == "up":
                await self.call_service(
                    "media_player/volume_up",
                    entity_id=self.media_player
                )
            else:
                await self.call_service(
                    "media_player/volume_down",
                    entity_id=self.media_player
                )
@Sennevds Sennevds added the enhancement New feature or request label Jun 2, 2020
@xaviml
Copy link
Owner

xaviml commented Jun 7, 2020

Hi @Sennevds,

Thanks for your feature request. I will be adding this for the next release. However, I will be using a FeatureSupport class that I already have for this.

I will let you know once it is ready for you to test.

Thanks :)

@Sennevds
Copy link
Author

Sennevds commented Jun 7, 2020

Perfect. That's the reason I didn't make a pull request. Hadn't had the time to check-out the complete code. The reason I needed the change is because I'm using a ir remote to send commands so set volume isn't supported.

@xaviml
Copy link
Owner

xaviml commented Jun 7, 2020

Will the volume_level attribute be available to read from the media_player then?

@Sennevds
Copy link
Author

Sennevds commented Jun 7, 2020

Not always(for me no) I think it's best to check if the property exists and has a value(I checked but it's not registered as a supported_feature otherwise you could check that)

@xaviml
Copy link
Owner

xaviml commented Jun 7, 2020

I asked this because ControllerX runs a loop when holding the button until a release action has been fired or the volume is at its maximum or minimum point. Therefore, if I cannot read the volume, then I will not know the volume to then stop the loop if needed.

For security reasons, the best will be to use the hold action if the volume_level is available, otherwise only the click action can be used. What do you think? Otherwise, if we allow the loop to run without having a stop condition than the release (which might not fired for any reason) it will stay in an infinite loop.

@Sennevds
Copy link
Author

Sennevds commented Jun 7, 2020

Hmm how do you handle the remote from Ikea that you turn(E1744 and ICTC-G-1) because for a remote with buttons it would be okay I think to press multiple times. Maybe a sort of time-out value?

@xaviml
Copy link
Owner

xaviml commented Jun 7, 2020

You are right. Those controllers do not have an option for that. What I will do is to set maximum loops to run, so if after N loops did not stop, then, I stop the loop.

@Sennevds
Copy link
Author

Sennevds commented Jun 7, 2020

I think that's the best solution. Now I'm thinking do the round controllers don't have a start and stop signal? I'm going to check that tonight. Because as long as I'm turning the controller the volume should go up no?

@xaviml
Copy link
Owner

xaviml commented Jun 7, 2020

Hi @Sennevds,

Yes, they do. They send a signal when starting the rotation and send another one when the rotation is finished.

@xaviml
Copy link
Owner

xaviml commented Jun 7, 2020

Hi @Sennevds,

I just added this feature request in the dev branch. Would you mind testing it? I tried myself and everything is working as expected. I set 20 as max loops to run (each loop is one volume up/down call).

Cheers,
Xavi M.

@Sennevds
Copy link
Author

Sennevds commented Jun 7, 2020

Super will try it tomorrow.

@Sennevds
Copy link
Author

Sennevds commented Jun 8, 2020

Looks like it works. sidenote the dimmer switch from ikea (ICTC-G-1) doesn't have a stop event in deconz. It has 4 events:

  • 1002: turn fast clockwise
  • 2002: turn clockwise
  • 3002: turn counter-clockwise
  • 4002: turn fast counter-clockwise

@xaviml
Copy link
Owner

xaviml commented Jun 13, 2020

Interesting.. that it does not have a stop action like z2m and zha does. I will add support to this controller for deconz, but instead of using the hold action, I will use the click action.

xaviml added a commit that referenced this issue Jun 13, 2020
@xaviml
Copy link
Owner

xaviml commented Jun 14, 2020

Feature added in ControllerX v2.8.0

@xaviml xaviml closed this as completed Jun 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants