Support `IMap` and `IMapView` #530

halildurmus · 2022-07-15T18:06:51Z

Closes #486

Adds support for IKeyValuePair, IMap, and IMapView interfaces
Adds PedometerStepKind enum (used as a key type for the above interfaces)
Adds PropertySet, MediaPropertySet, StringMap and ValueSet classes (implementations of the IMap)
Adds NotificationData class

After we land this, I'll send a PR that will include support for Windows.Data.Json and tests for IMap<String, IJsonValue?>.

As for the IMap<Object, Object?> type, I haven't been able to test it as the ResourceDictionary is the only API that supports it, and I think it only works in UWP apps.

halildurmus · 2022-07-17T15:32:20Z

lib/src/guid.dart

+  @override
+  bool operator ==(Object other) {
+    if (identical(this, other)) return true;
+
+    return other is GUID &&
+        other.Data1 == Data1 &&
+        other.Data2 == Data2 &&
+        other.Data3 == Data3 &&
+        other.Data4 == Data4;
+  }
+
+  @override
+  int get hashCode =>
+      Data1.hashCode ^ Data2.hashCode ^ Data3.hashCode ^ Data4.hashCode;


These overrides are required for Map<GUID, XX> types (e.g. Map<GUID, Object?>) to work properly.

halildurmus · 2022-07-17T15:36:38Z

lib/src/winrt/foundation/collections/ikeyvaluepair.dart

+  /// Gets the key of the key-value pair.
+  K get key {
+    if (isSameType<K, GUID>()) return _key_GUID as K;
+    if (isSameType<K, int>()) return _key_Uint32 as K;


I used Uint32 for int keys since it is the only int type to be supported for IKeyValuePair: https://github.com/timsneath/win32/blob/339c6e4ac04a9d2f2c1933c958bc0893be65dee2/lib/src/winrt_constants.dart#L147-L148

halildurmus · 2022-07-17T15:45:30Z

lib/src/winrt/foundation/collections/ikeyvaluepair.dart

+  V get _value_enum {
+    final retValuePtr = calloc<Int32>();


Used Int32 here for enum values since all supported enum types for IKeyValuePair are Int32 type (e.g. AppCapabilityAccessStatus, ChatMessageStatus).

timsneath

This is an astonishing piece of work. You have outclassed me! I've been busy in my free time fixing up another package that I own which has bitrotted a little, so I'm thrilled to see you continuing to make progress here. A few comments inline to check out.

At some point, we need to converge on a release, since there are folk waiting on more mundane fixes (e.g. some of the Win32 APIs that have been added over the last couple of months. I'm thinking of shipping a 3.0.0-pre based on what we've got so far to help out those who are waiting, and then we can keep iterating until you feel the new WinRT projection has settled down. WDYT?

timsneath · 2022-07-18T15:49:34Z

lib/src/winrt/foundation/collections/ikeyvaluepair.dart

+    }
+  }
+
+  PedometerStepKind get _key_PedometerStepKind {


Forgive me, but this seems weird here. Why would such a specific type as PedometerStepKind be embedded in a generic collection object like this? There could be hundreds of these, couldn't there? What if we decide (as is likely) to partition Windows.Device.Sensors into a different export from Windows.Foundation.Collections?

Here are the supported key-value pairs of IKeyValuePair according to IIDs taken from here:
https://github.com/timsneath/win32/blob/339c6e4ac04a9d2f2c1933c958bc0893be65dee2/lib/src/winrt_constants.dart#L62-L148

According to these, there is only one enum type (PedometerStepKind) used as a key type of IKeyValuePair. It seemed easier to handle it this way instead of adding a new parameter (keyCreator) that takes an enum constructor for the key type of ÌKeyValuePair which will allow me to create an instance of that enum type.

I can modify the implementation to handle these types with the keyCreator parameter I mentioned above if that's what you want.

I wonder if the best thing is to make the PedometerStepKind pieces an extension method in winrt/device/sensors? I don't know that this is the right answer, but it does feel like a very odd outlier. Are there other classes that might have a similar pattern, I wonder?

I wonder if the best thing is to make the PedometerStepKind pieces an extension method in winrt/device/sensors?

I've created extension methods for the PedometerStepKind pieces but I'm not sure if it's the way you wanted it. Could you take a look?

Are there other classes that might have a similar pattern, I wonder?

I don't think there are.

timsneath · 2022-07-18T15:54:54Z

lib/src/winrt/foundation/ipropertyvalue.dart

@@ -38,6 +33,93 @@ class IPropertyValue extends IInspectable {
  factory IPropertyValue.from(IInspectable interface) =>
      IPropertyValue.fromRawPointer(interface.toInterface(IID_IPropertyValue));

+  Object? get value {


I'm wondering if this should be implemented by keeping the interface autogenerated but adding an extension method for this helper method? The more that is autogenerated, the better, obviously -- both less prone to errors and more likely to catch errors in the generation. WDYT?

Yeah, that makes sense. Before doing that, I need to modify the generator to fix the deallocation bug I mentioned below.

timsneath · 2022-07-18T15:55:27Z

lib/src/winrt/foundation/ipropertyvalue.dart

@@ -363,24 +445,18 @@ class IPropertyValue extends IInspectable {
  GUID getGuid() {


Is this deallocation a bug in the generator code? Or is this fix unique to this interface?

Is this deallocation a bug in the generator code?

Yeah, it is a bug in the generator. retValuePtr should not be freed for structs (just like for COMObjects).

I don't know how can we avoid memory leaks from these as NativeFinalizer cannot be used with structs.

halildurmus · 2022-07-18T17:36:04Z

This is an astonishing piece of work. You have outclassed me! I've been busy in my free time fixing up another package that I own which has bitrotted a little, so I'm thrilled to see you continuing to make progress here. A few comments inline to check out.

Thank you!

At some point, we need to converge on a release, since there are folk waiting on more mundane fixes (e.g. some of the Win32 APIs that have been added over the last couple of months. I'm thinking of shipping a 3.0.0-pre based on what we've got so far to help out those who are waiting, and then we can keep iterating until you feel the new WinRT projection has settled down. WDYT?

That's a great idea. I saw your comment and I think these are great. I'll let you know if I come up with something to add there.

timsneath

LGTM!

halildurmus added enhancement New feature or request winrt labels Jul 15, 2022

halildurmus added this to In progress in WinRT projection via automation Jul 15, 2022

halildurmus added 27 commits July 17, 2022 09:57

Organize imports

930d179

Add PedometerStepKind enum

6521269

Update GUIDFromString method

ca7fa45

Add isSimilarType helper method

85805c8

Add IMap<K, V> IIDs

59a09bd

Override == operator and hashCode for GUID

558d110

Add helper methods

9d03586

Do not free retValuePtr for structs

bc5bdc4

Add value getter to IPropertyValue

92ccaa3

Project IKeyValuePair

731f183

Add toMap helper method

7e60bb9

Project IMapView

320d85d

Project IMap

2682f68

Add PropertySet, StringMap, ValueSet and MediaPropertySet

572b87a

Update excludedWindowsRuntimeTypes

b6442a8

Update exports

679e209

Add factory constructors to IMap

1002117

Expose the APIs that return IMap or IMapView correctly

e564c6f

Generate NotificationData

cd5a2be

There is no need to skip the test with the recent change to getGuid()

4513225

Add new TODOs

5ec4a88

Add isSupportedKeyValuePair helper method

87015c1

Improve documentation

6f4bb6e

Update documentation

2bef102

Use isTrue, isFalse matchers

7b08ecb

Add tests

dcefcd5

Add tests for IMapView<String, String?>

6f0251b

halildurmus force-pushed the map-mapview branch from 2d14e7a to 6f0251b Compare July 17, 2022 07:01

Simplify generic type check

142a590

halildurmus commented Jul 17, 2022

View reviewed changes

Return unmodifiable map

2e6c716

halildurmus marked this pull request as ready for review July 17, 2022 16:18

timsneath reviewed Jul 18, 2022

View reviewed changes

halildurmus added 7 commits July 20, 2022 16:54

Create an extension method to get the value stored in the IPropertyValue

23ccda3

Add PedometerReading

953ed0c

Create extension methods to support key types of PedometerStepKind

030b2bc

Make sure type is WinRT type

2e47dc2

Merge branch 'main' into map-mapview

b4f6ecc

Move helper functions to ipropertyvalue_helpers.dart

209eda6

Add allocator parameter to PropertySet

6ef6068

WinRT projection automation moved this from In progress to Reviewer approved Aug 5, 2022

timsneath approved these changes Aug 5, 2022

View reviewed changes

timsneath merged commit c35d945 into main Aug 5, 2022

WinRT projection automation moved this from Reviewer approved to Done Aug 5, 2022

timsneath deleted the map-mapview branch August 5, 2022 23:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support `IMap` and `IMapView` #530

Support `IMap` and `IMapView` #530

halildurmus commented Jul 15, 2022 •

edited

halildurmus Jul 17, 2022

halildurmus Jul 17, 2022

halildurmus Jul 17, 2022

timsneath left a comment

timsneath Jul 18, 2022

halildurmus Jul 18, 2022

timsneath Jul 20, 2022

halildurmus Jul 20, 2022

timsneath Jul 18, 2022

halildurmus Jul 18, 2022

halildurmus Jul 20, 2022

timsneath Jul 18, 2022

halildurmus Jul 18, 2022 •

edited

halildurmus commented Jul 18, 2022

timsneath left a comment

		@@ -363,24 +445,18 @@ class IPropertyValue extends IInspectable {
		GUID getGuid() {

Support IMap and IMapView #530

Support IMap and IMapView #530

Conversation

halildurmus commented Jul 15, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timsneath 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

Choose a reason for hiding this comment

halildurmus Jul 18, 2022 • edited

Choose a reason for hiding this comment

halildurmus commented Jul 18, 2022

timsneath left a comment

Choose a reason for hiding this comment

Support `IMap` and `IMapView` #530

Support `IMap` and `IMapView` #530

halildurmus commented Jul 15, 2022 •

edited

halildurmus Jul 18, 2022 •

edited