[NearbyInteraction][Xcode12] Add manual binding for NINearbyObjectDirectionNotAvailable #9346

whitneyschmidt · 2020-08-10T22:12:02Z

Add manual binding for NINearbyObject NINearbyObjectDirectionNotAvailable.

This should be updated again when we have support in for https://github.com/xamarin/maccore/issues/2274

monojenkins · 2020-08-10T22:20:14Z

Build failure
❌ Build failed or was aborted

dalexsoto · 2020-08-10T22:43:43Z

I think we need a test for this, maybe you can inspect the native values and make sure we match those on our side

dalexsoto

You will need this fix also I really encourage you to have a test :)

src/frameworks.sources

monojenkins · 2020-08-10T23:56:38Z

Build failure
❌ Build failed or was aborted

✅ Build succeeded
✅ API Diff (from stable)
⚠️ API Diff (from PR only) (🔥 breaking changes 🔥)
ℹ️ Generator Diff (please review changes)

whitneyschmidt · 2020-08-11T00:05:13Z

I think we need a test for this, maybe you can inspect the native values and make sure we match those on our side

I did the following:

Xcode:

vector_float3 vectVals = NINearbyObjectDirectionNotAvailable; 
//vectValues = (1.28042596E-19, 4.59163468E-41, 0, 0)

Vector3 vect = NINearbyObject.NINearbyObjectDirectionNotAvailable;
// vect = {0, 0, 0} (w/ 0 for length scalar)

Do these "match" if the ObjC values are so small?

monojenkins · 2020-08-11T02:19:17Z

Build success
✅ Build succeeded
✅ API Diff (from stable)
⚠️ API Diff (from PR only) (🔥 breaking changes 🔥)
ℹ️ Generator Diff (please review changes)
✅ Test run succeeded

rolfbjarne · 2020-08-11T08:59:29Z

I think we need a test for this, maybe you can inspect the native values and make sure we match those on our side

I did the following:

Xcode:
vector_float3 vectVals = NINearbyObjectDirectionNotAvailable; 
//vectValues = (1.28042596E-19, 4.59163468E-41, 0, 0)
Vector3 vect = NINearbyObject.NINearbyObjectDirectionNotAvailable;
// vect = {0, 0, 0} (w/ 0 for length scalar)
Do these "match" if the ObjC values are so small?

Hm, no, I think we'd need to be exactly the same. Maybe printing out and comparing the memory contents?

vector_float3* v = & NINearbyObjectDirectionNotAvailable;
uint8_t* ptr = (uint8_t*) v;
for (int i = 0; i < sizeof (vector_float3); i++) {
    printf ("%.2x ", ptr [i]);
}
printf ("\n");

Vector3 vect = NINearbyObject.NINearbyObjectDirectionNotAvailable;
unsafe {
    Vector3 *v = &vect;
    byte *ptr = (byte *) v;
    for (var i = 0; i < sizeof (Vector3); i++)
        Console.WriteLine ($"{ptr [i]:X2} ");
    Console.WriteLine ();
}

spouliot

since it's a manual API it should come with a unit test

spouliot · 2020-08-11T13:19:02Z

src/NearbyInteraction/NINearbyObject.cs

+        // TODO: https://github.com/xamarin/maccore/issues/2274
+        // We do not have generator support to trampoline Vector3 -> vector_float3 for Fields
+        [Field ("NINearbyObjectDirectionNotAvailable",  "NearbyInteraction")]
+        public static Vector3 NINearbyObjectDirectionNotAvailable {


remove NINearbyObject prefix since it's the current type name -> ObjectDirectionNotAvailable

spouliot · 2020-08-11T13:19:41Z

src/NearbyInteraction/NINearbyObject.cs

+                        _NINearbyObjectDirectionNotAvailable = *pointer;
+                    }
+                }
+                return (OpenTK.Vector3)_NINearbyObjectDirectionNotAvailable;


minor: that could be (Vector3)

whitneyschmidt · 2020-08-11T16:42:24Z

