Add Swift bindings and sample apps#834
Conversation
Andrey1994
left a comment
There was a problem hiding this comment.
wrote some comments, but looks good overall, main concern is about packaging
| if: (matrix.os == 'macos-14') | ||
| run: | | ||
| cd $GITHUB_WORKSPACE/swift_package | ||
| BRAINFLOW_LIB_DIR=$GITHUB_WORKSPACE/installed/lib swift run brainflow-swift-cli |
There was a problem hiding this comment.
add all examples to CI as in brainflow dosc https://brainflow.readthedocs.io/en/stable/Examples.html#python
There was a problem hiding this comment.
Addressed in c030c76. I added all documented Swift examples as SwiftPM executable products and added a macOS CI step that runs them all with BRAINFLOW_LIB_DIR=$GITHUB_WORKSPACE/installed/lib swift run <example>.
| @@ -1,4 +1,9 @@ | |||
| sphinx==4.0.2 | |||
| sphinxcontrib-applehelp==1.0.2 | |||
There was a problem hiding this comment.
why is it needed? doesnt look relevant to the binding
|
|
||
| stopStream() | ||
| readData() | ||
| let rows = rowCount |
There was a problem hiding this comment.
for demo app for ias lets add a plot for eeg channels, also keep it under swift_package folder, dont add it to the root folder.
So folder structure will be like:
- swift_package
- brainflow
- examples
|
|
||
| private func startStream() { | ||
| do { | ||
| let board = try BoardShim(board_id: BoardIds.SYNTHETIC_BOARD) |
There was a problem hiding this comment.
make it possible to use non synthetic board, devices we care about are Muse boards(no BLED dongle but native BLE boards should work)
| @@ -0,0 +1,44 @@ | |||
| # BrainFlow iOS Demo | |||
|
|
|||
There was a problem hiding this comment.
dont create folders like samples in the root folder. Instructions to build and use the binding should be in Docs
| import BrainFlow | ||
|
|
||
| let data = Array(0..<256).map { sin(Double($0) / 10.0) } | ||
| let psd = try DataFilter.get_psd(data: data, start_pos: 0, end_pos: data.count, sampling_rate: 250, window: WindowOperations.HANNING.rawValue) |
There was a problem hiding this comment.
dont use synathetic data like this, get it from synthetic board instead. Check how code samples referenced from docs are structured and make it the same way for consistency. Update docs to include swift binding code samples
| @@ -0,0 +1,13 @@ | |||
| import BrainFlow | |||
|
|
|||
| var data = Array(0..<256).map { sin(Double($0) / 10.0) } | |||
There was a problem hiding this comment.
same here and for all other examples
| @@ -0,0 +1,5 @@ | |||
| import BrainFlow | |||
|
|
|||
| let data = Array(0..<256).map(Double.init) | |||
| init(names: [String]) throws { | ||
| var errors = [String]() | ||
| for path in Self.candidatePaths(for: names) { | ||
| if let handle = dlopen(path, Self.openFlags) { |
There was a problem hiding this comment.
its good, I like dlopen approach like in other bindings, but how does packaging work? native libs should be installed and compiled separately? Did you find a way to add them to the package like we do in jar and whl? How will it work for ios and publishing to appstore?
|
Addressed the review feedback in c030c76:
Local verification:
|
|
Clarification: the previous review-response comment was accidentally posted through a GitHub connector authenticated as |
|
@kcoul could you help reviewing this? I dont have enough swift experience, may overlap with the work you are doing |
ed38a5f to
60f1fc1
Compare
Andrey1994
left a comment
There was a problem hiding this comment.
we cannot have copies of all headers from C interface in XCFrameworks format, lets come up with the script(CMake commands) to automate this copying step and dont have it as a part of the repo
| @@ -0,0 +1,40 @@ | |||
| # Swift API Parity | |||
There was a problem hiding this comment.
why do we have docs in swift api package folder, they are not rendered anywhere. Move it please to docs in the root folder instead
| for (const std::string &candidate : get_dlopen_candidates ()) | ||
| { | ||
| return false; | ||
| lib_instance = dlopen (candidate.c_str (), RTLD_LAZY | RTLD_DEEPBIND); |
There was a problem hiding this comment.
all of this makes its unknown which library is actually loaded, lets add log messages at least with the full path for the lib
|
|
||
| private: | ||
| #ifndef _WIN32 | ||
| std::vector<std::string> get_dlopen_candidates () const |
There was a problem hiding this comment.
how does it work for windows? I see this method is called from another one, but for _WIN32 macro its not defined anywhere?
There was a problem hiding this comment.
to make it cleaner change it to smth like ifdef APPLE all this logic for different paths, else just normal library path as before
| @@ -0,0 +1,498 @@ | |||
| #pragma once | |||
There was a problem hiding this comment.
is it a copypaste from c++ code? why is it needed here?
| @@ -0,0 +1,61 @@ | |||
| #pragma once | |||
|
|
|||
There was a problem hiding this comment.
if we need headers from C++ in swift, we should handle it as a part of CMake call, do not copypaste files manually
| #pragma once | ||
|
|
||
| #include "shared_export.h" | ||
|
|
There was a problem hiding this comment.
same, move file copy to CMake call and dont copy them
| @@ -0,0 +1,8 @@ | |||
| #pragma once | |||
| @@ -0,0 +1,498 @@ | |||
| #pragma once | |||
60f1fc1 to
953d653
Compare
953d653 to
08f0601
Compare
|
Addressed the latest Apple packaging review in 08f0601. What changed:
Local verification:
Current GitHub Actions runs for 08f0601 are action_required pending maintainer approval for fork PR workflows. |
|
Follow-up in 0eef31b adapts the Apple artifact workflow to Apple's SwiftPM binary framework guidance. What changed:
Validation:
GitHub Actions for 0eef31b are still action_required pending maintainer approval for fork PR workflows. |
Summary
swift_package/examples/apps, App Store readiness notes, docs, and macOS CI coverage for Swift build/test/CLI/examples.Verification
python3 tools/build.py --clear-build-dir --num-jobs 4 --no-synchroni --no-bluetooth --no-ble --no-tests --cmake-osx-architectures arm64env BRAINFLOW_LIB_DIR=../installed/lib swift buildenv BRAINFLOW_LIB_DIR=../installed/lib swift testenv BRAINFLOW_LIB_DIR=../installed/lib swift run <example>env BRAINFLOW_LIB_DIR=../installed/lib swift run brainflow-swift-clixcodebuild -quiet -project swift_package/examples/apps/ios/BrainFlowiOSDemo/BrainFlowiOSDemo.xcodeproj -scheme BrainFlowiOSDemo -configuration Debug -destination id=22FB561A-9754-45E2-92CF-D16F1F84FAC4 -derivedDataPath /private/tmp/brainflow-ios-derived-review buildRows: 32,Samples: 533and rendered EEG plotmake html SPHINXBUILD=/private/tmp/brainflow-docs-venv-20260518/bin/sphinx-buildgit diff --checkRows: 32,Samples: 521Notes
dlopenand does not vendor native binaries; native BrainFlow libraries must be built separately and placed inBRAINFLOW_LIB_DIR, system library paths,installed/lib, or an app bundle resource/framework directory.