Fix HA changed light attributes are all null iso missing for lights that are off (#1035)

Author: FrankBakkerNl

src/HassModel/NetDaemon.HassModel.CodeGenerator/CodeGeneration/AttributeTypeGenerator.cs

@@ -16,15 +16,15 @@ internal static class AttributeTypeGenerator
     ///
     ///     [JsonPropertyName("color_mode")]
     ///     public string? ColorMode { get; init; }
-    ///    
+    ///
     ///     [JsonPropertyName("color_temp")]
     ///     public double? ColorTemp { get; init; }
-    /// }   
+    /// }
     /// </example>
     public static RecordDeclarationSyntax GenerateAttributeRecord(EntityDomainMetadata domain)
     {
         var propertyDeclarations = domain.Attributes
-            .Select(a => AutoPropertyGetInit($"{a.ClrType.GetFriendlyName()}?", a.CSharpName)
+            .Select(a => AutoPropertyGetInit($"{(a.ClrType ?? typeof(object)).GetFriendlyName()}?", a.CSharpName)
                 .ToPublic()
                 .WithJsonPropertyName(a.JsonName));
 
@@ -34,4 +34,4 @@ public static RecordDeclarationSyntax GenerateAttributeRecord(EntityDomainMetada
 
         return domain.AttributesBaseClass != null ? record.WithBase(SimplifyTypeName(domain.AttributesBaseClass)) : record;
     }
-}
+}

src/HassModel/NetDaemon.HassModel.CodeGenerator/MetaData/EntityMetaData/AttributeMetaDataGenerator.cs

@@ -19,39 +19,39 @@ public static IEnumerable<EntityAttributeMetaData> GetMetaDataFromEntityStates(I
                 CSharpName: group.Key.ToValidCSharpPascalCase(),
                 ClrType: GetBestClrType(group.Select(g => g.Value))));
 
-        // We ignore possible duplicate CSharp names here, they will be handled later 
+        // We ignore possible duplicate CSharp names here, they will be handled later
         // by the MetaDataMerger
         return attributesByJsonName;
     }
-    private static Type GetBestClrType(IEnumerable<JsonElement> valueKinds)
+    private static Type? GetBestClrType(IEnumerable<JsonElement> valueKinds)
     {
         var distinctCrlTypes = valueKinds
             .Where(e => e.ValueKind != JsonValueKind.Null) // null fits in any type so we can ignore it for now
             .GroupBy(e => MapJsonType(e.ValueKind))
             .ToHashSet();
 
-        if (distinctCrlTypes.Count is 0 or > 1)
-        {
-            // Either all inputs where JsonValueKind.Null, or there are multiple possible
-            // input types. In either case, the return must be object
-            return typeof(object);
-        }
+        // If we have no items left that means all attributes had JsonValueKind.Null,
+        // this can occur eg for light brightness if all lights are currently off
+        // In that case we will not determine a clrType here because there might be a better type when this
+        // metadata gets merged
+        if (distinctCrlTypes.Count == 0) return null;
+
+        // Multiple types were found so we need to resort to object
+        if (distinctCrlTypes.Count > 1) return typeof(object);
 
         // For arrays, we want to enumerate the sub-elements of the array. If there's a single
         // element type, then we'll construct an IROList<subtype>, otherwise we'll construct
         // IROList<object>
         var clrTypeGroup = distinctCrlTypes.Single();
         if (clrTypeGroup.Key == typeof(IReadOnlyList<>))
         {
-            var listSubType = GetBestClrType(clrTypeGroup.SelectMany(el => el.EnumerateArray()));
+            var listSubType = GetBestClrType(clrTypeGroup.SelectMany(el => el.EnumerateArray())) ?? typeof(object);
             return clrTypeGroup.Key.MakeGenericType(listSubType);
         }
-        else
-        {
-            return clrTypeGroup.Key;
-        }
+
+        return clrTypeGroup.Key;
     }
