Repository navigation
Add PhysicalDeviceService for devicectl-backed physical device discovery - #196
Merged
Merged
Conversation
Expose reusable, devicectl-backed discovery of physical Apple devices at about the level of SimulatorService. - PhysicalDeviceService.ListAsync runs 'xcrun devicectl list devices' with a bounded timeout and cancellation. It returns a PhysicalDeviceDiscoveryResult whose Status distinguishes success (possibly empty), unsupported host, tool unavailable, execution failure, timeout and invalid output. - PhysicalDeviceInfo gains HardwareUdid/HasHardwareUdid (distinct from the CoreDevice Identifier), MarketingName, Reality, ConnectionState, Availability/IsReachable, IsSimulated, Kind and Connections. Existing members and DeviceCtlOutputParser.ParseDevices behave as before. - Entries for the same device over several transports are merged by hardware UDID. Simulators, which newer devicectl versions also list, are excluded. - DeviceCtlOutputParser.TryParseDevices is a strict parser that tells an empty list from invalid output. - ProcessUtils now kills the whole process tree on cancellation in the net10.0 build, so a cancelled or timed-out command leaves no orphans. - Hardware-free tests (fixtures, an injectable runner and a fake xcrun script) and README documentation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Reviewed by two other models and checked against how dotnet/macios consumes this library (its GetAvailableDevices task). - Add a PhysicalDeviceService (log, developerRoot) overload. dotnet/macios runs every Apple tool with DEVELOPER_DIR=SdkDevPath; the value is applied to the devicectl child process only, never to the current process. - Log failures at info level. The failure is already part of the result, and an ICustomLogger warning becomes an MSBuild build warning in macios. - Report an unaccepted Xcode license as ToolUnavailable instead of ExecutionFailed. - Report a timeout only when the timeout itself fired, so an unrelated OperationCanceledException from the runner is no longer misreported. - TryParseDevices rejects an entry without a text identifier, or with a non-text hardware udid, instead of returning a device nobody can address. - Stop JsonElement.TryGet* from throwing on a wrong value kind in the optional ecid and cpuType fields. A device with such a field is now kept; before, the lenient parser dropped it and every device after it, and the strict parser failed the whole document. This is the only lenient-path behavior change. - Correct the docs: only processes that are still running are killed on cancellation, and a connection reports its own pairing state. - Tests: kill leaked helper processes in TearDown and cover all of the above. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
dotnet/macios tests its devicectl handling by overriding a protected virtual ExecuteCtlAsync on the task and returning canned JSON. Give PhysicalDeviceService the same seam, so macios can adopt the service and keep that style of test. - Make DeviceCtlRunResult public, with FromJson, Completed, NotFound and UnsupportedHost factories. - Add protected virtual PhysicalDeviceService.ExecuteDeviceCtlAsync, the only place the service starts a process. The host check moves into its default implementation, so an override works on any host. It replaces the internal Func-based runner. - Add tests that run macios's three devicectl fixtures and the devicectl inputs of its EmptyJsons cases through the service, and map the result with a port of the task's mapping and sort. Add a sanitized legacy-iPad document modelled on a real device. - Document the pattern in the README. Word the docs on duplicate reports and on devices CoreDevice cannot manage after what was observed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- Spell the non-breaking spaces of the macios DEVICECTL_JSON_1 fixture as an escape, so the fixture and the Apple Watch description assertion match the originals exactly. A plain space had replaced them. - Sort like macios in the test-local mapping port: no explicit comparers. - DeviceCtlRunResult.NotFound rejects a null message, and the service reports a default message for a blank one, so a failed discovery always says what went wrong. - Document that a ExecuteDeviceCtlAsync override must return its task promptly: the timeout bounds the task, not a call that blocks the calling thread. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
rolfbjarne
approved these changes
Oct 20, 2026
Validate the service against a real iPhone (Xcode 27, devicectl JSON version 5, connected over USB, not paired). UnpairedIPhoneDocument is a sanitized copy of that layout with made-up identifiers. - The iPhone is listed with all its metadata, reachable (devicectl's "disconnected" tunnel state is shown as "available") while PairingState is still "unpaired". The macios mapping keeps it as ios-arm64 / Unpaired, and the lenient ParseDevices agrees with the service. - A third test feeds the same document without the deprecated properties, as `devicectl list devices --omit-deprecated-fields-in-json` writes it, because a document that has both would hide a fallback to the deprecated data. The CPU architecture then comes from the numeric type and unsigned subtype, the connection state from properties.connection.state, and HardwareModel (only in the deprecated hardwareProperties) is empty. - Document that HardwareModel can be empty, and that a reachable device is not necessarily paired. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Error handling needs correction, and the new public API and shared process-tree cancellation changes warrant final human review.
1 open issue
What changed in this PR
Adds reusable physical Apple-device discovery to Xamarin.MacDev, complementing SimulatorService and supporting downstream CLI integration.
Changes:
- Adds asynchronous discovery with cancellation, timeouts, Xcode selection and explicit outcomes.
- Extends device metadata, merges transports by hardware identity and excludes simulators.
- Adds documentation, hardware-free tests and dotnet/macios compatibility coverage.
| File | Description |
|---|---|
| Xamarin.MacDev/ProcessUtils.cs | Enables process-tree termination on modern .NET. |
| Xamarin.MacDev/PhysicalDeviceService.cs | Implements discovery and device merging. |
| Xamarin.MacDev/Models/PhysicalDeviceKind.cs | Defines device classifications. |
| Xamarin.MacDev/Models/PhysicalDeviceInfo.cs | Adds identity and connection metadata. |
| Xamarin.MacDev/Models/PhysicalDeviceDiscoveryStatus.cs | Defines discovery outcomes. |
| Xamarin.MacDev/Models/PhysicalDeviceDiscoveryResult.cs | Encapsulates devices and diagnostics. |
| Xamarin.MacDev/Models/PhysicalDeviceConnection.cs | Represents individual connections. |
| Xamarin.MacDev/Models/PhysicalDeviceAvailability.cs | Defines reachability states. |
| Xamarin.MacDev/DeviceCtlRunResult.cs | Exposes raw execution results. |
| Xamarin.MacDev/DeviceCtlOutputParser.cs | Adds strict parsing and metadata extraction. |
| Xamarin.MacDev/DeviceCtl.cs | Runs devicectl and manages temporary output. |
| tests/RecordingLogger.cs | Captures test diagnostics. |
| tests/ProcessUtilsTests.cs | Tests process-tree cancellation. |
| tests/ProcessTestHelper.cs | Supports process lifecycle tests. |
| tests/PhysicalDeviceServiceTests.cs | Tests discovery, filtering and merging. |
| tests/PhysicalDeviceServiceOverrideTests.cs | Tests the public override seam. |
| tests/PhysicalDeviceServiceFailureTests.cs | Covers failures, cancellation and timeouts. |
| tests/PhysicalDeviceInfoTests.cs | Tests public device models. |
| tests/MaciosParityTests.cs | Checks downstream compatibility scenarios. |
| tests/MaciosDeviceMapping.cs | Models downstream device mapping. |
| tests/MaciosDeviceCtlFixtures.cs | Supplies downstream JSON fixtures. |
| tests/FakeDeviceCtl.cs | Provides injectable test runners. |
| tests/DeviceCtlTests.cs | Exercises execution with fake xcrun scripts. |
| tests/DeviceCtlRunResultTests.cs | Tests execution-result factories. |
| tests/DeviceCtlOutputParserTests.cs | Extends parser regression coverage. |
| tests/DeviceCtlFixtures.cs | Supplies synthetic and hardware-derived fixtures. |
| README.md | Documents discovery usage and limitations. |
Review effort: Balanced
PhysicalDeviceService accepted a document that had an error object next to
a parseable result.devices array as Success, because the error was only
looked at when parsing the device list failed. With exit code 0 that
reported {"error":{"code":1000},"result":{"devices":[]}} as an empty list,
and a non-empty array would have been exposed as the devices of a failed
operation.
CreateResult now checks for the error object before it accepts the list and
returns ExecutionFailed with devicectl's message. Real devicectl writes
either info + result (success) or error + info (failure), so a successful
run is not affected.
TryParseDevices still validates structure only; its remarks and the README
now say that the exit code and the error object are checked by the service.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
rolfbjarne
approved these changes
Oct 21, 2026
This was referenced Oct 21, 2026
rmarinho
added a commit
to dotnet/maui-labs
that referenced
this pull request
Oct 22, 2026
Bump Xamarin.Apple.Tools.MaciOS for PhysicalDeviceService (dotnet/macios-devtools#196) and report paired, reachable iPhones and iPads from `xcrun devicectl` next to the simulators, as Android already does for physical devices. The device id is the hardware UDID, which is stable across connections, and devicectl runs only for queries that reach the Apple provider. Startup profiling stays simulator-only, so the profile target resolver skips physical devices instead of picking one by accident. Fixes #722 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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.

Fixes #195
Summary
Adds
PhysicalDeviceService, a public, reusable way to discover physical Apple devices throughxcrun devicectl list devices, at about the level ofSimulatorService. The downstream consumer is dotnet/maui-labs#722.How the issue's requirements are met
ListAsync (CancellationToken)andListAsync (TimeSpan, CancellationToken)(default 30 s). Timeout or cancellation terminates devicectl; thenet10.0build also terminates the processes it started that are still running (see Compatibility).PhysicalDeviceDiscoveryResult.StatusisSuccess(possibly empty),UnsupportedHost,ToolUnavailable,ExecutionFailed,TimedOutorInvalidOutput, withErrorMessageandExitCode. A devicectlerrorobject is alwaysExecutionFailed, even if the process exited with code 0 and the document also has a device list. Only invalid arguments and caller cancellation throw. Diagnostics go toICustomLogger, never stdout. A failed discovery is logged at info level, because it is already in the result and anICustomLoggerwarning becomes a build warning when the logger is an MSBuild task, as in dotnet/macios.new PhysicalDeviceService (logger, developerRoot)(for exampleAppleSdkSettings.DeveloperRoot, or MSBuild'sSdkDevPath) setsDEVELOPER_DIRfor the devicectl process only. The current process's environment is untouched, so services for different Xcode installations can run concurrently.HardwareUdid/HasHardwareUdid.Udidis unchanged and still falls back toIdentifier.Connectionslists each transport, best first. Entries without a hardware UDID are never merged.TransportType,PairingStateand the rawConnectionStateare preserved.Availability(Unknown/Unavailable/Available/Connected) andIsReachableare derived only from the connection (tunnel) state, so pairing is never treated as connected, and a missing or unrecognized state staysUnknown. Remembered, unreachable devices are included.Kind(iPhone, iPad, iPod, Apple Watch, Apple TV, Vision, Mac) is derived from DeviceClass, then ProductType, then Platform, and never from the name.MarketingName, OS version and build, hardware model and CPU architecture are preserved.protected virtualseam (see dotnet/macios below), JSON fixtures and a fakexcrunscript cover the matrix in the issue: empty, single and multiple devices, repeated enumeration, multiple transports, identity and metadata, connection states, new and legacy schemas, malformed output, command failure, missing tooling, timeout, cancellation, and supported and unsupported hosts.Compatibility
PhysicalDeviceInfogains members; existing members andToStringare unchanged.DeviceCtlOutputParser.ParseDevicesstays lenient and returns what it did before, including simulators, and the existing parser tests pass unmodified. Both parsers keep accepting trailing commas and comments. A new strictTryParseDevicestells an empty list from invalid output. It also rejects an entry without a textidentifier, or with a non-text hardwareudid, rather than returning a device nobody can address. The test seam adds two public members to maintain:DeviceCtlRunResultandPhysicalDeviceService.ExecuteDeviceCtlAsync.JsonElement.TryGet*throws on a value of another JSON kind, so a stringecid(or a non-numbercpuType) madeParseDevicesdrop that device and every device after it. Such optional fields are now skipped and the device is kept. I compared the existing members of whatParseDevicesreturns before and after this PR, on dotnet/macios's devicectl fixtures, real devicectl output and 24 edge cases (empty, malformed, partial). That case is the only difference.ProcessUtils.KillProcessnow usesKill (entireProcessTree: true)in thenet10.0build, so it applies to everyProcessUtilscaller. Without it, a grandchild holding the stdout pipe keeps a cancelled call hanging until the grandchild exits.netstandard2.0is unchanged.CreateProcessStartInfobecameinternalso tests can inspect the command line.entireProcessTreeonly reaches descendants while their parent is still running. A wrapper script that backgrounds a child and exits can leave that child behind, and the call then returnsTimedOutafter the timeout plus a short grace period instead of waiting for it. Realxcrunreplaces itself with devicectl, so there is no intermediate process.Findings worth knowing
devicectl list devices(JSON v5) also lists simulators (reality: simulated,visibilityClass: simulators). The service excludes them;ParseDevicesdeliberately does not, to keep its behavior.properties.connection.state(legacy:connectionProperties.tunnelState).properties.stateis an unrelated object, and the parser does not confuse the two.iPad6,3) attached to the Mac is listed as a sparse entry: no name, hardware UDID, OS version or CPU architecture, apairingStateofunsupportedand a tunnel state ofunavailable. The service reports it withAvailability.Unavailable, andUdidfalls back to the CoreDevice identifier.iPhone12,1, iOS 27.2) over USB is listed in full, and it shows two more v5 details.properties.hardware.cpuTypehas only numerictypeandsubtypeand noname, andsubtypeis written unsigned (18446744071562067970 for arm64e, where the deprecatedsubTypeis the signed -2147483646), so the parser decodes the CPU architecture from the numbers.hardwareModelis only in the deprecatedhardwareProperties, so it is read from there. devicectl's own_deprecationNoticesayshardwareProperties,devicePropertiesandconnectionProperties"will be removed in a future release", anddevicectl --omit-deprecated-fields-in-jsonalready writes the document without them (on the device it removes exactly those three and the notice, and adds nothing). The service does not pass the flag, but it handles that output: the same device, except thatHardwareModelis empty (documented on the property and in the README) and the CPU architecture still comes from the numbers.pairingStateunpairedand connection statedisconnected, which devicectl's own table prints asavailable. The service reportsAvailability.AvailableandIsReachable, withPairingStateunpaired, so a caller that deploys should checkPairingStateas well. Paired with the Mac (checked on aniPhone12,1, iOS 27.2, over USB), the same layout reportspairingStatepairedand connection stateconnected, and the service reportsAvailability.Connected. The paired iPhone also writesserialNumber,marketingNameandrealitytoproperties.hardware, which the unpaired one did not.xcrun devicectlexits 72 withunable to find utility "devicectl". AdeveloperRootthat does not exist fails withxcrun: error: missing DEVELOPER_DIR path. Both map toToolUnavailable, and so does xcrun refusing to run until the Xcode license is accepted.dotnet/macios
dotnet/macios consumes this repository as a submodule. Its
GetAvailableDevicesMSBuild task runs devicectl itself and, for devicectl, uses onlyDeviceCtlOutputParser.ParseDevicesfrom this library. That parser's output is unchanged on the task's devicectl fixtures, and those fixtures also parse under the strict parser.XamarinTask.ExecuteAsyncsetsDEVELOPER_DIRfromSdkDevPathfor each process it runs, which is what thedeveloperRootoverload is for.Same test seam. The task's tests subclass it and override
protected virtual ExecuteCtlAsyncto return canned JSON.PhysicalDeviceServicehas the equivalent:protected virtual Task<DeviceCtlRunResult> ExecuteDeviceCtlAsync (CancellationToken), the only place the service starts a process.DeviceCtlRunResultis now public, withFromJson (json),Completed (...),NotFound (...)andUnsupportedHost ()factories. A test subclass returnsDeviceCtlRunResult.FromJson (json)and the service interprets it exactly as it does real output. The README documents the pattern.Same tests.
tests/MaciosParityTests.csruns the task's devicectl-only tests through the service, the way the task's tests do (a subclass returning canned JSON):DeviceCtl1,DeviceCtl2_Mac,DeviceCtlJsonVersion5and the devicectl inputs ofEmptyJsons. It mapsDeviceswith a test-local port of the task's mapping and sort (tests/MaciosDeviceMapping.cs) and asserts the task's own expectations: three devices plus the discarded Apple Watch, the discarded Mac, the JSON v5 iPhone on iOS 27.0, and no devices for the degenerate inputs. The fixtures (tests/MaciosDeviceCtlFixtures.cs) are identical to the task's, compared character by character withmainat159f7aa0e, except that the non-breaking spaces in one Apple Watch name are written as an escape, because an editor can silently turn them into plain spaces. The tests that involve simctl (SimCtl*,BootedSimulatorsFirst,DeviceCtlDuplicateSimulatorandCtl1*) cover the task's own simulator handling, merging, filtering and sorting, so they are not repeated. The task'sDeviceCtlJsonVersion5test has no[Test]attribute, so it never runs there (checked onmainat159f7aa0e); the parity test runs it.Adopting it is not part of this PR. The shape would be a
protected virtual PhysicalDeviceService CreatePhysicalDeviceService ()in the task that defaults tonew PhysicalDeviceService (this, SdkDevPath)(the task is anICustomLogger),RunDeviceCtlAsynccallingListAsyncand mappingdiscovery.Devicesas it does today, and the test wrapper overriding the factory with a cannedPhysicalDeviceServicesubclass.Differences the task would see.
"",{},[],{"result":{}}) isInvalidOutputrather than an empty list.Devicesis empty then, so mappingDevicesgives the same result as today.ParseDevicesitself still returns an empty list.Connections. The task does not de-duplicate, and its sort comment assumes UDIDs are unique. I have not seen devicectl do this on real hardware.Unknown CPU architecture ''). The service lists it, withAvailability.Unavailable. It keeps the unpaired iPhone described above (ios-arm64, statusUnpaired), and there the lenientParseDevicesthe task calls today and the service give the same device. It also keeps the paired iPhone (ios-arm64, statusPaired).Testing
dotnet test tests/tests.csproj -c Release -f net10.0: 506 passed, 1 skipped (pre-existing), 0 failed. The baseline was 217 passed.dotnet format whitespace Xamarin.MacDev.sln --verify-no-changesis clean, and the library builds fornetstandard2.0andnet10.0with warnings as errors. The tests also compile fornet472, the Windows CI target.[Platform (Exclude = "Win")].xcrun(Xcode 27) and a legacy iPad attached,ListAsyncreturnsSuccesswith that one device (the simulators devicectl also lists are ignored), and the suite's real-xcruntest passes against it.xcrun(Xcode 27) and a CoreDevice-capable iPhone attached (iOS 27.2, USB, not paired),ListAsyncreturnsSuccesswith that device in about 0.3 s and every field filled in as described above. The simulators are ignored, the lenientParseDevicesand the strictTryParseDevicesaccept the output, andParseDevicesreturns the same device as the service.UnpairedIPhoneDocumentin the tests is a copy of that layout with made-up identifiers, run through the service and through the port of the task's mapping. The same iPhone listed with--omit-deprecated-fields-in-jsonalso gives the same device through the service (the same status, version, CPU architecture andios-arm64mapping), except for the emptyHardwareModel. A third test runs the fixture without the deprecated blocks, because a document that has both would hide a fallback to the deprecated data. Breaking the read ofproperties.connection.state, or the numeric CPU-type decoding, fails only that test.xcrun(Xcode 27) and an iPhone paired with the Mac (iPhone12,1, iOS 27.2, USB;pairingStatepaired, connection stateconnected),ListAsyncreturnsSuccessin about 0.3 s withAvailability.ConnectedandIsReachable, and the serial number, marketing name and reality are filled in. Both parsers accept the output. The same document listed with--omit-deprecated-fields-in-jsongives the same device through the service, except for the emptyHardwareModel, and the port of the task's mapping keeps it either way (ios-arm64, statusPaired). The tests'Devicebuilder writes these fields in the same places (properties.connectionandproperties.hardware). The service leaves no temporary file behind after a run.xcrun(Xcode 27, no physical devices) andDEVELOPER_DIRset to the Command Line Tools in the environment: the default service reportsToolUnavailable;developerRootset to the full Xcode reportsSuccesswith no devices; a nonexistentdeveloperRootreportsToolUnavailable; and the calling process'sDEVELOPER_DIRis unchanged afterwards.Review
Reviewed by two other models (Gemini 4 Argon and GPT-6.1 Sol), in four rounds.
developerRootoverload, info-level failure logging, the license mapping, the stricter timeout check and the stricterTryParseDevices. Both then re-reviewed those fixes and found nothing further. The one major finding I did not act on is the descendant-process limit above: it comes from sharedProcessUtilsbehavior, and the call stays bounded by the timeout plus the grace period.DeviceCtlRunResult.NotFoundcould produce a failure without a message, and that an override which blocks the calling thread is not bounded by the timeout (now documented). I did not add a guard for an override that returns a null task or result, because it already fails loudly and takes a null-forgiving!to happen in nullable-aware code, nor aTask.Runaround the override, because the contract is now documented. Both re-reviewed those fixes and found nothing further.WithoutDeprecatedPropertieshelper and the third test. Its wording notes (the README sentence on reachability, one assertion message and theHardwareModeldocs) were applied.errorobject next to a parseableresult.devicesarray (for example{"error":{"code":1000},"result":{"devices":[]}}) was reported asSuccess, because the error was only looked at when parsing the list failed. That was valid.CreateResultnow checks for the error object before it accepts the list. Real devicectl (Xcode 27) writesinfoandresulton success, anderrorandinfo(noresult, exit code 1) when a command fails, so a successful run is not affected. Five test cases cover it: an error next to an empty list, next to two devices, and the smallest documents (noinfo, no description). All five fail on the previous code. Both models reviewed the fix and found no blocking problems. I did not act on their one suggestion to makeTryParseDevicesreject such a document too: only the service calls it, so its remarks and the README now say that it validates the structure and that the service checks the exit code and theerrorobject.Not verified on real hardware
Behavior was validated with synthetic fixtures modelled on devicectl's v5 and legacy layouts, a fake
xcrunscript, and runs of the realxcrun devicectlagainst physical devices: a legacy iPad that CoreDevice reports asunsupportedandunavailable, a CoreDevice-capable iPhone that isunpairedanddisconnected(which devicectl prints asavailable), and an iPhone that ispairedandconnectedover USB. That confirms the reading ofdisconnectedas "reachable, not connected" (Availability.Available) and ofconnectedasAvailability.Connectedon real hardware. Three points still need a check:wired). Connections overlocalNetworkare still known only from sample data (a paired Apple Watch overlocalNetworkreportsdisconnected). The raw values are always preserved, so callers can apply their own interpretation.unsupportedentry without an OS version. Which OS versions devicectl lists fully is still unconfirmed, so the docs say such devices "may be missing from the list or listed with little information".sudo xcodebuild -license. That is matched on stderr and reported asToolUnavailable. The license is already accepted on the machine used here, so the exact message and exit code (69 in the tests) were not reproduced.