Async mount control #2

sebgod · 2026-01-09T06:25:55Z

This pull request introduces a new device management subcommand to the CLI, refactors device and profile listing logic for greater flexibility, and improves test reliability and API consistency. The most significant changes are the addition of the device CLI subcommand, the generalization of device listing methods, and updates to tests and supporting infrastructure.

CLI Feature Additions

Added a new DeviceSubCommand class that introduces the device CLI command with discover and list subcommands, allowing users to discover and list all connected devices except profiles. (src/TianWen.Lib.CLI/DeviceSubCommand.cs)
Registered the new device subcommand in the CLI program entry point to make it available to users. (src/TianWen.Lib.CLI/Program.cs)

Device and Profile Listing Refactor

Refactored IConsoleHost and its implementation to generalize device listing: replaced ListProfilesAsync with ListDevicesAsync<TDevice> and ListAllDevicesAsync, allowing listing of arbitrary device types and all devices. (src/TianWen.Lib.CLI/IConsoleHost.cs, src/TianWen.Lib.CLI/ConsoleHost.cs) [1] [2]
Updated profile subcommand logic to use the new generic device listing methods, centralizing and simplifying profile retrieval. (src/TianWen.Lib.CLI/ProfileSubCommand.cs) [1] [2] [3]

Test Infrastructure and Reliability

Updated tests to use cancellation tokens for async operations and replaced conditional skipping with Assert.SkipUnless for improved reliability and clearer intent. (src/TianWen.Lib.Tests/AscomDeviceTests.cs, src/TianWen.Lib.Tests/FindBestFocusTests.cs) [1] [2] [3] [4] [5] [6] [7]
Updated test logging infrastructure to use the correct logger provider version and added a static logger creation method for consistency. (src/TianWen.Lib.Tests/FakeExternal.cs) [1] [2] [3]

Miscellaneous Improvements

Updated default launch settings to use the new device list command for easier testing and development. (src/TianWen.Lib.CLI/Properties/launchSettings.json)
Minor cleanup: removed unused usings and improved code clarity in several files. (src/TianWen.Lib.CLI/IConsoleHost.cs, src/TianWen.Lib.Tests/AscomDeviceTests.cs, src/TianWen.Lib.Tests/FindBestFocusTests.cs) [1] [2] [3]

These changes collectively enhance the CLI's device management capabilities, improve code maintainability, and strengthen test coverage and reliability.

Copilot

Pull request overview

This pull request introduces comprehensive asynchronous control for mount operations, refactoring the device/profile listing logic for improved flexibility and test reliability. The changes include:

Conversion of synchronous mount driver operations to async patterns with proper cancellation token support
Introduction of a new device CLI subcommand for device discovery and listing
Refactoring serial connection interface to support async I/O operations
Addition of memory pooling utilities for improved performance
Test infrastructure improvements including xunit v3 migration and cancellation token usage

Reviewed changes

Copilot reviewed 38 out of 39 changed files in this pull request and generated 22 comments.

Show a summary per file

File	Description
StringHelper.cs	New utility for replacing non-printable characters with hex representation for better debugging
ArrayPoolHelper.cs	New helper class providing array pool rental with disposable wrapper for memory efficiency
SpanHelper.cs	Removed (functionality likely moved or no longer needed)
IMountDriver.cs	Converted all mount operation properties to async methods with cancellation token support
MeadeLX200ProtocolMountDriverBase.cs	Complete async refactoring of LX200 protocol implementation with proper locking and memory pooling
AscomTelescopeDriver.cs	Updated ASCOM mount driver to implement new async interface
Session.cs	Updated sequencing logic to use async mount operations throughout
ISerialConnection.cs	Refactored to async I/O with proper cancellation support
SerialConnection.cs	Implemented async serial I/O inheriting from new base class
SerialConnectionBase.cs	New abstract base class for serial connections with shared async I/O logic
FakeMeadeLX200SerialDevice.cs	Updated test fake to support async operations and improved concurrency handling
MeadeDeviceSource.cs	Implemented async device discovery with caching
FakeDeviceSource.cs	Adjusted fake device counts for testing
DeviceSubCommand.cs	New CLI command for device discovery and listing
ProfileSubCommand.cs	Refactored to use generic device listing methods
IConsoleHost.cs/ConsoleHost.cs	Generalized device listing with type-safe generic methods
Test files	Updated to use cancellation tokens and xunit v3 patterns

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-09T06:31:58Z

src/TianWen.Lib/Devices/MeadeLX200ProtocolMountDriverBase.cs

+    private async ValueTask SendAsync(ISerialConnection port, ReadOnlyMemory<byte> command, CancellationToken cancellationToken)
    {
-        if (_deviceInfo.SerialDevice is { } port && port.TryReadExactly(count, out response))
+        if (!Connected)
        {
-            return true;
+            throw new InvalidOperationException("Mount is not connected");
        }

-        response = default;
-        return false;
-    }
+        if (!CanUnpark && !CanSetPark && await AtParkAsync(cancellationToken))
+        {
+            throw new InvalidOperationException("Mount is parked, but it is not possible to unpark it");
+        }


The method acquires a port lock but never releases it in the error path when the AtParkAsync check fails. The semaphore will remain locked indefinitely, causing deadlock on subsequent operations. The Release() should be moved to a finally block to ensure it's always called.

Copilot · 2026-01-09T06:31:58Z

src/TianWen.Lib/Devices/MeadeLX200ProtocolMountDriverBase.cs

                }
            }
