lihas / NeuroPy

NeuroPy library written in python to connect, interact and get data from neurosky's MindWave EEG headset.
88 stars 54 forks source link

Checks #7

Closed ChristopheBossens closed 8 years ago

ChristopheBossens commented 8 years ago

Included error handling for connecting to the serial port. Also included a different threading library which should work better

lihas commented 8 years ago
  1. Have you tested it on the headset? I do not have mine with me right now.
  2. Also in lines 80-86 of NeuroPy.py shouldn't the serial port be closed before stopping the thread? Apart from these two doubts, I see the changes are fine to merge.
ChristopheBossens commented 8 years ago

Hi,

  1. I've tested it with the headset and spotted some minor issues. I also noticed that for some reason I run in to problems when I plug the USB Dongle in a different USB port. Seems that on my windows computer this causes windows to assign default device drivers for the USB serial port. In that case, the program can make a connection to the serial port and some data is received but it doesn't look like anything that the mindwave is sending. Particularly, the 'aa' byte is never in there and this causes the program gets stuck in the loop on lines 107-109. I'll first try to fix this issue before sending a final version. Did you have any problems with assigning correct device drivers? I've been using the Mindwave in a course where I teach psychology students to program some basic experiments and they reported similar issues.
  2. I've noted that this approach can produce an error in the scenario where we the thread is in the loop that looks for 'aa' sequences. If the serial port is closed at this point, the thread is still trying to to read from the serial port and this gives an error.

Greets!

On 17 December 2015 at 10:19, sahil notifications@github.com wrote:

  1. Have you tested it on the headset? I do not have mine with me right now.
  2. Also in lines 80-86 of NeuroPy.py shouldn't the serial port be closed before stopping the thread? Apart from these two doubts, I see the changes are fine to merge.

— Reply to this email directly or view it on GitHub https://github.com/lihas/NeuroPy/pull/7#issuecomment-165391517.

lihas commented 8 years ago

I dont have the headset with me, therefore I cannot test it. I had used it few years back for a project, then this library had worked satisfactorily. I am merging the changes.