-
Notifications
You must be signed in to change notification settings - Fork 46
Update Brainflow from 4.0.1 to 5.7.0 #280
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
base: master
Are you sure you want to change the base?
Update Brainflow from 4.0.1 to 5.7.0 #280
Conversation
…o Brianflow include folder.
…e upstream sources. Some devices have been refactored away since 4.0.1 release last incorporated into Neuromore.
…e upstream sources. Some devices have been refactored away since 4.0.1 release last incorporated into Neuromore. The ML pipeline looks to have been reworked.
|
@stellarpower
Again, thanks a lot for your work! |
49b2b37 to
db9fd1b
Compare
|
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:
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? |
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 😅.
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.