Skip to content

Commit 797964e

Browse files
rmarinhoCopilot
andcommitted
Address PR #282 review feedback: exit codes, type splitting, structured args
- AdbRunner: check exit codes consistently in ListDevicesAsync, WaitForDeviceAsync, StopEmulatorAsync - AdbRunner: bare catch {} → catch (Exception) in GetEmulatorAvdNameAsync - AdbDeviceInfo.cs: split 3 public types into separate files (AdbDeviceType.cs, AdbDeviceStatus.cs) - EnvironmentVariableNames: add AndroidAvdHome constant, use in AvdManagerRunner - EmulatorRunner.StartAvd: additionalArgs string? → IEnumerable<string>? for proper arg separation - EmulatorRunner.StartAvd: set RedirectStandardOutput/Error=false to prevent pipe deadlock Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 37c4ffe commit 797964e

7 files changed

Lines changed: 57 additions & 31 deletions

File tree

src/Xamarin.Android.Tools.AndroidSdk/EnvironmentVariableNames.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,11 @@ internal static class EnvironmentVariableNames
4646
/// (AVDs, preferences, etc.). Defaults to $HOME/.android.
4747
/// </summary>
4848
public const string AndroidUserHome = "ANDROID_USER_HOME";
49+
50+
/// <summary>
51+
/// Overrides the AVD storage directory.
52+
/// Defaults to $ANDROID_USER_HOME/avd or $HOME/.android/avd.
53+
/// </summary>
54+
public const string AndroidAvdHome = "ANDROID_AVD_HOME";
4955
}
5056
}

src/Xamarin.Android.Tools.AndroidSdk/Models/AdbDeviceInfo.cs

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,28 +3,6 @@
33

44
namespace Xamarin.Android.Tools;
55