-    
+
     private static Type MapJsonType(JsonValueKind kind) =>
         kind switch
         {

src/HassModel/NetDaemon.HassModel.CodeGenerator/MetaData/EntityMetaData/ClrTypeJsonConverter.cs

@@ -5,18 +5,18 @@ namespace NetDaemon.HassModel.CodeGenerator;
 /// <summary>
 /// Json(De)Serializes a System.Type using a 'friendly name'
 /// </summary>
-internal class ClrTypeJsonConverter : JsonConverter<Type>
+internal class ClrTypeJsonConverter : JsonConverter<Type?>
 {
-    public override Type Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
+    public override Type? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
     {
         var typeName = reader.GetString();
-        if (typeName == null) return typeof(object);
+        if (typeName == null) return null;
 
         return Type.GetType(typeName) ?? throw new InvalidOperationException($@"Type {typeName} is not found when deserializing");
     }
 
-    public override void Write(Utf8JsonWriter writer, Type value, JsonSerializerOptions options)
+    public override void Write(Utf8JsonWriter writer, Type? value, JsonSerializerOptions options)
     {
-        writer.WriteStringValue(value.ToString());
+        writer.WriteStringValue(value?.ToString());
     }
-}
+}

src/HassModel/NetDaemon.HassModel.CodeGenerator/MetaData/EntityMetaData/EntityDomainMetadata.cs

@@ -9,28 +9,28 @@ record EntitiesMetaData
 
 record EntityDomainMetadata(
     string Domain,
-    
-    [property: JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] 
+
+    [property: JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)]
     bool IsNumeric,
-    
+
     IReadOnlyList<EntityMetaData> Entities,
-    
+
     IReadOnlyList<EntityAttributeMetaData> Attributes
     )
 {
-    private static readonly HashSet<string> CoreInterfaces = 
+    private static readonly HashSet<string> CoreInterfaces =
         typeof(IEntityCore).Assembly.GetTypes()
             .Where(t => t.IsInterface && t.IsAssignableTo(typeof(IEntityCore)))
             .Select(t => t.Name)
             .ToHashSet();
-    
+
     private readonly string prefixedDomain = (IsNumeric && EntityIdHelper.MixedDomains.Contains(Domain)  ? "numeric_" : "") + Domain;
 
     [JsonIgnore]
     public string EntityClassName => $"{prefixedDomain}Entity".ToValidCSharpPascalCase();
 
     /// <summary>
-    /// Returns the name of the corresponding Core Interface if it exists, or null if it does not 
+    /// Returns the name of the corresponding Core Interface if it exists, or null if it does not
     /// </summary>
     [JsonIgnore]
     public string? CoreInterfaceName
@@ -54,4 +54,4 @@ public string? CoreInterfaceName
 
 record EntityMetaData(string id, string? friendlyName, string cSharpName);
 
-record EntityAttributeMetaData(string JsonName, string CSharpName, Type ClrType);
+record EntityAttributeMetaData(string JsonName, string CSharpName, Type? ClrType);

src/HassModel/NetDaemon.HassModel.CodeGenerator/MetaData/EntityMetaData/EntityMetaDataMerger.cs

@@ -9,19 +9,19 @@ internal static class EntityMetaDataMerger
 #pragma warning disable CS0618 // Type or member is obsolete
     private static readonly Type[] _possibleBaseTypes = typeof(LightAttributesBase).Assembly.GetTypes();
 #pragma warning restore CS0618 // Type or member is obsolete
-    
+
     // We need to merge the previously saved metadata with the current metadata from HA
     // We do this because sometimes entities do not provide all their attributes,
     // like a Light only has a brightness attribute when it is turned on
-    // 
+    //
     // data structure:
     // [ {
     //   "domain": "weather",
     //   "isNumeric": false,
     //   "entities": [ {
     //       "id": "",
     //       "friendlyName":""
-    //   }] 
+    //   }]
     //   "attributes" : [ {
     //       "jsonName": "temperature",
     //       "cSharpName": "Temperature",
@@ -49,7 +49,7 @@ public static EntitiesMetaData Merge(CodeGenerationSettings codeGenerationSettin
             previous = SetBaseTypes(previous);
             current = SetBaseTypes(current);
         }
-        
+
         return previous with
         {
             Domains = FullOuterJoin(previous.Domains, current.Domains, p => (p.Domain.ToLowerInvariant(), p.IsNumeric), MergeDomains)
@@ -72,11 +72,11 @@ public static EntitiesMetaData SetBaseTypes(EntitiesMetaData entitiesMetaData)
             Domains = entitiesMetaData.Domains.Select(m => DeriveFromBasetype(m, _possibleBaseTypes)).ToList()
         };
     }
