From ec058d270cd81649ef7ac639f07ef59965a80d74 Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Mon, 12 Oct 2026 19:58:29 -0400 Subject: [PATCH] feat(core): remove individual rail clamps on one plate Slice 3b task 1 (D8/D9): ClampRail gains RemovalMinutes (D9, default 5 minutes, validated finite >= 0) and Plate gains RemovedClamps (rail clamp ids taken off this sheet as a last resort) plus the computed ClampRemovalMinutes. Plate.SetClampRail gains an overload that attaches a rail with a removed-clamp set; zones become the rail's resolution for the sheet minus those ids, re-resolved on every resize/rotate so a removed clamp stays off and reappears removed if a later resize brings it back onto the sheet. ClampRail.Matches gains an overload comparing against the reduced resolution. IO: PlateDto.RemovedClamps and ClampRailDto.RemovalMinutes persist (missing removalMinutes falls back to the default; missing removedClamps means none). The writer's empty-plate drop logic no longer drops a sheet whose only content is a removal state the defaults can't recreate. The reader re-attaches the nest rail to a plate only when its saved zones equal the rail's resolution minus its saved removed ids, so a reduced-clamp sheet keeps following the rail (and the removal) through later resizes instead of freezing as static zones. Engine/desktop wiring (minimal-removal computation, Auto Nest last-resort removal, cut-time, UI) is Slice 3b tasks 2-3, not yet done. Tests: 16 new Core (ClampRailTests) + 5 new IO (ClampRailPersistenceTests) covering validation, resize/resolve behavior, round-trip, legacy missing-key defaults, and the empty-plate-with-removal-only retention case. Mutation check (reverting the RemovedClamps subtraction) turns 3 Core + 2 IO assertions red, confirming real coverage. Gates: Release OpenNest.Tests 3884 passed/31 skipped, OpenNest.IO.Tests 105 passed, OpenNest.Engine.Tests (Clamp filter) 98 passed; scoped dotnet format --verify-no-changes clean. --- OpenNest.Core/ClampRail.cs | 44 ++++++- OpenNest.Core/Plate.cs | 53 +++++++- .../ClampRailPersistenceTests.cs | 119 ++++++++++++++++++ OpenNest.IO/NestFormat.cs | 20 ++- OpenNest.IO/NestReader.cs | 21 +++- OpenNest.IO/NestWriter.cs | 23 +++- OpenNest.Tests/ClampRailTests.cs | 102 +++++++++++++++ 7 files changed, 367 insertions(+), 15 deletions(-) diff --git a/OpenNest.Core/ClampRail.cs b/OpenNest.Core/ClampRail.cs index 022acd78..65ac565d 100644 --- a/OpenNest.Core/ClampRail.cs +++ b/OpenNest.Core/ClampRail.cs @@ -15,7 +15,18 @@ namespace OpenNest /// public sealed class ClampRail : IEquatable { - public ClampRail(double width, double depth, IEnumerable centers) + /// + /// Planner default minutes to remove one clamp and put it back after cutting (D9), + /// used when a caller does not supply its own value. + /// + public const double DefaultRemovalMinutes = 5.0; + + public ClampRail( + double width, + double depth, + IEnumerable centers, + double removalMinutes = DefaultRemovalMinutes + ) { ArgumentNullException.ThrowIfNull(centers); if (!double.IsFinite(width) || width <= 0) @@ -28,6 +39,11 @@ namespace OpenNest nameof(depth), "Clamp depth must be finite and positive." ); + if (!double.IsFinite(removalMinutes) || removalMinutes < 0) + throw new ArgumentOutOfRangeException( + nameof(removalMinutes), + "Clamp removal time must be finite and zero or more." + ); var sorted = new SortedSet(); foreach (var center in centers) @@ -42,6 +58,7 @@ namespace OpenNest Width = width; Depth = depth; + RemovalMinutes = removalMinutes; Centers = sorted.ToArray(); } @@ -51,6 +68,12 @@ namespace OpenNest /// Clamp intrusion into the sheet from the origin-side edge (Y). public double Depth { get; } + /// + /// Minutes to remove one clamp and put it back after cutting (D9). Added to a sheet's + /// cut-time estimate once per clamp lists there. + /// + public double RemovalMinutes { get; } + /// Distinct clamp centers along X from the origin, ascending. public IReadOnlyList Centers { get; } @@ -101,6 +124,22 @@ namespace OpenNest return SameZones(plate.ClampZones, ResolveFor(plate.Size, plate.Quadrant)); } + /// + /// True when the plate's zones are exactly this rail's resolution for its sheet minus + /// the clamps named in (D8 last-resort removal, C13). + /// A null or empty set is the same check as . + /// + public bool Matches(Plate plate, IReadOnlyCollection removedClamps) + { + ArgumentNullException.ThrowIfNull(plate); + if (removedClamps == null || removedClamps.Count == 0) + return Matches(plate); + var expected = ResolveFor(plate.Size, plate.Quadrant) + .Where(z => !removedClamps.Contains(z.Id)) + .ToList(); + return SameZones(plate.ClampZones, expected); + } + internal static bool SameZones(IList current, IReadOnlyList expected) { if (current.Count != expected.Count) @@ -125,13 +164,14 @@ namespace OpenNest other is not null && Width.Equals(other.Width) && Depth.Equals(other.Depth) + && RemovalMinutes.Equals(other.RemovalMinutes) && Centers.SequenceEqual(other.Centers); public override bool Equals(object obj) => Equals(obj as ClampRail); public override int GetHashCode() { - var hash = HashCode.Combine(Width, Depth, Centers.Count); + var hash = HashCode.Combine(Width, Depth, RemovalMinutes, Centers.Count); foreach (var center in Centers) hash = HashCode.Combine(hash, center); return hash; diff --git a/OpenNest.Core/Plate.cs b/OpenNest.Core/Plate.cs index 3fa0ce84..e024bbb3 100644 --- a/OpenNest.Core/Plate.cs +++ b/OpenNest.Core/Plate.cs @@ -141,23 +141,66 @@ namespace OpenNest /// public ClampRail ClampRail { get; private set; } + private static readonly IReadOnlySet NoRemovedClamps = new HashSet(); + + /// + /// Rail clamp ids taken off this sheet as a last resort (D8) so a part that could not + /// be nested anywhere with every clamp in place still fits. Empty unless + /// is set; already excludes them, so + /// nothing else needs to consult this list to avoid them. Ids stay listed across a + /// resize/rotate even when the matching clamp is temporarily beyond the sheet, so it + /// re-appears removed if a later resize brings it back onto the sheet (C13). + /// + public IReadOnlySet RemovedClamps { get; private set; } = NoRemovedClamps; + + /// + /// Minutes this sheet's cut-time estimate gains from (D9): + /// each listed clamp adds once. Zero + /// when there is no rail (removal is meaningless without one to remove clamps from). + /// + public double ClampRemovalMinutes => + ClampRail == null ? 0 : RemovedClamps.Count * ClampRail.RemovalMinutes; + /// /// Follows and replaces the zones with its resolution for /// this sheet; null detaches and removes every zone. Unchanged zones raise no events. /// - public void SetClampRail(ClampRail rail) + public void SetClampRail(ClampRail rail) => SetClampRail(rail, null); + + /// + /// Follows with the clamps named in + /// taken off this sheet (D8): zones become the rail's resolution for this sheet minus + /// those ids. Null detaches, clears every zone and forgets the + /// removed set (removal only means something while following a rail). + /// + public void SetClampRail(ClampRail rail, IEnumerable removedClamps) { ClampRail = rail; - ReplaceClampZones( - rail == null ? Array.Empty() : rail.ResolveFor(Size, Quadrant) - ); + RemovedClamps = rail == null || removedClamps == null + ? NoRemovedClamps + : new HashSet(removedClamps, StringComparer.Ordinal); + ReplaceClampZones(ResolveRailZones()); } private void ResolveClampRail() { // Called from the constructor's Size assignment before the zone list exists. if (ClampRail != null) - ReplaceClampZones(ClampRail.ResolveFor(Size, Quadrant)); + ReplaceClampZones(ResolveRailZones()); + } + + private IReadOnlyList ResolveRailZones() + { + if (ClampRail == null) + return Array.Empty(); + var resolved = ClampRail.ResolveFor(Size, Quadrant); + if (RemovedClamps.Count == 0) + return resolved; + var kept = new List(resolved.Count); + foreach (var zone in resolved) + if (!RemovedClamps.Contains(zone.Id)) + kept.Add(zone); + return kept; } private void ReplaceClampZones(IReadOnlyList zones) diff --git a/OpenNest.IO.Tests/ClampRailPersistenceTests.cs b/OpenNest.IO.Tests/ClampRailPersistenceTests.cs index 5696c29c..7b924acb 100644 --- a/OpenNest.IO.Tests/ClampRailPersistenceTests.cs +++ b/OpenNest.IO.Tests/ClampRailPersistenceTests.cs @@ -1,5 +1,7 @@ using System.IO.Compression; using System.Text; +using System.Text.Json; +using System.Text.Json.Nodes; using OpenNest.Geometry; namespace OpenNest.IO.Tests; @@ -160,6 +162,123 @@ public class ClampRailPersistenceTests Assert.Null(plate.ClampRail); } + // D9: the rail's removal-minutes setting round-trips; a file without it defaults. + + [Fact] + public void RemovalMinutesRoundTrips() + { + var nest = new Nest("railed"); + nest.PlateDefaults.Size = new Size(48, 96); + nest.SetClampRail(new ClampRail(1.5, 0.625, MachineCenters, removalMinutes: 8)); + + var reopened = RoundTrip(nest); + + Assert.Equal(8, reopened.PlateDefaults.ClampRail.RemovalMinutes); + } + + [Fact] + public void MissingRemovalMinutesDefaultsOnLoad() + { + // The writer always emits removalMinutes; simulate an older file that predates it by + // stripping the key directly rather than relying on exact JSON formatting elsewhere. + var json = ReadNestJson(Write(RailNest())); + Assert.Contains("\"removalMinutes\"", json, StringComparison.Ordinal); + + var withoutKey = RewriteNestJson(Write(RailNest()), + text => RemoveJsonProperty(text, "removalMinutes")); + + var reopened = new NestReader(withoutKey).Read(); + + Assert.Equal(ClampRail.DefaultRemovalMinutes, reopened.PlateDefaults.ClampRail.RemovalMinutes); + } + + // D8/C13: last-resort per-sheet clamp removal persists and keeps following the rail. + + [Fact] + public void RemovedClampsRoundTripAndTheReducedRailStillFollowsResizes() + { + var nest = new Nest("railed"); + nest.PlateDefaults.Size = new Size(48, 120); + nest.SetClampRail(Machine()); + var plate = nest.CreatePlate(); + plate.SetClampRail(Machine(), new[] { "clamp-3" }); + + var reopened = RoundTrip(nest); + var reopenedPlate = Assert.Single(reopened.Plates); + + Assert.Equal(7, reopenedPlate.ClampZones.Count); + Assert.DoesNotContain(reopenedPlate.ClampZones, z => z.Id == "clamp-3"); + Assert.Equal(new[] { "clamp-3" }, reopenedPlate.RemovedClamps); + Assert.Same(reopened.PlateDefaults.ClampRail, reopenedPlate.ClampRail); // still rail-following + + // The plate still follows the rail minus its removed clamp after a resize (C13). + reopenedPlate.Size = new Size(48, 60); + Assert.Equal(new[] { "clamp-1", "clamp-2", "clamp-4" }, reopenedPlate.ClampZones.Select(z => z.Id)); + } + + [Fact] + public void AnEmptyPlateWithOnlyRemovedClampsIsNotDropped() + { + // Every clamp removed: ClampZones is empty, but the removal state must still be saved. + var nest = new Nest("railed"); + nest.PlateDefaults.Size = new Size(48, 96); + nest.SetClampRail(new ClampRail(1.5, 0.625, new[] { 6.0 })); + var plate = nest.CreatePlate(); + plate.SetClampRail(new ClampRail(1.5, 0.625, new[] { 6.0 }), new[] { "clamp-1" }); + Assert.Empty(plate.ClampZones); + + var reopened = RoundTrip(nest); + + var kept = Assert.Single(reopened.Plates); + Assert.Equal(new[] { "clamp-1" }, kept.RemovedClamps); + } + + [Fact] + public void LegacyFileWithoutRemovedClampsKeyLoadsWithNoneRemoved() + { + var withoutKey = RewriteNestJson(Write(RailNest()), + text => RemoveJsonProperty(text, "removedClamps")); + + var reopened = new NestReader(withoutKey).Read(); + + var plate = Assert.Single(reopened.Plates); + Assert.Empty(plate.RemovedClamps); + Assert.Equal(7, plate.ClampZones.Count); + } + + /// Removes a named property wherever it appears in the document, simulating a + /// key an older writer never emitted. Rebuilds via so the result + /// is always valid JSON regardless of whether the property was first, last or only one + /// in its object - unlike string surgery, which breaks on a dangling comma. + private static string RemoveJsonProperty(string json, string name) + { + var root = JsonNode.Parse(json)!; + var removed = RemovePropertyRecursive(root, name); + Assert.True(removed, $"Property '{name}' was not found to remove."); + return root.ToJsonString(new JsonSerializerOptions { WriteIndented = true }); + } + + private static bool RemovePropertyRecursive(JsonNode node, string name) + { + switch (node) + { + case JsonObject obj: + var removed = obj.Remove(name); + foreach (var child in obj.ToList()) + if (child.Value != null && RemovePropertyRecursive(child.Value, name)) + removed = true; + return removed; + case JsonArray arr: + var any = false; + foreach (var item in arr) + if (item != null && RemovePropertyRecursive(item, name)) + any = true; + return any; + default: + return false; + } + } + private static string ReadNestJson(MemoryStream zipBytes) { zipBytes.Position = 0; diff --git a/OpenNest.IO/NestFormat.cs b/OpenNest.IO/NestFormat.cs index baedf527..a08b6893 100644 --- a/OpenNest.IO/NestFormat.cs +++ b/OpenNest.IO/NestFormat.cs @@ -85,7 +85,11 @@ namespace OpenNest.IO return null; list.Add(x); } - return new ClampRailDto { Width = width, Depth = depth, Centers = list }; + // Missing or unreadable -> null; the reader then falls back to the default. + double? removalMinutes = TryNumber(rail, "removalMinutes", out var minutes) + ? minutes + : null; + return new ClampRailDto { Width = width, Depth = depth, Centers = list, RemovalMinutes = removalMinutes }; } public override void Write( @@ -101,6 +105,8 @@ namespace OpenNest.IO foreach (var center in value.Centers ?? new List()) writer.WriteNumberValue(center); writer.WriteEndArray(); + if (value.RemovalMinutes is double removalMinutes) + writer.WriteNumber("removalMinutes", removalMinutes); writer.WriteEndObject(); } @@ -123,6 +129,12 @@ namespace OpenNest.IO /// Clamp centers along X, measured from the plate origin. public List Centers { get; init; } = new(); + + /// + /// Minutes to remove one clamp and put it back after cutting (D9). Null in older + /// files and readers fall back to . + /// + public double? RemovalMinutes { get; init; } } public record DrawingDto @@ -157,6 +169,12 @@ namespace OpenNest.IO /// files, which load with no zones. /// public List Clamps { get; init; } = new(); + + /// + /// Rail clamp ids removed from this sheet as a last resort (D8). Missing or empty + /// means none were removed; already excludes them. + /// + public List RemovedClamps { get; init; } = new(); } public record ClampZoneDto diff --git a/OpenNest.IO/NestReader.cs b/OpenNest.IO/NestReader.cs index e88377d7..054dc1ef 100644 --- a/OpenNest.IO/NestReader.cs +++ b/OpenNest.IO/NestReader.cs @@ -329,7 +329,8 @@ namespace OpenNest.IO /// /// The saved machine clamp rail, or null when absent, empty or malformed (a bad rail - /// is ignored rather than failing the nest; each plate's saved zones still load). + /// is ignored rather than failing the nest; each plate's saved zones still load). A + /// missing removal-minutes value falls back to . /// private static ClampRail ReadClampRail(ClampRailDto dto) { @@ -337,7 +338,12 @@ namespace OpenNest.IO return null; try { - return new ClampRail(dto.Width, dto.Depth, dto.Centers); + return new ClampRail( + dto.Width, + dto.Depth, + dto.Centers, + dto.RemovalMinutes ?? ClampRail.DefaultRemovalMinutes + ); } catch (ArgumentException) { @@ -439,9 +445,14 @@ namespace OpenNest.IO } // A plate follows the nest rail only when its saved zones are exactly the - // rail's for this sheet; anything else is preserved as saved. - if (clampRail != null && clampRail.Matches(plate)) - plate.SetClampRail(clampRail); + // rail's for this sheet minus any saved removed-clamp ids (C13); otherwise + // the saved zones are kept as static, explicit data. + var removedClamps = p.RemovedClamps? + .Where(id => !string.IsNullOrEmpty(id)) + .ToHashSet(StringComparer.Ordinal) + ?? new HashSet(); + if (clampRail != null && clampRail.Matches(plate, removedClamps)) + plate.SetClampRail(clampRail, removedClamps); for (var partIndex = 0; partIndex < p.Parts.Count; partIndex++) { diff --git a/OpenNest.IO/NestWriter.cs b/OpenNest.IO/NestWriter.cs index dfd9fa8b..c6d2015b 100644 --- a/OpenNest.IO/NestWriter.cs +++ b/OpenNest.IO/NestWriter.cs @@ -142,6 +142,7 @@ namespace OpenNest.IO Width = pd.ClampRail.Width, Depth = pd.ClampRail.Depth, Centers = pd.ClampRail.Centers.ToList(), + RemovalMinutes = pd.ClampRail.RemovalMinutes, }, }; } @@ -207,18 +208,29 @@ namespace OpenNest.IO /// /// True when a plate created from the saved defaults would be this sheet's clamp - /// configuration: same size and quadrant, and zones the saved rail recreates. + /// configuration: same size and quadrant, no last-resort removal, and zones the + /// saved rail recreates. /// private bool DefaultsRecreateClamps(Plate plate) { var defaults = nest.PlateDefaults; return defaults.ClampRail is { } rail + && plate.RemovedClamps.Count == 0 && plate.Size.Width == defaults.Size.Width && plate.Size.Length == defaults.Size.Length && plate.Quadrant == defaults.Quadrant && rail.Matches(plate); } + /// Numeric suffix of a rail clamp id ("clamp-3" -> 3), for a natural sort of + /// instead of lexical ("clamp-10" before "clamp-2"). + /// A non-matching id sorts last by its own ordinal text. + private static int ClampIndex(string id) => + id != null && id.StartsWith("clamp-", StringComparison.Ordinal) + && int.TryParse(id.AsSpan(6), out var index) + ? index + : int.MaxValue; + private List BuildPlateDtos() { var list = new List(); @@ -233,7 +245,10 @@ namespace OpenNest.IO if ( plate.Parts.Count(p => !p.BaseDrawing.IsCutOff) == 0 && plate.CutOffs.Count == 0 - && (plate.ClampZones.Count == 0 || DefaultsRecreateClamps(plate)) + && ( + (plate.ClampZones.Count == 0 && plate.RemovedClamps.Count == 0) + || DefaultsRecreateClamps(plate) + ) ) continue; @@ -319,6 +334,10 @@ namespace OpenNest.IO Width = z.Bounds.Width, }) .ToList(), + RemovedClamps = plate.RemovedClamps + .OrderBy(ClampIndex) + .ThenBy(id => id, StringComparer.Ordinal) + .ToList(), } ); } diff --git a/OpenNest.Tests/ClampRailTests.cs b/OpenNest.Tests/ClampRailTests.cs index d0ca7708..2ff4b218 100644 --- a/OpenNest.Tests/ClampRailTests.cs +++ b/OpenNest.Tests/ClampRailTests.cs @@ -268,4 +268,106 @@ public class ClampRailTests Assert.NotEqual(Machine(), new ClampRail(1.5, 0.5, MachineCenters)); Assert.NotEqual(Machine(), new ClampRail(1.5, 0.625, MachineCenters.Take(7))); } + + // D9: minutes to remove one clamp and put it back, a variable setting on the rail. + + [Fact] + public void RemovalMinutesDefaultsToThePlannerValue() + { + Assert.Equal(5.0, Machine().RemovalMinutes); + Assert.Equal(5.0, ClampRail.DefaultRemovalMinutes); + } + + [Theory] + [InlineData(-1)] + [InlineData(double.NaN)] + [InlineData(double.PositiveInfinity)] + [InlineData(double.NegativeInfinity)] + public void RemovalMinutesMustBeFiniteAndZeroOrMore(double minutes) + { + Assert.Throws( + () => new ClampRail(1.5, 0.625, new[] { 6.0 }, minutes)); + } + + [Fact] + public void ZeroRemovalMinutesIsAllowed() + { + var rail = new ClampRail(1.5, 0.625, new[] { 6.0 }, removalMinutes: 0); + Assert.Equal(0, rail.RemovalMinutes); + } + + [Fact] + public void RailsWithDifferentRemovalMinutesAreNotEqual() + { + var fiveMinutes = new ClampRail(1.5, 0.625, MachineCenters, 5); + var tenMinutes = new ClampRail(1.5, 0.625, MachineCenters, 10); + Assert.NotEqual(fiveMinutes, tenMinutes); + Assert.NotEqual(fiveMinutes.GetHashCode(), tenMinutes.GetHashCode()); + } + + // D8/C13: last-resort per-sheet removal of individual rail clamps. + + [Fact] + public void RemovedClampsAreSubtractedFromTheRailResolution() + { + var plate = Sheet(48, 120); + plate.SetClampRail(Machine(), new[] { "clamp-3" }); + + Assert.Equal(7, plate.ClampZones.Count); + Assert.DoesNotContain(plate.ClampZones, z => z.Id == "clamp-3"); + Assert.Equal(new[] { "clamp-3" }, plate.RemovedClamps); + } + + [Fact] + public void RemovedClampStaysOffAfterResizeAndReappearsIfTheClampComesBackOntoTheSheet() + { + var plate = Sheet(48, 120); + plate.SetClampRail(Machine(), new[] { "clamp-8" }); // centered at 111" + + plate.Size = new Size(48, 60); // clamp-8 is beyond this sheet regardless + Assert.Equal(new[] { "clamp-1", "clamp-2", "clamp-3", "clamp-4" }, + plate.ClampZones.Select(z => z.Id)); + Assert.Equal(new[] { "clamp-8" }, plate.RemovedClamps); + + plate.Size = new Size(48, 120); // clamp-8 is back on the sheet, but still removed + Assert.Equal(7, plate.ClampZones.Count); + Assert.DoesNotContain(plate.ClampZones, z => z.Id == "clamp-8"); + } + + [Fact] + public void ClampRemovalMinutesSumsTheRailsPerClampTime() + { + var plate = Sheet(48, 120); + Assert.Equal(0, plate.ClampRemovalMinutes); // no rail yet + + plate.SetClampRail(new ClampRail(1.5, 0.625, MachineCenters, removalMinutes: 7), + new[] { "clamp-3", "clamp-4" }); + Assert.Equal(14, plate.ClampRemovalMinutes); + + plate.SetClampRail(null!); + Assert.Equal(0, plate.ClampRemovalMinutes); // detaching forgets the removed set too + Assert.Empty(plate.RemovedClamps); + } + + [Fact] + public void SetClampRailWithoutRemovedClampsBehavesAsBefore() + { + var plate = Sheet(48, 120); + plate.SetClampRail(Machine()); + + Assert.Empty(plate.RemovedClamps); + Assert.Equal(8, plate.ClampZones.Count); + } + + [Fact] + public void MatchesWithRemovedClampsComparesAgainstTheReducedResolution() + { + var plate = Sheet(48, 120); + plate.SetClampRail(Machine(), new[] { "clamp-3" }); + + Assert.True(Machine().Matches(plate, new[] { "clamp-3" })); + Assert.False(Machine().Matches(plate)); // full resolution: zones don't match + Assert.False(Machine().Matches(plate, Array.Empty())); + Assert.False(Machine().Matches(plate, new[] { "clamp-4" })); + } }