feat: implement Windows support for connectivity detection (#3)#6
Conversation
|
|
||
| let profile = match NetworkInformation::GetInternetConnectionProfile() { | ||
| Ok(profile) => profile, | ||
| Err(error) if error.code().0 == 0 => { |
There was a problem hiding this comment.
Instead of checking for a magic number 0, we should be able to use is_ok() on the HRESULT returned by error.code().
Then our usage becomes:
Err(error) if error.code().is_ok() => { ... } There was a problem hiding this comment.
Thanks, I updated this. I checked our current windows crate version and error.code().is_ok() is supported there, and it handles the Error::empty() / no-profile case we get from GetInternetConnectionProfile(). I replaced the 0 check and added tests to cover both the missing-profile case and a real failure HRESULT.
| @@ -0,0 +1,3 @@ | |||
| node_modules | |||
| dist | |||
| src-tauri/target | |||
There was a problem hiding this comment.
Can we please ignore src-tauri/gen and remove this folder from source control? This is ignored in our other plugin repositories.
There was a problem hiding this comment.
Updated the example app ignore rules to include src-tauri/gen, and removed that generated directory from source control so it won’t keep showing up in the repo.
| "build": "rollup -c", | ||
| "check-node-version": "check-node-version --npm 11.8.0", | ||
| "commitlint": "commitlint --from ${COMMITLINT_FROM:-710707e} --to ${COMMITLINT_TO:-HEAD}", | ||
| "commitlint": "commitlint --from 710707e", |
There was a problem hiding this comment.
Can we please undo this unrelated change?
There was a problem hiding this comment.
Yes, I’ll drop this from the PR. I changed it because the ${...:-...} expansion doesn’t work in PowerShell, but that’s separate from the scope here. I’ll keep the local workaround for now and we can handle a cross-shell commitlint fix in a follow-up PR.
There was a problem hiding this comment.
Interesting. We may need to revisit this elsewhere; Mac vs Windows I guess.
| tauri = { version = "2.9.3" } | ||
| thiserror = "2.0.17" | ||
| tracing = { version = "0.1.41", default-features = false, features = ["std", "log", "release_max_level_warn"] } | ||
| tracing = { version = "0.1.44", default-features = false, features = ["std", "log", "release_max_level_warn"] } |
There was a problem hiding this comment.
Can we please roll this back to 0.1.41 to match our other plugins?
There was a problem hiding this comment.
Rolled back to 0.1.41 to match the other plugins.
One note: our main app currently has tracing = "0.1.44", so that may be intentional there, but I’ve aligned this plugin with the plugin repos as requested.
| return Ok(ConnectionStatus::disconnected()); | ||
| } | ||
|
|
||
| let connection_cost = match profile.GetConnectionCost() { |
There was a problem hiding this comment.
Nit: This pattern can be simplified using inspect_err:
let connection_cost = profile
.GetConnectionCost()
.inspect_err(|error| warn!(%error, "failed to query Windows connection cost"))?;This could be applied also to the other properties too.
There was a problem hiding this comment.
Good suggestion! Updated the straightforward error paths.
2fcb6bf to
c3de9bf
Compare
9115e38 to
1f3fb93
Compare
614f4b7 to
c707f3e
Compare
c707f3e to
1c4e5c0
Compare
Adds Windows support for
connection_status()intauri-plugin-connectivity.NetworkInformationBackgroundDataUsageRestrictedin the Windowsconstrainedchecktracinglogs for plugin setup, command handling, and Windows status resolutionexamples/tauri-appapp for manual testingCloses #3