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

resolve_stream should have a timeout parameter #35

Open
jdevoldere opened this issue Sep 21, 2020 · 8 comments
Open

resolve_stream should have a timeout parameter #35

jdevoldere opened this issue Sep 21, 2020 · 8 comments

Comments

@jdevoldere
Copy link

As of now you can't specify a timeout which means a python program can get stuck indefinitely if no stream is found.

@dmedine
Copy link
Contributor

dmedine commented Sep 21, 2020

Unfortunately, this is not so simple to do without breaking the consistency that exists between the APIs for various programming languages. Some languages (and operating systems) have very different mechanisms for this. One option would be to add this feature to the C++ functions that sit at the bottom of each API. Of course, this still means updating each API to reflect the changes. I would be open to adding this to the TODO list and indeed it might already be on the list of features that some developers wish to implement, but in any case it will take some time before it is ever done.

In the meantime, you can try rolling your own: https://stackoverflow.com/questions/492519/timeout-on-a-function-call

@agricolab
Copy link
Contributor

I agree with @dmedine that it is better (i.e. likely way easier and safer) to implement this at the level of the wrapping language.

Besides, on my machine and in my experience, if no stream is present, pylsl.resolve_stream returns immediatly with an empty list, i.e []. Can you give a minimum example where it blocks if no stream is found?

@jdevoldere
Copy link
Author

@dmedine this library is probably a better and easier solution for such cases: https://pypi.org/project/func-timeout/

But after looking through the source code I've realized that resolve_stream is a legacy function (despite still being used in the pylsl examples) that points towards the newer resolve_byprop function which does have a timeout parameter, but resolve_stream only allows for 3 arguments to be passed so it excludes the timeout argument.

So really the solution is to just use resolve_byprop directly.

@agricolab I think you are confusing resolve_stream with the resolve_streams function.

@agricolab
Copy link
Contributor

agricolab commented Sep 21, 2020

The issue title says resolve_stream should have a timeout parameter, and i meant resolve_stream. But both functions return an empty list, if no streams are available. I found a working example though for your issue. You have to specify arguments, then it blocks., i.e

resolve_stream()
# returns []
resolve_stream('type','EEG')
# blocks

That is certainly unexpected behavior, and i would have expected that it returns immediately with an empty list, too.

@jdevoldere
Copy link
Author

@agricolab ah it defaults to resolve_streams depending on the arguments.

def resolve_stream(*args):
    if len(args) == 0:
        return resolve_streams()
    elif type(args[0]) in [int, float]:
        return resolve_streams(args[0])
    elif type(args[0]) is str:
        if len(args) == 1:
            return resolve_bypred(args[0])
        elif type(args[1]) in [int, float]:
            return resolve_bypred(args[0], args[1])
        else:
            if len(args) == 2:
                return resolve_byprop(args[0], args[1])
            else:
                return resolve_byprop(args[0], args[1], args[2])

@agricolab
Copy link
Contributor

Great. When resolve_stream either calls resolve_streams or resolve_byprop and both can be called in non-blocking mode, it should be trivial to add a timeout or default to non-blocking calls.

I would prefer to go non-blocking by default. But i wrote my own wrapper anyways, as i like a call by kwargs, so there's that.

def get_streaminfos_matching(**kwargs) -> List[StreamInfo]:
    """
    args
    ----
    **kwargs:
        keyword arguments to identify the desired stream
        
    returns
    -------
    sinfos: List[StreamInfo]
        a list of StreamInfo matching the kwargs
    
    
    Example::
    
        sinfos = get_streaminfos_matching(name="Liesl-Mock-EEG")
    """
    # find all available streams, check whether they are fitting with kwargs
    available_streams = pylsl.resolve_streams()
    fitting_streams = []
    if len(available_streams) == 0:
        return fitting_streams
    
    for st in available_streams:
        for k, v in kwargs.items():
            if eval("st." + k + "()") != v:
                break
        else:
            fitting_streams.append(st)

    return fitting_streams

@agricolab
Copy link
Contributor

After some consideration, i think i would agree with @jdevoldere that resolve_stream behaves unexpectedly. I would suggest to reimplement it in a manner that it does not block and returns immediately with an empty list if there is no stream fitting the arguments, whether arguments were supplied or not.

One alternative is to change the signature. In that case a signature and behaviour similar to queue.get, i.e. defaulting to queue.get(block=True, timeout=None). I am worried though that this will not be threadsafe in some environments which share the underlying dll. Therefore, a non-blocking default with an example in the docstring how to use this in a thread-safe blocking manner would be my suggestion.

@jdevoldere
Copy link
Author

@agricolab resolve_stream is a legacy function and is hence just in place to prevent older projects from breaking. So I think that rather than update resolve_stream we should just update the documentation (mainly the python examples).

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

No branches or pull requests

3 participants