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

[HomeKit] Updates for Xcode13 Beta 1-2 #12092

Merged
merged 13 commits into from
Aug 3, 2021

Conversation

tj-devel709
Copy link
Contributor

No description provided.

@tj-devel709 tj-devel709 added the note-highlight Worth calling out specifically in release notes label Jul 9, 2021
@tj-devel709 tj-devel709 added this to the xcode13.0 milestone Jul 9, 2021
@tj-devel709
Copy link
Contributor Author

@mandel-macaque There was no MacCatalyst.todo, but most of these interfaces say they support MacCatalyst in the web docs.
I tried including the HomeKit in the MacCatalyst section of the frameworks.source file and I kept getting this error in the build:

build/dotnet/maccatalyst/generated-sources/ObjCRuntime/Libraries.g.cs(163,68): error CS0117: 'Constants' does not contain a definition for 'HomeKitLibrary'
make[1]: *** [build/dotnet/maccatalyst/64/Xamarin.MacCatalyst.dll] Error 1

Is this something I should do?

@vs-mobiletools-engineering-service2
Copy link
Collaborator

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

1 tests failed, 85 tests passed.

Failed tests

  • Cecil-based tests/NUnit: Failed (Execution failed with exit code 4)

Pipeline on Agent XAMBOT-1104.BigSur'
Merge 3245e8b into 9d31e45

Copy link
Member

@mandel-macaque mandel-macaque left a comment

Choose a reason for hiding this comment

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

Please lets use Handler as with the other APIs:

src/homekit.cs Outdated Show resolved Hide resolved
src/homekit.cs Outdated Show resolved Hide resolved
@vs-mobiletools-engineering-service2
Copy link
Collaborator

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

2 tests failed, 84 tests passed.

Failed tests

  • monotouch-test/Mac Catalyst/Debug [dotnet]: Failed (Tests run: 2625 Passed: 2489 Inconclusive: 35 Failed: 1 Ignored: 135)
  • Cecil-based tests/NUnit: Failed (Execution failed with exit code 4)

Pipeline on Agent XAMBOT-1096.BigSur'
Merge 44d4790 into 797a2f5

Copy link
Member

@rolfbjarne rolfbjarne left a comment

Choose a reason for hiding this comment

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

@mandel-macaque There was no MacCatalyst.todo, but most of these interfaces say they support MacCatalyst in the web docs.
I tried including the HomeKit in the MacCatalyst section of the frameworks.source file and I kept getting this error in the build:

build/dotnet/maccatalyst/generated-sources/ObjCRuntime/Libraries.g.cs(163,68): error CS0117: 'Constants' does not contain a definition for 'HomeKitLibrary'
make[1]: *** [build/dotnet/maccatalyst/64/Xamarin.MacCatalyst.dll] Error 1

Is this something I should do?

You'll have to remove this line:

but that might very well lead to other things that need to be fixed, so it's probably best to just leave HomeKit out of Mac Catalyst for now to not block this PR.

src/HomeKit/HMEnums.cs Outdated Show resolved Hide resolved
@tj-devel709 tj-devel709 requested a review from manish July 12, 2021 15:26
@manish manish removed their request for review July 12, 2021 15:42
tools/common/Frameworks.cs Show resolved Hide resolved
@vs-mobiletools-engineering-service2
Copy link
Collaborator

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

1 tests failed, 88 tests passed.

Failed tests

  • monotouch-test/Mac Catalyst/Debug [dotnet]: Failed (Tests run: 2625 Passed: 2489 Inconclusive: 35 Failed: 1 Ignored: 135)

Pipeline on Agent XAMBOT-1101.BigSur'
Merge f3cc71d into 26a8abe

@tj-devel709
Copy link
Contributor Author

Unrelated test failure: https://github.com/xamarin/maccore/issues/2443

@rolfbjarne
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vs-mobiletools-engineering-service2
Copy link
Collaborator

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

1 tests failed, 88 tests passed.

Failed tests

  • framework-test/Mac Catalyst/Debug: TimedOut (Execution timed out after 1200 seconds.
    No test log file was produced)

Pipeline on Agent XAMBOT-1104.BigSur'
Merge f3cc71d into a202354

@spouliot
Copy link
Contributor

@tj-devel709 if HomeKit support is new for Catalyst 15 then all existing types needs to be annotated with [MacCatalyst (15,0)]. Otherwise they will appear available in earlier versions, but won't work

Copy link
Member

@mandel-macaque mandel-macaque left a comment

