Skip to content
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

cleanup and fixes #2

Merged
merged 4 commits into from Apr 28, 2019

Conversation

Projects
None yet
2 participants
@pabs3
Copy link
Contributor

commented Apr 27, 2019

I can build the code on Linux with these changes.

pabs3 added some commits Apr 27, 2019

Drop using namespace std::placeholders
Causes build failures with gcc & clang.

The code doesn't seem to use bind or placeholders.
Return a value from Engine::messageRecieve
Fixes: warning: control reaches end of non-void function [-Wreturn-type]

@shayneoneill shayneoneill merged commit 361b258 into bort-orbital:master Apr 28, 2019

@shayneoneill

This comment has been minimized.

Copy link
Contributor

commented Apr 28, 2019

Noice!

@shayneoneill

This comment has been minimized.

Copy link
Contributor

commented Apr 28, 2019

The std::placeholders thing was for an aborted attempt to use std::bind to provide partial function application to wrap the callbacks to RTMidi. I ended up abandoning this approach because it was daft and kind of incompatible with C++ methods (I'm bad at C++) and instead resorted to passing structs with pointers to the good things instead.

@shayneoneill

This comment has been minimized.

Copy link
Contributor

commented Apr 28, 2019

One issue though, is although this should build on Linux, I actually think it won't work, as RTMidi hasn't got its backend setup correctly to link with the Linux Platform midi drivers. I'll have to come up with a test suite that can detect these. It'll be pretty necessary for further work anyway

@pabs3

This comment has been minimized.

Copy link
Contributor Author

commented Apr 28, 2019

@shayneoneill

This comment has been minimized.

Copy link
Contributor

commented Apr 28, 2019

The main reason its included is because to run it with DEBUG mode, it needs to be compiled with the required flags.

However at this stage it all kind of just works, so perhaps this could be reverted to the system RTMIDI version. I think the codes in the CMakeList file to do that fairly straight forward. Maybe this needs to be conditional as building installers for Windows and MacOS would require packaging the dynamic versions, but this is perhaps a separate issue.

Probably the more pressing challenge is getting the QT stuff to work without the crappy hardwired directories.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.