-
Notifications
You must be signed in to change notification settings - Fork 94
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 in two new methods for iterable volume control. #55
base: main
Are you sure you want to change the base?
Conversation
@atlc thanks so much for the contribution! I'd like to find a way to reuse the existing https://github.com/jcarbaugh/python-roku/pull/55/files#diff-2a352684b33ccb00ba030ce51c0200c7R180 Instead of ITERABLE_FUNCTIONS on the line above, I'm wondering if it could just be What do you think? |
@atlc also, I'm happy to either let you investigate or I'll merge and take a stab at it myself, whichever you prefer. I'm prepping for a 4.0 release that I want to get out tomorrow night, so we can include this feature if we get it ready by then. Thanks! |
Hey @jcarbaugh, thanks for looking everything over! The reason why I decided to create a separate dict for the iterable functions is because that behavior is extensible to the channel up, channel down, left, right, down, up, and back functions and I thought that would be the most acceptable format. I am not a Python developer so I am not all that familiar with development conventions so I really appreciate the feedback. I'll take a stab at implementing it the second way, with the functionality extended to all the directional inputs, and the channel directions! |
Having that defined list instead of just being in the elif is compelling. Thinking about it more broadly, the volume up and down makes sense to repeat, but really any command could be repeated. What if call took an optional repeat parameter that would just invoke the given command multiple times? There would have to be a very short sleep between each call of the command to make sure that the previous one finished executing on the Roku. |
…ions to allow them to take a parameter of amount of times to loop
Really, adding the logic to an extra argument for the I won't have any time to work on this tomorrow unfortunately, so if you want to wait until after the release I can finish up implementing that Tuesday! Otherwise, I've got the rest of the PR up to date with all the other suggestions before the suggestion of shifting the functionality to PS - Thank you so much for making this awesome project, it's so nice not having to hunt for the remote and a laptop is so much harder to lose! 👍 |
I actually just wrote a simple button press function in my own app that does exactly that. The logic may be reusable here: def button_press(self, button, x=1):
if self.rc==[]: self.discover() # check for a connected roku object
if button not in self.rc.commands: return False # if the call is not a button command, return
for i in range(x):
try:
getattr(self.rc, button)()
except:
# re-iterate on exception, since this is almost always a temporary connection exception
# any other exceptions would have been caught already in self.discovery()
self.button_press(button, x-i) # subtract the current iteration from x arg to keep count I did consider adding in an arbitrary sleep in there, which I actually have done on some other commands which I wrapped. Some commands don't seem to need a sleep, while others would go far too quickly for the screen to catch up. I've found in practice that repeated forward and reverse commands can be strung together, but if you do this with more than say 4 then the interface can get a little laggy and must be /reset/ with a roku.play() command twice in a row (pause, unpause). All in all, "to sleep or not to sleep" may be a decision for the caller, but I'm not sure. Something about argument overload and protecting the user from themselves? |
Hrm. Looking at the code in core.py I'm not actually sure that my code will help at all. In that case, all I can offer is my recent experience both with using a time.sleep(1) and without: I've found it's useful to string together several up, down, left, or right commands, as the system is very responsive to this. As well, a literal command can be thrown in, as may be the case in automating a search, for example. Repeated forward, reverse, or replay commands tend to not work very well, with the worst offender being the replay command. You can witness this for yourself with a normal IR remote control: the system goes bonkers. I also found that when sending several forward commands (in order to modulate speed), sometimes the interface would get stuck on the heads up screen (the one with the play button and timeline) and the time would get all jumpy. It seemed to be an interface display issue and not a functional issue, as it didn't affect playback and was remedied by sending play() twice. I ran into this as I was writing a method to specify a time amount to skip forward or back, based on the knowledge that commercials tend to be specific lengths of time (none of which match a 20 second skip time, btw) I found that by timing the forward and reverse speeds at various multipliers, in combination with time.sleep calls, I was able to get surprisingly accurate skip timings. If there's interest, I may make a pull request to add that functionality to core (for now it lives in my wrapper.) |
This allows you to jump volume incrementally by passing an integer argument to either
volume_down_by
orvolume_up_by
.This functionality can also be extended to
ChannelUp
andChannelDown
by defining'channel_up_by': 'ChannelUp'
and'channel_down_by': 'ChannelDown'
in theITERABLE_FUNCTIONS
, but I don't have a TV tuner to test those commands.