Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/OriginalCircuit.Altium/Models/Sch/SchComponent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,15 @@ internal int CountChildPrimitives()
/// <inheritdoc />
public string? Comment { get; set; }

/// <summary>
/// Snapshot of <see cref="Comment"/> as it stood immediately after read (whichever source it was
/// derived from). The writer compares the live value against this baseline to tell an explicit edit
/// of the convenience property from an untouched load, so it only pushes a change — and only then
/// disables the byte-faithful replay path — when the caller actually mutated <see cref="Comment"/>.
/// Null for components built from scratch (no baseline to compare against).
/// </summary>
internal string? CommentAsRead { get; set; }

/// <inheritdoc />
public string? DesignatorPrefix { get; set; }

Expand Down
19 changes: 19 additions & 0 deletions src/OriginalCircuit.Altium/Serialization/Readers/SchDocReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using OriginalCircuit.Altium.Serialization.Compound;
using OriginalCircuit.Altium.Serialization.Dto.Sch;
using System.Globalization;
using System.Linq;
using System.Text;
using PinElectricalType = OriginalCircuit.Altium.Models.Sch.PinElectricalType;

Expand Down Expand Up @@ -335,6 +336,23 @@ private void ReadFileHeader(CompoundFileAccessor accessor, SchDocument document,
document.AddPrimitive(primitive);
}
}

// Override the placeholder Comment (set from DesignItemId in CreateComponent, before children
// were known) with the actual child "Comment" parameter's value, once attached. Mirrors
// SchLibReader's post-processing so both readers derive Comment from the same field and it is
// writable/round-trippable via the same value SchDocWriter/SchLibWriter sync back to.
foreach (var component in document.Components.Cast<SchComponent>())
{
foreach (var param in component.Parameters)
{
if (string.Equals(param.Name, "Comment", StringComparison.OrdinalIgnoreCase))
{
component.Comment = param.Value;
component.CommentAsRead = param.Value;
break;
}
}
}
}

