brainflow-dev / brainflow

BrainFlow is a library intended to obtain, parse and analyze EEG, EMG, ECG and other kinds of data from biosensors
https://brainflow.org/
MIT License
1.3k stars 328 forks source link

Add swift binding #327

Open Andrey1994 opened 3 years ago

Andrey1994 commented 3 years ago

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

ScottThomasMiller commented 3 years ago

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?

Andrey1994 commented 3 years ago

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)

    public void prepare_session () throws BrainFlowError
    {
        int ec = instance.prepare_session (board_id, input_json);
        if (ec != ExitCode.STATUS_OK.get_code ())
        {
            throw new BrainFlowError ("Error in prepare_session", ec);
        }
    }
Andrey1994 commented 3 years ago

So I would say let's create BoardShim, DataFilter, and other classes like in existing bindings

Andrey1994 commented 3 years ago

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):

class BoardShimDynLibLoader {
    handler = nil;
    static func get_handler() {
        if (handler == nil)
            handler = dlopen("libBoardContoller.dylib", RTLD_NOW)
        return handler;
}

class BoardShim {
    func prepare_session() {
        typealias prepare_session = @convention(c) (CInt, Cchar *) -> CInt
        handler = BoardShimDynLibLoader.get_handler()
        var prepare_session = dlsym(handle, "prepare_session")
        if (prepare_session != nil) {
            let f = unsafeBitCast(sym, prepare_session .self)
            let result = f(board_id, params)
       }
    }    
}

Idea is to create a singleton that will call dlopen only once and find addresses of required functions manually. Is it worse or better?(I don't know swift best practices)

ScottThomasMiller commented 3 years ago

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".

ScottThomasMiller commented 3 years ago

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.

Andrey1994 commented 3 years ago

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 is_prepared from BoardShim class wraps method is_prepared from low-level API, you need to do the same

ScottThomasMiller commented 3 years ago

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?

Andrey1994 commented 3 years ago

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)

Andrey1994 commented 3 years ago

Also, you need to duplicate enums like BoardIds, etc. All of that is pretty small and simple except main classes(BoardShim and DataFilter)

ScottThomasMiller commented 3 years ago

Here's what I have so far: https://github.com/ScottThomasMiller/ScottsML/tree/main/Swift/BrainFlowBindings

Andrey1994 commented 3 years ago

Looks good so far!

ScottThomasMiller commented 3 years ago

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.

Andrey1994 commented 3 years ago

"I'm making them all "const char *" in board_controller.h instead."

Its not ok, some other bindings don't like const char *. Lets find a way to keep board_controller.h as is(add type casting somewhere on swift side)

Andrey1994 commented 3 years ago

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 char * in C header

ScottThomasMiller commented 3 years ago

Understood. I'll take a look.

ScottThomasMiller commented 3 years ago

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.

ScottThomasMiller commented 3 years ago

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?

Andrey1994 commented 3 years ago

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

Andrey1994 commented 3 years ago

More info if you are interested: https://brainflow.org/2021-03-09-new-major-version/

Andrey1994 commented 3 years ago

"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)

ScottThomasMiller commented 3 years ago

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 3.524325 1.759109 -0.000000 -1.844007 -3.426920 -13.467129 -5.613274 1.408918 8.663252 15.039456 25.759608 10.453843 -4.274830 -20.293409 -36.518251 -37.565274 -15.633888 9.210492 37.694390 57.364029 71.815144 36.607924 -16.633953 -52.961736 -81.124755 -80.163680 -42.131472 16.581405 73.079982 76.795649 103.085663 39.600036 -24.570546 -104.618697 -83.167381 -145.595706 -60.288164 49.365395 92.544312 81.063926 75.094505 62.654041 -34.156802 -145.984865 -55.713966 -154.178495 -120.002921 61.347603 192.979051 25.696258 167.315720 142.702001 -41.541240 -212.566271 19.168347 -103.980679 -190.077269 60.032875 162.163131 -79.682029 138.539331 188.915072 -126.283773 -70.084075 161.071326 -49.621182 -255.658966 42.593981 120.334711 -172.826360 18.479719 280.558184 -107.601329 -134.791289 88.188602 6.314293 -107.612166 111.620795 17.905098 -113.680189 0.962317 0.981579 0.918030 0.800336 0.999687 0.882748 0.990145 0.992720 0.860554 0.829110 0.930930 0.922554 0.806216 0.938207 0.963169 0.919554 0.833327 0.924387 0.995097 0.818384 0.822444 0.968036 0.898841 0.996125 0.838067 0.831681 0.943121 0.962370 0.885325 0.996443 1.080530 1.061077 0.940015 1.095128 1.063627 5432.291190 5411.946023 4528.449774 5425.031334 5443.923847 4680.233026 4931.376847 4703.288257 4516.483963 5402.541694 36.593704 36.602607 36.596204 36.605233 36.601470 1079.024751 982.201564 956.497338 973.923423 1078.052445 990.825113 1073.140646 1009.932247 1022.242693 1039.514976 85.979647 94.859912 84.274026 81.926733 94.004491 1629995103.728148 1629995103.731914 1629995103.736948 1629995103.740714 1629995103.745530 0.000000 0.000000 0.000000 0.000000 0.000000

And here is the output from my Swift code:

100.00000 101.00000 102.00000 103.00000 104.00000 -0.00000 -1.69165 -3.44543 -4.99500 -6.62305 1.44753 8.28995 14.16592 18.66274 24.80209 -4.34256 -20.31284 -28.98679 -38.16113 -37.66824 10.10112 37.05097 56.79074 59.00251 49.33110 -13.89766 -40.02346 -85.40910 -53.10395 -22.44765 15.85307 71.53188 92.72715 47.15099 -12.59683 -36.72619 -107.82567 -85.19615 -23.98563 51.49561 33.87915 152.06859 56.80625 -23.14171 -99.88292 -38.42393 -166.58373 -80.56335 88.75330 134.13172 50.69680 81.78940 35.05108 -97.93932 -73.78425 -73.65167 -203.25009 14.83315 190.58374 19.88039 123.58570 184.52759 -109.25622 -174.56790 28.48647 -58.28310 -147.67590 200.37148 175.14706 -140.58163 42.38645 127.63895 -263.29436 -66.21680 320.38816 -213.30514 -70.51221 116.57182 -25.88802 -137.70446 84.85873 18.95906 -127.68043 151.27837 151.23315 0.91632 0.89045 0.96501 0.85506 0.99279 0.89318 0.95777 0.92006 0.98815 0.93787 0.99624 0.95188 0.87109 0.92150 0.92891 0.80789 0.80934 0.86136 0.90575 0.91562 0.93334 0.91119 0.84393 0.97291 0.84798 0.99401 0.98418 0.89540 0.96639 0.88983 1.02263 1.09288 0.94792 0.92615 1.07662 4576.38763 5264.11655 5197.22065 4898.32205 5393.38484 5318.84261 5102.69790 5163.85087 5157.62249 5268.54776 36.60431 36.59329 36.60232 36.60342 36.60356 931.45280 952.83389 1080.53608 1041.21333 1031.47774 922.83384 1055.93884 953.47350 976.94542 1008.56613 94.12669 89.46505 93.62441 96.79821 93.19209 1629995240.96847 1629995240.97224 1629995240.97725 1629995240.98102 1629995240.98603 0.00000 0.00000 0.00000 0.00000 0.00000

Andrey1994 commented 3 years ago

Yes, that's 100% correct!

ScottThomasMiller commented 3 years ago

Nice. What does the first row signify?

Andrey1994 commented 3 years ago

package_num, for real boards it can be used to track package loss

ScottThomasMiller commented 3 years ago

And the 2nd-to-last line:

1629995240.96847 1629995240.97224 1629995240.97725 1629995240.98102 1629995240.98603

Are those the timestamps?

Andrey1994 commented 3 years ago

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

ScottThomasMiller commented 3 years ago

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:

func streamEEG() {
    do {
        print("preparing session")
        try board.prepareSession()
        print("setting gain to x1")
        try setGain(setting: .x1)
        while try !board.isPrepared() {
            sleep(1)
        }
        try board.startStream()
        print("streaming EEG")
        while true {
            let matrix = try board.getBoardData()
            guard matrix.count > 0 else {
                continue
            }
            print("matrix dim: [\(matrix.count),\(matrix[0].count)]")
            print(matrix)
        }
    }
    catch {
        try? logMessage (logLevel: LogLevels.LEVEL_ERROR.rawValue, message: error.localizedDescription)
        try? board.isPrepared() ? try? board.releaseSession() : print("session already closed")
    }
}
ScottThomasMiller commented 3 years ago

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.

ScottThomasMiller commented 3 years ago

Oh: also I had moved the setGain call to after the isPrepared loop. That might have something to do with it.

Andrey1994 commented 3 years ago

"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

Andrey1994 commented 3 years ago

Do you have plans to push swift binding to brainflow upstream and\or add other methods from BoardController(and probably DataFilter)?

ScottThomasMiller commented 3 years ago

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.

Andrey1994 commented 3 years ago

Great, thanks!

ScottThomasMiller commented 3 years ago

Your "Java Signal Filtering" example contains the following line of code:

