-
Notifications
You must be signed in to change notification settings - Fork 27
Fix duplicate DisplayName ArgumentException #1072
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from 4 commits
c475254
abd64cb
8cbc7cb
4039c9e
da1fff4
068dc6c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,12 @@ | ||
| // Copyright (c) 2026 Phoenix Contact GmbH & Co. KG | ||
| // Licensed under the Apache License, Version 2.0 | ||
|
|
||
| using Microsoft.Extensions.Logging; | ||
| using Moryx.AbstractionLayer.Resources; | ||
| using Moryx.ControlSystem.Cells; | ||
| using Moryx.Factory; | ||
| using Moryx.FactoryMonitor.Endpoints.Models; | ||
| using Moryx.Logging; | ||
| using Moryx.Orders; | ||
| using Moryx.Serialization; | ||
| using Moryx.Tools; | ||
|
|
@@ -16,21 +18,30 @@ namespace Moryx.FactoryMonitor.Endpoints.Converter; | |
| /// </summary> | ||
| internal class Converter | ||
| { | ||
| /// <summary> | ||
| /// Logger for this resource | ||
| /// </summary> | ||
| public IModuleLogger Logger { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Constructor | ||
| /// </summary> | ||
| /// <param name="serialization"></param> | ||
| public Converter(ICustomSerialization serialization) | ||
|
|
||
| public Converter(ICustomSerialization serialization, IModuleLogger logger = null) | ||
| { | ||
| Serialization = serialization; | ||
| Logger = logger; | ||
| } | ||
|
|
||
| protected ICustomSerialization Serialization { get; } | ||
|
|
||
| public ResourceChangedModel ToResourceChangedModel(Resource current) | ||
| { | ||
| if (current == null) return null; | ||
| if (current == null) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| var cellEntry = EntryConvert.EncodeObject(current.Descriptor, Serialization); | ||
|
|
||
|
|
@@ -44,19 +55,19 @@ public ResourceChangedModel ToResourceChangedModel(Resource current) | |
|
|
||
| public static ActivityChangedModel ToActivityChangedModel(ICell current) | ||
| { | ||
| if (current == null) return null; | ||
|
|
||
| return new ActivityChangedModel | ||
| return current == null | ||
| ? null | ||
| : new ActivityChangedModel | ||
| { | ||
| ResourceId = current.Id, | ||
| }; | ||
| } | ||
|
|
||
| public static CellStateChangedModel ToCellStateChangedModel(Resource current) | ||
| { | ||
| if (current == null) return null; | ||
|
|
||
| return new CellStateChangedModel | ||
| return current == null | ||
| ? null | ||
| : new CellStateChangedModel | ||
| { | ||
| Id = current.Id, | ||
| }; | ||
|
|
@@ -69,34 +80,64 @@ public static CellStateChangedModel ToCellStateChangedModel(Resource current) | |
| /// <param name="baseType">Type of the resource</param> | ||
| internal Dictionary<string, CellPropertySettings> ToCellPropertySettings(Entry cellEntry, Type baseType) | ||
| { | ||
| if (cellEntry == null) return null; | ||
| if (cellEntry == null) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| var cellProperties = new Dictionary<string, CellPropertySettings>(); | ||
| if (cellEntry.GetType().IsArray || cellEntry.SubEntries.Count > 0) | ||
| { | ||
| foreach (var subEntry in cellEntry.SubEntries) | ||
| { | ||
| var results = ToCellPropertySettings(subEntry, baseType); | ||
| if (results == null) continue; | ||
| if (results == null) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| foreach (var item in results) | ||
| { | ||
| cellProperties.Add(item.Key, item.Value); | ||
| if (cellProperties.ContainsKey(item.Key)) | ||
| { | ||
| Logger?.LogWarning($"Duplicate DisplayName '{item.Key}' in resource '{baseType.Name}'. Overwriting previous value. Ensure unique DisplayNames."); | ||
|
||
|
|
||
| cellProperties[item.Key] = item.Value; | ||
| } | ||
| else | ||
| { | ||
| cellProperties[item.Key] = item.Value; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| else | ||
| { | ||
| var entryVisualizer = Serialization.GetProperties(baseType) | ||
| .FirstOrDefault(x => x.Name == cellEntry.Identifier)?.GetCustomAttribute<EntryVisualizationAttribute>(); | ||
| if (entryVisualizer == null) return null; | ||
| if (entryVisualizer == null) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| var property = new CellPropertySettings | ||
| { | ||
| IsDisplayed = false, | ||
| CurrentValue = cellEntry.Value.Current, | ||
| IconName = entryVisualizer?.Icon, | ||
| ValueUnit = entryVisualizer?.Unit, | ||
| }; | ||
| cellProperties.Add(cellEntry.DisplayName, property); | ||
|
|
||
| if (cellProperties.ContainsKey(cellEntry.DisplayName)) | ||
|
||
| { | ||
| Logger?.LogWarning($"Duplicate DisplayName '{cellEntry.DisplayName}' in resource '{baseType.Name}'. Overwriting previous value."); | ||
|
|
||
| cellProperties[cellEntry.DisplayName] = property; | ||
| } | ||
| else | ||
| { | ||
| cellProperties[cellEntry.DisplayName] = property; | ||
| } | ||
| } | ||
| return cellProperties; | ||
| } | ||
|
|
@@ -189,4 +230,4 @@ public static TransportRouteModel ToTransportRouteModel(TransportPathModel pathM | |
| } | ||
|
|
||
| public static FactoryStateModel ToFactoryStateModel(IManufacturingFactory factory) => new() { Id = factory.Id }; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's an internal component. Why this nullability? Complexity could be reduced here when you don't allow null. The logger can also be a private field instead of a public property.