Fix deadlocks, TCP stability, and ASCOM registration issues#69
Open
EricSchubert wants to merge 1 commit intoOpenAstroTech:masterfrom
Open
Fix deadlocks, TCP stability, and ASCOM registration issues#69EricSchubert wants to merge 1 commit intoOpenAstroTech:masterfrom
EricSchubert wants to merge 1 commit intoOpenAstroTech:masterfrom
Conversation
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
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.
Please see attached markdown files for all detailed changes and fixes in this PR.
BUILD_FIXES.md
CHANGES.md
Summary
This PR fixes 11 bugs identified while using the ASCOM driver with N.I.N.A. and OATControl
on a BTT SKR Pico v1.0 running firmware V1.13.9. The issues range from permanent UI/thread
deadlocks to silent communication failures and COM registration problems.
Full details for each fix (problem, change, and rationale) are documented in
CHANGES.mdand
BUILD_FIXES.md, which I'll attach as comments below.Runtime Bug Fixes (
OATCommunications,ASCOM.Driver)Fix 1 —
Slew()deadlock (OatmealTelescopeCommandHandlers.cs)doneEvent.Set()was missing from the:MS#callback —await doneEvent.WaitAsync()hungforever. Added
doneEvent.Set()inside the:MS#callback.Fix 2 —
SetLocation()deadlock (OatmealTelescopeCommandHandlers.cs)A premature
await doneEvent.WaitAsync()appeared before any commands were sent, making alllocation/time commands unreachable. Removed the spurious wait.
Fix 3 —
RefreshMountState()deadlock (OatmealTelescopeCommandHandlers.cs)doneEvent.Set()was insideif (status.Success)— a comm failure meant it was never calledand the method hung. Moved
Set()outside theifblock.Fix 4 — TCP timeout reported as success (
TcpCommunicationHandler.cs)A catch block substituted
"0#"as a fake response, whichCommandResponsetreated assuccess. Replaced with
string.Emptyand an explicitsucceededflag.Fix 5 —
DoubleFullResponseonly reads once over TCP (TcpCommunicationHandler.cs)DoubleFullResponsewas grouped with single-response types — only one read happened, leavingthe second
#-terminated reply in the buffer and corrupting the next command's response.Split into its own
caseblock with two reads, matchingSerialCommunicationHandler.Fix 6 — TCP stream leak on write failure (
TcpCommunicationHandler.cs)An early
returnon write failure bypassedstream.Close(), leaking the socket handle.Added
stream.Close()before the early return.Fix 7 — TCP reconnects on every command (
TcpCommunicationHandler.cs)stream.Close()after every command closed the underlying socket, so_client.Connectedwasalways
falseand every command triggered a full TCP handshake. Added a_streamfield tokeep the connection open; reconnects only when
_stream == null.Fix 8 —
SendMessage()silently returns empty when disconnected (SharedResources.cs)When
SharedSerial.Connectedwasfalse,SendMessage()returnedstring.Empty—indistinguishable from a legitimate empty response. Now throws
ASCOM.NotConnectedException.Fix 9 —
PollUntilZero()hangs forever on mount stall (Driver.cs)No timeout — a stalled mount or repeated
"255"error returns fromCommandString()causedFindHome(),Park(), and synchronous slews to block the driver thread permanently. AddedmaxAttempts = 120(2-minute cap); throwsTimeoutExceptionon expiry.Fix 10 —
LocalServer32registry path not quoted (LocalServer.cs)Application.ExecutablePathwas written without quotes. Paths with spaces causedCO_E_SERVER_EXEC_FAILUREat COM activation. Wrapped in double quotes.Fix 11 —
SetupDialog()invisible when called from N.I.N.A. (Driver.cs)COM-started processes are denied foreground privilege by Windows.
SetupDialogFormopened butwas silently hidden behind other windows. Added
SetForegroundWindow()on the main formhandle before
ShowDialog()to regain foreground status.Build / Project Fixes (
OpenAstroTracker.csproj)Build Fix 4 — Wrong NuGet HintPaths for ASCOM assemblies
HintPaths pointed to
packages\(inside the project dir) instead of..\packages\(theactual NuGet restore location). ASCOM DLLs were not CopyLocal'd to
bin/Debug/, causingReflectionTypeLoadExceptionat COM activation on a clean build.Build Fix 5 — Debug build targets wrong CPU architecture
Debug
PropertyGrouphad<PlatformTarget>AnyCPU</PlatformTarget>. On 64-bit Windows thiswrote the COM registration to the 64-bit registry hive; 32-bit ASCOM clients (N.I.N.A., SGP,
etc.) look in
Wow6432Nodeand couldn't find the driver. Changed tox86to match theRelease configuration and the official installer.
Test Environment