Choose a reason for hiding this comment

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

Nees to be merged with main which contains Xcode13 and changes in the framework.

@tj-devel709
Copy link
Contributor Author

tj-devel709 commented Jul 20, 2021

@spouliot So I added what I think are the appropriate MacCatalyst attributes, but xtro is saying they are all unknown. Is there anything else I am supposed to do expect for add homekit to frameworks.sources?
https://gist.github.com/tj-devel709/0c8984d3f225a2b0999d145d52e51870

@vs-mobiletools-engineering-service2
Copy link
Collaborator

🔥 Tests failed catastrophically on Build (no summary found). 🔥

Result file $(TEST_SUMMARY_PATH) not found.

Pipeline on Agent
Merge 2484cec into 09f911b

@vs-mobiletools-engineering-service2
Copy link
Collaborator

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

1 tests failed, 88 tests passed.

Failed tests

  • Xtro/Mac: Failed (Test run failed.)

Pipeline on Agent XAMBOT-1107.BigSur'
Merge d554000 into eea8142

@rolfbjarne
Copy link
Member

@tj-devel709 if HomeKit support is new for Catalyst 15 then all existing types needs to be annotated with [MacCatalyst (15,0)]. Otherwise they will appear available in earlier versions, but won't work

Looks like the HomeKit framework was added to Mac Catalyst in 14.0: https://developer.apple.com/documentation/homekit?language=objc

@spouliot So I added what I think are the appropriate MacCatalyst attributes, but xtro is saying they are all unknown. Is there anything else I am supposed to do expect for add homekit to frameworks.sources?
https://gist.github.com/tj-devel709/0c8984d3f225a2b0999d145d52e51870

Maybe sharpie didn't include the HomeKit headers for Mac Catalyst for some reason? Running gen-maccatalyst in tests/xtro-sharpie creates a maccatalyst-15.0.g.cs file without any HomeKit bindings.

CC @dalexsoto

@spouliot
Copy link
Contributor

From xtro's perspective nothing is excluded for MacCatalyst.

* macosx 15.0, x86_64
sharpie query -bind ios15.0-macabi-x86_64.pch > maccatalyst-15.0.g.cs

Right now we're using the (sharpie generated) x86_64 pre-compiled headers. That makes me wonder if this could be something arch-dependent... it's something that's bound to happen at some point :(

@tj-devel709 you might want to create a small catalyst app with Xcode and see if you can use any HomeKit API.

@spouliot
Copy link
Contributor

Switching from arm64 does not change anything (wrt HomeKit) but made me realize that macOS does not support HomeKit. It might be filtered (because of that) by sharpie or even Xcode (might need some opt-in) ?

@tj-devel709
Copy link
Contributor Author

tj-devel709 commented Jul 21, 2021

@spouliot @rolfbjarne @dalexsoto
So I'm not sure if this is sufficient but I included Mac in an iOS Xcode project (which I think is how you enable Mac Catalyst), included HomeKit, and then called some things that are supported in Mac Catalyst (14,0) according to the web docs and Xcode does not complain about it!
https://gist.github.com/tj-devel709/07da0aff16246e04154e0a7f49acc9a4

@mandel-macaque
Copy link
Member

@tj-devel709 can you please update the wiki and point to this PR for HomeKit, is confusing when I want to track the status of beta3.

@tj-devel709
Copy link
Contributor Author

@tj-devel709 can you please update the wiki and point to this PR for HomeKit, is confusing when I want to track the status of beta3.

Updated!

@tj-devel709
Copy link
Contributor Author

Created an issue regarding the xtro tests for MacCatalyst not recognizing the HomeKit selectors! https://github.com/xamarin/maccore/issues/2476

@vs-mobiletools-engineering-service2
Copy link
Collaborator

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

1 tests failed, 91 tests passed.

Failed tests

  • monotouch-test/Mac Catalyst/Debug [dotnet]: Failed (Tests run: 2629 Passed: 2486 Inconclusive: 35 Failed: 2 Ignored: 141)

Pipeline on Agent XAMBOT-1096.BigSur'
Merge a6cf406 into c0651c9

src/frameworks.sources Show resolved Hide resolved
@mandel-macaque
Copy link
Member

Needs to be merged with main AFAIK beta4 added changes to this.

@vs-mobiletools-engineering-service2
Copy link
Collaborator

✅ [PR Build] Tests passed on Build. ✅

Tests passed on Build.

API diff

✅ API Diff from stable

