-
Notifications
You must be signed in to change notification settings - Fork 87
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
Fix the macOS version for Monterey #68
Conversation
Did they break the API in Monterey? |
Kinda, they changed some threading related behavior I think.
I just tested it on an older macOS version, and it appears to be broken. I guess I could check on which version it is and use the old method on older macOS. |
Okay, it seems to be working correctly on both macOS versions (High Sierra and Monterey) now. |
It's weird to me that such a fundamental thing would break on macOS. I think that it must be that we're doing something wrong, that probably introduces additional bugs. Can we try to find a solution that works on both versions? |
Yeah it's indeed weird, even some Xcode provided bluetooth tools seem to be broken.
Yeah I guess that's a good idea, I will setup a High Sierra VM (to make testing a bit faster) and test a few more things. |
This didn't work, couldn't get bluetooth working in a macOS VM. However I did discover a solution that works on both versions, but wakes the cpu every second on Monterey (which should only be needed on High Sierra) std::unique_lock<std::mutex> lk(macOSBluetoothConnector->disconnectionMutex);
while (macOSBluetoothConnector->running) {
[[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:.1]];
macOSBluetoothConnector->disconnectionConditionVariable.wait_for(lk, std::chrono::milliseconds(1000), [&]() {
return !macOSBluetoothConnector->running;
});
}
lk.unlock(); I guess it is a decent solution for both versions. |
Not quite sure that I understand the macOS concurrency model, but it sounds like it works this way. Merging :) |
This brings back functionality, but it still consumes one full cpu core, so it isn't ready for merging. Closes #62