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

Updates on adding Synchroni Devices #754

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

Conversation

AranRhiod
Copy link

Adding support for the following Synchroni Devices:
Synchroni_Uno
Synchroni_Trio
Synchroni_Pento
Synchroni_Octo
OB3000

AranRhiod and others added 30 commits August 1, 2024 23:33
Requires information of boards,
Require setup of child board class and implementation of virtual methods
needbuild.cmakenew files and directories to target include directories
Need implementation of the Synchroni board class
added info and binding of:
Uno
Tro
Pento
Octo
Neo Synchroni board
TODO added
Changed boardIDs to be universal for synchroni devices.
update on read_data()
Signed-off-by: Andrey Parfenov <[email protected]>
Data parsing implmentation in progress.
stream ends abnormally with no data  during wait time with current command send for start_stream. checking command with hardware department
This reverts commit df382a8.
yecq and others added 10 commits December 25, 2024 16:22
Added information for Synchroni Pento, Uno device.

Board ID name change for distinguishing Octo and Pento 8 Channel device
Dropping tests that was not excluded from previous merge in fork
roping tests that was not excluded from previous merge
Copy link
Member

@Andrey1994 Andrey1994 left a comment

Choose a reason for hiding this comment

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

would be much easier wo need to hide communication protocol and just implement parsing in BrainFlow using BleLibBoard... but its fine, I think brainflow part for adding new board is ok

.gitignore Outdated
build/
*.DS_Store
/tryinstall/
Copy link
Member

Choose a reason for hiding this comment

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

this should be removed

Copy link
Author

Choose a reason for hiding this comment

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

removed and discarding changes resulted from changes in gitignore

@@ -1,4 +1,9 @@
cmake_minimum_required (VERSION 3.13)
if (APPLE)
Copy link
Member

Choose a reason for hiding this comment

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

why is it changed?

Copy link

Choose a reason for hiding this comment

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

Because MAC version requires XCFramework (new format of library), so it will require latest version of CMake

NEUROPAWN_KNIGHT_BOARD = 57
SYNCHRONI_TRIO_3_CHANNELS_BOARD = 58
SYNCHRONI_OCTO_8_CHANNELS_BOARD = 59
SYNCHRONI_PENTO_8_CHANNELS_BOARD = 62
Copy link
Member

Choose a reason for hiding this comment

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

60 and 61 are missed, see no reasons for such gaps, so please change board ids

Copy link
Author

Choose a reason for hiding this comment

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

fixed ID in new commit

NEUROPAWN_KNIGHT_BOARD(57);
NEUROPAWN_KNIGHT_BOARD(57),
SYNCHRONI_TRIO_3_CHANNELS_BOARD(58),
SYNCHRONI_OCTO_8_CHANNELS_BOARD(59),
Copy link
Member

Choose a reason for hiding this comment

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

60 is missed, such ids should match each other among languages otherwise it will not work

Copy link
Author

Choose a reason for hiding this comment

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

Fixed id in new commit

NEUROPAWN_KNIGHT_BOARD(57)
SYNCHRONI_TRIO_3_CHANNELS_BOARD(58)
SYNCHRONI_OCTO_8_CHANNELS_BOARD(59)

Copy link
Member

Choose a reason for hiding this comment

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

60 is missed

Copy link
Author

Choose a reason for hiding this comment

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

fixed ID in new commit

@@ -37,17 +37,22 @@ def main():
params.timeout = args.timeout
params.file = args.file
params.master_board = args.master_board

Copy link
Member

Choose a reason for hiding this comment

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

this file should not be changed

Copy link
Author

Choose a reason for hiding this comment

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

Changes removed

{"timestamp_channel", 4},
{"marker_channel", 5},
{"num_rows", 6},
{"eeg_channels", {1, 2}},
Copy link
Member

Choose a reason for hiding this comment

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

is it really ecg channel only? for the cases where electrode can be connected to everything and there is no way to know it in front we just write eeg_channels emg_channels and ecg_channels to the same values(e.g. openbci ganglion)

Copy link

Choose a reason for hiding this comment

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

We recommend user use according to IFU

std::tuple<std::string> info =
std::make_tuple (params.mac_address);
return func ((void*)&info);
// return s_close_device((void*)params.mac_address.c_str());
Copy link
Member

Choose a reason for hiding this comment

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

would be good to remove such comments here and in other places

Copy link
Author

Choose a reason for hiding this comment

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

CLeaned up synchroni_board.cpp

@@ -0,0 +1,205 @@
name: Cpp Release
Copy link
Member

Choose a reason for hiding this comment

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

all changes from simpleble should be removed from this change, it doesnt even use ble_board anymore

Copy link
Author

Choose a reason for hiding this comment

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

Resetting the SimpleBLE folder to #753.

@@ -0,0 +1,102 @@
//
Copy link
Member

Choose a reason for hiding this comment

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

is there any difference between all these macos headers for different archs? I guess no, so can the headers folder be made common for all of them?

Copy link

Choose a reason for hiding this comment

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

Because MAC version requires XCFramework (new format of library), this is part of released xcframework

@AranRhiod AranRhiod requested a review from Andrey1994 January 7, 2025 19:10
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