I think we need a test for this, maybe you can inspect the native values and make sure we match those on our side

I did the following:
Xcode:
vector_float3 vectVals = NINearbyObjectDirectionNotAvailable; 
//vectValues = (1.28042596E-19, 4.59163468E-41, 0, 0)
Vector3 vect = NINearbyObject.NINearbyObjectDirectionNotAvailable;
// vect = {0, 0, 0} (w/ 0 for length scalar)
Do these "match" if the ObjC values are so small?

Hm, no, I think we'd need to be exactly the same. Maybe printing out and comparing the memory contents?

vector_float3* v = & NINearbyObjectDirectionNotAvailable;
uint8_t* ptr = (uint8_t*) v;
for (int i = 0; i < sizeof (vector_float3); i++) {
    printf ("%.2x ", ptr [i]);
}
printf ("\n");

Vector3 vect = NINearbyObject.NINearbyObjectDirectionNotAvailable;
unsafe {
    Vector3 *v = &vect;
    byte *ptr = (byte *) v;
    for (var i = 0; i < sizeof (Vector3); i++)
        Console.WriteLine ($"{ptr [i]:X2} ");
    Console.WriteLine ();
}

Thanks @rolfbjarne - I was able to test these out and the byte-by-byte values in memory are all 00, from VS as well as Xcode 👍

spouliot · 2020-08-11T17:11:53Z

src/NearbyInteraction/NINearbyObject.cs

+// Authors:
+//	Whitney Schmidt  <[email protected]>
+//
+// Copyright 2020 Microsoft Inc.


minor: it's not Inc. it's Corp. ;-)

spouliot · 2020-08-11T17:41:02Z

tests/monotouch-test/NearbyInteraction/NINearbyObjectTest.cs

+				byte* ptr = (byte*) v;
+				byte zero = 0;
+				for (var i = 0; i < sizeof (Vector3); i++)
+					Assert.True (ptr[i] == zero, "Expected {0} but was {1}", zero.ToString(), ptr[i].ToString());


minor:

