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

result_timeout kwarg for all Sender functions #49

Merged
merged 1 commit into from
Nov 23, 2015

Conversation

luckydonald
Copy link
Owner

fixes #48 ?
@the-glu can you test that, please?

@the-glu
Copy link

the-glu commented Oct 29, 2015

Well, it will work in "my" case, but doesn't allow to override the timeout if the default one is used.
The comment is also wrong since you copy-pasted it from my PR who allow it.

Why a new pull request ? I can fix #48 ;)

@luckydonald
Copy link
Owner Author

This was what I did briefly in the online editor. But you shouldn't want use the default timeout, if you specify your own?

@the-glu
Copy link

the-glu commented Nov 4, 2015

I don't understand :)

I don't think the default timeout should be used, I a custom is specified.

@luckydonald
Copy link
Owner Author

Well, it will work in "my" case, but doesn't allow to override the timeout if the default one is used.

The default one is only used when you don't override it, and t.
Why do you need to change only the default?

Can you tell me step by step, what you are trying to archive?
Maybe when I understand, I can help you correctly.

@the-glu
Copy link

the-glu commented Nov 19, 2015

I want to be able to set the timeout at a provided value in all cases (if there is a timeout set in functions definitions or the default one is used) . That what I did in #48 .

@luckydonald
Copy link
Owner Author

I believe that is possible here too, with actual less code.
I really can't see any difference. You assign just a second variable for something already possible with result_timeout?

result_timeout = functions[function_name][FUNC_TIME]  #moved upwards
if "result_timeout" in kwargs:
    result_timeout = kwargs["result_timeout"]  # overrides functions[...][FUNC_TIME]
self._do_command( ... , answer_timeout=result_timeout , ...)

That should be equivalent to your version:

custom_timeout = None
 if "custom_timeout" in kwargs:
    custom_timeout = kwargs["custom_timeout"]
result_timeout = functions[function_name][FUNC_TIME]

self._do_command( ... , answer_timeout=result_timeout if custom_timeout is None else custom_timeout , ...)
# overrides functions[...][FUNC_TIME] aka. result_timeout 

except for the kwarg name of course.

@luckydonald
Copy link
Owner Author

Actually, in your version I cannot set it to None explicitly.

luckydonald added a commit that referenced this pull request Nov 23, 2015
`result_timeout` keyword parameter for all Sender functions.

 How long, in seconds, we wait for the cli to answer the send command. Set to None to use the global default timeout (`Sender.default_answer_timeout`) instead of the default timeout for the given command. To use the default timeout for that command omit this parameter.
@luckydonald luckydonald merged commit 9319b89 into master Nov 23, 2015
@luckydonald luckydonald deleted the sender-result_timeout branch November 23, 2015 19:22
@luckydonald
Copy link
Owner Author

If you see a case the code above doesn't handle please tell me.
And thanks again for your code.
closes #48

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.

2 participants