Thanks, @ScottThomasMiller, I will review it soon and will do some minor changes

Per Andrey, I will move all global functions in BoardShim.swift to static functions within the type definition.

I am working on the binding for perform_wavelet_transform in DataFilter.swift. The Java version returns a Pair type. The internet says a Java Pair is sort of like a Swift dictionary, with key/value pairs. But in your example code, you seem to use the return value like a normal tuple. Should my binding return a plain vanilla tuple, or a dictionary?

Its a tuple of two arrays

For perform_fft, which returns Complex[], I can think of three options off the top of my head:

1. Write my own Complex data type in Swift, modeling it after https://introcs.cs.princeton.edu/java/97data/Complex.java.html or similar.
2. Use the Complex data type already defined by Apple in the Swift Numerics package.
3. Simply return a tuple of [Double] arrays.

Option 1 will take some time. Option 2 will introduce a dependency on a package that is not included in the standard Swift library. Option 3 forces the caller to do their own numeric processing of complex numbers.

How does the process of installing new packages(dependencies) look like in Swift? If it's smth like in python or in java(declare them in files like setup.py or pom.xml and they are downloaded automatically) its ok to introduce Swift Numerics as a dependency.

I think we need to add smth like this https://developer.apple.com/documentation/xcode/creating_a_standalone_swift_package_with_xcode and declare it as a dependency in project file

It's pretty easy. In my Xcode project for my iOS app, I selected File-> Swift Packages->Add package dependency. And then I entered the Github URL for the Swift Numerics package: https://github.com/apple/swift-numerics.git. And then I followed the prompts to assign various dependencies. Xcode downloaded the package and set the build dependencies per the prompts.

And then, at the top of DataFilter.swift I added:

Import ComplexModule

Now I can declare the FFT function as returning an array of Complex Doubles, i.e.:

static func performFFT (data: inout [Double], startPos: Int32, endPos: Int32, window: Int32) throws -> [Complex<Double>]

I will look into the package bundling, per the link you sent.

Correction: I have to import the Numerics package itself, not ComplexModule. Otherwise it seems to be working fine.

Its fine. We need to create brainflow package like its described at the link I've shared and add it as a dependency

DataFilter.swift is 100% coded but only about 10% tested. I need help with its unit tests, specifically with designing a small handful of hardcoded input data arrays for which we know the expected output.

I am working on the package. It's not easy. It won't build without the BrainFlow libraries. I think I need to create a framework for the C++ libs, and then include that framework as a dependency in the package.

<< DataFilter.swift is 100% coded but only about 10% tested. I need help with its unit tests, specifically with designing a small handful of hardcoded input data arrays for which we know the expected output.

You should take a look at tests for other languages, you can find them here https://github.com/brainflow-dev/brainflow/tree/master/tests/python for example. Also, need to add it to CI pipelines https://github.com/brainflow-dev/brainflow/blob/master/.github/workflows/run_unix.yml

<< I think I need to create a framework for the C++ libs, and then include that framework as a dependency in the package.

No, you need to reuse CMake pipelines as for other languages and copy libs to a correct folder for swift. These libs should be packages together with swift binding

About copying libs, there are multiple files like build.cmake in different folders and there are lines like this one https://github.com/brainflow-dev/brainflow/blob/master/src/board_controller/build.cmake#L168 it should be added for swift. Easiest option to find them all is find . - name build.cmake

In band_power_all.py, what are the expected values for band[0] and band[1]?

If you use all eeg channels from synthetic board these values should be low

There's a bug in DataFilter.getAvgBandPowers(). It's returning empty arrays. I'm looking into it.

The bug was in my unit test, not in DataFilter. I updated my unit test, so that now, at the end of it, it does the following:

            let bands = try DataFilter.getAvgBandPowers(data: data, channels: EEGchannels,
                                                        samplingRate: samplingRate, applyFilters: true)
            
            let avgSum = bands.0.reduce(0, +)
            let stdSum = bands.1.reduce(0, +)
            XCTAssert((bands.0.count == 5) && (bands.1.count == 5) &&
                      (avgSum > 0) && (avgSum <= 1) && (stdSum > 0) && (stdSum < 10))

I ran the test successfully, 10 times in a row.

Good, thanks! Let's also add 3rd main component(MLModel) and I will help to update the packaging

Will do, as soon as I finish testing DataFilter.

In https://github.com/brainflow-dev/brainflow/blob/master/tests/python/transforms.py, the following call to perform_fft() seems to be missing start/stop or length parameters:

        # demo for fft, len of data must be a power of 2
        fft_data = DataFilter.perform_fft(data[channel], WindowFunctions.NO_WINDOW.value)

No, for python it gets them from the shape of ndarray. It's done because of some language-specific implementations of arrays in java and numpy

https://github.com/brainflow-dev/brainflow/blob/master/java-package/brainflow/src/main/java/brainflow/DataFilter.java#L440

here is a comment about it. In c\c++ or in python you can use pointer offsets or slices, in java I didn't find anything like that and decided to add offsets as parameters

I can rewrite DataFilter.performFFT() so that it is modeled after the Python, instead of the Java. Or I can overload it with both signatures.

Its up to you