style, missing space before ( (twice)

zero.ToString() allocates and it's a constant in the loop

the assert error will tell you a byte is not 0 but won't tell you where it is (I)

Assert.That (ptr [i], Is.EqualTo (zero), $"Position {i}");

The above assert will provide the expected and actual values (and turn them to string only if needed/failing) so the only missing part (to diagnose the failure) is the position i.

spouliot · 2020-08-11T17:43:25Z

tests/monotouch-test/NearbyInteraction/NINearbyObjectTest.cs

+		public void DirectionNotAvailable ()
+		{
+			if (!TestRuntime.CheckXcodeVersion (12, 0))
+				Assert.Ignore ("Requires iOS 14.0");


It's a special case but, since it's all 0 and C# struct are zeroized then the test should work even before iOS 14.

I tried running on iOS 13.2 w/o the version check and received the following error:

2020-08-11 13:55:20.518482-0400 monotouchtest[43690:16037527] [FAIL] NINearbyObjectTest.DirectionNotAvailable : System.NullReferenceException : Object reference not set to an instance of an object

Since NearbyInteraction is a new framework in Xcode12, I think this is expected.

I also tried out the following to double-check:

NSCoder coder = new NSCoder (); NINearbyObject n = new NINearbyObject (coder);

It produced this error:
[FAIL] NINearbyObjectTest.DirectionNotAvailable : System.Exception : Could not create an native instance of the type 'NearbyInteraction.NINearbyObject': the native class hasn't been loaded.

I believe this is correct behavior since the framework isn't available < iOS 14, lmk if I'm missing something here!

NINearbyObjectTest.DirectionNotAvailable : System.NullReferenceException : Object reference not set to an instance of an object

Reading a field with dlsym does not require the field to exists - but maybe it did not like not finding the framework itself. The stack trace would show what cause the NRE.

Anyway not a huge deal, you can leave the version check there :)

NSCoder coder = new NSCoder (); NINearbyObject n = new NINearbyObject (coder);

I believe this is correct behavior since the framework isn't available < iOS 14

Yes. You can't do as it will call code initWithCoder on a class (NINearbyObject) that has not been loaded into memory.

However the field is different since it's static (no instance has to be created) and return a type (Vector3) that already exists. Nothing in your manual code requires the framework to be loaded. In fact it should work (a managed exception is acceptable, not a crash) if someone was to put NINearbyObject.DirectionNotAvailable in a debugger watch on an older OS version.

That makes sense - if there's no need to instantiate an object to access the field, then I see how it would work.

Someone tried it out on an older simulator and got this stack trace: https://gist.github.com/whitneyschmidt/e5d239d4f83473d29244c7cee86adcf3

Stack trace seems incomplete. The first frame is inside nunitlite code, nothing about the test code itself.

I have a breakpoint set on Vector3 vect = NINearbyObject.DirectionNotAvailable;. When I try and step into it, I end up with the stack trace above 🤔 Is there a better way to try and see what's going on?

monojenkins · 2020-08-11T18:06:00Z

Build failure
❌ Build failed or was aborted

✅ Build succeeded
✅ API Diff (from stable)
⚠️ API Diff (from PR only) (🔥 breaking changes 🔥)
ℹ️ Generator Diff (please review changes)

mandel-macaque

+1 once @spouliot concerns are addressed.

monojenkins · 2020-08-11T20:21:06Z

Build failure
✅ Build succeeded
✅ API Diff (from stable)
⚠️ API Diff (from PR only) (🔥 breaking changes 🔥)
ℹ️ Generator Diff (please review changes)
🔥 Test run failed 🔥

Test results

8 tests failed, 68 tests passed.

Failed tests

monotouch-test/tvOS - simulator/Debug: BuildFailure
monotouch-test/tvOS - simulator/Debug (LinkSdk): BuildFailure
monotouch-test/tvOS - simulator/Debug (static registrar): BuildFailure
monotouch-test/tvOS - simulator/Release (all optimizations): BuildFailure
monotouch-test/watchOS 32-bits - simulator/Debug: BuildFailure
monotouch-test/watchOS 32-bits - simulator/Debug (LinkSdk): BuildFailure
monotouch-test/watchOS 32-bits - simulator/Debug (static registrar): BuildFailure
monotouch-test/watchOS 32-bits - simulator/Release (all optimizations): BuildFailure

rolfbjarne

👍

rolfbjarne · 2020-08-12T06:19:34Z

tests/monotouch-test/NearbyInteraction/NINearbyObjectTest.cs

+			if (!TestRuntime.CheckXcodeVersion (12, 0))
+				Assert.Ignore ("Requires iOS 14.0");


Really minor, so no need to fix this, but for the future this is shorter:

Suggested change

if (!TestRuntime.CheckXcodeVersion (12, 0))

Assert.Ignore ("Requires iOS 14.0");

TestRuntime.AssertXcodeVersion (12, 0);

spouliot · 2020-08-12T12:58:45Z

Tests failures are due to the new test being executed on tvOS and watchOS, both not supporting ARKit.
Wrap your source file inside #if __IOS__ and #endif to exclude other platforms.

monojenkins · 2020-08-12T19:13:10Z

Build success
✅ Build succeeded
✅ API Diff (from stable)
⚠️ API Diff (from PR only) (🔥 breaking changes 🔥)
ℹ️ Generator Diff (please review changes)
✅ Test run succeeded

whitneyschmidt added 7 commits August 7, 2020 11:52

nichanges + apiclassptrtest debugging code

d57265a

actually add ni bindings

23ec667

updates to fix intro failure

281fb3f

link to maccore issue

d822bb3

updates for nearbyinteraction manual code + fix NIErrorDomain binding

0d5b796

merge xcode12

d6a9bbe

remove noise

ad09cdd

whitneyschmidt added this to the xcode12 milestone Aug 10, 2020

whitneyschmidt requested review from dalexsoto, rolfbjarne and mandel-macaque August 10, 2020 22:12

remove .ignore

9889c07

whitneyschmidt requested a review from spouliot as a code owner August 10, 2020 22:15

dalexsoto requested changes Aug 10, 2020

View reviewed changes

src/frameworks.sources Outdated Show resolved Hide resolved

whitneyschmidt commented Aug 10, 2020

View reviewed changes

src/frameworks.sources Outdated Show resolved Hide resolved

fix typos

d523686

spouliot suggested changes Aug 11, 2020

View reviewed changes

add unit test, fix up seb suggestions

0074fe0

whitneyschmidt requested review from spouliot and dalexsoto August 11, 2020 16:42

spouliot reviewed Aug 11, 2020

View reviewed changes

spouliot approved these changes Aug 11, 2020

View reviewed changes

fix Assert to avoid allocating string every time

ab47613

mandel-macaque approved these changes Aug 11, 2020

View reviewed changes

rolfbjarne approved these changes Aug 12, 2020

View reviewed changes

fix test failures by wrapping with __IOS__, clean up version check

0904e2a

whitneyschmidt merged commit fb665c8 into xamarin:xcode12 Aug 12, 2020

whitneyschmidt mentioned this pull request Aug 12, 2020

[Xcode12][NearbyInteraction] New framework, updated through beta 4 #9336

Merged

spouliot mentioned this pull request Aug 12, 2020

Xcode 12 final audit checklist #8943

Open

46 tasks

whitneyschmidt added iOS Issues affecting iOS notes-mention Deserves a mention in release notes labels Sep 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NearbyInteraction][Xcode12] Add manual binding for NINearbyObjectDirectionNotAvailable #9346

