search

alexandrebarachant / muse-lsl

Python script to stream EEG data from the muse 2016 headset
BSD 3-Clause "New" or "Revised" License
629 stars 184 forks source link

Error with `logging` fixed. #215

Closed aureateAnatidae closed 5 months ago

aureateAnatidae commented 6 months ago

Removed the timeout string from the logging.info() statement, preventing an exception. Adds previous connection timeout functionality. Fixed previous --timeout argument to use AUTO_DISCONNECT_DELAY as the #of seconds to wait until automatically disconnecting the device and ceasing stream, instead of the connection --timeout argument.

aureateAnatidae commented 6 months ago

By the way, the current pull request by Makowski adding the --lsl_time argument has no merge conflicts. Even if it adds a merge conflict for my --timeout argument, I'd rather have his added first. I think it's a lot more important.

jdpigeon commented 5 months ago

Thanks for turning me on to the issues with timeout while connecting to bands! Also, for pointing out the confusing variety of different timeout constants we had in the project.

I've just cleaned up our timeout code and merged in another feature from @narodnik that adds the ability to define a number of retry attempts when connecting to bands. In my hands, this seems pretty useful for helping ensure connection happens, and with the default of 1 retry, seems to be good enough to get bands connected the majority of the time.

I'm curious whether you think this new retry feature is good enough for your usecase. If so, we probably don't need to expose the timeout variable directly, as for bleak I believe the retries approach is a better way to address flaky connectivity.

aureateAnatidae commented 5 months ago

I agree actually, from my limited knowledge of bleak, retries would be better. Timeouts only helped on Debian for some reason (I do not know why at all), and retrying (after clearing the device data/cache) is a lot more consistent of a method on my current setup👍.