studio
studio copied to clipboard
Update Brainflow from 4.0.1 to 5.7.0
I've been advised by that in order to support FreeEEG32 properly we need to use a version of Brainflow at least past this PR back in 2021.
I've therefore bumped the dependency to the latest release. Wee bit concerned about the build system long-term, copying files into the source tree manually and then ironing out the issues, but leaving that aside :sweat_smile:.
My FreeEEG32 board still doesn't connect properly, but thanks to improved logging I get a message from Brainflow regarding JSON, so I will diagnose that and open a separate PR.
I started from 1.7.1 as I wanted to build the latest stable release, and thought this might be a suitable candidate for a 1.7.2, but afraid only afterwards realised I probably should have worked off the master to ease merging.
@stellarpower Thanks a lot for doing this. Updating brainflow is high on our agenda for this year. I will try to have a more detailed review soon. For now I spotted these things:
- Please remove all changes on the Github CI files
- Please check changes on file
brainflow/utils/shared_export.h
. Last time I explicitly removed the exports there or else the brainflow exports showed up on our executable like on the brainflow shared library. This PR here adds them back in. At least last version of brainflow did not seem to fully support or at least not favor static linking (in terms of more nicely controlling the export configuration) - I don't see any changes on the Visual Studio project files. Any chance you can update those as well?
- Please don't just add all brainflow third party stuff directly into
deps/include/brainflow/third_party
. (a)kissfft
andjson
are already indeps/include
and should be shared from there (e.g. we use it as well) (b) if there are new dependencies, please add them todeps/include
folder as well (c) if prebuilt binaries are absolutely unavoidable (=closed source), please add them to the appropriate platform folders indeps/prebuilt
. You can find some closed source SDK libraries there already. - Please try to regularly pull from our
master
branch to keep things up to date.
Again, thanks a lot for your work!
Am wondering, is it easier if I alter the MR to be pulled into a feature branch, and that way you can use what's there as a springboard, and if you know what you wanna do or how you want things set out, you can work on it directly? And then I can also pull back from there to my fork to make any edits there and then submit those back. And then there's potentially only one set of conflicts to fix when you're happy with how things are and ready to merge it all back into the master?
(1) was just as I was trying to get it to build on Windows for someone who uses it there, so I've pushed those off to a branch on the side now. On (3) also, am not a Windows user, so not sure if there's much I can usefully do.
Longer-term, I think I'd quite like to take a look at the build and dependency setup, as I think the way things are now probably all creates quite a bit more work than is needed. Ideally I think bumping a dependency should be a new version in one or two lines of configs and then just any breaking changes this new versions introduces that need fixed in our code. So I don't know if you have any opinions on this, or requirements for how things need to work at neuromore, but I was going to suggest using something similar to how I have set some of my own projects out.
Earthly + VCPkg is a combination I've used before with pretty good success. VCPkg is also designed around building self-standing static binaries, which seems to suit how Studio is set up presently. Earthly is a tool that tries to bridge the world between CI and local development, so that you get reproducible builds both when working on code on your local machine as well as then building in the cloud. This also extends to cross-compiling, something I've not tried yet but this was the reason for the junk commits under (1), as I couldn't get the CI going for various reasons. I have also hit problems that were hard to reproduce when running parallel builds using make, and I think this is a known issue, with how make by default splits up jobs and carries on even if one has failed - so I think it would probably make sense to migrate to a higher-level build system. But I understand this may complicate things for integration with Windows/MSVC.
So at a high-level the rough process I am thinking of would involve:
- Replacing the hardcoded makefiles with a more modern buildsystem * CMake would be an obvious choice * However there are more modern alternatives to this that make life easier available. I am looking to give xmake a run for its money in the near future myself.
- Using a package manager for dependencies and removing third-party files that are checked in at specific versions and then altered: * Bumping any package version should be as simple as e.g. changing a version string in some JSON * I'd need to analyse how many of Neuromore's dependencies are available in repositories, but I think if any are not available, provisioning these as packages should be no more work than it is now manually integrating these into studio, and then the wider community can benefit aswell from the recipe. * This is also where xmake may help, as it can pull dependencies from all the major package managers (vcpkg, conan, build2) as well as its own repository
- Using earthly as an option for performing local builds, and using this as the main mechanism for the cloud build * Earthfiles are similar to Dockerfiles, with added features, and Earthly has been designed for very simple integration with CI pipelines, so most of the work on this is basically already done within the GitHub actions you guys have. * And ideally this would make it relatively trivial to perform a reproducible buuld on a development machine, possibly for a differing OS, to check that nothing in the local setup is impacting the build process and thus make it more likely CI checks will pass upon pushing up.
Do you have any thoughts on all this? Appreciate it's quite a major refactor I'm suggesting, but at the same time, I think as you mentioned, if upgrading a version is a task that ends up on the backlog list, IMO it hopefully should be worth it to make those sorts of tasks much quicker to tick off.
Cheers!
Also, FYI - I believe this bug in brainflow is affecting Neuromore builds on linux right now - my FreeEEG32 is inaccessible when using the packaged versions. I'll wait and see what Andrey says, as I'm not sure what is at fault, but if it is confirmed and gets fixed upstream in some form, then it would probably make sense to wait on a slightly newer release of Brainflow before integrating it all manually. If not, it owuld be possible to workaround by symlinking or update-alternatives or similar in the CI build.
Above bug in brainflow is now closed. We should consider waiting for the next suitable release of Brainflow and then pulling in any changes from there, as this issue broke CI builds for linux and serial devices.
@cyberjunk are you okay if I change this PR to point to a feature branch, and then it can be reworked as necessary before considering any inclusion back into your upstream master?