View API diff

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

🎉 All 92 tests passed 🎉

Pipeline on Agent XAMBOT-1096.BigSur'
Merge 4933df6 into 448cc6e

!missing-null-allowed! 'System.String HomeKit.HMCharacteristicMetadata::get_ManufacturerDescription()' is missing an [NullAllowed] on return type
!missing-null-allowed! 'System.String HomeKit.HMService::get_AssociatedServiceType()' is missing an [NullAllowed] on return type
!missing-null-allowed! 'System.Void HomeKit.HMCharacteristic::UpdateAuthorizationData(Foundation.NSData,System.Action`1<Foundation.NSError>)' is missing an [NullAllowed] on parameter #0
!missing-null-allowed! 'System.Void HomeKit.HMCharacteristic::WriteValue(Foundation.NSObject,System.Action`1<Foundation.NSError>)' is missing an [NullAllowed] on parameter #0
Copy link
Member

Choose a reason for hiding this comment

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

Minor: missing end of line.

Copy link
Member

Choose a reason for hiding this comment

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

Why aren't we fixing this now?

Comment on lines +1 to +42
# Only supported in iOS 8
!missing-protocol! HMAccessoryBrowserDelegate not bound

# Only supported in iOS 8
!missing-type! HMAccessoryBrowser not bound
!missing-selector! HMAccessoryBrowser::delegate not bound
!missing-selector! HMAccessoryBrowser::discoveredAccessories not bound
!missing-selector! HMAccessoryBrowser::setDelegate: not bound
!missing-selector! HMAccessoryBrowser::startSearchingForNewAccessories not bound
!missing-selector! HMAccessoryBrowser::stopSearchingForNewAccessories not bound

# Only supported in iOS 11.3
!missing-type! HMAccessorySetupPayload not bound
!missing-selector! HMAccessorySetupPayload::initWithURL: not bound
!missing-selector! HMAccessorySetupPayload::initWithURL:ownershipToken: not bound
!missing-selector! HMHome::addAndSetupAccessoriesWithPayload:completionHandler: not bound


# Only supported in iOS 13
!missing-type! HMAccessoryOwnershipToken not bound
!missing-selector! HMAccessoryOwnershipToken::initWithData: not bound

# Only supported in iOS 13
!missing-type! HMAddAccessoryRequest not bound
!missing-selector! HMAddAccessoryRequest::accessoryCategory not bound
!missing-selector! HMAddAccessoryRequest::accessoryName not bound
!missing-selector! HMAddAccessoryRequest::home not bound
!missing-selector! HMAddAccessoryRequest::payloadWithOwnershipToken: not bound
!missing-selector! HMAddAccessoryRequest::payloadWithURL:ownershipToken: not bound
!missing-selector! HMAddAccessoryRequest::requiresOwnershipToken not bound
!missing-selector! HMAddAccessoryRequest::requiresSetupPayloadURL not bound

# Webdocs say it is supported in MacCatalyst 13, but the parameter type HMAddAccessoryRequest
# is only supported in iOS 13
!missing-protocol-member! HMHomeManagerDelegate::homeManager:didReceiveAddAccessoryRequest: not found

# Only supported in iOS 15
!missing-type! HMAccessorySetupManager not bound
!missing-selector! HMAccessorySetupManager::addAndSetUpAccessoriesForTopology:completionHandler: not bound

# Deprecated
!missing-selector! HMHome::removeUser:completionHandler: not bound
Copy link
Member

Choose a reason for hiding this comment

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

All this with Xcode 13 beta 4 is not needed. I merged with main and using export AUTO_SANITIZE=1 all of them are gone.

@vs-mobiletools-engineering-service2
Copy link
Collaborator

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

1 tests failed, 91 tests passed.

Failed tests

  • monotouch-test/Mac Catalyst/Debug [dotnet]: Failed (Tests run: 2629 Passed: 2490 Inconclusive: 35 Failed: 1 Ignored: 138)

Pipeline on Agent XAMBOT-1104.BigSur'
Merge 5eb4404 into f375881

Copy link
Member

@mandel-macaque mandel-macaque left a comment

Choose a reason for hiding this comment

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

Approving. We will clean NullAllowed later.

@mandel-macaque
Copy link
Member

unrelated issues: https://github.com/xamarin/maccore/issues/2443

@mandel-macaque mandel-macaque merged commit 6ae2ca0 into xamarin:main Aug 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
note-highlight Worth calling out specifically in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants