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

[HealthKit] Update bindings for Xcode 14 beta 1-4 #15612

Merged
merged 26 commits into from
Sep 1, 2022

Conversation

tj-devel709
Copy link
Contributor

@tj-devel709 tj-devel709 commented Aug 3, 2022

This one has been a toughy ๐Ÿ˜ฎโ€๐Ÿ’จ
@mandel-macaque I am creating the draft PR and here is a gist with the MacModern failures that you mentioned you would take a closer look at: https://gist.github.com/tj-devel709/24f5c63d4b698a0e2387524a0d2666c6

There were also monotouch-test failures that seem to be related to enabling mac that I could use some help with as well.

Lastly, @chamons there was a failing Cecil test regarding ios in DidGenerateEvent method found here: https://gist.github.com/tj-devel709/2c8aee7325c0c2751e29269d3eb77fef. I did not touch this method so I am not sure why this would be a problem.
Here is the Cecil failure: https://gist.github.com/tj-devel709/cfb2576c75de2ee100a8f2b7ee6e3923

Thanks for the help!

Edit *: Issues mentioned in the code comments
https://github.com/xamarin/maccore/issues/2609
https://github.com/xamarin/maccore/issues/2610

@tj-devel709 tj-devel709 added the notes-mention Deserves a mention in release notes label Aug 3, 2022
@tj-devel709 tj-devel709 added this to the xcode14 milestone Aug 3, 2022
@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@mandel-macaque mandel-macaque added the do-not-merge Do not merge this pull request label Aug 3, 2022
@tj-devel709
Copy link
Contributor Author

tj-devel709 commented Aug 3, 2022

Oh interesting, hopefully not related to the other errors, but I missed the api-annotations-dotnet/MacCatalyst-HealthKit.todo file and will get to this later today!

Monotouch-Tests iOS also do not seem to like my manual file and/or test - /Users/builder/azdo/_work/1/s/xamarin-macios/tests/monotouch-test/HealthKit/HKCategoryValueSleepAnalysisTest.cs(27,22): error CS0103: The name 'HKCategoryValueSleepAnalysisAsleep' does not exist in the current context [/Users/builder/azdo/_work/1/s/xamarin-macios/tests/xharness/tmp-test-dir/monotouch-test21/monotouch-test.csproj]

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@tj-devel709
Copy link
Contributor Author

@rolfbjarne Addressed!

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

src/healthkit.cs Outdated

[Watch (9,0), MacCatalyst (16,0), Mac (13,0), iOS (16,0), NoTV]
[Export ("initWithRoute:dateInterval:dataHandler:")]
NativeHandle Constructor (HKWorkoutRoute workoutRoute, NSDateInterval dateInterval, Action<HKWorkoutRouteQuery, NSArray<CLLocation>, bool, NSError> dataHandler);
Copy link
Member

Choose a reason for hiding this comment

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

This needs a dotnet array so you will need a custom delegate

delegate void HKWorkoutRouteQueryDataHandler (HKWorkoutRouteQuery query, [NullAllowed] CLLocation [] routeData, bool done, [NullAllowed] NSError error);

src/healthkit.cs Outdated

[Async]
[Export ("removeAttachment:fromObject:completion:")]
void RemoveAttachment (HKAttachment attachment, HKObject @object, Action<bool, NSError> completion);
Copy link
Member

Choose a reason for hiding this comment

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

what does the bool represents here? Probably this is a good candidate to do a delegate instead of an Action, same for the rest in this class.

Copy link
Contributor

@SotoiGhost SotoiGhost left a comment

Choose a reason for hiding this comment

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

Everything looks good, just address Alex's comments.

Comment on lines 2025 to 2026
GLKit \
HealthKit \
Copy link
Contributor

Choose a reason for hiding this comment

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

nit

Suggested change
GLKit \
HealthKit \
GLKit \
HealthKit \

@tj-devel709
Copy link
Contributor Author