private static SchComponent CreateComponent(Dictionary<string, string> parameters)
Expand All @@ -347,6 +365,7 @@ private static SchComponent CreateComponent(Dictionary<string, string> parameter
Name = dto.LibReference ?? dto.DesignItemId ?? string.Empty,
Description = dto.ComponentDescription,
Comment = dto.DesignItemId,
CommentAsRead = dto.DesignItemId,
PartCount = Math.Max(0, dto.PartCount - 1),
UniqueId = dto.UniqueId,
Location = new CoordPoint(CoordFromDxp(dto.LocationX, dto.LocationXFrac), CoordFromDxp(dto.LocationY, dto.LocationYFrac)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,10 @@ private string GetSectionKeyFromRefName(string refName)
if (string.Equals(param.Name, "Designator", StringComparison.OrdinalIgnoreCase))
component.DesignatorPrefix = param.Value;
else if (string.Equals(param.Name, "Comment", StringComparison.OrdinalIgnoreCase))
{
component.Comment = param.Value;
component.CommentAsRead = param.Value;
}
}

// Parse PinSymbolLineWidth auxiliary stream to set per-pin SymbolLineWidth values.
Expand Down
15 changes: 12 additions & 3 deletions src/OriginalCircuit.Altium/Serialization/Writers/SchDocWriter.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Globalization;
using System.Linq;
using OriginalCircuit.Altium.Models.Sch;
using OriginalCircuit.Altium.Serialization.Compound;
using OriginalCircuit.Altium.Serialization.Binary;
Expand Down Expand Up @@ -266,12 +267,20 @@ private static void WriteFileHeader(CompoundFileAccessor cf, SchDocument documen
using var ms = new MemoryStream();
using var writer = new BinaryFormatWriter(ms, leaveOpen: true);

// Push SchComponent.Comment into its backing "Comment" parameter before deciding how to
// serialize: the byte-faithful path below replays captured bytes rather than live model state,
// so a Comment edit must be detected here to force the typed path, or it would be silently lost.
var commentEdited = false;
foreach (var component in document.Components.Cast<SchComponent>())
commentEdited |= SchLibWriter.SyncComponentComment(component);

// Byte-faithful path: when the document was read from a file and is unedited, walk the captured
// record order (each entry linked to its model object) and re-emit each record's parameters
// verbatim — preserving order, duplicate keys and unmodeled parameters. Files built from scratch,
// edited (primitive added/removed), or containing binary-pin records fall through to the typed
// serialization below.
if (document.ReadOrderedRecords is { Count: > 0 } orderedRecords &&
// edited (primitive added/removed or Comment changed), or containing binary-pin records fall
// through to the typed serialization below.
if (!commentEdited &&
document.ReadOrderedRecords is { Count: > 0 } orderedRecords &&
document.HeaderParametersOrdered is { Count: > 0 } headerOrdered &&
document.LoadedPrimitiveCount == document.CountModeledPrimitives())
{
Expand Down
31 changes: 31 additions & 0 deletions src/OriginalCircuit.Altium/Serialization/Writers/SchLibWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using OriginalCircuit.Altium.Serialization.Binary;
using System.Globalization;
using System.IO.Compression;
using System.Linq;
using System.Text;

namespace OriginalCircuit.Altium.Serialization.Writers;
Expand Down Expand Up @@ -239,8 +240,38 @@ private static string BuildOrderedParamString(List<KeyValuePair<string, string>>
return sb.ToString();
}

/// <summary>
/// Pushes <see cref="SchComponent.Comment"/> into the child "Comment" parameter's <c>Value</c>
/// (creating the parameter if the component has none), so edits made through the convenience
/// property are reflected in the field Altium actually displays and persists. Without this, the
/// property is populated on read but has nowhere to go on write. Returns true if a parameter's
/// value changed or one was added, so callers with a byte-faithful replay path (SchDocWriter) know
/// to fall back to typed serialization instead of echoing stale captured bytes.
/// </summary>
internal static bool SyncComponentComment(SchComponent component)
{
if (component.Comment == null || component.Comment == component.CommentAsRead)
return false;

var param = component.Parameters.FirstOrDefault(p =>
string.Equals(p.Name, "Comment", StringComparison.OrdinalIgnoreCase)) as SchParameter;

if (param != null)
{
if (param.Value == component.Comment)
return false;
param.Value = component.Comment;
return true;
}

component.AddParameter(SchParameter.Create("Comment").WithValue(component.Comment).Build());
return true;
}

private static void WriteComponent(CompoundFileAccessor cf, SchComponent component, Dictionary<string, string> sectionKeys)
{
SyncComponentComment(component);

var sectionKey = sectionKeys.TryGetValue(component.Name, out var key)
? key
: GetSectionKeyFromName(component.Name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,42 @@ public void FromScratch_SignalHarness_RoundTrips()
Assert.Equal(200, rtHarness.Color);
}

[Fact]
public void WriteThenRead_ComponentComment_RoundTrips()
{
var doc = new SchDocument();
var comp = SchComponent.Create("R1").WithComment("10k").Build();
doc.AddComponent(comp);

var readBack = RoundTrip(doc);

Assert.Equal("10k", ((SchComponent)readBack.Components[0]).Comment);
}

[SkippableFact]
public void WriteThenRead_RealFile_MutatedComment_Persists()
{
// Regression test for a bug where SchDocWriter's byte-faithful replay path (taken whenever the
// primitive count is unchanged) echoed the original captured bytes verbatim, silently dropping
// an edit to SchComponent.Comment because the edit doesn't add or remove a primitive.
var testDataPath = GetTestDataPath();
var filePath = Path.Combine(testDataPath, "DAC.SchDoc");
if (!File.Exists(filePath)) { Skip.If(true, "Test data not available"); return; }

var original = (SchDocument)new SchDocReader().Read(File.OpenRead(filePath));
var comp = (SchComponent)original.Components[0];
var before = comp.Comment;
comp.Comment = "MUTATED_COMMENT_VALUE";

using var ms = new MemoryStream();
new SchDocWriter().Write(original, ms);
ms.Position = 0;
var rt = (SchDocument)new SchDocReader().Read(ms);

Assert.NotEqual("MUTATED_COMMENT_VALUE", before);
Assert.Equal("MUTATED_COMMENT_VALUE", ((SchComponent)rt.Components[0]).Comment);
}

[SkippableFact]
public void WriteThenRead_RealFiles_PreservesComponentProperties()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,36 @@ public void Component_PreservesAllProperties()
Assert.Equal(2, comp.PartCount);
}

[Fact]
public void Component_Comment_RoundTrips()
{
// SchComponent.Comment is populated on read from the child "Comment" parameter but, without a
// sync step before write, has nowhere to go back to — WithComment() was silently a no-op.
var original = new SchLibrary();
var component = SchComponent.Create("RESISTOR").WithComment("10k").Build();
original.Add(component);

var readBack = RoundTrip(original);

Assert.Equal("10k", ((SchComponent)readBack.Components.First()).Comment);
}

[Fact]
public void Component_MutatedComment_Persists()
{
var original = new SchLibrary();
var component = SchComponent.Create("RESISTOR").WithComment("10k").Build();
original.Add(component);

var afterFirstRoundTrip = RoundTrip(original);
var comp = (SchComponent)afterFirstRoundTrip.Components.First();
comp.Comment = "22k";

var afterSecondRoundTrip = RoundTrip(afterFirstRoundTrip);

Assert.Equal("22k", ((SchComponent)afterSecondRoundTrip.Components.First()).Comment);
}

[Fact]
public void MultipleComponents_PreservesAll()
{
Expand Down