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

wrap CBPeripherals as CBMPeripheralNative on restore #106

Merged

Conversation

kennylovrin
Copy link
Contributor

I found when trying to restore state that it was tricky as the state contains CBPeripheral objects, but the rest of the code more or less relies on CBMPeripheral everywhere. One option would be to make the CBPeripheralNative.init public, but I think manipulating the state is more consistent with the CoreBluetoothMock API in general.

wraps the CBPeripherals in the restored state as CBMPeripheralNative
before sending the state to the delegate to allow it to be more
consistent and compatible with code that expects CBMPeripheral.
@CLAassistant
Copy link

CLAassistant commented Feb 19, 2024

CLA assistant check
All committers have signed the CLA.

@philips77
Copy link
Member

Hello,
Thank you for the PR. Did you test it, and does it fix your issue? I have no sample to test state restoration.

Copy link
Member

@philips77 philips77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good!

@philips77 philips77 merged commit a9652af into NordicSemiconductor:main Feb 19, 2024
1 check passed
@kennylovrin
Copy link
Contributor Author

kennylovrin commented Feb 19, 2024

Hello, Thank you for the PR. Did you test it, and does it fix your issue? I have no sample to test state restoration.

Hi, I see it's already merged now, it's not a huge change. But yes I've tested it and I am using it currently. It does solve my issue in the sense that I now get objects that are more easily compatible with the rest of the library.

@philips77
Copy link
Member

Splendid! I will do a new release shortly.

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.

3 participants