[NearbyInteraction][Xcode12] Add manual binding for NINearbyObjectDirectionNotAvailable #9346

whitneyschmidt commented Aug 10, 2020

monojenkins commented Aug 10, 2020

dalexsoto commented Aug 10, 2020

dalexsoto left a comment

monojenkins commented Aug 10, 2020

whitneyschmidt commented Aug 11, 2020

monojenkins commented Aug 11, 2020

rolfbjarne commented Aug 11, 2020

spouliot left a comment

spouliot Aug 11, 2020

spouliot Aug 11, 2020

whitneyschmidt commented Aug 11, 2020

spouliot Aug 11, 2020

spouliot Aug 11, 2020

spouliot Aug 11, 2020

whitneyschmidt Aug 11, 2020

spouliot Aug 11, 2020

whitneyschmidt Aug 11, 2020

spouliot Aug 12, 2020

whitneyschmidt Aug 12, 2020

monojenkins commented Aug 11, 2020

mandel-macaque left a comment

monojenkins commented Aug 11, 2020

Failed tests

rolfbjarne left a comment

rolfbjarne Aug 12, 2020

spouliot commented Aug 12, 2020

monojenkins commented Aug 12, 2020

		if (!TestRuntime.CheckXcodeVersion (12, 0))
		Assert.Ignore ("Requires iOS 14.0");

	if (!TestRuntime.CheckXcodeVersion (12, 0))
	Assert.Ignore ("Requires iOS 14.0");
	TestRuntime.AssertXcodeVersion (12, 0);

[NearbyInteraction][Xcode12] Add manual binding for NINearbyObjectDirectionNotAvailable #9346

[NearbyInteraction][Xcode12] Add manual binding for NINearbyObjectDirectionNotAvailable #9346

Conversation

whitneyschmidt commented Aug 10, 2020

monojenkins commented Aug 10, 2020

dalexsoto commented Aug 10, 2020

dalexsoto left a comment

Choose a reason for hiding this comment

monojenkins commented Aug 10, 2020

whitneyschmidt commented Aug 11, 2020

monojenkins commented Aug 11, 2020

rolfbjarne commented Aug 11, 2020

spouliot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whitneyschmidt commented Aug 11, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monojenkins commented Aug 11, 2020

mandel-macaque left a comment

Choose a reason for hiding this comment

monojenkins commented Aug 11, 2020

Test results

Failed tests

rolfbjarne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spouliot commented Aug 12, 2020

monojenkins commented Aug 12, 2020