From b228a6936497d16001c6c6ee48adad834a768f99 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Tue, 25 Aug 2026 13:21:45 -0400 Subject: [PATCH] Identify a marker by its device, without moving what NTLARS reports A part marker has no machine number of its own - the device is what a backup belongs to, and C:\Enrollment\asset-id.txt is what names it. Reading that file inside the shared machine-number lookup looked like the tidy place for it and was wrong: marker bays run eDNC too, so Backup-NtlarsSettings.ps1 reaches the same lookup, and it would have started posting a machine's DNC settings under the marker's asset number where today it exits quietly. The marker script asks for the fallback explicitly instead, which is what -MachineNumberFallback is for. Also from review: Returning a byte array bare lets the pipeline enumerate it, so an EMPTY file came back as $null, ComputeHash threw, and with ErrorActionPreference=Continue the manifest entry silently kept the previous file's hash - a corrupted manifest stored as a good revision. Empty parameter files are plausible. Reading registry values directly under StrictMode throws when one is absent, so the "no Port Id" guard was unreachable and a partially configured marker bay reported itself as having no marker at all, permanently and quietly. Values are now read through a presence check with the fleet's own defaults. The outside-window quiet state embedded the hour, so the say-once module said it hourly. Command replies now have their checksum verified like data blocks do, since a corrupted listing entry would otherwise be trusted and a file requested under a name the controller does not have. A controller that genuinely holds nothing is no longer reported as a failed capture. A snapshot that could not be written to the share now counts as a missed backup like the rest, and the header no longer claims orphaned snapshots get tidied - they are kept on purpose, because a timed-out post may still have been recorded. --- .../common/lib/ShopdbBackupClient.psm1 | 46 +++++++++++++- .../common/lib/TelesisMarker.psm1 | 62 ++++++++++++++++--- .../common/scripts/Backup-PartMarker.ps1 | 38 ++++++++++-- 3 files changed, 131 insertions(+), 15 deletions(-) diff --git a/playbook/shopfloor-setup/common/lib/ShopdbBackupClient.psm1 b/playbook/shopfloor-setup/common/lib/ShopdbBackupClient.psm1 index ea01f66..355f882 100644 --- a/playbook/shopfloor-setup/common/lib/ShopdbBackupClient.psm1 +++ b/playbook/shopfloor-setup/common/lib/ShopdbBackupClient.psm1 @@ -46,6 +46,16 @@ $script:LOGDIR = 'C:\Logs\Shopfloor' $script:SHOPDBREG = 'HKLM:\SOFTWARE\GE\ShopDB' $script:KEYFILE = 'C:\Enrollment\shopdb-key.txt' $script:PCCONFIG = 'C:\Enrollment\pc-config.txt' +# The DEVICE this PC drives, by asset number - a part marker, a CMM, a Keyence. +# Written at staging and read by the computers collector, which resolves it with +# _asset_by_number and refuses a name it cannot find rather than minting a +# phantom. Marker and measuring-tool bays have no machine number of their own +# ("keyence / genspect / part-marker have no per-bay id" in +# Report-AssetToShopDB.ps1), so for those this file IS the identity. +$script:ASSETIDFILE = 'C:\Enrollment\asset-id.txt' +# 0.12.0 shipped this name before the file was generalised; a bay staged then +# still has it. +$script:LEGACYASSETIDFILE = 'C:\Enrollment\measuringtool-id.txt' # The action ShopDB reported for the most recent post, per kind: 'created', # 'noop', or '' when nothing has been posted. Send-ShopdbBackup returns only @@ -144,6 +154,26 @@ function Get-ShopdbCollectorKey { } +function Get-ShopdbAssetId { + <# + The asset number of the device this PC drives, or ''. + + Same file and same precedence the computers collector uses, so a bay + cannot report its marker under one identity and its backup under + another. + #> + foreach ($file in @($script:ASSETIDFILE, $script:LEGACYASSETIDFILE)) { + if (Test-Path $file) { + try { + $value = ([string](Get-Content -LiteralPath $file -First 1 -EA Stop)).Trim() + if ($value) { return $value } + } catch { } + } + } + return '' +} + + function Get-ShopdbMachineNumber { <# The machine number this PC reports, from pc-config.txt. @@ -168,6 +198,19 @@ function Get-ShopdbMachineNumber { if ($val) { return ([string]$val).Trim() } } catch { } } + + # NO asset-id.txt fallback here, deliberately. It belongs to the CALLER. + # + # A part-marker bay has no machine number of its own but does have the DNC + # registry tree, so Backup-NtlarsSettings.ps1 passes its key check and + # reaches this function. A shared fallback would make it post the machine's + # DNC settings under the MARKER's asset number - a kind that resolves by + # machine number, handed a device asset number - where today it exits + # quietly. That is a behaviour change to a script running in production. + # + # A kind that legitimately identifies itself by device passes + # -MachineNumberFallback { Get-ShopdbAssetId } instead. The per-kind + # mechanism already exists; this is what it is for. return '' } @@ -479,4 +522,5 @@ Export-ModuleMember -Function Initialize-ShopdbBackup, Send-ShopdbBackup, Get-ShopdbRegValue, Get-ShopdbCollectorKey, Get-ShopdbMachineNumber, Get-ShopdbIntervalHours, Get-ShopdbLogPath, Get-ShopdbLastBackupAction, Test-ShopdbBackupNoop, - Add-ShopdbBackupSkip, Get-ShopdbBackupSkips, Clear-ShopdbBackupSkips + Add-ShopdbBackupSkip, Get-ShopdbBackupSkips, Clear-ShopdbBackupSkips, + Get-ShopdbAssetId diff --git a/playbook/shopfloor-setup/common/lib/TelesisMarker.psm1 b/playbook/shopfloor-setup/common/lib/TelesisMarker.psm1 index dbd5a9b..7974b38 100644 --- a/playbook/shopfloor-setup/common/lib/TelesisMarker.psm1 +++ b/playbook/shopfloor-setup/common/lib/TelesisMarker.psm1 @@ -84,19 +84,43 @@ function Get-TelesisPortConfig { } $props = Get-ItemProperty -Path $script:MARKKEY -ErrorAction Stop - $portid = [string]$props.'Port Id' + # Every value is read through a presence check because this module runs + # under StrictMode Latest, where touching a registry value that is not + # there throws PropertyNotFoundException. Reading them directly made the + # "no Port Id" guard below unreachable, and turned a PARTIALLY configured + # marker bay into "no Telesis marker configured on this PC" - permanent, + # quiet, and the exact failure this feature exists to prevent. + $names = $props.PSObject.Properties.Name + function Read-MarkValue { + param([string]$Name, [string]$Default = '') + if ($names -contains $Name) { + $value = $props.$Name + if ($null -ne $value) { return ([string]$value).Trim() } + } + return $Default + } + + $portid = Read-MarkValue 'Port Id' if (-not $portid) { throw "No 'Port Id' under $script:MARKKEY - marker serial port is not configured." } + # 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 } + [pscustomobject]@{ - PortName = $portid.Trim() - BaudRate = [int]("$($props.Baud)".Trim() -replace '\D', '') - Parity = switch -Regex ("$($props.Parity)".Trim()) { + PortName = $portid + BaudRate = $baud + Parity = switch -Regex (Read-MarkValue 'Parity' 'None') { '^(?i)odd' { 'Odd' } '^(?i)even' { 'Even' } default { 'None' } } - DataBits = [int]("$($props.'Data Bits')".Trim() -replace '\D', '') - StopBits = if ("$($props.'Stop Bits')".Trim() -eq '2') { 'Two' } else { 'One' } + DataBits = $databits + StopBits = if ((Read-MarkValue 'Stop Bits' '1') -eq '2') { 'Two' } else { 'One' } } } @@ -232,7 +256,20 @@ function Invoke-TelesisCommand { if ($start -lt 0 -or $end -lt $start) { throw "Malformed reply to command '$Type' '$Data'." } - return $text.Substring($start + 1, $end - $start - 1) + $reply = $text.Substring($start + 1, $end - $start - 1) + + # Check the reply's own BCC, not just the data blocks'. A corrupted byte in + # a LISTING entry would otherwise be trusted, and the wrong filename would + # be requested and stored under a name nothing on the controller has. + $tail = $text.Substring($end + 1) + if ($tail.Length -ge 3) { + $expected = New-TelesisBcc -Type $Type -Data ([Text.Encoding]::ASCII.GetBytes($reply)) + if ($tail.Substring(0, 3) -ne $expected) { + throw ("Bad checksum on the reply to command '{0}' '{1}' (got {2}, expected {3})." -f + $Type, $Data, $tail.Substring(0, 3), $expected) + } + } + return $reply } @@ -394,7 +431,14 @@ function Read-TelesisFile { $final = [Text.Encoding]::ASCII.GetBytes($finaltext) $Port.Write($final, 0, $final.Length) - return $data + # The comma is load-bearing. A bare `return $data` is ENUMERATED by the + # pipeline: the caller gets Object[] of boxed bytes for a normal file, a + # scalar for a one-byte file, and $null for an EMPTY one. A null then + # reaches ComputeHash in the manifest builder, which throws, and under + # ErrorActionPreference=Continue the assignment is skipped so the entry + # silently keeps the PREVIOUS file's hash - a corrupted manifest stored as + # a good revision. Empty parameter files are entirely plausible. + return ,$data } @@ -418,7 +462,7 @@ function Write-TelesisFile { $header = '{0};{1:D6};{2}' -f $FileName, $Data.Length, $script:BLOCKSIZE [void](Invoke-TelesisCommand -Port $Port -Type 'D' -Data $header) - $total = [Math]::Ceiling($Data.Length / [double]$script:BLOCKSIZE) + $total = [int][Math]::Ceiling($Data.Length / [double]$script:BLOCKSIZE) while ($true) { # ACK + 5-digit block number + CR, from the controller. $request = New-Object Text.StringBuilder diff --git a/playbook/shopfloor-setup/common/scripts/Backup-PartMarker.ps1 b/playbook/shopfloor-setup/common/scripts/Backup-PartMarker.ps1 index 228cd25..95f139a 100644 --- a/playbook/shopfloor-setup/common/scripts/Backup-PartMarker.ps1 +++ b/playbook/shopfloor-setup/common/scripts/Backup-PartMarker.ps1 @@ -26,12 +26,18 @@ # changed would look exactly like one whose backup died six months ago. So an # unchanged controller still posts, gets action=noop, and stays green. # -# ORDER OF OPERATIONS - write to the share BEFORE posting, then clean up on a -# no-op. A failed post then leaves a real directory the next run tidies, rather +# ORDER OF OPERATIONS - write to the share BEFORE posting, then delete again on +# a no-op. That way a failed post leaves a real snapshot on the share rather # than a database row pointing at a directory that was never created. Deleting # after a no-op is safe precisely because a no-op proves identical content # already exists in the previous snapshot. # +# A snapshot whose POST failed is deliberately KEPT, and nothing tidies it +# later. It cannot safely be deleted here: a client-side timeout does not mean +# the server rejected it, so the revision may exist and point at this very +# directory. An orphan directory costs a few hundred kilobytes; deleting a +# snapshot ShopDB has recorded costs the backup itself. +# # PORT CONTENTION IS THE MAIN HAZARD. MarkTelesis holds the serial port whenever # the marker is in service, and so does a tech running Backup470 by hand. This # never takes the port from either - it checks, skips the cycle, and says so @@ -206,7 +212,11 @@ if (-not $Force) { 'WARNING: no successful capture in {0:N1} days ({1}). Every shift change has been busy - check whether the marker application is holding the port permanently.' -f $daysold, $(if (Test-Path $markerfile) { 'last attempt ' + $since.LastWriteTime.ToString('yyyy-MM-dd HH:mm') } else { 'never captured on this PC' })) } else { - Write-ShopdbQuietState -Kind $KIND -State ('outside-window-{0}' -f $now.Hour) -Message ( + # State is the CONDITION, not the clock. Embedding the hour made + # it "new" every hour, so the say-once module wrote the same line + # ~20 times a day per bay - a small rerun of the 3,234-line log it + # was built to stop. + Write-ShopdbQuietState -Kind $KIND -State 'outside-window' -Message ( 'Outside the capture window ({0}). The marker port is shared with eDNC, so the daily capture waits for a shift change. Use -Force to override.' -f (($WindowHours | ForEach-Object { '{0:00}:00' -f $_ }) -join ', ')) } @@ -239,8 +249,17 @@ try { exit 0 } +# -MachineNumberFallback, not a shared default: a marker bay legitimately has +# no machine number - the DEVICE is what the backup belongs to, and +# asset-id.txt is what names it. Putting this in the shared lookup would change +# what the NTLARS backup reports on these same bays, which run eDNC too. +# +# Safe on the wire: the part-marker kind resolves the reporting PC to its own +# marker by hostname first, and falls back to matching this value against +# assets.assetnumber - which is exactly what asset-id.txt holds. $ctx = Initialize-ShopdbBackup -Kind $KIND -Force:$Force ` - -BaseUrl $(if ($BaseUrl) { $BaseUrl } else { '' }) + -BaseUrl $(if ($BaseUrl) { $BaseUrl } else { '' }) ` + -MachineNumberFallback { Get-ShopdbAssetId } if (-not $ctx.Proceed) { exit 0 } # --- Capture everything off the controller ---------------------------------- @@ -307,7 +326,12 @@ try { if ($port -and $port.IsOpen) { $port.Close() } } -if ($null -eq $files -or $files.Count -eq 0) { +# $null means the capture broke partway. An EMPTY hashtable means the capture +# worked and the controller genuinely holds nothing - a factory-reset or +# just-replaced unit. Treating those the same made a wiped controller log a +# capture failure every window for ever, and escalate to the loud warning, when +# in fact the backup was working perfectly and reporting the truth. +if ($null -eq $files) { # A failed capture is a missed backup too, and it is the more worrying kind: # port contention resolves itself, a controller that will not answer does # not. Counted with its own reason so the two are distinguishable in the @@ -361,6 +385,10 @@ try { } | ConvertTo-Json -Depth 4 | Set-Content -LiteralPath (Join-Path $snapshotdir 'manifest.json') -Encoding UTF8 } catch { + # Counted like the others: a snapshot captured but not stored is just as + # missed as one never captured, and share credentials failing on a bay is + # exactly the kind of quiet breakage this count exists to surface. + Add-ShopdbBackupSkip -Kind $KIND -Reason 'could not write to the share' Write-ShopdbBackupLog -Kind $KIND -Message "Could not write $snapshotdir : $_" exit 0 }