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

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

julfy
Copy link

@julfy julfy commented Jan 20, 2020

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
Copy link

rivasd commented Jan 21, 2020

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

@julfy
Copy link
Author

julfy commented Jan 21, 2020

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

@wjcroft
Copy link

wjcroft commented Jan 21, 2020

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
Copy link
Author

julfy commented Jan 21, 2020

@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
Copy link

wjcroft commented Jan 22, 2020

@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
Copy link

wjcroft commented Jan 22, 2020

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
Copy link

rivasd commented Jan 22, 2020

@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
Copy link
Author

julfy commented Jan 23, 2020

@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 retiutut self-assigned this Apr 16, 2020
@retiutut retiutut self-requested a review April 16, 2020 19:30
@retiutut
Copy link
Member

@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
Copy link

wjcroft commented Sep 16, 2020

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/

@retiutut retiutut removed their assignment Feb 23, 2021
@zeyus
Copy link

zeyus commented May 3, 2022

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants