Add Schick 33/Elite USB sensor deployment automation script#42
Open
AlrightLad wants to merge 6 commits intoDTC-Inc:mainfrom
Open
Add Schick 33/Elite USB sensor deployment automation script#42AlrightLad wants to merge 6 commits intoDTC-Inc:mainfrom
AlrightLad wants to merge 6 commits intoDTC-Inc:mainfrom
Conversation
Automated deployment script for Schick 33/Elite USB Interface Driver v5.16 with Eaglesoft version-aware installation paths (IOSS vs Legacy). Installation Flows: - Legacy (Eaglesoft < 24.20): CDR Elite > Rename DLL > CDR Patch > AE USB Driver - IOSS (Eaglesoft >= 24.20): Uninstall legacy > CDR Elite > Clean Shared Files (preserve CDRData.dll + OMEGADLL.dll) > CDR Patch > IOSS Autorun > Configure service Key Features: - Auto-detects Eaglesoft version from registry to select install path - Downloads installers from Backblaze B2 (Patterson-Eaglesoft bucket) and Microsoft - Programmatic USB sensor disconnect/reconnect via PnP device APIs, eliminating the need to physically unplug and replug the sensor during deployment - Stops AutoDetectServer.exe and uninstalls legacy CDR components per Patterson docs - Configures IOSS service: delayed auto-start, restart-on-failure recovery, Local System - Validates DLLs, service state, and exports JSON results for HALO ticket integration - Silent install parameters for all components (CDR Elite, IOSS, AE USB, MSXML 4.0) Download Sources: - CDR Elite 5.16, CDR Patch 2808, IOSS v3.2, AE USB Interface/Firmware: Backblaze B2 - MSXML 4.0 SP3 (KB2758694): Microsoft Download Center Prerequisite Checks: - PowerShell 5.1+, Administrator rights, TLS 1.2, Internet connectivity - Core Isolation (Memory Integrity) warning for USB driver compatibility - RDP session detection warning for USB device visibility
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new PowerShell deployment script that detects Eaglesoft version, resolves InstallMode (Auto/Legacy/IOSS), downloads and verifies installers, runs prerequisite checks, executes Legacy or IOSS installation flows (including USB/device disable/enable and Shared Files/DLL handling), validates the installation, and exports JSON results. Changes
Sequence DiagramsequenceDiagram
participant Script as Deployment Script
participant Registry as Windows Registry
participant Prereq as Prerequisite Checks
participant Downloader as Download Pipeline
participant Legacy as Legacy Installers
participant IOSS as IOSS Installer/Service
participant Service as Windows Service Manager
participant USB as USB Sensor Manager
participant Results as Results Export
Script->>Registry: Query Eaglesoft version & uninstall entries
Registry-->>Script: Version details
Script->>Script: Resolve InstallMode (Auto/Legacy/IOSS)
Script->>Prereq: Run environment & connectivity checks
Prereq-->>Script: Pass/Fail
Script->>Downloader: Determine & fetch required installers (hash verify)
Downloader-->>Script: Files ready/cached
alt Legacy mode
Script->>USB: Disable USB sensor devices
Script->>Legacy: Install MSXML4, CDR Elite, CDR Patch, AE USB driver (silent)
Script->>Script: Rename DLLs / handle Shared Files where needed
Script->>USB: Re-enable USB sensors
else IOSS mode
Script->>Service: Stop AutoDetectServer (if running)
Script->>USB: Disable USB sensor devices
Script->>IOSS: Uninstall legacy components, clear Shared Files (preserve listed DLLs)
Script->>IOSS: Install MSXML4, CDR Elite, CDR Patch, Install IOSS service
Script->>Service: Configure IOSS service (start type, recovery, account)
Script->>USB: Re-enable USB sensors
end
Script->>Script: Validate artifacts & service status
Script->>Results: Export JSON summary to OutputPath
Results-->>Script: Export complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
app-eaglesoft/Deploy-SchickSensor (3)
248-250:Get-WmiObjectis deprecated; preferGet-CimInstance.
Get-WmiObjectis deprecated starting in PowerShell 3.0 and removed in PowerShell 7+. If this script ever runs under PS 7, this call will fail. Replace with the CIM equivalent.Proposed fix
- $aeDriver = Get-WmiObject Win32_PnPSignedDriver -ErrorAction SilentlyContinue | - Where-Object { $_.DeviceName -like "*AE*USB*" -or $_.Description -like "*Schick*" } + $aeDriver = Get-CimInstance Win32_PnPSignedDriver -ErrorAction SilentlyContinue | + Where-Object { $_.DeviceName -like "*AE*USB*" -or $_.Description -like "*Schick*" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-eaglesoft/Deploy-SchickSensor` around lines 248 - 250, Replace the deprecated Get-WmiObject call with Get-CimInstance when querying Win32_PnPSignedDriver: call Get-CimInstance -ClassName Win32_PnPSignedDriver -ErrorAction SilentlyContinue (or use a -Filter if you want server-side filtering), then apply the same Where-Object filter on DeviceName/Description to set $aeDriver and $state.AEUSBDriverInstalled; keep the variable names ($aeDriver, $state.AEUSBDriverInstalled) and the existing -ErrorAction behavior so the logic remains identical.
338-345: Internet connectivity check targets google.com instead of the actual download host.Corporate/dental-office firewalls may block google.com while allowing access to the Backblaze B2 bucket. Consider testing against the actual download endpoint to avoid false negatives.
Proposed fix
try { - $null = Invoke-WebRequest -Uri "https://www.google.com" -UseBasicParsing -TimeoutSec 10 + $null = Invoke-WebRequest -Uri "https://s3.us-west-002.backblazeb2.com" -UseBasicParsing -TimeoutSec 10 -Method Head $checks += @{ Name = "Internet"; Status = "PASS"; Message = "Connected" } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-eaglesoft/Deploy-SchickSensor` around lines 338 - 345, The Internet check currently calls Invoke-WebRequest against "https://www.google.com"; update the Invoke-WebRequest call to use the actual download host (the Backblaze B2 bucket download endpoint) instead so firewall rules won't produce false negatives, keep the existing -TimeoutSec 10 and -UseBasicParsing flags, and adjust the $checks entry (Name = "Internet"; Message) if you want to reference the specific host; ensure the catch still sets $passed = $false and records a FAIL when the real endpoint is unreachable.
840-870: Extract duplicated device-discovery logic into a shared helper function.The device-search routines in
Reset-USBSensor(lines 840–870) andDisable-USBSensorForInstall(lines 964–994) are nearly identical. Create a shared helper likeFind-SchickUSBDevicesto avoid maintenance burden and reduce inconsistencies. Note: the two versions differ slightly (Status filter, variable scope, comparison method), so the helper should be designed to handle both cases.Additionally,
Reset-USBSensoris not called anywhere in the script; onlyReset-USBSensorFallbackis invoked (line 1034) and theDisable-USBSensorForInstall/Enable-USBSensorAfterInstallpair is used in the main flow (lines 1164, 1195, 1210, 1250). IfReset-USBSensoris dead code, remove it; if reserved for standalone use, document that intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-eaglesoft/Deploy-SchickSensor` around lines 840 - 870, The duplicated USB device discovery in Reset-USBSensor and Disable-USBSensorForInstall should be refactored into a single helper (e.g., Find-SchickUSBDevices) that accepts options for status filtering, property source (FriendlyName vs HardwareIds), and comparison method (object equality vs InstanceId comparison) and returns a unified collection of matching devices; update Reset-USBSensor and Disable-USBSensorForInstall to call this helper (use parameters to preserve their slight behavioral differences), and either remove Reset-USBSensor if it is dead code or add a short comment/docstring explaining it is intentionally retained for standalone use (reference functions: Reset-USBSensor, Disable-USBSensorForInstall, Enable-USBSensorAfterInstall, Reset-USBSensorFallback, and the new Find-SchickUSBDevices).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app-eaglesoft/Deploy-SchickSensor`:
- Around line 744-752: Install-AEUSBDriver currently returns $true for any
non-zero $process.ExitCode, hiding real failures; change the logic so that
Install-AEUSBDriver only returns $true for $process.ExitCode -eq 0 or for a
defined whitelist of acceptable exit codes (e.g., 3010), otherwise log the exit
code as an error and return $false; implement this by introducing an
$acceptableExitCodes array (or switch) checked against $process.ExitCode, update
the Write-Log call to use Error/Warning with the exit code included, and ensure
Install-AEUSBDriver uses that check instead of always returning $true.
- Around line 1281-1288: The banner box borders are too short; update the
Write-Host here-string (the multi-line string that begins with Write-Host @" and
contains "Schick Sensor Deployment Script v1.0.0") so the top/bottom corner
lines use the same width as the longest content line: either compute the max
content length and programmatically build the top/bottom with "╔" + "═" repeated
N + "╗" and "╚" + "═" repeated N + "╝", or manually expand the existing
"╔════════…╗" and "╚════════…╝" to match the inner lines so the vertical "║"
characters align with the corners. Ensure spacing inside matches for all content
lines.
- Around line 1154-1197: The Disable-USBSensorForInstall call in
Invoke-LegacyInstallation can leave devices disabled if any install step returns
early; wrap the main installation sequence inside a try/finally block so
Enable-USBSensorAfterInstall always runs (even on early return/exception).
Specifically, in Invoke-LegacyInstallation, call Disable-USBSensorForInstall
before try, place the existing step sequence (MSXML4, Install-CDRElite,
Rename-CDRImageProcessDLL, Install-CDRPatch, Install-AEUSBDriver) inside the
try, and put Enable-USBSensorAfterInstall in the finally; preserve and return
the original success/failure result (or rethrow) so behavior isn’t silently
changed. Apply the identical try/finally pattern to Invoke-IOSSInstallation to
ensure sensors are re-enabled on all exit paths.
- Around line 407-414: The downloader currently reuses any existing non-empty
file at $destination (Test-Path + file size) without integrity checks; update
Get-Installer to consult $Script:Config.Downloads (add a SHA256 field per
installer), and after a cache hit or after download compute the file's SHA256
via Get-FileHash and compare it to the expected SHA256 from the installerConfig;
on mismatch, Write-Log an error including expected and actual hashes,
Remove-Item -Force the corrupt file and return $null (or trigger a
re-download/failure path) so the script never executes a file that fails hash
verification.
- Around line 571-578: The wildcard list assigned to $legacyProducts is too
broad (e.g., "*CDR*") and may match unrelated apps; tighten the patterns in the
$legacyProducts array to exact product names used by Schick/Patterson (e.g.,
include full product strings like "*Schick CDR Elite*" or "*Patterson CDR USB*")
and/or add a secondary filter when enumerating installed products (the
Get-ItemProperty / Where-Object step that checks DisplayName) to require the
Publisher contains "Patterson" or "Schick" so only vendor-specific matches are
uninstalled; update references to $legacyProducts and the Where-Object filter
accordingly.
- Around line 793-804: The three invocations of sc.exe (the config and failure
commands that use $serviceName) silently drop output with Out-Null and never
check $LASTEXITCODE, so failures are swallowed; capture each sc.exe invocation's
output (don't pipe to Out-Null), check $LASTEXITCODE immediately after the call,
and on non-zero exit log the error via Write-Log including the exit code and
captured output and then return/throw failure (instead of continuing to return
$true); apply this pattern to the & sc.exe config $serviceName start=
delayed-auto, & sc.exe failure $serviceName ..., and & sc.exe config
$serviceName obj= "LocalSystem" calls so service configuration failures are
detected and propagated.
- Around line 199-206: The catch currently cleans $eaglesoft.DisplayVersion into
$cleanVersion and re-casts with [Version], which can still throw; change that
second cast to a safe parse (use [Version]::TryParse or a guarded try/catch) to
validate $cleanVersion before returning: if TryParse succeeds return the parsed
Version, otherwise return $null so the caller can default to Legacy mode;
reference $eaglesoft.DisplayVersion, $cleanVersion and the [Version] cast in
your fix.
---
Nitpick comments:
In `@app-eaglesoft/Deploy-SchickSensor`:
- Around line 248-250: Replace the deprecated Get-WmiObject call with
Get-CimInstance when querying Win32_PnPSignedDriver: call Get-CimInstance
-ClassName Win32_PnPSignedDriver -ErrorAction SilentlyContinue (or use a -Filter
if you want server-side filtering), then apply the same Where-Object filter on
DeviceName/Description to set $aeDriver and $state.AEUSBDriverInstalled; keep
the variable names ($aeDriver, $state.AEUSBDriverInstalled) and the existing
-ErrorAction behavior so the logic remains identical.
- Around line 338-345: The Internet check currently calls Invoke-WebRequest
against "https://www.google.com"; update the Invoke-WebRequest call to use the
actual download host (the Backblaze B2 bucket download endpoint) instead so
firewall rules won't produce false negatives, keep the existing -TimeoutSec 10
and -UseBasicParsing flags, and adjust the $checks entry (Name = "Internet";
Message) if you want to reference the specific host; ensure the catch still sets
$passed = $false and records a FAIL when the real endpoint is unreachable.
- Around line 840-870: The duplicated USB device discovery in Reset-USBSensor
and Disable-USBSensorForInstall should be refactored into a single helper (e.g.,
Find-SchickUSBDevices) that accepts options for status filtering, property
source (FriendlyName vs HardwareIds), and comparison method (object equality vs
InstanceId comparison) and returns a unified collection of matching devices;
update Reset-USBSensor and Disable-USBSensorForInstall to call this helper (use
parameters to preserve their slight behavioral differences), and either remove
Reset-USBSensor if it is dead code or add a short comment/docstring explaining
it is intentionally retained for standalone use (reference functions:
Reset-USBSensor, Disable-USBSensorForInstall, Enable-USBSensorAfterInstall,
Reset-USBSensorFallback, and the new Find-SchickUSBDevices).
Addresses all CodeRabbit automated review comments from PR DTC-Inc#42. CRITICAL FIXES: 1. USB Sensors Left Disabled on Failure - Wrapped Invoke-LegacyInstallation and Invoke-IOSSInstallation in try/finally - Enable-USBSensorAfterInstall now ALWAYS executes, even if installation fails - Prevents technicians from encountering disabled sensors after failed deployments 2. Installer Integrity Verification - Added SHA256 field to all installer configs (currently $null with TODO markers) - New Test-InstallerHash function verifies downloads against expected hashes - Corrupted/mismatched files are automatically removed and re-downloaded - Hash verification runs on both cached files and fresh downloads 3. Overly Broad Uninstall Patterns - Tightened patterns: "CDR Patch-2808*" instead of "*CDR Patch*" - Added Publisher filter requiring Patterson/Schick/Sirona - Prevents accidental uninstall of CD-R burning software or Call Detail Record tools CODE QUALITY IMPROVEMENTS: 4. PowerShell 7+ Compatibility - Replaced deprecated Get-WmiObject with Get-CimInstance for driver detection 5. Connectivity Check - Now tests against actual Backblaze B2 endpoint instead of google.com - Falls back to Microsoft download endpoint if B2 unreachable - Avoids false negatives from corporate firewalls blocking google.com 6. Service Configuration Error Handling - Added $LASTEXITCODE checks after all sc.exe calls - Logs specific exit codes when service configuration fails 7. AE USB Driver Exit Codes - Only accepts known success codes: 0 (success), 3010 (reboot required), 1641 (reboot initiated) - Previously returned $true for ALL non-zero codes 8. Version Parsing - Replaced unsafe [Version] cast with [Version]::TryParse pattern - Handles malformed version strings without throwing unhandled exceptions 9. USB Discovery Consolidation - Extracted Find-SchickUSBDevices shared helper function - Reduces code duplication between Reset-USBSensor and Disable/Enable functions - Single point of maintenance for device detection patterns 10. Banner Alignment - Standardized all banner borders to 71 characters - Fixed visual misalignment in header and completion messages NOTE: SHA256 hashes are set to $null and must be populated after first verified download of each installer from trusted source.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
app-eaglesoft/Deploy-SchickSensor (3)
147-151: O(n²) log accumulation — prefer aList[object].
$Script:Results.Log += @{...}copies the entire array on every call. Switch the log to[System.Collections.Generic.List[object]].♻️ Proposed refactor
Initialize (line ~1402):
- Log = @() + Log = [System.Collections.Generic.List[object]]::new()In
Write-Log(line ~148):- $Script:Results.Log += @{ - Timestamp = $timestamp - Level = $Level - Message = $Message - } + $Script:Results.Log.Add(@{ + Timestamp = $timestamp + Level = $Level + Message = $Message + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-eaglesoft/Deploy-SchickSensor` around lines 147 - 151, $Script:Results.Log uses array concatenation via "$Script:Results.Log += @{...}" which copies the array on every call; change the backing store to a [System.Collections.Generic.List[object]] (e.g., initialize $Script:Results.Log = [System.Collections.Generic.List[object]]::new()) and update Write-Log to call $Script:Results.Log.Add($entry) instead of using +=; ensure any code that reads $Script:Results.Log enumerates its items or calls .ToArray() if an array is still required elsewhere.
84-88:AEUSBFirmwareis configured but never downloaded or used.
Get-RequiredInstallersdoesn't include"AEUSBFirmware"in either theLegacyorIOSSswitch arms, making this a dead config entry. Either add it to the appropriate install flow or remove it to avoid confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-eaglesoft/Deploy-SchickSensor` around lines 84 - 88, AEUSBFirmware is defined but never used because Get-RequiredInstallers does not return it; either remove the AEUSBFirmware entry or add it to the appropriate installer list and flow. Locate the Get-RequiredInstallers function and add "AEUSBFirmware" to the Legacy or IOSS return/array (and any callers that build the download/install list), and ensure the existing download/install logic consumes the same key (AEUSBFirmware) so the URL/FileName/SHA256 are actually downloaded and validated; alternatively delete the AEUSBFirmware block if it is intentionally unused.
1207-1210:CDRImageProcess.dllis not validated after Legacy installation.Comment at line 1288 documents that CDR Patch creates
CDRImageProcess.dll v5.15.1877in Legacy mode too, butTest-Installationonly adds it to$requiredDLLsfor IOSS. A failed CDR Patch in Legacy mode goes undetected.♻️ Proposed fix
$requiredDLLs = @("CDRData.dll", "OMEGADLL.dll") - if ($Mode -eq 'IOSS') { - $requiredDLLs += "CDRImageProcess.dll" - } + # CDR Patch creates CDRImageProcess.dll in both modes + $requiredDLLs += "CDRImageProcess.dll"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-eaglesoft/Deploy-SchickSensor` around lines 1207 - 1210, Test-Installation currently only adds "CDRImageProcess.dll" to the $requiredDLLs array when $Mode -eq 'IOSS', but comments state the CDR Patch installs CDRImageProcess.dll (v5.15.1877) in Legacy mode too, so a failed patch under Legacy is not detected; update Test-Installation to include "CDRImageProcess.dll" in $requiredDLLs for Legacy installations as well (or make the check mode-conditional to include both 'IOSS' and 'Legacy'), and ensure the presence/version check for CDRImageProcess.dll mirrors how other required DLLs are validated so failures in Legacy are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app-eaglesoft/Deploy-SchickSensor`:
- Line 216: The regex used to clean $versionString contains a typo '\.§' so
trailing dots aren't removed; update the replacement for $cleanVersion to strip
trailing periods by replacing the '\.§' token with the proper end-of-string
anchor '\.$' (or use a dedicated replacement like -replace '\.$','') so that
$cleanVersion has no trailing dot before [Version]::TryParse is called.
- Around line 866-874: Install-IOSS currently treats any non-zero exit code as
success (logs Warning but returns $true), swallowing failures; update
Install-IOSS so it only returns $true for exit codes 0 and 3010 and returns
$false (or throws) for any other exit code, logging an Error with the exit code;
similarly change Set-IOSSServiceConfiguration to treat failure as error (not
just Warning) and return/propagate failure like Install-AEUSBDriver does—look
for the Install-IOSS and Set-IOSSServiceConfiguration functions and replicate
the same exit-code handling and logging pattern used in Install-AEUSBDriver.
- Around line 269-271: The registry query for MSXML4 only checks the 64-bit
Uninstall hive and misses 32-bit installs; update the check in
Get-CurrentInstallState (the block that sets $msxml and $state.MSXML4Installed)
to query both HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall\* and
HKLM:\SOFTWARE\WOW6432Node\Microsoft\Windows\CurrentVersion\Uninstall\* (or
enumerate both registry views) and treat $state.MSXML4Installed as true if
either query returns an item whose DisplayName -like "*MSXML 4*". Ensure you
still use -ErrorAction SilentlyContinue and preserve the existing
$msxml/$state.MSXML4Installed variable names.
- Around line 668-671: The publisher filter inside the Where-Object is broken
because the inner ForEach-Object rebinding of $_ replaces the product object;
capture the current product into a local variable (e.g. $product = $_) before
invoking $validPublishers | ForEach-Object and then compare $product.Publisher
-like $_ inside the loop (or use Any/Where against $validPublishers referencing
$product.Publisher) so the publisher check evaluates against the product's
Publisher property rather than a string-bound $_.
- Around line 325-328: The version check currently uses
($PSVersionTable.PSVersion.Major -lt 5) which incorrectly allows PowerShell 5.0;
update the condition to require at least 5.1 by comparing the version object
directly (e.g. if ($PSVersionTable.PSVersion -lt [version]"5.1") ) or by adding
a Minor check (e.g. Major -lt 5 -or (Major -eq 5 -and Minor -lt 1)); update the
same block that sets $checks and $passed to use this new condition so PowerShell
5.0 will fail.
- Around line 1258-1377: The functions Invoke-LegacyInstallation and
Invoke-IOSSInstallation are emitting helper call outputs into the pipeline
causing $installSuccess to become an array; suppress those outputs and ensure
the functions return a single boolean. For each helper call that currently
returns booleans but is not captured (Disable-USBSensorForInstall,
Rename-CDRImageProcessDLL, Uninstall-LegacyComponents,
Enable-USBSensorAfterInstall) change the invocations to discard pipeline output
(e.g. use [void]Disable-USBSensorForInstall or assign to $null) and explicitly
check critical results (capture Rename-CDRImageProcessDLL into a variable and if
it fails return $false immediately). Also ensure both functions explicitly
return a single boolean (return $true / return $false or at the end return
([bool]$installSuccess)) so callers receive a scalar, not an array.
---
Nitpick comments:
In `@app-eaglesoft/Deploy-SchickSensor`:
- Around line 147-151: $Script:Results.Log uses array concatenation via
"$Script:Results.Log += @{...}" which copies the array on every call; change the
backing store to a [System.Collections.Generic.List[object]] (e.g., initialize
$Script:Results.Log = [System.Collections.Generic.List[object]]::new()) and
update Write-Log to call $Script:Results.Log.Add($entry) instead of using +=;
ensure any code that reads $Script:Results.Log enumerates its items or calls
.ToArray() if an array is still required elsewhere.
- Around line 84-88: AEUSBFirmware is defined but never used because
Get-RequiredInstallers does not return it; either remove the AEUSBFirmware entry
or add it to the appropriate installer list and flow. Locate the
Get-RequiredInstallers function and add "AEUSBFirmware" to the Legacy or IOSS
return/array (and any callers that build the download/install list), and ensure
the existing download/install logic consumes the same key (AEUSBFirmware) so the
URL/FileName/SHA256 are actually downloaded and validated; alternatively delete
the AEUSBFirmware block if it is intentionally unused.
- Around line 1207-1210: Test-Installation currently only adds
"CDRImageProcess.dll" to the $requiredDLLs array when $Mode -eq 'IOSS', but
comments state the CDR Patch installs CDRImageProcess.dll (v5.15.1877) in Legacy
mode too, so a failed patch under Legacy is not detected; update
Test-Installation to include "CDRImageProcess.dll" in $requiredDLLs for Legacy
installations as well (or make the check mode-conditional to include both 'IOSS'
and 'Legacy'), and ensure the presence/version check for CDRImageProcess.dll
mirrors how other required DLLs are validated so failures in Legacy are caught.
app-eaglesoft/Deploy-SchickSensor
Outdated
Comment on lines
+866
to
+874
| if ($process.ExitCode -eq 0 -or $process.ExitCode -eq 3010) { | ||
| Write-Log "IOSS installed successfully" -Level Success | ||
| return $true | ||
| } | ||
| else { | ||
| Write-Log "IOSS installation returned exit code: $($process.ExitCode)" -Level Warning | ||
| # Continue anyway - service configuration will verify | ||
| return $true | ||
| } |
Contributor
There was a problem hiding this comment.
Install-IOSS returns $true for all exit codes — IOSS failures are silently swallowed.
Non-zero exit codes produce a Warning log but still return $true. Set-IOSSServiceConfiguration failure is also only a warning (line 1362), so a completely failed IOSS install propagates as "success" and the script exits 0 or 2 (warnings) instead of 1. This is the same pattern that was correctly fixed for Install-AEUSBDriver in the previous commit.
🐛 Proposed fix (mirror Install-AEUSBDriver pattern)
if ($process.ExitCode -eq 0 -or $process.ExitCode -eq 3010) {
Write-Log "IOSS installed successfully" -Level Success
return $true
}
else {
- Write-Log "IOSS installation returned exit code: $($process.ExitCode)" -Level Warning
- # Continue anyway - service configuration will verify
- return $true
+ Write-Log "IOSS installation failed with exit code: $($process.ExitCode)" -Level Error
+ return $false
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app-eaglesoft/Deploy-SchickSensor` around lines 866 - 874, Install-IOSS
currently treats any non-zero exit code as success (logs Warning but returns
$true), swallowing failures; update Install-IOSS so it only returns $true for
exit codes 0 and 3010 and returns $false (or throws) for any other exit code,
logging an Error with the exit code; similarly change
Set-IOSSServiceConfiguration to treat failure as error (not just Warning) and
return/propagate failure like Install-AEUSBDriver does—look for the Install-IOSS
and Set-IOSSServiceConfiguration functions and replicate the same exit-code
handling and logging pattern used in Install-AEUSBDriver.
Resolves second round of CodeRabbit review comments on PR DTC-Inc#42. CRITICAL - Pipeline output pollution (script always exited 0): Helper functions (Disable-USBSensorForInstall, Enable-USBSensorAfterInstall, Uninstall-LegacyComponents, Rename-CDRImageProcessDLL) emitted boolean return values to the pipeline without capture, causing $installSuccess to become an array instead of a boolean. The -not check always evaluated to $false, meaning installation failures were never detected and the script exited 0 regardless. Fixed by prefixing all helper calls with $null = and explicitly checking Rename-CDRImageProcessDLL result to fail the installation on error. CRITICAL - Install-IOSS returned $true on all exit codes: Non-zero exit codes logged a warning but still returned $true, silently swallowing installation failures. Now only accepts exit codes 0, 3010, and 1641 (same pattern as the previously fixed Install-AEUSBDriver). MAJOR - Publisher filter was completely broken: Nested ForEach-Object rebound $_ from the product object to the $validPublishers string, causing .Publisher to always return $null. Every product failed the publisher check, defeating the safety filter. Fixed by capturing $productPublisher = $_.Publisher before the inner loop. Minor fixes: - Regex typo: \.\u00a7 (section sign) replaced with \.+$ (end-of-string anchor) so trailing dots are actually stripped during version string cleanup - PowerShell version check: -lt [Version]'5.1' instead of Major -lt 5 to correctly reject PowerShell 5.0 which lacks required PnP/CIM cmdlets - MSXML4 registry: Now searches both 64-bit and WOW6432Node hives to detect 32-bit installs and avoid unnecessary reinstalls - Removed unused AEUSBFirmware config entry (defined but never downloaded) - CDRImageProcess.dll now validated in both Legacy and IOSS modes since CDR Patch creates it in both installation paths - Results Log switched from array += O(n^2) to Generic.List[object].Add() O(1)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
app-eaglesoft/Deploy-SchickSensor (2)
1004-1005:Select-Object -Uniquemay not reliably deduplicate PnP device objects.
Select-Object -Uniquerelies on default object equality, which forCimInstance/PnP device objects isn'tInstanceId-based. The earlierInstanceIdcheck (line 998) only guards the hardware-ID branch. Devices matched by both the friendly-name search and the hardware-ID search could still appear twice.Proposed fix
- return ($foundDevices | Select-Object -Unique) + return ($foundDevices | Sort-Object -Property InstanceId -Unique)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-eaglesoft/Deploy-SchickSensor` around lines 1004 - 1005, The current dedupe uses Select-Object -Unique on $foundDevices which doesn't reliably deduplicate CimInstance/PnP device objects because object equality isn't based on InstanceId; replace the final return `return ($foundDevices | Select-Object -Unique)` with a dedupe by InstanceId (e.g. group by InstanceId and take the first item from each group) so devices matched via friendly-name and hardware-ID are collapsed; ensure you reference the existing InstanceId field used earlier when grouping (use Group-Object -Property InstanceId | ForEach-Object { $_.Group[0] } or equivalent).
476-497: No retry mechanism for installer downloads.
Get-Installermakes a single download attempt. Transient network errors (DNS hiccups, B2 rate-limiting, TCP resets) will immediately abort the entire deployment. A simple 2–3 attempt retry with exponential back-off would significantly improve reliability for a production script that runs on dental workstations with variable connectivity.Sketch: add retry loop around download
- try { - $ProgressPreference = 'SilentlyContinue' - Invoke-WebRequest -Uri $url -OutFile $destination -UseBasicParsing -TimeoutSec 300 + $maxRetries = 3 + $attempt = 0 + $downloaded = $false + + while (-not $downloaded -and $attempt -lt $maxRetries) { + $attempt++ + try { + $ProgressPreference = 'SilentlyContinue' + Invoke-WebRequest -Uri $url -OutFile $destination -UseBasicParsing -TimeoutSec 300 + $downloaded = $true + } + catch { + Write-Log "Download attempt $attempt/$maxRetries failed for $Name`: $_" -Level Warning + if ($attempt -lt $maxRetries) { + $delay = [math]::Pow(2, $attempt) * 5 + Write-Log " Retrying in $delay seconds..." -Level Info + Start-Sleep -Seconds $delay + } + else { + Write-Log "Failed to download $Name after $maxRetries attempts" -Level Error + return $null + } + } + } - $fileSize = (Get-Item $destination).Length - Write-Log "Downloaded $Name successfully ($([math]::Round($fileSize/1MB, 2)) MB)" -Level Success + $fileSize = (Get-Item $destination).Length + Write-Log "Downloaded $Name successfully ($([math]::Round($fileSize/1MB, 2)) MB)" -Level Success🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-eaglesoft/Deploy-SchickSensor` around lines 476 - 497, Get-Installer currently calls Invoke-WebRequest once and returns on error; wrap the download block (the Invoke-WebRequest call inside Get-Installer) in a retry loop of 2–3 attempts with exponential backoff (e.g., initial 2s, then double) so transient network failures are retried, logging each attempt and the caught $_ error via Write-Log (include attempt number), and only return $destination after a successful download and Test-InstallerHash passes; if Test-InstallerHash fails remove the file and abort immediately, and if all attempts fail log a final error and return $null. Preserve setting $ProgressPreference and TimeoutSec, and reference Invoke-WebRequest, Test-InstallerHash, and Get-Installer in your changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app-eaglesoft/Deploy-SchickSensor`:
- Around line 710-714: The $uninstallString may contain the executable plus
embedded args, but the code strips quotes and passes the whole string into
Start-Process -FilePath, which breaks paths/args; update the block that handles
$uninstallString to parse the executable path and its existing arguments (e.g.,
extract a quoted path if present or take the first token as the executable),
assign the executable to a $exePath variable and the remaining to $existingArgs,
then build an ArgumentList that preserves vendor args and appends your silent
flags (e.g., merge $existingArgs with "/S","/SILENT","/VERYSILENT","/NORESTART"
as appropriate), and call Start-Process -FilePath $exePath -ArgumentList
$argumentList -Wait -NoNewWindow -ErrorAction SilentlyContinue; continue to
Write-Log "Uninstall attempted" after the process call.
- Around line 1330-1338: Move the Uninstall-LegacyComponents call into the
protected try/finally region inside Invoke-IOSSInstallation so
Enable-USBSensorAfterInstall always runs; specifically, ensure
Disable-USBSensorForInstall is still called before the try, then call
Uninstall-LegacyComponents inside the try block (before other install steps) so
any exception triggers the finally which calls Enable-USBSensorAfterInstall.
- Around line 964-970: The device matching is too broad: update the
$devicePatterns array and the hardware ID check used by
Disable-USBSensorForInstall so FTDI devices aren't disabled indiscriminately;
either remove the "*FTDI*" entry from $devicePatterns (variable $devicePatterns)
and rely on the Schick-specific VID_20D6 match, or if you must detect Schick
devices on FTDI silicon, replace the loose VID_0403 match in the hardware ID
regex ("VID_0403|VID_20D6|Schick") with a VID:PID pair (e.g., VID_0403_PID_xxxx)
for the known Schick PID so only that exact FTDI device is matched by
Disable-USBSensorForInstall. Ensure you update both the $devicePatterns list and
the hardware ID match so they stay consistent.
---
Nitpick comments:
In `@app-eaglesoft/Deploy-SchickSensor`:
- Around line 1004-1005: The current dedupe uses Select-Object -Unique on
$foundDevices which doesn't reliably deduplicate CimInstance/PnP device objects
because object equality isn't based on InstanceId; replace the final return
`return ($foundDevices | Select-Object -Unique)` with a dedupe by InstanceId
(e.g. group by InstanceId and take the first item from each group) so devices
matched via friendly-name and hardware-ID are collapsed; ensure you reference
the existing InstanceId field used earlier when grouping (use Group-Object
-Property InstanceId | ForEach-Object { $_.Group[0] } or equivalent).
- Around line 476-497: Get-Installer currently calls Invoke-WebRequest once and
returns on error; wrap the download block (the Invoke-WebRequest call inside
Get-Installer) in a retry loop of 2–3 attempts with exponential backoff (e.g.,
initial 2s, then double) so transient network failures are retried, logging each
attempt and the caught $_ error via Write-Log (include attempt number), and only
return $destination after a successful download and Test-InstallerHash passes;
if Test-InstallerHash fails remove the file and abort immediately, and if all
attempts fail log a final error and return $null. Preserve setting
$ProgressPreference and TimeoutSec, and reference Invoke-WebRequest,
Test-InstallerHash, and Get-Installer in your changes.
…eRabbit review Addresses five issues identified in the third round of automated code review on PR DTC-Inc#42 to harden reliability and prevent unintended side effects. Device deduplication: - Replace Select-Object -Unique with Sort-Object -Property InstanceId -Unique in Find-SchickUSBDevices, as Select-Object uses reference equality on CimInstance objects and silently fails to deduplicate Download resilience: - Add retry loop (3 attempts) with exponential backoff (2s, 4s) to Get-Installer so transient network failures don't abort the entire deployment - Clean up partial downloads between retry attempts UninstallString argument parsing: - Parse quoted and unquoted exe paths from embedded arguments before passing to Start-Process, fixing cases where registry UninstallString values like '"C:\path\uninst.exe" /I{GUID}' were passed whole as the -FilePath parameter FTDI device matching scope: - Remove broad *FTDI* friendly name search pattern that could match Arduino boards, lab equipment, and other unrelated VID_0403 devices - VID_0403 hardware ID matches now require the device FriendlyName to contain Schick, Dental, AE, or Sensor as a secondary filter - VID_20D6 (Schick) continues to match unconditionally USB sensor safety in IOSS flow: - Move Uninstall-LegacyComponents call inside the try/finally block in Invoke-IOSSInstallation so USB sensors are guaranteed to be re-enabled even if legacy component removal throws an exception
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
app-eaglesoft/Deploy-SchickSensor (1)
1001-1004:"*AE*USB*"friendly-name pattern may match non-dental USB devices.This pattern could match devices like "Creative AE-5 USB Audio" or similar hardware whose friendly name happens to contain "AE" and "USB". Combined with the class filter (
USB,Image,Ports,HIDClass), the risk is reduced but not eliminated — a USB audio device inHIDClasswould still match. Consider tightening to"*AE USB*"(no inner wildcard) or"Schick*AE*USB*"to match only the known Patterson product name.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app-eaglesoft/Deploy-SchickSensor` around lines 1001 - 1004, The device pattern array $devicePatterns contains a too-broad entry "*AE*USB*" which can match non-dental USB devices; update the pattern in $devicePatterns to a tighter match such as "*AE USB*" (remove the inner wildcard) or prefixed to the vendor/product like "Schick*AE*USB*" so only the intended Patterson/Schick sensor friendly-names are matched by the pattern used when enumerating devices.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app-eaglesoft/Deploy-SchickSensor`:
- Around line 640-757: Uninstall-LegacyComponents currently logs uninstall
exceptions in the catch block but always returns $true; change it to track
per-product failures (e.g., add a $failedUninstalls counter or list when the
catch is hit for a given $product or when Start-Process returns non-zero), and
at the end return $false if any failures occurred instead of unconditionally
returning $true; update the caller Invoke-IOSSInstallation to stop suppressing
the result (remove `$null =`) and check the Boolean return from
Uninstall-LegacyComponents so installation aborts or warns when legacy removals
failed.
- Around line 486-493: When a downloaded file fails Test-InstallerHash, don't
return immediately; remove the corrupted $destination and continue the retry
loop so subsequent attempts can try again. In the block where you call
Test-InstallerHash (the code that currently does Remove-Item -Path $destination
-Force and return $null), replace the immediate return with logic to continue to
the next iteration (e.g., log the failure, Remove-Item, and let the enclosing
retry loop proceed or explicitly continue), ensuring the retry counter/exit
conditions enforced by the surrounding download loop remain intact so you only
exit early on final failure or download exceptions.
- Around line 935-980: The function Set-IOSSServiceConfiguration currently logs
sc.exe and Start-Service failures but always returns $true; add a boolean
failure tracker (e.g. $hadFailure = $false) inside Set-IOSSServiceConfiguration,
set $hadFailure = $true when any sc.exe invocation has $LASTEXITCODE -ne 0 (the
config/failure/obj calls) and also set $hadFailure = $true if Start-Service is
attempted and the subsequent $iossService.Status is not 'Running'; finally
return $false when $hadFailure is true (otherwise return $true) so callers see a
real failure result. Use the existing symbols ($LASTEXITCODE, the sc.exe
commands, Start-Service, $iossService, and the final return) to locate where to
add and update the flag.
---
Duplicate comments:
In `@app-eaglesoft/Deploy-SchickSensor`:
- Around line 62-93: The Downloads entries for CDRElite, CDRPatch, AEUSBDriver,
IOSS, and MSXML4 currently have SHA256 = $null so Test-InstallerHash will skip
validation; compute the canonical SHA256 for each installer and populate the
SHA256 fields in the $Script:Config.Downloads objects before main runs (or
update the deployment process to inject them), referencing the exact keys
(CDRElite, CDRPatch, AEUSBDriver, IOSS, MSXML4) so Test-InstallerHash performs
verification; if you cannot provide hashes now, add a Write-Log -Level Error
call immediately before any installer execution (where Test-InstallerHash would
be called) to escalate the missing-hash condition instead of a warning.
---
Nitpick comments:
In `@app-eaglesoft/Deploy-SchickSensor`:
- Around line 1001-1004: The device pattern array $devicePatterns contains a
too-broad entry "*AE*USB*" which can match non-dental USB devices; update the
pattern in $devicePatterns to a tighter match such as "*AE USB*" (remove the
inner wildcard) or prefixed to the vendor/product like "Schick*AE*USB*" so only
the intended Patterson/Schick sensor friendly-names are matched by the pattern
used when enumerating devices.
…cision Addresses five issues from the fourth round of automated code review on PR DTC-Inc#42, focused on ensuring failures are never silently swallowed. Device pattern precision: - Tighten "*AE*USB*" friendly name pattern to "*AE USB*" in Find-SchickUSBDevices to prevent false matches on unrelated hardware such as "Creative AE-5 USB Audio" sound cards Download integrity retry: - Hash verification failures after successful download now continue the retry loop with exponential backoff instead of returning null immediately - Covers transient CDN corruption, truncated responses, and edge proxies serving stale content that may resolve on subsequent attempts Uninstall failure propagation: - Uninstall-LegacyComponents now captures exit codes from both MSI and EXE uninstall processes, counts failures per product, and returns $false when any component fails to remove - Invoke-IOSSInstallation checks the return value and halts installation, since Patterson documentation requires clean removal of legacy CDR/Schick components before IOSS can be installed Service configuration failure tracking: - Set-IOSSServiceConfiguration tracks sc.exe non-zero exit codes and service start failures via $configSuccess flag, returning the actual result instead of unconditional $true - Startup type and logon account failures are now logged at Error level rather than Warning to reflect their operational impact SHA256 hash pre-flight awareness: - Config comments updated from TODO to ACTION REQUIRED with instructions for populating hashes via Get-FileHash on a trusted machine - New prerequisite check enumerates all installers with null SHA256 fields and emits a WARN-level alert listing which downloads will proceed without integrity verification
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app-eaglesoft/Deploy-SchickSensor`:
- Around line 66-95: The Downloads map in $Script:Config currently leaves SHA256
for CDRElite, CDRPatch, AEUSBDriver, IOSS, and MSXML4 as $null; replace each
$null with the verified SHA256 hex string for the corresponding installer (keys:
CDRElite, CDRPatch, AEUSBDriver, IOSS, MSXML4) obtained by downloading the
installer on a trusted machine and running Get-FileHash -Algorithm SHA256, then
paste the resulting hash string into the SHA256 field for that entry; keep the
filenames/URLs unchanged and ensure the runtime prerequisite check that
validates these hashes continues to run against these populated values.
- Around line 993-1000: The sc.exe failure call block doesn't set $configSuccess
= $false on non-zero exit like the other config blocks, so update the failure
branch in the sc.exe failure/$serviceName section to set $configSuccess = $false
when $LASTEXITCODE -ne 0; keep the existing Write-Log Warning (or enhance
message) and mirror the pattern used in the other blocks that check
$LASTEXITCODE and set $configSuccess accordingly (refer to the $configSuccess
variable, sc.exe failure call, Write-Log, $LASTEXITCODE, and $serviceName).
Comment on lines
+66
to
+95
| $Script:Config = @{ | ||
| # Full download URLs with SHA256 hashes for integrity verification | ||
| # To populate hashes: download each installer on a trusted machine, then run: | ||
| # (Get-FileHash -Path ".\filename.exe" -Algorithm SHA256).Hash | ||
| Downloads = @{ | ||
| CDRElite = @{ | ||
| Url = "https://s3.us-west-002.backblazeb2.com/public-dtc/repo/vendors/Patterson-Eaglesoft/CDRElite5_16/CDRElite/CDR%20Elite%20Setup.exe" | ||
| FileName = "CDR Elite Setup.exe" | ||
| SHA256 = $null # ACTION REQUIRED: Populate from verified download | ||
| } | ||
| CDRPatch = @{ | ||
| Url = "https://s3.us-west-002.backblazeb2.com/public-dtc/repo/vendors/Patterson-Eaglesoft/CDRElite5_16/CDRElite/Patch/CDRPatch-2808.msi" | ||
| FileName = "CDRPatch-2808.msi" | ||
| SHA256 = $null # ACTION REQUIRED: Populate from verified download | ||
| } | ||
| AEUSBDriver = @{ | ||
| Url = "https://s3.us-west-002.backblazeb2.com/public-dtc/repo/vendors/Patterson-Eaglesoft/AEUSBInterfaceSetup.exe" | ||
| FileName = "AEUSBInterfaceSetup.exe" | ||
| SHA256 = $null # ACTION REQUIRED: Populate from verified download | ||
| } | ||
| IOSS = @{ | ||
| Url = "https://s3.us-west-002.backblazeb2.com/public-dtc/repo/vendors/Patterson-Eaglesoft/IOSS_v3.2/IOSS_v3.2/Autorun.exe" | ||
| FileName = "IOSS_Autorun.exe" | ||
| SHA256 = $null # ACTION REQUIRED: Populate from verified download | ||
| } | ||
| MSXML4 = @{ | ||
| Url = "https://download.microsoft.com/download/1/E/E/1EE06E22-A56F-4E76-B6F6-E7670B4F8163/msxml4-KB2758694-enu.exe" | ||
| FileName = "msxml4-KB2758694-enu.exe" | ||
| SHA256 = $null # ACTION REQUIRED: Populate from verified download | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
All five SHA256 fields are $null — hashes must be populated before production use.
The prerequisite check at lines 380–391 correctly warns at runtime, but as-is, every installer download is unverified. Since this script runs elevated and executes downloaded binaries, filling these hashes is the single highest-priority pre-merge action item.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app-eaglesoft/Deploy-SchickSensor` around lines 66 - 95, The Downloads map in
$Script:Config currently leaves SHA256 for CDRElite, CDRPatch, AEUSBDriver,
IOSS, and MSXML4 as $null; replace each $null with the verified SHA256 hex
string for the corresponding installer (keys: CDRElite, CDRPatch, AEUSBDriver,
IOSS, MSXML4) obtained by downloading the installer on a trusted machine and
running Get-FileHash -Algorithm SHA256, then paste the resulting hash string
into the SHA256 field for that entry; keep the filenames/URLs unchanged and
ensure the runtime prerequisite check that validates these hashes continues to
run against these populated values.
Comment on lines
+993
to
+1000
| # Set recovery options: restart on first, second, and subsequent failures | ||
| $null = & sc.exe failure $serviceName reset= 86400 actions= restart/60000/restart/60000/restart/60000 2>&1 | ||
| if ($LASTEXITCODE -eq 0) { | ||
| Write-Log " Set recovery options: Restart on failure (60s delay)" -Level Info | ||
| } | ||
| else { | ||
| Write-Log " Failed to set recovery options (exit code: $LASTEXITCODE)" -Level Warning | ||
| } |
Contributor
There was a problem hiding this comment.
sc.exe failure non-zero exit doesn't set $configSuccess = $false — inconsistent with sibling calls.
Lines 989–990 (config start=) and 1008–1009 (config obj=) correctly set $configSuccess = $false on failure, but this block only logs a Warning and continues. While recovery options are arguably less critical, this means the function can return $true even though a configuration command failed, and the caller at line 1466 won't see the issue.
Proposed fix
else {
Write-Log " Failed to set recovery options (exit code: $LASTEXITCODE)" -Level Warning
+ $configSuccess = $false
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app-eaglesoft/Deploy-SchickSensor` around lines 993 - 1000, The sc.exe
failure call block doesn't set $configSuccess = $false on non-zero exit like the
other config blocks, so update the failure branch in the sc.exe
failure/$serviceName section to set $configSuccess = $false when $LASTEXITCODE
-ne 0; keep the existing Write-Log Warning (or enhance message) and mirror the
pattern used in the other blocks that check $LASTEXITCODE and set $configSuccess
accordingly (refer to the $configSuccess variable, sc.exe failure call,
Write-Log, $LASTEXITCODE, and $serviceName).
Completes installer integrity verification by populating SHA256 hashes from user-verified downloads against the Backblaze B2 bucket and Microsoft Download Center. SHA256 hashes populated: - CDRElite: C4CA8729EFBAAD03EFE58C52398B0EC8FF5A6BD50F999672D5F700C999F9DD31 - CDRPatch: 1E2F4B768AFD485F730F3F74D98996B80C761A2D0F676097573B15AB414E4148 - AEUSBDriver: C74779873D125EF6175D00AD52FA07277535F89D6415CC7207D76AA120D545A5 - IOSS: 9A3C29D60EF7BF8076E6BF94E5B4D8659A5C730F5B87D61C84A55BF9E293249A - MSXML4: 52D80E6E7BA94437199A2E6B51F00C7F483269C02BEDEC2253C4602F5209B31B MSXML4 download URL fix: - Previous URL (/download/1/E/E/1EE06E22-A56F-4E76-B6F6-E7670B4F8163/...) now returns HTTP 404 from Microsoft servers - Updated to current working URL from Microsoft Download Center: /download/a/7/6/a7611ffc-4f68-4fb1-a931-95882ec013fc/msxml4-KB2758694-enu.exe With all hashes populated, the prerequisite SHA256 check now passes cleanly without requiring -DownloadOnly bypass, and all downloaded installers will be cryptographically verified before execution.
Gumbees
requested changes
Mar 23, 2026
Contributor
There was a problem hiding this comment.
5:39 PM ... Schick Sensor Deploy Script PR Review
PR review for Deploy-SchickSensor.ps1 in the msp-script-library. Script was written without referencing the CLAUDE.md or any existing scripts in the repo. Gave up doing line-by-line review after 3 comments because the whole thing needs a rewrite.
Review (copy/paste for PR)
i stopped reviewing at line 104 because this doesnt follow our model at all. every script in this repo follows the same three-part structure and this one ignores all of it. all of this is documented in the CLAUDE.md at the root of the repo.
whats wrong:
- no RMM variable declaration block at the top. every script has this. its how techs know what to set in ninja.
- no
$RMMcheck, no$ValidInputloop, noRead-Hostprompts. this script has zero dual-mode support. it only runs one way. - no
Start-Transcript/Stop-Transcript. instead theres a customWrite-Logfunction and a JSON export system. thats not how we log things here. Start-Transcript. thats it. - hardcoded URLs buried in a config hashtable. URLs are never static in a script. they go in parameters, RMM variables, or user input. i already commented this on line 72.
- hardcoded installation paths.
C:\Program Files (x86)\Schick Technologies\...should be built from environment variables, not string literals. Invoke-WebRequestfor downloads. we useStart-BitsTransfer. i commented this on line 508.- mixed case URLs. lowercase only. always.
- PascalCase filename. our convention is lowercase-with-hyphens.
$Script:Configmonolithic hashtable pattern. no other script in this repo does this. variables go flat at the top in the RMM block.- 1600 lines with an ASCII art banner, custom logging framework, JSON results export, USB device lifecycle management... this is a driver install script not an application.
actual bugs:
- regex on lines ~770-785 has mismatched parentheses.
'^"([^"]+)"\s*(.*))is missing a closing quote. this wont run. Set-IOSSServiceConfigurationdoesnt set$configSuccess = $falseon thesc.exe failureblock but does on the other twosc.execalls. so the function can return true when a config command failed.Test-InstallerHashsilently returns$truewhen the expected hash is null. so hash verification is effectively disabled and nobody would know.
what needs to happen:
read the CLAUDE.md. rewrite this using the template. look at app-duo/duo-agent-install.ps1 for the pattern. RMM vars at top, interactive/RMM input handling, Start-Transcript wrapping all logic. move every URL and path into parameters. replace Invoke-WebRequest with Start-BitsTransfer. replace Write-Log with transcript. if the IOSS and Legacy paths are different enough, consider splitting into two scripts.
the detection logic and installation sequence are fine. the problem is the script was written from scratch without referencing anything else in this repo.
| # (Get-FileHash -Path ".\filename.exe" -Algorithm SHA256).Hash | ||
| Downloads = @{ | ||
| CDRElite = @{ | ||
| Url = "https://s3.us-west-002.backblazeb2.com/public-dtc/repo/vendors/Patterson-Eaglesoft/CDRElite5_16/CDRElite/CDR%20Elite%20Setup.exe" |
Contributor
There was a problem hiding this comment.
URLS should never be static in a script. They should be configurable via launch parameters, environment variables, or user input variables.
Also rename everything to use lowercase. Never use mixed case in urls. it-just-needs-to-look-like-this.
| SharedFiles = "C:\Program Files (x86)\Schick Technologies\Shared Files" | ||
| SchickBase = "C:\Program Files (x86)\Schick Technologies" | ||
| TempDownload = "$env:TEMP\SchickInstall" | ||
| } |
Contributor
There was a problem hiding this comment.
Why are these static and not environmental?
| for ($attempt = 1; $attempt -le $maxRetries; $attempt++) { | ||
| try { | ||
| $ProgressPreference = 'SilentlyContinue' | ||
| Invoke-WebRequest -Uri $url -OutFile $destination -UseBasicParsing -TimeoutSec 300 |
Contributor
There was a problem hiding this comment.
Do not use Invoke-WebRequest. It is slow. Use BITS-Transfer
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated deployment script for Schick 33/Elite USB Interface Driver v5.16 with Eaglesoft version-aware installation paths (IOSS vs Legacy).
Installation Flows:
Key Features:
Download Sources:
Prerequisite Checks:
Summary by CodeRabbit
New Features
Improvements