[Network] Add support for Xcode 14 beta 6. #15841

mandel-macaque · 2022-09-01T16:03:21Z

No description provided.

tj-devel709

A few small things, but LGTM otherwise!

tests/monotouch-test/Network/NWEndpointTests.cs

src/Network/NWProtocolFramerOptions.cs

src/Network/NWAdvertiseDescriptor.cs

src/Network/NWBrowserDescriptor.cs

src/Network/NWEndpoint.cs

rolfbjarne · 2022-09-01T16:15:11Z

src/Network/NWEthernetChannel.cs

+			if (parameters is null)
+				ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (parameters));
+
+			InitializeHandle (nw_ethernet_channel_create_with_parameters (ethernetType, networkInterface.Handle, parameters.Handle));


You can use GetNonNullHandle here instead of manually checking above.

But I need to check nevertheless to throw the exception.

No, GetNonNullHandle throws the exception:

xamarin-macios/src/ObjCRuntime/INativeObject.cs

Lines 30 to 33 in 7511281

static public NativeHandle GetNonNullHandle (this INativeObject self, string argumentName)

{

if (self is null)

ThrowHelper.ThrowArgumentNullException (argumentName);

rolfbjarne · 2022-09-01T16:20:10Z

src/Network/NWProtocolQuicOptions.cs

+		[iOS (16,0)]
+		[Watch (9,0)]
+#endif
+		public byte StreamType => nw_quic_get_stream_type (GetCheckedHandle ());


Looks like this can return an enum

But we do not know the enum values :/

/* * @result * Returns the type of the QUIC stream, stored in nw_quic_stream_type_t. * If the type can not be determined, returns nw_quic_stream_type_unknown.

/** * @typedef nw_quic_stream_type_t * @abstract * Represents the type of a QUIC stream. */ typedef enum { /*! @const nw_quic_stream_type_unknown A QUIC stream whose direction can not be determined. */ nw_quic_stream_type_unknown = 0, /*! @const nw_quic_stream_type_bidirectional A bidirectional QUIC stream. */ nw_quic_stream_type_bidirectional = 1, /*! @const nw_quic_stream_type_unidirectional An unidirectional QUIC stream. */ nw_quic_stream_type_unidirectional = 2, } nw_quic_stream_type_t;

I missed that, thx

stephen-hawley · 2022-09-01T19:59:08Z

src/Network/NWProtocolFramerOptions.cs

+		[DllImport (Constants.NetworkLibrary)]
+		static extern void nw_framer_options_set_object_value (OS_nw_protocol_options options, string key, NativeHandle value);
+
+		public void SetValue<T> (string key, T? value) where T : NSObject


If value is null won't value.GetHandle throw a NRE?

GetHandle is an extension that checks if the ptr is null.

tests/monotouch-test/Network/NWBrowserDescriptorTest.cs

src/Network/NWBrowserDescriptor.cs

src/Network/NWAdvertiseDescriptor.cs

Co-authored-by: Rolf Bjarne Kvinge <[email protected]> Co-authored-by: TJ Lambert <[email protected]>

dalexsoto

Just double check your tests results! But looks good so far!

src/Network/NWEndpoint.cs

rolfbjarne · 2022-09-02T14:18:29Z

src/Network/NWEthernetChannel.cs

+			if (parameters is null)
+				ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (parameters));
+
+			InitializeHandle (nw_ethernet_channel_create_with_parameters (ethernetType, networkInterface.Handle, parameters.Handle));


No, GetNonNullHandle throws the exception:

xamarin-macios/src/ObjCRuntime/INativeObject.cs

Lines 30 to 33 in 7511281

static public NativeHandle GetNonNullHandle (this INativeObject self, string argumentName)