@dalexsoto Updated to use the delegates!

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@@ -564,7 +564,7 @@ interface HKCorrelationType {

delegate void HKHealthStoreGetRequestStatusForAuthorizationToShareHandler (HKAuthorizationRequestStatus requestStatus, NSError error);
delegate void HKHealthStoreRecoverActiveWorkoutSessionHandler (HKWorkoutSession session, NSError error);
delegate void HKHealthStoreCompletionHandler (bool success, [NullAllowed] NSError error);
delegate void HKHealthStoreCompletionHandler (bool success, NSError error);
Copy link
Contributor

Choose a reason for hiding this comment

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

Weird... what was the compiler issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for bringing this up @SotoiGhost !
Here is a gist of the errors / warnings that were not present when not including the [NullAllowed]
https://gist.github.com/tj-devel709/756ac73e32f3ff8ae234d8ba7ecdd7cc

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Collaborator

โœ… API diff for current PR / commit

Legacy Xamarin (No breaking changes)
.NET (No breaking changes)

โ— API diff vs stable (Breaking changes)

Legacy Xamarin (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
.NET (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
Legacy Xamarin (stable) vs .NET

โ„น๏ธ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 7975c6ecf3eb6874305a9db4511bf5a155606911 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

๐Ÿ’ป [PR Build] Tests on macOS Mac Catalina (10.15) passed ๐Ÿ’ป

โœ… All tests on macOS Mac Catalina (10.15) passed.

Pipeline on Agent
Hash: 7975c6ecf3eb6874305a9db4511bf5a155606911 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

๐Ÿ“š [PR Build] Artifacts ๐Ÿ“š

Packages generated

View packages

Pipeline on Agent XAMBOT-1169.Monterey
Hash: 7975c6ecf3eb6874305a9db4511bf5a155606911 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

โŒ [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) failed โŒ

Failed tests are:

  • monotouch-test

Pipeline on Agent
Hash: 7975c6ecf3eb6874305a9db4511bf5a155606911 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

๐Ÿ”ฅ [CI Build] Test results ๐Ÿ”ฅ

Test results

โŒ Tests failed on VSTS: simulator tests

0 tests crashed, 2 tests failed, 226 tests passed.

Failures

โŒ interdependent_binding_projects tests

1 tests failed, 6 tests passed.
  • interdependent-binding-projects/watchOS 32-bits - simulator/Debug: TimedOut

Html Report (VSDrops) Download

โŒ introspection tests

1 tests failed, 12 tests passed.
  • introspection/watchOS 32-bits - simulator/Debug (watchOS 6.0): Crashed Known issue: HE0038)

Html Report (VSDrops) Download

Successes

โœ… bcl: All 69 tests passed. Html Report (VSDrops) Download
โœ… cecil: All 1 tests passed. Html Report (VSDrops) Download
โœ… dotnettests: All 1 tests passed. Html Report (VSDrops) Download
โœ… fsharp: All 7 tests passed. Html Report (VSDrops) Download
โœ… framework: All 8 tests passed. Html Report (VSDrops) Download
โœ… generator: All 2 tests passed. Html Report (VSDrops) Download
โœ… install_source: All 1 tests passed. Html Report (VSDrops) Download
โœ… linker: All 65 tests passed. Html Report (VSDrops) Download
โœ… mac_binding_project: All 1 tests passed. Html Report (VSDrops) Download
โœ… mmp: All 2 tests passed. Html Report (VSDrops) Download
โœ… mononative: All 12 tests passed. Html Report (VSDrops) Download
โœ… monotouch: All 23 tests passed. Html Report (VSDrops) Download
โœ… msbuild: All 2 tests passed. Html Report (VSDrops) Download
โœ… mtouch: All 1 tests passed. Html Report (VSDrops) Download
โœ… xammac: All 3 tests passed. Html Report (VSDrops) Download
โœ… xcframework: All 8 tests passed. Html Report (VSDrops) Download
โœ… xtro: All 2 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: [PR build]

@tj-devel709 tj-devel709 merged commit 7bd9838 into xamarin:xcode14 Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notes-mention Deserves a mention in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants