Improves property value conversion and extraction (#1183)

* Refactors property extraction to use model units

Uses the model's units when extracting properties to
ensure consistency and accuracy of converted values.

Extracts property sets as a static function to provide
re-usability without the class instance.

* Refactors Revit category extraction

Improves Revit category extraction by utilizing UI units for property conversion, ensuring consistent unit handling.

Additionally, refactors the extractor to a static class.

* Improves property conversion and handling

Introduces robust property conversion and handling logic.

Leverages user interface units for length, area, and volume property conversions,
ensuring consistency with the Navisworks UI.

Enhances property data handling by using dictionaries to represent
numerical properties with associated units, providing more context
for downstream applications.

Adds property name sanitization to ensure compatibility with
Speckle's object model.

* Removes the unused `Speckle.Converter.Navisworks.Helpers` import from the PropertySetsExtractor class to reduce clutter and improve code maintainability.

Relates to CNX-2784

* Standardizes numerical property dictionary creation.

Simplifies the creation of numerical property dictionaries
by removing the `internalDefinitionName` parameter from the
`NumObj` method. This ensures a consistent format for numerical
properties across the connector.

Relates to CNX-2784

* Refactors property conversion to service

Moves property conversion logic into a dedicated service.

This improves code organization and testability and allows
to reuse logic and manage UI units consistently.

* Refactors property conversion for consistency

Standardizes property conversion by introducing a dedicated `IPropertyConverter` service.

This change ensures consistent handling of property values across different extractors,
improving data accuracy and reducing inconsistencies in quantity extraction.
It also adds resets of the property converter to ensure clean conversions for each item.

Relates to CNX-2784

* Update Converters/Navisworks/Speckle.Converters.NavisworksShared/Helpers/PropertyHelpers.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update Converters/Navisworks/Speckle.Converters.NavisworksShared/Helpers/PropertyHelpers.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update Converters/Navisworks/Speckle.Converters.NavisworksShared/Helpers/PropertyHelpers.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Moves UI Units cache to service

Moves the UI Units cache logic from the helpers to a dedicated service.

This improves the separation of concerns and makes the code more maintainable and testable.

Relates to CNX-2784

* Fixes unit label typos.

Corrects minor spelling errors in the unit labels for
centimeters, millimeters, micrometers, and kilometers.

Relates to CNX-2784

* Ensures correct units are used on send

Uses the UI Units Cache service to ensure the correct
units are being applied to objects when sending them to Speckle.

Relates to CNX-2784

* Enhances the property helper functionality to support additional features. Adjusts the constructor parameters to accommodate new requirements.

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Connectors/Navisworks/Speckle.Connectors.NavisworksShared/Operations/Send/NavisworksRootObjectBuilder.cs

@@ -1,4 +1,4 @@
-using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging;
 using Speckle.Connector.Navisworks.HostApp;
 using Speckle.Connector.Navisworks.Services;
 using Speckle.Connectors.Common.Builders;
@@ -25,7 +25,9 @@ public class NavisworksRootObjectBuilder(
   ISdkActivityFactory activityFactory,
   NavisworksMaterialUnpacker materialUnpacker,
   NavisworksColorUnpacker colorUnpacker,
-  IElementSelectionService elementSelectionService
+  IElementSelectionService elementSelectionService,
+  IUiUnitsCache uiUnitsCache,
+  InstanceStoreManager instanceStoreManager
 ) : IRootObjectBuilder<NAV.ModelItem>
 {
   private bool SkipNodeMerging { get; set; }
@@ -257,12 +259,14 @@ public class NavisworksRootObjectBuilder(
 
     (string name, string path) = GetContext(convertedBase.applicationId);
 
+    var units = uiUnitsCache.Ensure();
+
     return new NavisworksObject
     {
       name = name,
       displayValue = convertedBase["displayValue"] as List<Base> ?? [],
       properties = convertedBase["properties"] as Dictionary<string, object?> ?? [],
-      units = converterSettings.Current.Derived.SpeckleUnits,
+      units = units.ToString(),
       applicationId = convertedBase.applicationId,
       ["path"] = path
     };

Converters/Navisworks/Speckle.Converters.NavisworksShared/DataExtractors/PropertySetsExtractor.cs

@@ -1,10 +1,14 @@
-using Speckle.Converter.Navisworks.Settings;
+using Speckle.Converter.Navisworks.Services;
+using Speckle.Converter.Navisworks.Settings;
 using Speckle.Converters.Common;
 using static Speckle.Converter.Navisworks.Helpers.PropertyHelpers;
 
 namespace Speckle.Converter.Navisworks.ToSpeckle;
 
-public class PropertySetsExtractor(IConverterSettingsStore<NavisworksConversionSettings> settingsStore)
+public class PropertySetsExtractor(
+  IConverterSettingsStore<NavisworksConversionSettings> settingsStore,
+  IPropertyConverter propertyConverter
+)
 {
   internal Dictionary<string, object?>? GetPropertySets(NAV.ModelItem modelItem)
   {
@@ -18,6 +22,17 @@ public class PropertySetsExtractor(IConverterSettingsStore<NavisworksConversionS
     return propertyDictionary;
   }
 
+  private static NAV.Units GetModelUnits(NAV.ModelItem modelItem)
+  {
+    NAV.ModelItem? ancestor = modelItem;
+    while (ancestor != null && !ancestor.HasModel)
+    {
+      ancestor = ancestor.Parent;
+    }
+
+    return ancestor != null ? ancestor.Model.Units : NAV.Units.Meters;
+  }
+
   /// <summary>
   /// Extracts property sets from a NAV.ModelItem and adds them to a dictionary,
   /// PropertySets are specific to the host application source appended to Navisworks and therefore
@@ -28,6 +43,9 @@ public class PropertySetsExtractor(IConverterSettingsStore<NavisworksConversionS
   private Dictionary<string, object?> ExtractPropertySets(NAV.ModelItem modelItem)
   {
     var propertySetDictionary = new Dictionary<string, object?>();
+    var modelUnits = GetModelUnits(modelItem);
+
+    propertyConverter.Reset();
 
     foreach (var propertyCategory in modelItem.PropertyCategories)
     {
@@ -40,23 +58,18 @@ public class PropertySetsExtractor(IConverterSettingsStore<NavisworksConversionS
 
       foreach (var property in propertyCategory.Properties)
       {
-        string sanitizedName = SanitizePropertyName(property.DisplayName);
-        var propertyValue = ConvertPropertyValue(property.Value, settingsStore.Current.Derived.SpeckleUnits);
-
+        var sanitizedName = SanitizePropertyName(property.DisplayName);
+        var propertyValue = propertyConverter.ConvertPropertyValue(property.Value, modelUnits, property.DisplayName);
         if (propertyValue != null)
         {
           propertySet[sanitizedName] = propertyValue;
         }
       }
 
-      if (propertySet.Count <= 0)
+      if (propertySet.Count > 0)
       {
-        continue;
+        propertySetDictionary[SanitizePropertyName(propertyCategory.DisplayName)] = propertySet;
       }
-
-      string categoryName = SanitizePropertyName(propertyCategory.DisplayName);
-
-      propertySetDictionary[categoryName] = propertySet;
     }
 
     return propertySetDictionary;

Converters/Navisworks/Speckle.Converters.NavisworksShared/DataExtractors/RevitBuiltInCategoryExtractor.cs

@@ -1,8 +1,10 @@
-using static Speckle.Converter.Navisworks.Helpers.PropertyHelpers;
+using Speckle.Converter.Navisworks.Services;
+using Speckle.InterfaceGenerator;
 
 namespace Speckle.Converter.Navisworks.ToSpeckle;
 
-public sealed class RevitBuiltInCategoryExtractor
+[GenerateAutoInterface]
+public class RevitBuiltInCategoryExtractor(IPropertyConverter converter) : IRevitBuiltInCategoryExtractor
 {
   private const int ANCESTOR_AND_SELF_COUNT = 4; // It seems like this is the maximum depth found needed in practice
   private const string REVIT_CAT_GROUP = "LcRevitData_Element";
@@ -13,28 +15,28 @@ public sealed class RevitBuiltInCategoryExtractor
   /// Attempts to map a Navisworks/Revit display category from the given model item or its ancestors
   /// to a known Revit built-in category constant (e.g., "OST_Walls").
   /// </summary>
-  internal static bool TryGetBuiltInCategory(
-    NAV.ModelItem item,
-    out string mapped,
-    int maxDepth = ANCESTOR_AND_SELF_COUNT
-  )
+  public bool TryGetBuiltInCategory(NAV.ModelItem item, out string mapped, int maxDepth = ANCESTOR_AND_SELF_COUNT)
   {
     mapped = string.Empty;
 
-    // Look up the category value, starting at this item and walking up to maxDepth ancestors
+    // Find the category VariantData up the hierarchy
     var v = FindRevitCategoryInHierarchy(item, maxDepth);
-    if (v == null)
+    if (v is null)
     {
       return false;
     }
 
-    var name = ConvertPropertyValue(v, "")?.ToString();
+    converter.Reset();
+
+    // Convert using per-object model units and current UI units
+    var nameObj = converter.ConvertPropertyValue(v, item.Model.Units, item.DisplayName);
+    var name = nameObj?.ToString();
     if (string.IsNullOrWhiteSpace(name))
     {
       return false;
     }
 
-    name = name?.Trim();
+    name = name!.Trim();
 
     // Map display name to OST_* built-in category constant
     var builtInCategory = DisplayNameToRevitBuiltInCategory(name);

Converters/Navisworks/Speckle.Converters.NavisworksShared/DataHandlers/HierarchicalPropertyHandler.cs

@@ -11,7 +11,8 @@ public class HierarchicalPropertyHandler(
   PropertySetsExtractor propertySetsExtractor,
   ModelPropertiesExtractor modelPropertiesExtractor,
   ClassPropertiesExtractor classPropertiesExtractor,
-  IConverterSettingsStore<NavisworksConversionSettings> settingsStore
+  IConverterSettingsStore<NavisworksConversionSettings> settingsStore,
+  IRevitBuiltInCategoryExtractor revitCategoryExtractor
 ) : BasePropertyHandler(propertySetsExtractor, modelPropertiesExtractor)
 {
   private static string PseudoClassPropertiesKey => "_pseudoClassProperties";
@@ -22,7 +23,7 @@ public class HierarchicalPropertyHandler(
     var propertyDict = classPropertiesExtractor.GetClassProperties(modelItem) ?? [];
 
     // Interop-lite mapping for Revit built-in categories
-    if (_mapRevit && RevitBuiltInCategoryExtractor.TryGetBuiltInCategory(modelItem, out var builtInCategory))
+    if (_mapRevit && revitCategoryExtractor.TryGetBuiltInCategory(modelItem, out var builtInCategory))
     {
       PropertyHelpers.AddPropertyIfNotNullOrEmpty(
         propertyDict,

Converters/Navisworks/Speckle.Converters.NavisworksShared/Helpers/PropertyHelpers.cs

@@ -1,4 +1,4 @@
-using System.Globalization;
+using System.Globalization;
 using System.Text.RegularExpressions;
 using Speckle.Objects.Geometry;
 
@@ -8,6 +8,9 @@ public static class PropertyHelpers
 {
   private static readonly HashSet<string> s_excludedCategories = ["Geometry", "Metadata"];
 
+  /// <summary>
+  /// Adds a property to an object (either a Base object or a Dictionary) if the value is not null or empty.
+  /// </summary>
   private static readonly Dictionary<NAV.VariantDataType, Func<NAV.VariantData, string, dynamic?>> s_typeHandlers =
     new()
     {
@@ -111,3 +114,27 @@ public static class PropertyHelpers
   internal static bool IsCategoryToBeSkipped(NAV.PropertyCategory propertyCategory) =>
     s_excludedCategories.Contains(propertyCategory.DisplayName);
 }
+
+internal static class UnitLabels
+{
+  internal static string Linear(NAV.Units u) =>
+    u switch
+    {
+      NAV.Units.Kilometers => "Kilometers",
+      NAV.Units.Meters => "Metres",
+      NAV.Units.Centimeters => "Centimeters",
+      NAV.Units.Millimeters => "Millimeters",
+      NAV.Units.Micrometers => "Micrometers",
+      NAV.Units.Miles => "Miles",
+      NAV.Units.Yards => "Yards",
+      NAV.Units.Feet => "Feet",
+      NAV.Units.Inches => "Inches",
+      NAV.Units.Mils => "Mils",
+      NAV.Units.Microinches => "Microinches",
+      _ => "Metres"
+    };
+
+  internal static string Area(NAV.Units u) => $"Square {Linear(u).ToLower()}";
+
+  public static string Volume(NAV.Units u) => $"Cubic {Linear(u).ToLower()}";
+}

Converters/Navisworks/Speckle.Converters.NavisworksShared/Services/PropertyConversion.cs

@@ -0,0 +1,91 @@
+using System.Globalization;
+using Speckle.Converter.Navisworks.Helpers;
+using Speckle.InterfaceGenerator;
+
+namespace Speckle.Converter.Navisworks.Services;
+
+[GenerateAutoInterface]
+public class PropertyConverter(IUiUnitsCache uiUnitsCache) : IPropertyConverter
+{
+  public void Reset() => uiUnitsCache.Reset();
+
+  public object? ConvertPropertyValue(NAV.VariantData? value, NAV.Units modelUnits, string propDisplayName) =>
+    value == null
+      ? null
+      : _handlers.TryGetValue(value.DataType, out var f)
+        ? f(value, (modelUnits, propDisplayName))
+        : value.DataType is NAV.VariantDataType.None or NAV.VariantDataType.Point2D
+          ? null
+          : value.ToString();
+
+  private readonly Dictionary<
+    NAV.VariantDataType,
+    Func<NAV.VariantData, (NAV.Units model, string name), object?>
+  > _handlers =
+    new()
+    {
+      { NAV.VariantDataType.Boolean, (v, _) => v.ToBoolean() },
+      { NAV.VariantDataType.DisplayString, (v, _) => v.ToDisplayString() },
+      { NAV.VariantDataType.IdentifierString, (v, _) => v.ToIdentifierString() },
+      { NAV.VariantDataType.Int32, (v, _) => v.ToInt32() },
+      { NAV.VariantDataType.Double, (v, _) => v.ToDouble() },
+      // Angle as dictionary with units
+      { NAV.VariantDataType.DoubleAngle, (v, t) => NumObj(t.name, v.ToDoubleAngle(), "Degrees") },
+      // Length → dictionary in UI units
+      {
+        NAV.VariantDataType.DoubleLength,
+        (v, t) =>
+        {
+          var uiUnits = uiUnitsCache.Ensure();
+
+          var k = NAV.UnitConversion.ScaleFactor(t.model, uiUnits);
+          return NumObj(t.name, v.ToDoubleLength() * k, UnitLabels.Linear(uiUnits));
+        }
+      },
+      // Area → dictionary in UI units^2
+      {
+        NAV.VariantDataType.DoubleArea,
+        (v, t) =>
+        {
+          var uiUnits = uiUnitsCache.Ensure();
+          var k = NAV.UnitConversion.ScaleFactor(t.model, uiUnits);
+          k *= k;
+          return NumObj(t.name, v.ToDoubleArea() * k, UnitLabels.Area(uiUnits));
+        }
+      },
+      // Volume → dictionary in UI units^3
+      {
+        NAV.VariantDataType.DoubleVolume,
+        (v, t) =>
+        {
+          var uiUnits = uiUnitsCache.Ensure();
+          var k = NAV.UnitConversion.ScaleFactor(t.model, uiUnits);
+          k = k * k * k;
+          return NumObj(t.name, v.ToDoubleVolume() * k, UnitLabels.Volume(uiUnits));
+        }
+      },
+      { NAV.VariantDataType.DateTime, (v, _) => v.ToDateTime().ToString(CultureInfo.InvariantCulture) },
+      { NAV.VariantDataType.NamedConstant, (v, _) => v.ToNamedConstant().DisplayName },
+      { NAV.VariantDataType.None, (_, _) => null },
+      { NAV.VariantDataType.Point2D, (_, _) => null },
+      {
+        NAV.VariantDataType.Point3D,
+        (v, t) =>
+        {
+          var uiUnits = uiUnitsCache.Ensure();
+          var k = NAV.UnitConversion.ScaleFactor(t.model, uiUnits);
+          var p = v.ToPoint3D();
+
+          return new Speckle.Objects.Geometry.Point(p.X * k, p.Y * k, p.Z * k, UnitLabels.Linear(uiUnits));
+        }
+      }
+    };
+
+  private static Dictionary<string, object> NumObj(string name, double value, string units) =>
+    new()
+    {
+      ["name"] = name,
+      ["value"] = value,
+      ["units"] = units
+    };
+}

Converters/Navisworks/Speckle.Converters.NavisworksShared/Services/UIUnits.cs

@@ -0,0 +1,71 @@
+using Autodesk.Navisworks.Api.Interop;
+using Speckle.InterfaceGenerator;
+using static Autodesk.Navisworks.Api.Interop.LcUOption;
+
+namespace Speckle.Converter.Navisworks.Services;
+
+[GenerateAutoInterface]
+public class UiUnitsCache : IUiUnitsCache
+{
+  private NAV.Units? _ui;
+
+  public NAV.Units Ensure()
+  {
+    if (_ui.HasValue)
+    {
+      return _ui.Value;
+    }
+
+    UiUnitsUtil.TryGetUiLinearUnits(out var ui);
+    _ui = ui;
+    return _ui.Value;
+  }
+
+  public void Reset() => _ui = null;
+}
+
+public static class UiUnitsUtil
+{
+  // disp_units: 0=linear_format
+  public static bool TryGetUiLinearUnits(out NAV.Units uiUnits)
+  {
+    using var opt = new LcUOptionLock();
+    var root = GetRoot(opt);
+    var disp = root.GetSubOptions("interface").GetSubOptions("disp_units");
+
+    int code = -1;
+
+    using var v = new NAV.VariantData();
+    disp.GetValue(0, v);
+    var s = v.ToString();
+    var colon = s.LastIndexOf(':');
+    var open = s.IndexOf('(', colon + 1);
+    if (colon >= 0 && open > colon && !int.TryParse(s.Substring(colon + 1, open - colon - 1), out code))
+    {
+      code = -1;
+    }
+
+    uiUnits = code switch
+    {
+      0 => NAV.Units.Kilometers,
+      1 => NAV.Units.Meters,
+      2 => NAV.Units.Centimeters,
+      3 => NAV.Units.Millimeters,
+      4 => NAV.Units.Micrometers,
+      5 => NAV.Units.Miles,
+      6 => NAV.Units.Miles,
+      7 => NAV.Units.Yards,
+      8 => NAV.Units.Yards,
+      9 => NAV.Units.Feet,
+      10 => NAV.Units.Feet,
+      11 => NAV.Units.Feet,
+      12 => NAV.Units.Inches,
+      13 => NAV.Units.Inches,
+      14 => NAV.Units.Mils,
+      15 => NAV.Units.Microinches,
+      _ => NAV.Units.Meters
+    };
+
+    return code >= 0;
+  }
+}

Converters/Navisworks/Speckle.Converters.NavisworksShared/Speckle.Converters.NavisworksShared.projitems

@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8"?>
+<?xml version="1.0" encoding="utf-8"?>
 <Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
   <PropertyGroup>
     <MSBuildAllProjects>$(MSBuildAllProjects);$(MSBuildThisFileFullPath)</MSBuildAllProjects>
@@ -39,5 +39,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)ToSpeckle\NavisworksRootToSpeckleConverter.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)ToSpeckle\Raw\BoundingBoxToSpeckleRawConverter.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)ToSpeckle\TopLevel\ModelItemTopLevelConverterToSpeckle.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Services\PropertyConversion.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Services\UIUnits.cs"/>
   </ItemGroup>
-</Project>
+</Project>