-
Notifications
You must be signed in to change notification settings - Fork 514
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
[CHIP] Add support for Xcode13 beta2. #12025
[CHIP] Add support for Xcode13 beta2. #12025
Conversation
[DesignatedInitializer] | ||
IntPtr Constructor (ChipDevice device, byte endpoint, DispatchQueue queue); | ||
|
||
[Async (ResultTypeName = "ChipReadAttributeResult")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When should we use this ResultTypeName with Async?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is related when the handler returns more than one value:
delegate void ChipResponseHandler ([NullAllowed] NSError error, [NullAllowed] NSDictionary data);
The generator will tell you.
src/chip.cs
Outdated
} | ||
|
||
[Mac (12,0), Watch (8,0), TV (15,0), iOS (15,0), MacCatalyst (15,0)] | ||
[BaseType (typeof(ChipCluster), Name="CHIPBinding")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[BaseType (typeof(ChipCluster), Name="CHIPBinding")] | |
[BaseType (typeof (ChipCluster), Name="CHIPBinding")] |
same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the missing spaces in the basetypes, it looks good to me!
❌ [PR Build] Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View API diffAPI & Generator diffℹ️ API Diff (from PR only) (please review changes) GitHub pagesResults can be found in the following github pages (it might take some time to publish): Test results12 tests failed, 77 tests passed.Failed tests
Pipeline on Agent XAMBOT-1101.BigSur' |
Co-authored-by: TJ Lambert <50846373+tj-devel709@users.noreply.github.com>
🔥 Tests failed catastrophically on Build (no summary found). 🔥Result file $(TEST_SUMMARY_PATH) not found. |
…n-macios into chip-xcode13-beta1
🔥 Tests failed catastrophically on Build (no summary found). 🔥Result file $(TEST_SUMMARY_PATH) not found. |
#if !MONOMAC | ||
if (t.Namespace == "Chip" && Runtime.Arch == Arch.SIMULATOR) // namespace not present in sims as of Xcode13 beta 1 | ||
continue; | ||
#endif | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic should go in the Skip
method, possibly in the MacApiProtocolTest
subclass.
[Async (ResultTypeName = "ChipReadAttributeResult")] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The [Async] attribute should be one liner lower.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After the comments are addressed 👍
src/chip.cs
Outdated
|
||
[Async (ResultTypeName = "ChipReadAttributeResult")] | ||
[Export ("disableNetwork:breadcrumb:timeoutMs:responseHandler:")] | ||
void DisableNetwork (NSData networkID, ulong breadcrumb, uint timeoutMs, ChipResponseHandler responseHandler); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are other ID
that needs to be Id
void DisableNetwork (NSData networkID, ulong breadcrumb, uint timeoutMs, ChipResponseHandler responseHandler); | |
void DisableNetwork (NSData networkId, ulong breadcrumb, uint timeoutMs, ChipResponseHandler responseHandler); |
src/chip.cs
Outdated
bool PairDevice (ulong deviceID, string address, ushort port, ushort discriminator, uint setupPinCode, [NullAllowed] out NSError error); | ||
|
||
[Export ("pairDeviceWithoutSecurity:address:port:error:")] | ||
bool PairDeviceWithoutSecurity (ulong deviceID, string address, ushort port, [NullAllowed] out NSError error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same search and replace.
bool PairDeviceWithoutSecurity (ulong deviceID, string address, ushort port, [NullAllowed] out NSError error); | |
bool PairDeviceWithoutSecurity (ulong deviceId, string address, ushort port, [NullAllowed] out NSError error); |
Co-authored-by: Alex Soto <alex@alexsoto.me>
Co-authored-by: Alex Soto <alex@alexsoto.me>
❌ [PR Build] Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View API diffAPI & Generator diffℹ️ API Diff (from PR only) (please review changes) GitHub pagesResults can be found in the following github pages (it might take some time to publish): Test results11 tests failed, 78 tests passed.Failed tests
Pipeline on Agent XAMBOT-1104.BigSur' |
@@ -222,6 +222,7 @@ public void GatherFrameworks () | |||
case "Metal": | |||
case "MetalKit": | |||
case "MetalPerformanceShaders": | |||
case "CHIP": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dalexsoto out of curiosity, why doesn't the default case handle this?
xamarin-macios/tools/common/Target.cs
Lines 251 to 258 in 401befe
if (App.IsSimulatorBuild && !App.IsFrameworkAvailableInSimulator (framework.Name)) { | |
if (App.LinkMode != LinkMode.None) { | |
ErrorHelper.Warning (5223, Errors.MX5223, framework.Name, App.PlatformName); | |
} else { | |
Driver.Log (3, Errors.MX5223, framework.Name, App.PlatformName); | |
} | |
continue; | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rolfbjarne good question, I did not have a close look at it I wonder if there is a casing issue somewhere @mandel-macaque looks like we'll need to check deeper
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm looking atm.
❌ [PR Build] Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View API diffAPI & Generator diffℹ️ API Diff (from PR only) (please review changes) GitHub pagesResults can be found in the following github pages (it might take some time to publish): Test results4 tests failed, 85 tests passed.Failed tests
Pipeline on Agent XAMBOT-1104.BigSur' |
✅ [PR Build] Tests passed on Build. ✅Tests passed on Build. API diff✅ API Diff from stable View API diffAPI & Generator diffℹ️ API Diff (from PR only) (please review changes) GitHub pagesResults can be found in the following github pages (it might take some time to publish): 🎉 All 89 tests passed 🎉Pipeline on Agent XAMBOT-1104.BigSur' |
No description provided.