-    
+
     private static EntityDomainMetadata DeriveFromBasetype(EntityDomainMetadata domainMetadata, IReadOnlyCollection<Type> possibleBaseTypes)
     {
         var baseType = possibleBaseTypes.FirstOrDefault(t => t.Name == domainMetadata.AttributesClassName + "Base");
-        
+
         var basePropertyJsonNames = baseType?.GetProperties()
             .Select(p => p.GetCustomAttribute<JsonPropertyNameAttribute>()?.Name ?? p.Name)
             .ToHashSet() ?? new HashSet<string>();
@@ -93,60 +93,57 @@ private static EntityDomainMetadata MergeDomains(EntityDomainMetadata previous,
         // Only keep entities from Current but merge the attributes
         return current with
         {
-            Attributes = MergeAttributeSets(previous.Attributes, current.Attributes).ToList()
+            Attributes = FullOuterJoin(previous.Attributes, current.Attributes, a => a.JsonName, MergeAttributes).ToList()
         };
     }
 
-    private static IEnumerable<EntityAttributeMetaData> MergeAttributeSets(
-        IReadOnlyCollection<EntityAttributeMetaData> previousAttributes,
-        IEnumerable<EntityAttributeMetaData> currentAttributes)
-    {
-        return FullOuterJoin(previousAttributes, currentAttributes, a => a.JsonName, MergeAttributes);
-    }
-    
     private static EntityAttributeMetaData MergeAttributes(EntityAttributeMetaData previous, EntityAttributeMetaData current)
     {
         // for Attributes matching by the JsonName keep the previous
-        // this makes sure the preferred CSharpName stays the same
+        // this makes sure the preferred CSharpName stays the same, we only merge the types
         return previous with
         {
-            // In case the ClrType derived from the current metadata is not the same as the previous
-            // we will use 'object' for safety
-            
-            ClrType = previous.ClrType == current.ClrType ? previous.ClrType : typeof(object),
-            // There is a possible issue here when one of the ClrTypes is 'object'
-            // It can be object because either all inputs where JsonValueKind.Null, or there are
-            // multiple possible input types.
-            // Right here we dont actually know which it was, we will assume there were multiple,
-            // so the resulting type will stay object 
+            ClrType = MergeTypes(previous.ClrType, current.ClrType)
         };
     }
 
+    private static Type? MergeTypes(Type? previous, Type? current)
+    {
+        // null for previous or current type means we did not get any non-null values to determine a type from
+        // so if previous or current is null we use the other.
+        // if for some reason the type has changed we use object to support both.
+        return
+            previous == current ? previous :
+            previous is null ? current :
+            current is null ? previous :
+            typeof(object);
+    }
+
     private static EntityDomainMetadata HandleDuplicateCSharpNames(EntityDomainMetadata entitiesMetaData)
     {
         // This hashset will initially have all Member names in the base class.
-        // We will then also add all new names to this set so we are sure they will all be unique 
+        // We will then also add all new names to this set so we are sure they will all be unique
         var reservedCSharpNames = entitiesMetaData.AttributesBaseClass?
             .GetMembers().Select(p => p.Name).ToHashSet() ?? new HashSet<string>();
 
         var withDeDuplicatedCSharpNames = entitiesMetaData.Attributes
             .GroupBy(t => t.CSharpName)
             .SelectMany(s => DeDuplicateCSharpNames(s.Key, s, reservedCSharpNames)).ToList();
-        
+
         return entitiesMetaData with
         {
             Attributes = withDeDuplicatedCSharpNames
         };
     }
-    
+
     private static IEnumerable<EntityAttributeMetaData> DeDuplicateCSharpNames(
-        string preferredCSharpName, IEnumerable<EntityAttributeMetaData> items, 
+        string preferredCSharpName, IEnumerable<EntityAttributeMetaData> items,
         ISet<string> reservedCSharpNames)
     {
         var list = items.ToList();
         if (list.Count == 1 && reservedCSharpNames.Add(preferredCSharpName))
         {
-            // Just one Attribute with this preferredCSharpName AND it was not taken yet 
+            // Just one Attribute with this preferredCSharpName AND it was not taken yet
             return new[] { list[0]};
         }
 
@@ -167,28 +164,28 @@ string ReserveNextAvailableName()
     }
 
     /// <summary>
-    /// Full outer join two sets based on a key and merge the matches 
+    /// Full outer join two sets based on a key and merge the matches
     /// </summary>
     /// <returns>
     /// All items from previous and current that dont match
     /// A merged item for all matches based in the Merger delegate
     /// </returns>
     private static IEnumerable<T> FullOuterJoin<T, TKey>(
         IEnumerable<T> previous,
-        IEnumerable<T> current, 
-        Func<T, TKey> keySelector, 
+        IEnumerable<T> current,
+        Func<T, TKey> keySelector,
         Func<T,T,T> merger) where TKey : notnull
     {
         var previousLookup = previous.ToDictionary(keySelector);
         var currentLookup = current.ToLookup(keySelector);
-            
+
         var inPrevious = previousLookup
             .Select(p => (previous: p.Value, current: currentLookup[p.Key].FirstOrDefault()))
-            .Select(t => t.current == null 
-                ? t.previous // Item in previous doe snot exist in current, return previous 
+            .Select(t => t.current == null
+                ? t.previous // Item in previous doe snot exist in current, return previous
                 : merger(t.previous, t.current)); // match, so call merge delegate
-        
+
         var onlyInCurrent = currentLookup.Where(l => !previousLookup.ContainsKey(l.Key)).SelectMany(l => l);
         return inPrevious.Concat(onlyInCurrent);
     }
-}
+}