{

if (self is null)

ThrowHelper.ThrowArgumentNullException (argumentName);

rolfbjarne · 2022-09-02T14:20:12Z

src/Network/NWProtocolQuicOptions.cs

+		[iOS (16,0)]
+		[Watch (9,0)]
+#endif
+		public byte StreamType => nw_quic_get_stream_type (GetCheckedHandle ());


/* * @result * Returns the type of the QUIC stream, stored in nw_quic_stream_type_t. * If the type can not be determined, returns nw_quic_stream_type_unknown.

/** * @typedef nw_quic_stream_type_t * @abstract * Represents the type of a QUIC stream. */ typedef enum { /*! @const nw_quic_stream_type_unknown A QUIC stream whose direction can not be determined. */ nw_quic_stream_type_unknown = 0, /*! @const nw_quic_stream_type_bidirectional A bidirectional QUIC stream. */ nw_quic_stream_type_bidirectional = 1, /*! @const nw_quic_stream_type_unidirectional An unidirectional QUIC stream. */ nw_quic_stream_type_unidirectional = 2, } nw_quic_stream_type_t;

rolfbjarne · 2022-09-02T14:21:13Z

src/Network/NWProtocolFramerOptions.cs

+			if (value == IntPtr.Zero)
+				return null;


This IntPtr.Zero check is unnecessary

rolfbjarne · 2022-09-02T15:13:17Z

src/Network/NWProtocolQuicOptions.cs

@@ -276,7 +276,7 @@ public SecProtocolOptions SecProtocolOptions
 		[Watch (9,0)]
 #endif
 		[DllImport (Constants.NetworkLibrary)]
-		static extern byte nw_quic_get_stream_type (OS_nw_protocol_metadata stream_metadata);
+		static extern NWQuicStreamType nw_quic_get_stream_type (OS_nw_protocol_metadata stream_metadata);


The native function still returns a byte, so it should be declared as such.

rolfbjarne

Actually this is still an issue: #15841 (review)

src/Network/NWEndpoint.cs

mandel-macaque added 2 commits August 31, 2022 21:49

[Network] Add support for Xcode 14 up to beta 6.

9e028f1

Merge branch 'xcode14' into netwroking-xcode14

e198364

mandel-macaque added the notes-mention Deserves a mention in release notes label Sep 1, 2022

tj-devel709 approved these changes Sep 1, 2022

View reviewed changes

tests/monotouch-test/Network/NWEndpointTests.cs Outdated Show resolved Hide resolved

src/Network/NWProtocolFramerOptions.cs Outdated Show resolved Hide resolved

src/Network/NWProtocolFramerOptions.cs Outdated Show resolved Hide resolved

rolfbjarne requested changes Sep 1, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

stephen-hawley requested changes Sep 1, 2022

View reviewed changes

mandel-macaque commented Sep 1, 2022

View reviewed changes

tests/monotouch-test/Network/NWBrowserDescriptorTest.cs Outdated Show resolved Hide resolved

mandel-macaque commented Sep 1, 2022

View reviewed changes

tests/monotouch-test/Network/NWBrowserDescriptorTest.cs Outdated Show resolved Hide resolved

mandel-macaque commented Sep 1, 2022

View reviewed changes

src/Network/NWBrowserDescriptor.cs Outdated Show resolved Hide resolved

mandel-macaque commented Sep 1, 2022

View reviewed changes

src/Network/NWAdvertiseDescriptor.cs Outdated Show resolved Hide resolved

Apply suggestions from code review

0fd306c

Co-authored-by: Rolf Bjarne Kvinge <[email protected]> Co-authored-by: TJ Lambert <[email protected]>

stephen-hawley approved these changes Sep 1, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

dalexsoto approved these changes Sep 2, 2022

View reviewed changes

rolfbjarne reviewed Sep 2, 2022

View reviewed changes

rolfbjarne requested changes Sep 2, 2022

View reviewed changes

Address reviews.

3ee69be

rolfbjarne reviewed Sep 2, 2022

View reviewed changes

Native uses byte.

ebcdea3

rolfbjarne approved these changes Sep 2, 2022

View reviewed changes

rolfbjarne requested changes Sep 2, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

Address reviews.

ee41826

This comment has been minimized.

Sign in to view

rolfbjarne requested changes Sep 5, 2022

View reviewed changes

src/Network/NWEndpoint.cs Show resolved Hide resolved

Address reviews.

3c8d88d

rolfbjarne approved these changes Sep 6, 2022

View reviewed changes

mandel-macaque added this to the xcode14 milestone Sep 6, 2022

mandel-macaque mentioned this pull request Sep 6, 2022

[DeviceDiscoveryUI] Added support for Xcode 14 b1 #15762

Closed

This comment has been minimized.

Sign in to view

Fix a number of failing tests.

1bf80ec

This comment has been minimized.

Sign in to view

Fix duplicated attrs.

2ee68f0

This comment has been minimized.

Sign in to view

Merge branch 'xcode14' into netwroking-xcode14

9361aaf

mandel-macaque merged commit 0eef1b6 into xamarin:xcode14 Sep 8, 2022

mandel-macaque deleted the netwroking-xcode14 branch September 8, 2022 14:55

	static public NativeHandle GetNonNullHandle (this INativeObject self, string argumentName)
	{
	if (self is null)
	ThrowHelper.ThrowArgumentNullException (argumentName);

[Network] Add support for Xcode 14 beta 6. #15841

[Network] Add support for Xcode 14 beta 6. #15841

Conversation

mandel-macaque commented Sep 1, 2022

tj-devel709 left a 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

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

dalexsoto left a 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

rolfbjarne Sep 2, 2022 • edited Loading

Choose a reason for hiding this comment

rolfbjarne left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rolfbjarne Sep 2, 2022 •

edited

Loading