I completed the initial phase of unit testing for DataFilter, using the Python tests as models. So far my unit tests cover only the following DataFilter methods:

deTrend
getAvgBandPowers
getBandPower
getCSP
getNearestPowerOfTwo
getPSDwelch
getWindow
performBandpass
performBandstop
performDownsampling
performFFT
performHighpass
performIFFT
performInverseWaveletTransform
performLowpass
performRollingFilter
performWaveletDenoising
performWaveletTransform
removeEnvironmentalNoise

I will start including my unit test modules in the next commit.

Is it unit tests or integration tests? We currently use tests as code samples at BrainFlow docs

What you see here https://brainflow.readthedocs.io/en/stable/Examples.html#python is the same as what we run in CI pipelines to test code. And code sample on the docs page should be more like an integration test

These are unit tests for Xcode. They test only that my bindings return data, and that the output data is different from the input data, except testBandPowerAll() which also compares averages. I am starting to include negative tests for specific exceptions.

Per your suggestion I followed https://github.com/brainflow-dev/brainflow/tree/master/tests/python. From that page I implemented the following:

testBandPower
testBandPowerAll
testCSP
testDenoising
testDownsampling
testSignalFiltering
testTransforms
testWindowing

For the CI pipeline should I follow the Examples.html#python page instead? Is it possible to use one set of tests for both Xcode unit and Github CI?

I have something working for CI that is ready to push. I added a new Xcode project in swift-package/BrainFlowCI. The command lines to run the unit tests look like this:

nice, feel free to push.

Also, there were some breaking changes in brainflow 5.0.0, in MLModule predict now should return an array, in bandpass\bandstop now we use start and stop freqs and wavelets are enums now

Also, there were some breaking changes in brainflow 5.0.0, in MLModule predict now should return an array, in bandpass\bandstop now we use start and stop freqs and wavelets are enums now

CI tests pushed, and merge conflicts resolved. I am looking at the 5.0.0 changes now.

#441 I tried to write them in description for this pr

How do I merge the 5.0.0 files with those in my branch? Like this?

https://stackoverflow.com/questions/33484153/branch-is-behind-main

in a folder where you cloned it run:

git remote add upstream https://github.com/brainflow-dev/brainflow
git fetch upstream
git merge upstream/master

It will show you some conflicts, need to solve them and run:

git add %file with conflict%
git merge --continue

I'm working my way down the list. I am at this one:

java: add method overloading for enums in addition to ints. e.g: BoardIds board_id = BoardIds.SYNTHETIC_BOARD, before it had to be int board_id = BoardIds.SYNTHETIC_BOARD.get_code()

I think we get that one for free with Swift. I can already declare smth like:

var boardId: BoardIds = .SYNTHETIC_BOARD

Do I need to do anything for this one?

No, it was only for java and its speciffic to enums in java, in other languages they are integers from the box

I see what I need to do for the last one, but do I need to worry about the next-to-last one:

cpp: methods for fft now return size

Its also not relevant

I finished coding the v5 upgrades and am almost finished testing them. My CI code throws an exception when trying to prepare an ML classifier, but only for specific combos of metric+classifier. The error is:

[2022-06-27 12:07:58.718] [ml_logger] [error] Unable to create Brainflow model params with these arguments. Exception: [json.exception.type_error.302] type must be string, but is null

good, I will review it one more time and this time more carefully. I want to merge your changes before #510 because it will require changes in Swift binding also and if we merge this PR first I will port them to swift by myself.

About metrics and classifiers, indeed not all combinations are supported, could you tell me which one you tried? Exception should be this one https://github.com/brainflow-dev/brainflow/blob/master/src/ml/ml_module.cpp#L67 and probably I need to add log message here. If you see an exception in json its a little strange

This one works:

let concentrationParams = BrainFlowModelParams(metric: .MINDFULNESS, classifier: .DYN_LIB_CLASSIFIER)

This one does not work:

let relaxationParams = BrainFlowModelParams(metric: .RESTFULNESS, classifier: .ONNX_CLASSIFIER)

its expected, onnx classifier and dyn lib classifiers can work only with USER_DEFINED metric.
But I dont like the error message you see, will fix it

Correction: none of them are working. I was mislead by Xcode. Not even the ones that should work, for example mindfulness+default.

it should work and tests in our languages are passed

It was my bug. Fixed. Upgrades complete, including the CI tests.

also, probably I missed it but I didnt see methods like release_all_sessions they should be in all 3 classes

I seem to have a lot of accidentally pushed flotsam in my branch. I apologize and will clean it all up shortly.

Everything under swift-package/BrainFlowCI)/BrainFlowCI.xcodeproj/ was created by Xcode and I assume should be in the repo.

Everything under swift-package/BrainFlowCI)/BrainFlowCI.xcodeproj/ was created by Xcode and I assume should be in the repo.

I thought some of shell scripts\commands generate it or xcode creates it automatically, in this case probably we dont need it in the repo

I had to merge one more change which will require swift package update. Here you can find more info https://brainflow.org/2022-07-15-brainflow-5-1-0/ and #510

CI is especially important now because I relocated and have no access to macos anymore... so its the only way for me to see that it actually works and passes tests