Support finding a compiled api with API::UNSPECIFIED

[ejconlon]

trafficstars

Hello, I'm working on Haskell FFI bindings for this library and found that using API::UNSPECIFIED doesn't quite behave as I expect. Creating an observer simply fails, rather than picking a valid API. Here's a diff that works for me:

diff --git a/libremidi/vendor/libremidi/backends.hpp b/libremidi/vendor/libremidi/backends.hpp
index a101f46..bbf7161 100644
--- a/libremidi/vendor/libremidi/backends.hpp
+++ b/libremidi/vendor/libremidi/backends.hpp
@@ -156,7 +156,7 @@ template <typename F>
 auto for_backend(libremidi::API api, F&& f)
 {
   static constexpr auto is_api
-      = [](auto& backend, libremidi::API api) { return backend.API == api; };
+      = [](auto& backend, libremidi::API api) { return backend.API == api || api == libremidi::API::UNSPECIFIED; };
   std::apply([&](auto&&... b) { ((is_api(b, api) && (f(b), true)) || ...); }, available_backends);
 }
 }

Is this the solution?

Thanks!

[jcelerier]

~hello ! I actually pushed some commits that change this behaviour this very morning, could you try on the latest commit if that was not the case?~

nevermind, I didn't push the change for observers

[jcelerier]

Patch looks sound nevertheless, will apply

[jcelerier]

btw, are you ok with me linking the bindings in the README ? :)

[ejconlon]

Thanks for addressing this!

Linking is fine with me.

Would you be open to exposing more methods in the C API? For example, finding which APIs are available, and what their names are. At the moment I’m trying to but the new bindings to use, but I might have time later to send a patch for those new methods.

[jcelerier]

yes definitely, the C api is very provisional right now and not stable yet. For instance all the names aren't homogenous. Stable version will be part of the 5.0 release.

[jcelerier]

heya! I just added some of the requested methods. there will still be a bit of breakage before stable release..

[ejconlon]

Thank you. I am still wrangling foreign pointers and the Haskell side! Will pull and try the new methods soon.

[jcelerier]

heya :) did you have time to try ?

[ejconlon]

Ah sorry I will do it this week and close the issue if it’s all good!

[ejconlon]

Ok - get_version works but I think the libremidi-c.h and cpp files are out of sync:

LIBREMIDI_EXPORT
void libremidi_midi1_available_apis(void* ctx, void (*)(void*, libremidi_api));
LIBREMIDI_EXPORT
void libremidi_midi2_available_apis(void* ctx, void (*)(void*, libremidi_api));

vs

void libremidi_available_midi1_apis(void* ctx, void (*cb)(void* ctx, libremidi_api))
void libremidi_available_midi2_apis(void* ctx, void (*cb)(void* ctx, libremidi_api))

Let me know which naming you prefer and I'll submit PR to fix. (My slight preference is *available_midi*)

[ejconlon]

Also, I think libremidi-c.cpp:75 needs to test for string equality in the old c way:

if (strcmp(b.name, name) == 0)

strcmp is a smell but maybe it's ok if the known-good string is first argument?

[ejconlon]

With those changes, my test of the bindings passes: https://github.com/ejconlon/libremidi-haskell/blob/master/libremidi/test/Main.hs#L14

[jcelerier]

libremidi_midi1_available_apis

this one, as it mirrors the C++ namespaces libremidi::midi{1,2}

thanks for noticing the bug in backend name comparison!

that said for it I think it'd be better to have the backend_name be a std::string_view in the C++ side, it can still be constexpr and will avoid this sort of bug entirely. I'll do that patch :)

[jcelerier]

Merged the string_view change

[ejconlon]

LGTM thank you! Released https://hackage.haskell.org/package/libremidi

[jcelerier]

amazing ! i'll add it to the README