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

[Platform] Apply API review fixes for MTRDevicePairingDelegate #22537

Closed
bzbarsky-apple opened this issue Sep 12, 2022 · 3 comments
Closed

[Platform] Apply API review fixes for MTRDevicePairingDelegate #22537

bzbarsky-apple opened this issue Sep 12, 2022 · 3 comments
Assignees
Labels
darwin p2 priority 2 work v1.1

Comments

@bzbarsky-apple
Copy link
Contributor

bzbarsky-apple commented Sep 12, 2022

Reproduction steps

  1. Change signatures of callback methods to take controller as first argument (and maybe generally change all delegates to take object they are hanging off of as their first argument). Example API here:
     // (PASE established)
     - (void)deviceController:(MTRDeviceController *)deviceController
         establishedCommissioningSessionForNewNodeID:(NSNumber *)newNodeID;
     - (void)deviceController:(MTRDeviceController *)deviceController
         failedToEstablishCommissioningSessionForNewNodeID:(NSNumber *)newNodeID
         error:(NSError *)error;
     // Device Commissioning
     - (void)deviceController:(MTRDeviceController *)deviceController
         commissionedNodeWithID:(NSNumber *)nodeID;
     - (void)deviceController:(MTRDeviceController *)deviceController
         failedToCommissionNodeWithID:(NSNumber *)nodeID;
         error:(NSError * _Nullable)error;
    
  2. Remove the "pairing deleted" notification, which does not seem to actually exist anyway.

May have a bit of overlap with #22531 but the conflicts should not be too bad. Otherwise, this looks independent of the other API change bits going on.

Part of #22420

Platform

darwin

Platform Version(s)

No response

Type

Manually tested with SDK

(Optional) If manually tested please explain why this is only manually tested

No response

Anything else?

No response

@stale
Copy link

stale bot commented Mar 18, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale Stale issue or PR label Mar 18, 2023
@bzbarsky-apple bzbarsky-apple removed the stale Stale issue or PR label Mar 18, 2023
@woody-apple woody-apple added the p2 priority 2 work label Apr 4, 2023
@vivien-apple
Copy link
Contributor

Looking again it seems that has been fixed by the combination of #22638 and #22682.

@bzbarsky-apple
Copy link
Contributor Author

@vivien-apple the current naming does not read as nicely as the API proposal above, and we don't have separate notifications for commissioning success/failure, but that might just not be worth the churn at this point, I agree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
darwin p2 priority 2 work v1.1
Projects
None yet
Development

No branches or pull requests

3 participants