+            finally
+            { 


There's a trailing whitespace after the opening brace on line 868 that should be removed for consistency with the rest of the codebase.

Copilot · 2026-01-09T06:31:58Z

src/TianWen.Lib/Devices/IMountDriver.cs

    public async ValueTask<bool> WaitForSlewCompleteAsync(CancellationToken cancellationToken)
    {
-        var period = TimeSpan.FromMilliseconds(250);
+        var period = TimeSpan.FromMilliseconds(251);


The WaitForSlewCompleteAsync method has a hardcoded period of 251 milliseconds which appears to be an arbitrary change from the original 250. This should either be 250 milliseconds for consistency, or if the change to 251 was intentional, it should be documented why this specific value is needed.

Copilot · 2026-01-09T06:31:59Z

src/TianWen.Lib/Connections/ISerialConnection.cs

-    bool TryReadExactly(int count, [NotNullWhen(true)] out ReadOnlySpan<byte> message);
+    ValueTask<bool> TryWriteAsync(ReadOnlyMemory<byte> data, CancellationToken cancellationToken);
+
+    ValueTask<bool> TryWriteAsync(string data, CancellationToken cancellationToken) => TryWriteAsync(Encoding.GetBytes(data), cancellationToken);


The method signature parameter name is missing. The parameter should have a name like 'data' to match the method body usage and the interface definition.

Copilot · 2026-01-09T06:31:59Z

src/TianWen.Lib/Connections/SerialConnection.cs

-    /// Encoding used for decoding byte messages (used for display/logging only)
-    /// </summary>
-    public Encoding Encoding { get; }
+    public override string DisplayName => throw new NotImplementedException();


The DisplayName property is marked as 'throw new NotImplementedException()' which will cause runtime failures when this property is accessed. This needs to return the actual port name.

Suggested change

public override string DisplayName => throw new NotImplementedException();

public override string DisplayName => _port.PortName;

Copilot · 2026-01-09T06:32:03Z

src/TianWen.Lib/Devices/Fake/FakeMeadeLX200SerialDevice.cs

+    private volatile bool _isTracking = false;
+    private volatile bool _isSlewing = false;
+    private volatile bool _highPrecision = false;
+    private volatile int _trackingFrequency = 601; // TODO simulate tracking and tracking rate


Field '_trackingFrequency' can be 'readonly'.

Copilot · 2026-01-09T06:32:03Z

src/TianWen.Lib/Connections/SerialConnectionBase.cs

+            if (await TryReadExactlyRawAsync(buffer.AsMemory(0, count), cancellationToken))
+            {
+                return Encoding.GetString(buffer.AsSpan(0, count));
+            }
+            else
+            {
+                return null;
+            }


Both branches of this 'if' statement return - consider using '?' to express intent better.

Suggested change

if (await TryReadExactlyRawAsync(buffer.AsMemory(0, count), cancellationToken))

{

return Encoding.GetString(buffer.AsSpan(0, count));

}

else

{

return null;

}

return await TryReadExactlyRawAsync(buffer.AsMemory(0, count), cancellationToken)

? Encoding.GetString(buffer.AsSpan(0, count))

: null;

Copilot · 2026-01-09T06:32:04Z

src/TianWen.Lib/Devices/IMountDriver.cs

+        if (Connected)
+        {
+            return await DestinationSideOfPierAsync(await GetRightAscensionAsync(cancellationToken), await GetDeclinationAsync(cancellationToken), cancellationToken);
+        }
+        else
+        {
+            return PointingState.Unknown;
+        }


Both branches of this 'if' statement return - consider using '?' to express intent better.

Copilot · 2026-01-09T06:32:04Z

src/TianWen.Lib/Devices/IMountDriver.cs

+        if (Connected)
+        {
+            return ConditionHA(await GetSiderealTimeAsync(cancellationToken) - await GetRightAscensionAsync(cancellationToken));
+        }
+        else
+        {
+            return double.NaN;
+        }


Both branches of this 'if' statement return - consider using '?' to express intent better.

Copilot · 2026-01-09T06:32:04Z

src/TianWen.Lib/Devices/IMountDriver.cs

+        if (!double.IsNaN(raMount) && !double.IsNaN(decMount) && !double.IsNaN(az) && !double.IsNaN(alt))
+        {
+            return (raMount, decMount, az, alt);
+        }
+        else
+        {
+            return null;
+        }


Both branches of this 'if' statement return - consider using '?' to express intent better.

sebgod added 3 commits January 9, 2026 07:54

Make IMountDriver fully ascync, update to xunit3

9df448d

Borken

206404c

Unborken

d41a199

sebgod requested a review from Copilot January 9, 2026 06:25

Copilot started reviewing on behalf of sebgod January 9, 2026 06:26 View session

Copilot AI reviewed Jan 9, 2026

View reviewed changes

DO not push on pr

82d022d

sebgod merged commit 047f558 into main Jan 9, 2026
2 checks passed

sebgod deleted the async-mount branch January 9, 2026 06:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Async mount control #2

Async mount control #2

Uh oh!

sebgod commented Jan 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 9, 2026

Uh oh!

Copilot AI Jan 9, 2026

Uh oh!

Copilot AI Jan 9, 2026

Uh oh!

Copilot AI Jan 9, 2026

Uh oh!

Copilot AI Jan 9, 2026

Uh oh!

Copilot AI Jan 9, 2026

Uh oh!

Copilot AI Jan 9, 2026

Uh oh!

Copilot AI Jan 9, 2026

Uh oh!

Copilot AI Jan 9, 2026

Uh oh!

Copilot AI Jan 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	public override string DisplayName => throw new NotImplementedException();
	public override string DisplayName => _port.PortName;

Async mount control #2

Async mount control #2

Uh oh!

Conversation

sebgod commented Jan 9, 2026

CLI Feature Additions

Device and Profile Listing Refactor

Test Infrastructure and Reliability

Miscellaneous Improvements

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants