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

Optionally check for LSL stream name #51

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

charlesbmi
Copy link
Contributor

@charlesbmi charlesbmi commented Apr 18, 2023

In addition to checking for a matching LSL stream source ID.

This is useful in the case that a device uses its serial number as its source ID and generates more than one LSL stream: (e.g., a marker stream and a data stream coming from the same device).

This issue comes up with the Neurosity Crown, for example:

>>> import pylsl
>>> streams = pylsl.resolve_streams('eeg')
>>> [(stream.name(), stream.source_id()) for stream in streams]
[('Crown-459 Markers', '4599f267492346b97fda95666a33d868'), ('Crown-459', '4599f267492346b97fda95666a33d868')]

In this case, it is necessary to specify both the host name and source ID to select the correct stream.

Also extends the pylsl Stream connection timeout to the default of 1 second, as recommended by https://github.com/labstreaminglayer/pylsl/blob/master/pylsl/pylsl.py#L535-L538:

Warning: If this is too short (<0.5s) only a subset
(or none) of the outlets that are present on the network may
be returned. (default 1.0)

@teonbrooks
Copy link
Member

nice observation, good to know that was the source_id naming behavior for the Crown.

as an fyi, a convention we try to keep for the signature is that verbose is the last argument in the list. we bump it to the end when adding a argument.

I wonder if we should make the change to the _BaseClient instead of the LSLClient. I know that it would likely just be used by LSL clients but it would maintain the underlying object representation for all the different clients.

@charlesbmi
Copy link
Contributor Author

That sounds good to me!

mne_realtime/base_client.py Outdated Show resolved Hide resolved
mne_realtime/lsl_client.py Outdated Show resolved Hide resolved
@@ -106,14 +110,15 @@ def _connect(self):
pylsl = _check_pylsl_installed(strict=True)
print(f'Looking for LSL stream {self.host}...')
# resolve_byprop is a bit fragile
streams = pylsl.resolve_streams(wait_time=min(0.1, self.wait_max))
streams = pylsl.resolve_streams(wait_time=min(1.0, self.wait_max))
ids = list()
for stream_info in streams:
Copy link
Member

Choose a reason for hiding this comment

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

if I'm understanding this correctly then i would also create a separate error for when the host results in multiple instances and host_name is None

something like
if len(streams) != len(np.unique(streams)) and host_name is None:
ValueError('This LSL stream does not have a unique name, please also specify host_name')

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.

3 participants