Bug 43205 - SiriKit API Inconsistencies
Summary: SiriKit API Inconsistencies
Status: VERIFIED FIXED
Alias: None
Product: iOS
Classification: Xamarin
Component: Xamarin.iOS.dll ()
Version: XI 9.99 (iOS 10 previews)
Hardware: Macintosh Mac OS
: Normal normal
Target Milestone: Xcode8 (iOS10)
Assignee: Alex Soto [MSFT]
URL:
Depends on:
Blocks:
 
Reported: 2016-08-10 02:47 UTC by Adam
Modified: 2016-09-14 04:44 UTC (History)
5 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
VERIFIED FIXED

Description Adam 2016-08-10 02:47:02 UTC
I am currently trying to work with SiriKit in the preview and have come across some API inconsistencies.

In creating an IntentHandle and implementing the IINSendMessageIntentHandling, I notice that there is a Protocol definition for ResolveContent but no overrides. I feel that there should be an override but maybe I am implementing that incorrectly.

Second and more importantly the NeedsValue is a property from the Base class INIntentResolutionResult but in the iOS docs its a method. Also it returns a type of INIntentResolutionResult but it should return INStringResolutionResult, as I can't cast a base class into a derived class.

        public class IntentHandle: INExtension, IINSendMessageIntentHandling
	{
		public IntentHandle()
		{
		}

		
		public override ResolveContent(INSendMessageIntent intent, Action<INStringResolutionResult> completion)
		{

			if (intent.Content != "")
			{
				completion(INStringResolutionResult.GetSuccess(intent.Content));
			}
			else {
				completion(INStringResolutionResult.NeedsValue);
			}
		}
	}
Comment 1 Vincent Dondain [MSFT] 2016-08-10 13:57:09 UTC
Hey,

So let me try to clarify what's happening here:

"In creating an IntentHandle and implementing the IINSendMessageIntentHandling, I notice that there is a Protocol definition for ResolveContent but no overrides. I feel that there should be an override but maybe I am implementing that incorrectly."

INSendMessageIntentHandling in not declared with the [Model] attribute in our bindings so you don't have a concrete class you can use that would provide the override (note that you would only get to override the required methods, optional methods would not be declared with the virtual keyword).

In any case you're doing:

public class IntentHandle: INExtension, IINSendMessageIntentHandling

So you chose to use the generated interface way of consuming protocols (good piece of information here: https://developer.xamarin.com/guides/cross-platform/macios/binding/objective-c-libraries/#Binding_Protocols [New in MonoTouch 7.0 part]).

Therefore you just have to add the method signature of the class that's defined in IINSendMessageIntentHandling. Note that interfaces will only have the *required* methods that you can simply copy and paste. Optional methods you'd have to also add the [Export] attribute in our C# code.

"Second and more importantly the NeedsValue is a property from the Base class INIntentResolutionResult but in the iOS docs its a method."

That is fine, static method vs static property is equivalent, it's just syntax.

"Also it returns a type of INIntentResolutionResult but it should return INStringResolutionResult, as I can't cast a base class into a derived class."

I think we're indeed missing something here, we should be getting INStringResolutionResult and I don't see how to get it or fix it with the current bindings (static method on the base class). [cc:alex]
Comment 2 Alex Soto [MSFT] 2016-08-11 04:38:26 UTC
We have an idea which is to introduce the following methods instead of the properties

public static T GetNeedsValue<T> () where T : INIntentResolutionResult
{
    return ObjCRuntime.Runtime.GetINativeObject<T> (INIntentResolutionResult.NeedsValue, true);
}

public static T GetNotRequired<T> () where T : INIntentResolutionResult
{
    return ObjCRuntime.Runtime.GetINativeObject<T> (INIntentResolutionResult.NotRequired, true);
}

public static T GetUnsupported<T> () where T : INIntentResolutionResult
{
    return ObjCRuntime.Runtime.GetINativeObject<T> (INIntentResolutionResult.Unsupported, true);
}

where Unsupported, NotRequired and NeedsValue are pointers to the native object.

But we are exploring other options where the cast happens automagically so users does not have to "guess" the type they need to "cast" to.
Comment 3 Sebastien Pouliot 2016-08-18 15:58:27 UTC
I'd rather not require something like:

> completion (INStringResolutionResult.GetNeedsValue<INStringResolutionResult> ());

The problem here is that ObjC makes INIntentResolutionResult and INStringResolutionResult static members different (as it returns `instancetype`) while (by default) C# makes them identical.

I think we need something like
https://github.com/xamarin/xamarin-macios/pull/623

which was a similar issue (wrt language, not usage). That would let the code be written as

> > completion (INStringResolutionResult.NeedsValue);

which feels more natural to read (and write).


Sadly that means quite a lot of extra/near-duplicated code inside bindings :( and we should investigate how to have the generator do this for us (but that's a longer term goal).
Comment 4 Alex Soto [MSFT] 2016-08-24 15:35:22 UTC
Proposal: https://github.com/xamarin/xamarin-macios/pull/655
Comment 5 Sebastien Pouliot 2016-08-25 00:56:38 UTC
merged in xamarin-macios/xcode8 901d960c1d1b230adc8a7150913fd5d3b0c166ae