                DataFilter.perform_lowpass (data[eeg_channels[i]], BoardShim.get_sampling_rate (board_id), 20.0, 4,
                        FilterTypes.BESSEL.get_code (), 0.0);

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?

Andrey1994 commented 3 years ago

yes

ScottThomasMiller commented 3 years ago

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]?

Andrey1994 commented 3 years ago

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.

ScottThomasMiller commented 3 years ago

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.

Andrey1994 commented 3 years ago

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 https://github.com/brainflow-dev/brainflow/issues/108 but have no time and don't fully understand how to make it the same for all bindings

Andrey1994 commented 3 years ago

Hi, how is it going? If you need some help you can create a WIP PR

ScottThomasMiller commented 3 years ago

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.

ScottThomasMiller commented 3 years ago

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?

Andrey1994 commented 3 years ago

instance.get_num_rows is a method from low-level library(BoardController) and this method is called from BoardShim.get_num_rows. Later on in user-code and\or in other methods from BoardShim class you can call BoardShim.get_num_rows while instance object is private and not for end users

ScottThomasMiller commented 3 years ago

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.

Andrey1994 commented 3 years ago

<<I'm not yet sure how to do the same in Swift, nor am I certain it's necessary.

This stuff with instance in java is kinda bridging header in swift. It describes the interface of BoardController.dll and allows you to call C\C++ methods. You don't need that.

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

ScottThomasMiller commented 3 years ago

In my fork should I duplicate ./brainflow/settings.xml and ./brainflow/pom.xml?

Andrey1994 commented 3 years ago

no, these files are specific for maven packaging(like setup.py in python)

ScottThomasMiller commented 3 years ago

I created my fork and within it a branch named swift-bindings. I cloned that branch to my laptop, added my .swift files, committed and pushed them, and now am ready to create my pull request. How do I make it WIP?

Here's a screenshot:

image

Andrey1994 commented 3 years ago

Green Button "Create pull Request" click on triangle close to it