6-
/// <summary>
7-
/// Represents the type of an Android device.
8-
/// </summary>
9-
public enum AdbDeviceType
10-
{
11-
Device,
12-
Emulator
13-
}
14-
15-
/// <summary>
16-
/// Represents the status of an Android device.
17-
/// </summary>
18-
public enum AdbDeviceStatus
19-
{
20-
Online,
21-
Offline,
22-
Unauthorized,
23-
NoPermissions,
24-
NotRunning,
25-
Unknown
26-
}
27-
286
/// <summary>
297
/// Represents an Android device or emulator from 'adb devices -l' output.
308
/// Mirrors the metadata produced by dotnet/android's GetAvailableAndroidDevices task.
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
namespace Xamarin.Android.Tools;
5+
6+
/// <summary>
7+
/// Represents the status of an Android device.
8+
/// </summary>
9+
public enum AdbDeviceStatus
10+
{
11+
Online,
12+
Offline,
13+
Unauthorized,
14+
NoPermissions,
15+
NotRunning,
16+
Unknown
17+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
namespace Xamarin.Android.Tools;
5+
6+
/// <summary>
7+
/// Represents the type of an Android device.
8+
/// </summary>
9+
public enum AdbDeviceType
10+
{
11+
Device,
12+
Emulator
13+
}

src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,11 @@ public async Task<IReadOnlyList<AdbDeviceInfo>> ListDevicesAsync (CancellationTo
6767
{
6868
var adb = RequireAdb ();
6969
using var stdout = new StringWriter ();
70+
using var stderr = new StringWriter ();
7071
var psi = CreateAdbProcess (adb, "devices", "-l");
71-
await ProcessUtils.StartProcess (psi, stdout, null, cancellationToken).ConfigureAwait (false);
72+
var exitCode = await ProcessUtils.StartProcess (psi, stdout, stderr, cancellationToken).ConfigureAwait (false);
73+
74+
ProcessUtils.ThrowIfFailed (exitCode, "adb devices -l", stderr.ToString ());
7275

7376
var devices = ParseAdbDevicesOutput (stdout.ToString ());
7477

@@ -103,8 +106,8 @@ public async Task<IReadOnlyList<AdbDeviceInfo>> ListDevicesAsync (CancellationTo
103106
}
104107
} catch (OperationCanceledException) {
105108
throw;
106-
} catch {
107-
// Silently ignore failures (emulator may not support this command)
109+
} catch (Exception) {
110+
// Expected: emulator may not support 'emu avd name' command
108111
}
109112

110113
return null;
@@ -125,7 +128,9 @@ public async Task WaitForDeviceAsync (string? serial = null, TimeSpan? timeout =
125128
cts.CancelAfter (effectiveTimeout);
126129

127130
try {
128-
await ProcessUtils.StartProcess (psi, null, null, cts.Token).ConfigureAwait (false);
131+
using var stderr = new StringWriter ();
132+
var exitCode = await ProcessUtils.StartProcess (psi, null, stderr, cts.Token).ConfigureAwait (false);
133+
ProcessUtils.ThrowIfFailed (exitCode, "adb wait-for-device", stderr.ToString ());
129134
} catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) {
130135
throw new TimeoutException ($"Timed out waiting for device after {effectiveTimeout.TotalSeconds}s.");
131136
}
@@ -137,8 +142,11 @@ public async Task StopEmulatorAsync (string serial, CancellationToken cancellati
137142
throw new ArgumentException ("Serial must not be empty.", nameof (serial));
138143

139144
var adb = RequireAdb ();
145+
using var stderr = new StringWriter ();
140146
var psi = CreateAdbProcess (adb, "-s", serial, "emu", "kill");
141-
await ProcessUtils.StartProcess (psi, null, null, cancellationToken).ConfigureAwait (false);
147+
var exitCode = await ProcessUtils.StartProcess (psi, null, stderr, cancellationToken).ConfigureAwait (false);
148+
149+
ProcessUtils.ThrowIfFailed (exitCode, $"adb -s {serial} emu kill", stderr.ToString ());
142150
}
143151

144152
/// <summary>

src/Xamarin.Android.Tools.AndroidSdk/Runners/AvdManagerRunner.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ internal static List<AvdInfo> ParseAvdListOutput (string output)
172172
static string GetAvdRootDirectory ()
173173
{
174174
// ANDROID_AVD_HOME takes highest priority
175-
var avdHome = Environment.GetEnvironmentVariable ("ANDROID_AVD_HOME");
175+
var avdHome = Environment.GetEnvironmentVariable (EnvironmentVariableNames.AndroidAvdHome);
176176
if (!string.IsNullOrEmpty (avdHome))
177177
return avdHome;
178178

src/Xamarin.Android.Tools.AndroidSdk/Runners/EmulatorRunner.cs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,20 +54,24 @@ void ConfigureEnvironment (ProcessStartInfo psi)
5454
AndroidEnvironmentHelper.ConfigureEnvironment (psi, getSdkPath (), getJdkPath?.Invoke ());
5555
}
5656

57-
public Process StartAvd (string avdName, bool coldBoot = false, string? additionalArgs = null)
57+
public Process StartAvd (string avdName, bool coldBoot = false, IEnumerable<string>? additionalArgs = null)
5858
{
5959
var emulatorPath = RequireEmulatorPath ();
6060

6161
var args = new List<string> { "-avd", avdName };
6262
if (coldBoot)
6363
args.Add ("-no-snapshot-load");
64-
if (!string.IsNullOrEmpty (additionalArgs))
65-
args.Add (additionalArgs);
64+
if (additionalArgs != null)
65+
args.AddRange (additionalArgs);
6666

6767
var psi = ProcessUtils.CreateProcessStartInfo (emulatorPath, args.ToArray ());
6868
ConfigureEnvironment (psi);
6969

7070
var process = new Process { StartInfo = psi };
71+
// Start directly instead of ProcessUtils.StartProcess because this is a
72+
// long-running background process — we don't want to await its completion.
73+
psi.RedirectStandardOutput = false;
74+
psi.RedirectStandardError = false;
7175
process.Start ();
7276

7377
return process;

0 commit comments

Comments
 (0)