src/HassModel/NetDaemon.HassModel.Tests/CodeGenerator/AttributeMetaDataGeneratorTest.cs

@@ -36,8 +36,52 @@ public void SameAttributeDifferentArrayTypes_obect()
         copy.Should().Be(metadata.First());
     }
 
+    [Fact]
+    public void AllNullShouldBeNull()
+    {
+        var entityStates = new HassState[] {
+            new() { AttributesJson = new { size = (object?)null, }.AsJsonElement() },
+            new() { AttributesJson = new { size = (object?)null, }.AsJsonElement() },
+        };
+
+        var metadata = AttributeMetaDataGenerator.GetMetaDataFromEntityStates(entityStates);
+
+        metadata.Should().BeEquivalentTo(new[] { new EntityAttributeMetaData("size", "Size", null) });
+    }
+
+    [Fact]
+    public void IncludeSaveAndMergeState()
+    {
+        // first we have a few lights that are ON and therefore have a brightness propert as a double
+        var entityStates = new HassState[] {
+            new() {EntityId = "light.attic", AttributesJson = new { brightness = 2.1d, }.AsJsonElement() },
+            new() {EntityId = "light.livingroom", AttributesJson = new { brightness = 2.2d, }.AsJsonElement() },
+        };
+
+        var metadata = AttributeMetaDataGenerator.GetMetaDataFromEntityStates(entityStates).ToList();
+        // lets pretend it gets saved and reloaded from disk
+        metadata = SerializeAndDeserialize(metadata);
+
+        // now get new metadata while the lights are off (brightness = null)
+        var newEntityStates = new HassState[] {
+            new() {EntityId = "light.attic", AttributesJson = new { brightness = (object?)null, }.AsJsonElement() },
+            new() {EntityId = "light.livingroom", AttributesJson = new { brightness = (object?)null, }.AsJsonElement() },
+        };
+
+        var newMetadata = AttributeMetaDataGenerator.GetMetaDataFromEntityStates(newEntityStates).ToList();
+
+        // merge the previous and current metadata
+        var merged = EntityMetaDataMerger.Merge(new CodeGenerationSettings(),
+            new EntitiesMetaData { Domains = new []{new EntityDomainMetadata("light", false, Array.Empty<EntityMetaData>(), metadata!)}},
+            new EntitiesMetaData { Domains = new []{new EntityDomainMetadata("light", false, Array.Empty<EntityMetaData>(), newMetadata)}}
+            );
+
+        // We should still have Brightness as a double
+        merged.Domains.First().Attributes.Should().BeEquivalentTo(new[] { new EntityAttributeMetaData("brightness", "Brightness", typeof(double)) });
+    }
+
 
-    private static EntityAttributeMetaData? SerializeAndDeserialize(EntityAttributeMetaData input)
+    private static T? SerializeAndDeserialize<T>(T input)
     {
         var serializeOptions = new JsonSerializerOptions()
         {
@@ -47,7 +91,7 @@ public void SameAttributeDifferentArrayTypes_obect()
         };
 
         var json = JsonSerializer.Serialize(input, serializeOptions);
-        return JsonSerializer.Deserialize<EntityAttributeMetaData>(json, serializeOptions);
+        return JsonSerializer.Deserialize<T>(json, serializeOptions);
     }
 
 }