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

Various fixes #108

Merged
merged 7 commits into from
Jul 1, 2022
Merged

Various fixes #108

merged 7 commits into from
Jul 1, 2022

Conversation

reaperhulk
Copy link
Contributor

This PR makes a variety of changes to the project, some of which may be desirable, some of which could be better. I've tried to separate each commit so they can be cherry-picked or improved as you desire. 😄

The changes:

  1. Resized the menu bar images so they appear the correct size. When the new images were committed several years ago they were apparently contributed at 100px/200px/300px height. This looks great, but macOS (at least as of Xcode 13.2.1 in macOS 12.1) doesn't properly scale them. So instead we scale them to 22px/44px/66px respectively.
  2. Fix the submodule commit hash (and extraneous submodule that used a relative path) to point at an existing commit that is compatible with the current project source. I did not upgrade to latest here.
  3. Added a bit of debug code in various places just so I could understand the errors I was seeing better.
  4. Switched from xpc_connection_send_message_with_reply to xpc_connection_send_message_with_reply_sync because it appeared there was a race condition in the async code in the action selector assuming xpc_connection_send_message_with_reply immediately returns. Maybe I'm wrong about this though (please let me know if so!)
  5. Updated a few calls to non-deprecated versions. There are a variety of other project settings that Xcode would like to update, but that's a much larger change so I've deferred it for now.
  6. Fixed helper crashes caused by multiple calls to IOHIDManagerScheduleWithRunLoop with the same hidManager. In macOS 10.15+ this causes an assertion failure.
  7. Added a very primitive dialog that tells you yubiswitch requires Accessibility enabled to function (which then redirects you to the correct System Preferences pane and tab).

As a consequence of these changes yubiswitch does now properly disable on start (previously I had to toggle it on and off a few times to reliably get it to disable).

not entirely sure this is correct, but unless the async code actually
blocks until the block executes then this was incorrect previously
there are more to do here still though
This could be much friendlier, but it works
repeated calls to IOHIDManagerScheduleWithRunLoop cause an assertion
failure in 10.15+.

This code is brittle in the presence of multiple rapid calls (esp in
conjunction with the automated disables/enables that yubiswitch can emit
on timers and other events), but this will make it a bit better
@stevenharman
Copy link

@reaperhulk Thank you so much for this! I hope @pallotron has a chance to incorporate these and cut a new build 🔜. I've got an M1 personal machine and we've started to move to M1s for work as well. I really miss YubiSwitcher on my M1!

@pallotron pallotron merged commit 99fdf55 into pallotron:master Jul 1, 2022
@pallotron
Copy link
Owner

Thanks! Sorry about late merge. I don't have much time to dedicate to personal projects.

In fact I should probably seek a new owner for this project :)

@reaperhulk
Copy link
Contributor Author

You might want to shift this repo to its own GitHub org to make it easier to expand maintainers and manage it. I’m not familiar with best practices for managing an OSS project that also needs an apple developer subscription for signing/notarizarion though. I would be willing to help out (although I emphasize that I am not an objective-C developer, I was just motivated to get this working 😂)

@davidrothera
Copy link
Collaborator

I can help out and have a developer account to perform releases.

@erikdw
Copy link

erikdw commented Sep 27, 2022

@reaperhulk : thanks so much for this fix, I'm looking forward to taking advantage of it.
@davidrothera : would you be able to somehow create a new image with this fix in it?

@davidrothera
Copy link
Collaborator

Sure thing, I'll do it this evening.

@pallotron
Copy link
Owner

Let me add you as maintainer on this project. Thanks @davidrothera

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants