OpenBCI / OpenBCI_Cyton_Library

Repository for OpenBCI Cyton Arduino Libraries
MIT License
87 stars 88 forks source link

Allow writing to SD card with sampling rate over 250 #96

Open julfy opened 4 years ago

julfy commented 4 years ago

Description

Now, if communication is performed over bluetooth, board will force sampling rate to be 250 regardless of the rate that was set before. It was done because RFduino system can't handle amount of data generated on higher sampling rates. This hack, however, prevents data from being collected to SD card on higher sampling rates as well, even though the board can easily handle it.

This pr removes the SPS forcing and sets condition around streaming channel data to serial port allowing writes only if SPS == 250. There are some minor changes to doc and logging as well.

NB

Please don't forget to assign a proper version number, or let me know what it should be

rivasd commented 4 years ago

Hi! have you tested the stability of this over longer sessions (30min+)? I am very interested in this fix

julfy commented 4 years ago

@rivasd I have not, do you have a reason to think it might be a problem?

wjcroft commented 4 years ago

Julfy, thanks for submitting this.

Does your mod work also with the Daisy? It appears to be testing explicitly for 250 Hz sample rate?

William

julfy commented 4 years ago

@wjcroft I've tested it with daisy on 1000hz. It shouldn't matter whether daisy is attached or not I think, as curSampleRate is the same in both cases, it's just that the code does some sneaky downsampling

wjcroft commented 4 years ago

@andrewjaykeller, how does this PR look to you? A number of users on the Forum have requested this over the years. Also mentioning @conorrussomanno @produceconsumerobot

wjcroft commented 4 years ago

The most recent forum post. There are probably six other threads asking for a solution on this.

https://openbci.com/forum/index.php?p=/discussion/2435/over-250-sps-to-sd-card-using-bluetooth-dongle

rivasd commented 4 years ago

@julfy Well I know that, for example, space on the SD card is pre-allocated using a hardcoded 250Hz sampling rate, which we could also fix in this PR. I've also had a lot of glitches in general with SD card saving (aka most not working) over bluetooth with 32Gb SanDisk class 10 cards and have still not figured out why, but that was over long repeated sessions over a half day with three headsets in the same room

julfy commented 4 years ago

@rivasd I see. That indeed better be changed. Meanwhile, I've confirmed that the board has no problems with collecting the data for 35 minutes with Daisy attached on 1000hz. For the reference, 2HR file size allocation seems to be enough only for ~1160000 16 channel samples. I didn't test anything in between, but 12HR setting was enough for 35min 1000hz 16ch.

retiutut commented 4 years ago

@rivasd and @julfy Thank you for your work on this matter. We are busy working on the OpenBCI GUI at the moment, but I think this is an appropriate firmware update for the Cyton. Ty @wjcroft for bringing this to my attention today.

Is this PR "good to go"?

wjcroft commented 3 years ago

Another user just requested this. The PR needs a quick code review.

https://openbci.com/forum/index.php?p=/discussion/2699/wifi-shield-not-available-in-store-need-higher-sample-rate/

zeyus commented 2 years ago

I've been using this to successfully record data in combination with #102 with a sample rate of 1000Hz. I've tested it with the GUI as well and it behaves as normal, including when I manually specify the sample rate up to 250Hz (as expected with this PR).

One thing I noticed is that the GUI doesn't seem to do a soft reset / manually set the requested sample rate, which leads to a blank stream if the board had been used to record at a higher rate. This isn't an issue with the PR though, more a note that the GUI should probably include that as part of the initial connect via serial dongle.

A final note, and it might be something funky with my FW, but using BrainFlow before updating, I'd get responses from sent commands (unless streaming) and now it seems hit and miss. On this same note, I don't see a reason to prohibit responses via serial connection if "streaming" above 250Hz because there is no real stream data sent, which might mean it's nice to have a switch for starting stream + SD or SD only. Considering for me it's more important to know the status of the commands (including SD writing start, etc) I might look into adding this myself, just in the middle of exams at the moment so not sure of the timeframe.

Thanks!