-
-
Notifications
You must be signed in to change notification settings - Fork 335
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
Add swift binding #327
Comments
I am trying to create a bridging header for Swift 5 and Xcode 12.5.1. Must my Swift code instantiate the BoardShim object, or can it just call prepare_session() and start_stream() directly? |
The idea of bindings is to provide high-level API which is easier to use than plain C methods exposed from low-level library. So, we created BoardShim class for each binding. Users are expected to work with BoardShim methods instead calling low-level prepare_session and other methods directly. Example from Java:(https://github.com/brainflow-dev/brainflow/blob/master/java-package/brainflow/src/main/java/brainflow/BoardShim.java)
|
So I would say let's create BoardShim, DataFilter, and other classes like in existing bindings |
I dont know swift but why do we need a bridging header at all?(and what is it?) I think it can look like this(pseudocode inspired from python binding):
Idea is to create a singleton that will call |
The bridging header tells Swift how to resolve the object names in your Swift code. Without it you get errors such as, "Cannot find 'prepare_stream' in scope". It's typically a .h file which simply includes other .h files. You have to register it in the Xcode build settings, so that Swift knows where to look. Your example is similar to this: https://stackoverflow.com/questions/35229149/interacting-with-c-classes-from-swift I should be able to implement something like the that, because I already have all of BrainFlow's C++ headers. I am still learning Swift, but I think that your idea is basically a wrapper, and wrappers are very "Swifty". |
I managed to call the low-level functions directly, such as prepare_session() and get_eeg_channels(), but so far I am unable to instantiate BoardShim(). I need some help. Everything I've found so far, I can't get working. |
You should not call methods from C++ BoardShim class. This class even doesn't exist in low-level API(low-level API is for bindings, high-level API is for users). Instead, you need to create a new class in Swift and call it BoardShim, inside this class you need to wrap methods from low-level API. Take a look at the cpp-package for reference. https://github.com/brainflow-dev/brainflow/blob/master/cpp-package/src/board_shim.cpp#L104 Here method |
OK I think I finally understand: you want me to write BoardShim.swift modeled after board_shim.cpp. That seems completely doable. I will also have to write swift bindings for all of BoardShim's dependencies, such as BrainFlowException, BrainFlowArray, BrainFlowInputParams, etc. And nowhere in all of those bindings will I need to instantiate any C++ classes, correct? |
Correct, but probably you dont need BrainFlowArray(its specific for c++ for better perf, you can do the same as in java for example). Also, there are two more classes: DataFilter and MLModule, its the same there.(I can help and do it for some of them) |
Also, you need to duplicate enums like BoardIds, etc. All of that is pretty small and simple except main classes(BoardShim and DataFilter) |
Here's what I have so far: https://github.com/ScottThomasMiller/ScottsML/tree/main/Swift/BrainFlowBindings |
Looks good so far! |
Thanks. Swift doesn't like binding its String type to "char *" parameters in its bridging headers, so I'm making them all "const char *" in board_controller.h instead. Also I think Swift will automatically make BoardIds iterable if I add the CaseIterable protocol to its type definition. I'll try that shortly. |
"I'm making them all "const char *" in board_controller.h instead." Its not ok, some other bindings don't like |
I cannot say that I fully understand what exactly they are doing here https://forums.swift.org/t/how-to-pass-swift-string-to-c-function-in-swift-5/28416 but seems like its doable with |
Understood. I'll take a look. |
I reverted board_controller.h to its original state. For the board parameter string I am now converting the JSON string inside BoardShim.init() like so: self.cParams = UnsafeMutablePointer<CChar>(mutating: jsonParams) And then I pass cParams into the low-level functions. It seems to be working OK. Swift gives me a warning about creating a dangling pointer, at the preceding line of code. From what I've read, if the low-level functions never modify or destroy the JSON parameters, then we can safely ignore the warning. |
I need some more help, this time with BrainFlowArray. My C++ is weak. I cannot translate BrainFlowArray into Swift all by myself. From what little I do understand of it, BrainFlowArray provides various matrix operations, including reshaping the input buffer in a sort of numpy fashion. For the app I am building, all I need is a 2D matrix of doubles, so I will focus on that initially. For testing purposes, to make sure I reshape the buffer correctly, what do you suggest? Is there a simulator mode I can use to compare results against? |
You dont need to port BrainFlowArray to Swift, its numpy-like optimization specific for C++. Key idea there is memory layout(2d array is 1d array in fact) You can do the same as in java https://github.com/brainflow-dev/brainflow/blob/master/java-package/brainflow/src/main/java/brainflow/BoardShim.java#L779 there its just standard 2d array |
More info if you are interested: https://brainflow.org/2021-03-09-new-major-version/ |
"For the app I am building, all I need is a 2D matrix of doubles, so I will focus on that initially. For testing purposes, to make sure I reshape the buffer correctly, what do you suggest?" Run it using synthetic board and check that rows are correct(first row is package num, etc) |
Using the synthetic board, I am comparing my output against the output of your C++ example. It looks like I am reshaping my buffer correctly. In both cases we are using get_current_board_data(5). Here is the output from your C++ example: 98.000000 99.000000 100.000000 101.000000 102.000000 And here is the output from my Swift code: 100.00000 101.00000 102.00000 103.00000 104.00000 |
Yes, that's 100% correct! |
Nice. What does the first row signify? |
package_num, for real boards it can be used to track package loss |
And the 2nd-to-last line: 1629995240.96847 1629995240.97224 1629995240.97725 1629995240.98102 1629995240.98603 Are those the timestamps? |
Yes, you can call method get_board_descr to get info about these rows programmatically, or take a look at https://github.com/brainflow-dev/brainflow/blob/master/src/board_controller/brainflow_boards.cpp |
I am testing my Swift bindings by streaming EEG data from my OpenBCI Cyton+Daisy. All of the package IDs are coming back even-numbered, as if I am missing every other sample. Do I have another bug? The issue does not occur with the synthetic board. Here's my code:
|
The headset battery died, so I swapped it out for a freshly recharged battery, re-ran my test, and the issue went away. So: next item on my to-do list is to add code to check the battery level! Also, when the issue was happening, my calls to set the gains were causing BrainFlow to log warnings about rescaling the data, something about how your code uses x24 to convert int to uV. But with the fresh battery, those warnings are no longer printed. |
Oh: also I had moved the setGain call to after the isPrepared loop. That might have something to do with it. |
"Also, when the issue was happening, my calls to set the gains were causing BrainFlow to log warnings about rescaling the data, something about how your code uses x24 to convert int to uV. But with the fresh battery, those warnings are no longer printed." It cannot be so, these warnings doesn't depend on it and printed everytime. About even numbers, it can be ok for daisy, it sends 2 packages and probably I keep it as is |
Do you have plans to push swift binding to brainflow upstream and\or add other methods from BoardController(and probably DataFilter)? |
Yes. I plan to push my bindings when they are 100% coded and tested. So far I have coded 27 BoardShim functions, plus all the enums, the BrainFlowInputParams, and BrainFlowException. Next up will be DataFilter. |
Great, thanks! |
Your "Java Signal Filtering" example contains the following line of code:
The example fetches 30 samples. In the preceding line of code, does data[eeg_channels[i]] represent a 30-element vector of doubles for the ith channel? |
yes |
Follow-up question: in the preceding example, data is a 2D array of doubles, which is why you are iterating over each channel, calling the filter once for each channel. Is it OK if I instead use a slice of the 1D buffer, before it's been reshaped, to call the filter only once for all channels? For example, if I have 10 samples and 16 channels, then the first 10 elements of the 1D buffer are package IDs, and the next 160 elements are EEG channel data. Is it OK to call the filter just once using data[10...169]? |
No, it's not ok. These 2 APIs are independent and BoardShim should return 2d array while existing methods for signal processing operate on a 1d array. Also, from the user's point of view, it allows you to apply different filters for different channels. |
I see what you mean, and indeed I plan to provide matching functionality in my bindings. But what I would like to also do is to overload get_board_data() with a second function which returns the 1D buffer. Users will still be able to call the original function and get the 2D array. The two APIs will remain independent. My question is more about whether the filter will still return the correct values if I pass in all channels at once. |
For correct implementation of 1d array it should be like in C++ binding, we can add it later on. For beginning, I think we should make it like in java. As is filter function will not understand that its multiple arrays joined together in 1d array, it will treat it as a single 1d array and will generate wrong result. To fix it method signature for filtering should be changed. I was going to implement smth like that before #108 but have no time and don't fully understand how to make it the same for all bindings |
Hi, how is it going? If you need some help you can create a WIP PR |
It's going well. BoardShim is completely coded, but not yet 100% tested. I'm working on building a unit test module for it. DataFilter is only about 25% coded. |
Actually I do have one remaining question about BoardShim: in your BoardShim.java, the get_num_rows binding calls instance.get_num_rows, but the get_board_data and get_current_board_data bindings call BoardShim.get_num_rows. Why is that? |
|
I'm not yet sure how to do the same in Swift, nor am I certain it's necessary. I am ready to push BoardShim.swift and its dependencies. Please give me instructions for committing and pushing. |
<<I'm not yet sure how to do the same in Swift, nor am I certain it's necessary. This stuff with <<I am ready to push BoardShim.swift and its dependencies. Please give me instructions for committing and pushing. Create WIP PR from non-master branch of your fork and if you see a checkbox like "allow edit to maintainers" click on it. So, I will be able to make changes and help you a little. Make sure that swift binding matches the common structure in terms of folder and file names. Would be good to add a few tests to CI pipelines but it can be done later on |
In my fork should I duplicate ./brainflow/settings.xml and ./brainflow/pom.xml? |
no, these files are specific for maven packaging(like setup.py in python) |
Green Button "Create pull Request" click on triangle close to it |
"Create draft pull request"? |
yes |
Because of the Swift scope rules, I had to move the logger functions of DataFilter inside its type definition. Otherwise it would have caused name collisions with the analogous BoardShim functions. For consistency I should probably do the same for the BoardShim logger functions. What do you think? |
I think all methods like |
In fact there should not be any global functions, only class methods and static class methods |
And lets move this discussion to PR comments |
Use other languages(java, c#, python) as an API reference.
Useful link: https://stackoverflow.com/questions/34669958/swift-how-to-call-a-c-function-loaded-from-a-dylib
The text was updated successfully, but these errors were encountered: