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

chore(corebluetooth): Use objc2 and its framework crates #381

Merged
merged 5 commits into from May 14, 2024

Conversation

madsmtm
Copy link

@madsmtm madsmtm commented May 1, 2024

objc2 is the successor to objc, which improves the way memory-management is done using the objc2::rc::Id type, which is effectively a typed StrongPtr. Additionally it provides the msg_send_id! macro, which ensures that the Objective-C memory management rules are followed correctly. Finally, it has encoding verification when debug assertions are enabled, which verify the calling convention at runtime, and can catch errors with using an incorrect method signature. The second commit fixes the errors that was found with this.

Compared to cocoa, objc2-foundation provides type-safe wrappers around each class, which means that you can't accidentally call e.g. allKeys on something that is not a dictionary, and it allows us to naturally use Options for nullable values. The third commit replaces the manual interface to Foundation types in src/corebluetooth/framework.rs with the types from objc2-foundation.

This PR is only the first part of the transition; in the future we will have objc2-core-bluetooth, see madsmtm/objc2#608, which should eliminate the need all of src/corebluetooth/framework.rs. But in the meantime, this still improves the safety significantly.

Feel free to ask if something is unclear!

@qdot
Copy link
Contributor

qdot commented May 1, 2024

Wow, thanks so much for this! The macOS portion of the codebase is probably the least understood part (I really just ripped out what Servo had and poked it until it worked), so I'm happy to see some maintenance happening to hopefully make it safer and more maintainable. I'll try to review this and get it in soon.

@qwandor qwandor changed the base branch from master to dev May 1, 2024 09:50
@qdot qdot merged commit d59b29b into deviceplug:dev May 14, 2024
4 checks passed
@madsmtm madsmtm deleted the objc2 branch May 14, 2024 22:28
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.

None yet

3 participants