feat: supported connection types#17
Conversation
| let devices = manager.get_property::<Vec<OwnedObjectPath>>("Devices")?; | ||
| let mut device_types = Vec::with_capacity(devices.len()); | ||
|
|
||
| for device in devices { |
There was a problem hiding this comment.
The two ? operators in this loop mean a single per-device D-Bus failure aborts the entire NetworkManager query, discarding types already resolved for other adapters. device_details elsewhere in this file handles the same situation with warn! + continue.
Could we use a similar approach here?
There was a problem hiding this comment.
Updated the NetworkManager device loop to warn and skip individual devices that fail, preserving types resolved from the remaining adapters. Added a regression test covering a per-device failure.
| }; | ||
| use windows::Win32::Foundation::{ERROR_BUFFER_OVERFLOW, NO_ERROR}; | ||
| use windows::Win32::NetworkManagement::IpHelper::{ | ||
| GAA_FLAG_INCLUDE_ALL_INTERFACES, GAA_FLAG_SKIP_ANYCAST, GAA_FLAG_SKIP_DNS_SERVER, |
There was a problem hiding this comment.
With GAA_FLAG_INCLUDE_ALL_INTERFACES virtual adapters surface as "ethernet". So a Wi-Fi-only machine running WSL2 would return ["wifi","ethernet"], which diverges from Linux (which filters virtual interfaces) and from the README's "physical transport classes" contract.
Should we filter known-virtual adapters to match Linux?
There was a problem hiding this comment.
Good catch! Filtered adapters using MIB_IF_ROW2.HardwareInterface from GetIfEntry2() instead of name-based heuristics. This keeps inactive physical adapters while excluding interfaces Windows identifies as virtual. Added a regression test and updated the README mapping.
| * The result is deduplicated, excludes `'unknown'`, and represents transport | ||
| * classes the device can use rather than the currently selected connection. | ||
| */ | ||
| export async function supportedConnectionTypes(): Promise<ConnectionType[]> { |
There was a problem hiding this comment.
connectionStatus has an error-propagation test but supportedConnectionTypes doesn't — should we add this for parity? Also, an empty-array case would also cover the documented "none reported" path.
There was a problem hiding this comment.
Added parity coverage for supportedConnectionTypes(): backend errors propagate to callers, and no reported transports resolves to an empty array.
8e1c602 to
061e075
Compare
| // operational state, matching the supported-hardware contract. The skip | ||
| // flags avoid populating address lists we do not inspect. | ||
| // https://learn.microsoft.com/en-us/windows/win32/api/iphlpapi/nf-iphlpapi-getadaptersaddresses | ||
| GetAdaptersAddresses( |
There was a problem hiding this comment.
The ERROR_BUFFER_OVERFLOW path still retries exactly once; per the MS docs the resized call can overflow again if adapters change in between, which then becomes a hard DetectionFailed.
Would a small bounded loop (≤3 attempts) be more robust?
There was a problem hiding this comment.
Replaced the single resize retry with a bounded three-attempt loop, matching Microsoft's example. Each overflow uses the newly reported buffer size. Added tests covering third-attempt success and exhaustion after three overflows.
| } | ||
|
|
||
| @Test | ||
| fun `test that supported connection types are deduped and ordered`() { |
There was a problem hiding this comment.
This case sets all three feature flags to true, so activeTransportTypes doesn't affect the result. Could we add a case where a transport is absent from features but present in activeTransportTypes?
There was a problem hiding this comment.
Added a case where Wi-Fi is declared by system features and Ethernet is supplied only through activeTransportTypes, verifying active networks supplement feature detection while preserving stable ordering.
velocitysystems
left a comment
There was a problem hiding this comment.
Thanks @polw1. Just a couple of remaining minor observations.
| } | ||
| ``` | ||
|
|
||
| #### Query supported transport classes |
There was a problem hiding this comment.
Nit: The shared contract says transports are returned "even when that transport is not currently active." On Android, a transport not declared as a PackageManager system feature is reported only while that network is active.
Can we soften the wording here and in the JSDoc (index.ts) to note that on Android, transports not declared as system features are reported only while currently active?
There was a problem hiding this comment.
Nice catch. I softened the README wording and added the Android limitation to the JSDoc. Transports not declared as PackageManager system features are now documented as being reported only while currently active.
| /// | ||
| /// On platforms without an implementation, this returns [`Error::Unsupported`]. | ||
| #[command] | ||
| pub(crate) async fn supported_connection_types<R: Runtime>( |
There was a problem hiding this comment.
Currently the failure semantics vary between platforms. Windows returns Err(DetectionFailed); Linux returns Ok(vec![]); Android returns a list. A consumer can't uniformly distinguish "none supported" from "detection failed."
Can we align this between the platforms?
There was a problem hiding this comment.
Fixed. Linux now distinguishes successful empty detection from enumeration failure. Ok([]) means detection completed successfully with no supported transports; failed NetworkManager and sysfs enumeration returns DetectionFailed, consistent with the other implementations. I also documented these semantics in the Rust and TypeScript APIs.
Windows exposes Ethernet-like virtual adapters alongside physical network hardware. Reporting them as supported transports creates policy options the device cannot actually use, so adapter classification must use the hardware flag from GetIfEntry2. That hardware query is fallible. Treat ERROR_NO_DATA as a successful empty enumeration, preserve usable partial results, and return DetectionFailed when native errors prevent any supported adapter from being classified. This keeps Ok([]) reserved for successful detection.
Callers use an empty supported-transport list to mean that enumeration completed but no recognized physical transports were found. Linux previously produced the same value when NetworkManager device reads failed, leaving callers unable to distinguish an empty device from failed detection. Preserve usable partial results, but surface the first read error when no recognized transport can be recovered. Document the empty-result and Android active-transport limitations so the public contract matches platform behavior.
Error is imported only for desktop builds, so unqualified intra-doc links fail when Rustdoc documents Android or iOS targets. Use crate-qualified paths so documentation resolves independently of platform-gated imports.
0e89346 to
ad0e723
Compare
Adds a new
supportedConnectionTypes()API for querying the physical transport classes a device can use, separate from the current connectionStatus() API.Implemented support for:
macOS and iOS remain unsupported for this new API.
Details
Adds the new Tauri command supported_connection_types
Exposes supportedConnectionTypes() from the TypeScript package
Returns a deduplicated ConnectionType[]
Excludes unknown
Uses stable ordering: wifi, ethernet, cellular
Updates Tauri permissions for the new command
Adds platform-specific implementations:
Adds manual testing docs for Linux and Android
Updates the example app to display both current connection status and supported connection types