From f1755751bb2b31eae48ca589ba2c436bca3b3b9c Mon Sep 17 00:00:00 2001 From: cproudlock Date: Wed, 26 Aug 2026 15:06:47 -0400 Subject: [PATCH] Part marker: a misconfigured bay is now loud, and the listing loop is bounded Every failure of Get-TelesisPortConfig was filed as "no Telesis marker configured on this PC" and exited 0. That is correct for the many PCs which are not marker bays, and wrong for a bay whose registry key EXISTS and could not be read: a half-configured marker was silently never backed up again, which is exactly the failure shape this feature was built to prevent. The absent-key case now throws a distinguishable exception and stays quiet; anything else is logged every cycle and counted as a skip. Get-TelesisFileList was unbounded. The receiver drives that loop, so a controller repeating a name or answering noise on a marginal cable would spin for ever - and not harmlessly, because it holds the MARKING port while GE-Enforce waits on the script with no timeout, so one chatty controller wedges the enforcement cycle and the operator cannot mark a part. Now capped by count, by elapsed time, and by repeat detection, all of which THROW: a partial listing presented as a complete snapshot would be worse than failing. Baud and Data Bits parse with TryParse. Note the narrower reason than it looks: [int]'' is 0 in PowerShell, so empty and non-numeric values were already handled by the zero-check. What was not handled is a numeric string too large for Int32, which throws on the cast and would have been filed as "not a marker bay". Verified in the win11 VM as SYSTEM, against both the old and new module: absent key stays quiet, a key without Port Id is loud, garbage and out-of-range baud both fall back to 9600, a real 19200 is still honoured, a repeating controller stops at once and endless unique names stop at the cap. --- .../common/lib/TelesisMarker.psm1 | 66 +++++++++++++++++-- .../common/scripts/Backup-PartMarker.ps1 | 16 ++++- 2 files changed, 75 insertions(+), 7 deletions(-) diff --git a/playbook/shopfloor-setup/common/lib/TelesisMarker.psm1 b/playbook/shopfloor-setup/common/lib/TelesisMarker.psm1 index 7974b38..422aef7 100644 --- a/playbook/shopfloor-setup/common/lib/TelesisMarker.psm1 +++ b/playbook/shopfloor-setup/common/lib/TelesisMarker.psm1 @@ -79,8 +79,14 @@ function Get-TelesisPortConfig { [CmdletBinding()] param() + # A distinguishable exception, not a bare string. The caller must be able to + # tell "this PC is not a marker bay" (quiet, correct, permanent) from "this + # marker bay is misconfigured" (loud, and someone needs to fix it). Filing + # both as the former is how a broken bay stops being backed up and nothing + # anywhere says so. if (-not (Test-Path $script:MARKKEY)) { - throw "No marker configuration at $script:MARKKEY - this PC does not drive a Telesis marker." + throw [System.Management.Automation.ItemNotFoundException]::new( + "No marker configuration at $script:MARKKEY - this PC does not drive a Telesis marker.") } $props = Get-ItemProperty -Path $script:MARKKEY -ErrorAction Stop @@ -106,10 +112,27 @@ function Get-TelesisPortConfig { # Defaults match the fleet's own configuration (9600-8-N-1 in WJPRT.reg), so # a key missing one value still produces a usable port rather than a zero # baud rate that fails at Open with an unhelpful message. - $baud = [int]((Read-MarkValue 'Baud' '9600') -replace '\D', '') - if (-not $baud) { $baud = 9600 } - $databits = [int]((Read-MarkValue 'Data Bits' '8') -replace '\D', '') - if (-not $databits) { $databits = 8 } + # TryParse, not a cast, for the OUT-OF-RANGE case. + # + # An empty or non-numeric value was already handled: PowerShell coerces + # [int]'' to 0 (verified in the VM), so the -replace leaves '' and the + # zero-check below substitutes the default. What is NOT handled is a numeric + # string too large for Int32 - '99999999999999' throws on the cast, and the + # caller files that as "this PC is not a marker bay", so a real bay would + # stop being backed up with nothing saying why. + # + # Narrow, but the whole guard is here for garbage in a registry value, and + # leaving one shape of garbage fatal defeats the point. + [int]$baud = 0 + if (-not [int]::TryParse(((Read-MarkValue 'Baud' '9600') -replace '\D', ''), + [ref]$baud) -or $baud -le 0) { + $baud = 9600 + } + [int]$databits = 0 + if (-not [int]::TryParse(((Read-MarkValue 'Data Bits' '8') -replace '\D', ''), + [ref]$databits) -or $databits -le 0) { + $databits = 8 + } [pscustomobject]@{ PortName = $portid @@ -350,12 +373,45 @@ function Get-TelesisFileList { 'Fonts' { @{ Type = '4'; Start = 'R'; Step = ''; Extension = '.TFF' } } } + # BOUNDED, three ways. The receiver drives this loop, so a controller that + # keeps answering - repeating a name, or answering noise on a marginal cable + # - would spin here for ever. That is not a hang in a background job: this + # holds the MARKING port, and GE-Enforce waits on the script with no + # timeout, so one chatty controller wedges the whole enforcement cycle and + # the operator cannot mark a part. + # + # A truncated listing is a bad outcome too, so both limits are far above any + # real controller (a full bay carries tens of patterns, and a listing takes + # well under a second per name) and both THROW rather than returning a short + # list quietly. A partial snapshot presented as complete is the failure this + # whole feature exists to avoid. + $MAXNAMES = 2000 + $BUDGETSECONDS = 90 + $names = New-Object System.Collections.Generic.List[string] + $seen = New-Object System.Collections.Generic.HashSet[string] + $clock = [System.Diagnostics.Stopwatch]::StartNew() + $name = Invoke-TelesisCommand -Port $Port -Type $spec.Type -Data $spec.Start while ($name) { + if (-not $seen.Add($name)) { + throw ("$Class listing repeated '$name' after $($names.Count) entries - " + + 'the controller is looping rather than ending the list.') + } $names.Add($name) + + if ($names.Count -ge $MAXNAMES) { + throw ("$Class listing exceeded $MAXNAMES entries - refusing to keep " + + 'the marking port while the controller talks indefinitely.') + } + if ($clock.Elapsed.TotalSeconds -ge $BUDGETSECONDS) { + throw ("$Class listing exceeded $BUDGETSECONDS seconds after " + + "$($names.Count) entries - releasing the marking port.") + } + $name = Invoke-TelesisCommand -Port $Port -Type $spec.Type -Data $spec.Step } + $clock.Stop() return [pscustomobject]@{ Class = $Class Extension = $spec.Extension diff --git a/playbook/shopfloor-setup/common/scripts/Backup-PartMarker.ps1 b/playbook/shopfloor-setup/common/scripts/Backup-PartMarker.ps1 index e11e57b..c2d950b 100644 --- a/playbook/shopfloor-setup/common/scripts/Backup-PartMarker.ps1 +++ b/playbook/shopfloor-setup/common/scripts/Backup-PartMarker.ps1 @@ -328,11 +328,23 @@ if ($busy) { try { $portconfig = Get-TelesisPortConfig -} catch { - # Every PC type that is not a part-marker bay lands here, permanently. +} catch [System.Management.Automation.ItemNotFoundException] { + # The registry key is absent: this PC is not a part-marker bay. Quiet and + # permanent, which is correct - most of the fleet lands here every cycle. Write-ShopdbQuietState -Kind $KIND -State 'no-marker-config' -Message ( "No Telesis marker configured on this PC ($_). Nothing to do.") exit 0 +} catch { + # The key EXISTS and reading it failed: a marker bay that is misconfigured, + # or a registry permission problem. Filing this as "not a marker bay" - as + # this used to - meant a half-configured bay was never backed up again and + # nothing anywhere said so. It is the silent-failure shape this whole + # feature exists to avoid, so it is logged every cycle and counted as a + # skip, not swallowed. + $message = "Marker configuration present but unreadable: $_" + Write-ShopdbBackupLog -Kind $KIND -Message $message + Add-ShopdbBackupSkip -Kind $KIND -Reason 'marker-config-unreadable' + exit 1 } # -MachineNumberFallback, not a shared default: a marker bay legitimately has