wifictl dbus_nm: correct wire types — NmState 0..70 + OwnedObjectPath
The redbear-wifictl NM-shaped D-Bus interface returned wire-incompatible types that no compliant client (Qt, GNOME, kf6-networkmanager-qt) could parse: 1. Root State returned a NmDeviceState value (up to 120 = Failed), but Qt's qnetworkmanagerservice.h:65-74 defines NmState as 0..=70 (Unknown..ConnectedGlobal). Returning a device-state value in the manager-state property is wire-incompatible. 2. get_devices() and get_access_points() returned Vec<String>, but the D-Bus spec requires Vec<o> (array of object paths). zbus serializes Vec<String> as 'as' which clients reject. 3. active_access_point returned String, should be o. The sentinel '/' for 'no active AP' is a valid object path per spec. Fixes: - Add new NmState enum with the correct 8 variants (Unknown=0, Asleep=10, Disconnected=20, Disconnecting=30, Connecting=40, ConnectedLocal=50, ConnectedSite=60, ConnectedGlobal=70). - Add NmState::from_device_state(NmDeviceState) mapping table: Unmanaged/Unavailable/Disconnected/Failed -> Disconnected; Prepare/Config/NeedAuth/IpConfig/IpCheck -> Connecting; Activated -> ConnectedGlobal; Unknown -> Unknown. - Root state property now returns NmState::from_device_state( device.state).as_u32(), guaranteed 0..=70. - get_devices(), get_all_devices(), active_connections (property), get_access_points(), get_all_access_points() all return Vec<OwnedObjectPath> via a shared try_path() helper that maps malformed strings to fdo::Error::Failed instead of panicking. - active_access_point() property returns fdo::Result<OwnedObjectPath>; uses '/' sentinel when no AP is active. - 5 new host tests verify NmState numeric values (the Qt-required 0/10/20/.../70 sequence), from_device_state mapping, and the 'range never exceeds 70' invariant. Verified: 35 unit + 2 integration tests pass.
This commit is contained in:
@@ -118,6 +118,91 @@ impl NmDeviceState {
|
||||
}
|
||||
}
|
||||
|
||||
/// Overall NetworkManager state — the value carried by the
|
||||
/// `org.freedesktop.NetworkManager.State` property and the `StateChanged`
|
||||
/// signal. This is a *distinct* enum from [`NmDeviceState`]: it is a coarser
|
||||
/// roll-up over the smaller range `0..=70` defined by the NetworkManager spec
|
||||
/// and consumed by clients such as Qt's `QNetworkConfigurationManager`
|
||||
/// (`qnetworkmanagerservice.h:65-74`). A per-device state value (which can
|
||||
/// reach `120`) must never leak onto this property.
|
||||
#[repr(u32)]
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub enum NmState {
|
||||
Unknown = 0,
|
||||
Asleep = 10,
|
||||
Disconnected = 20,
|
||||
Disconnecting = 30,
|
||||
Connecting = 40,
|
||||
ConnectedLocal = 50,
|
||||
ConnectedSite = 60,
|
||||
ConnectedGlobal = 70,
|
||||
}
|
||||
|
||||
impl Default for NmState {
|
||||
fn default() -> Self {
|
||||
NmState::Unknown
|
||||
}
|
||||
}
|
||||
|
||||
impl NmState {
|
||||
/// Numeric value used on the D-Bus wire (matches the NetworkManager
|
||||
/// `NMState` enum, range `0..=70`).
|
||||
pub fn as_u32(self) -> u32 {
|
||||
self as u32
|
||||
}
|
||||
|
||||
/// Human-readable label for diagnostics.
|
||||
pub fn as_str(self) -> &'static str {
|
||||
match self {
|
||||
NmState::Unknown => "unknown",
|
||||
NmState::Asleep => "asleep",
|
||||
NmState::Disconnected => "disconnected",
|
||||
NmState::Disconnecting => "disconnecting",
|
||||
NmState::Connecting => "connecting",
|
||||
NmState::ConnectedLocal => "connected-local",
|
||||
NmState::ConnectedSite => "connected-site",
|
||||
NmState::ConnectedGlobal => "connected-global",
|
||||
}
|
||||
}
|
||||
|
||||
/// Roll up a per-device state into the overall NetworkManager state.
|
||||
///
|
||||
/// NetworkManager computes its top-level state from the "best" device
|
||||
/// state. For this single-device daemon the mapping is direct:
|
||||
///
|
||||
/// | device state | manager state |
|
||||
/// |---------------------------------------|---------------------|
|
||||
/// | `Unknown` | `Unknown` |
|
||||
/// | `Unmanaged`/`Unavailable`/ | `Disconnected` |
|
||||
/// | `Disconnected`/`Failed` | |
|
||||
/// | `Prepare`/`Config`/`NeedAuth`/ | `Connecting` |
|
||||
/// | `IpConfig`/`IpCheck` | |
|
||||
/// | `Activated` | `ConnectedGlobal` |
|
||||
pub fn from_device_state(state: NmDeviceState) -> Self {
|
||||
match state {
|
||||
// The device state is genuinely undetermined.
|
||||
NmDeviceState::Unknown => NmState::Unknown,
|
||||
|
||||
// No usable connectivity.
|
||||
NmDeviceState::Unmanaged
|
||||
| NmDeviceState::Unavailable
|
||||
| NmDeviceState::Disconnected
|
||||
| NmDeviceState::Failed => NmState::Disconnected,
|
||||
|
||||
// Any active transition towards a connection.
|
||||
NmDeviceState::Prepare
|
||||
| NmDeviceState::Config
|
||||
| NmDeviceState::NeedAuth
|
||||
| NmDeviceState::IpConfig
|
||||
| NmDeviceState::IpCheck => NmState::Connecting,
|
||||
|
||||
// Fully up. Without a connectivity checker we cannot distinguish
|
||||
// local/site/global, so report the strongest reachable value.
|
||||
NmDeviceState::Activated => NmState::ConnectedGlobal,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Default)]
|
||||
pub struct NmAccessPoint {
|
||||
pub ssid: String,
|
||||
@@ -145,12 +230,23 @@ impl NmAccessPoint {
|
||||
mod nm_iface {
|
||||
use std::sync::{Arc, Mutex};
|
||||
|
||||
use super::NmWifiDevice;
|
||||
use super::{NmState, NmWifiDevice};
|
||||
use zbus::{
|
||||
blocking::connection::Builder as ConnectionBuilder, fdo, interface,
|
||||
zvariant::ObjectPath,
|
||||
zvariant::{ObjectPath, OwnedObjectPath},
|
||||
};
|
||||
|
||||
/// Convert a daemon-produced path string into an [`OwnedObjectPath`],
|
||||
/// mapping a malformed path to a clean `fdo::Error` instead of panicking.
|
||||
/// Every path this daemon constructs (`DEVICE_PATH`, `access_point_path`,
|
||||
/// and the `/` sentinel) is well-formed, so this only fails on programmer
|
||||
/// error — but returning `fdo::Result` keeps the conversion explicit and
|
||||
/// avoids `.expect()` on dynamic path strings.
|
||||
fn try_path(s: &str) -> fdo::Result<OwnedObjectPath> {
|
||||
OwnedObjectPath::try_from(s)
|
||||
.map_err(|e| fdo::Error::Failed(format!("invalid object path '{s}': {e}")))
|
||||
}
|
||||
|
||||
/// Well-known D-Bus service name for NetworkManager.
|
||||
pub(crate) const BUS_NAME: &str = "org.freedesktop.NetworkManager";
|
||||
/// Object path for the root NetworkManager object.
|
||||
@@ -195,12 +291,12 @@ mod nm_iface {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn device_path(&self) -> String {
|
||||
DEVICE_PATH.to_string()
|
||||
pub(crate) fn device_path(&self) -> fdo::Result<OwnedObjectPath> {
|
||||
try_path(DEVICE_PATH)
|
||||
}
|
||||
|
||||
pub(crate) fn nm_state(&self) -> u32 {
|
||||
snapshot(&self.device).state.as_u32()
|
||||
NmState::from_device_state(snapshot(&self.device).state).as_u32()
|
||||
}
|
||||
|
||||
pub(crate) fn wireless_enabled_flag(&self) -> bool {
|
||||
@@ -211,12 +307,12 @@ mod nm_iface {
|
||||
#[interface(name = "org.freedesktop.NetworkManager")]
|
||||
impl NmRoot {
|
||||
/// GetDevices — returns the object paths of all managed devices.
|
||||
fn get_devices(&self) -> fdo::Result<Vec<String>> {
|
||||
Ok(vec![self.device_path()])
|
||||
fn get_devices(&self) -> fdo::Result<Vec<OwnedObjectPath>> {
|
||||
Ok(vec![self.device_path()?])
|
||||
}
|
||||
|
||||
/// GetAllDevices — same set as GetDevices for this single-device daemon.
|
||||
fn get_all_devices(&self) -> fdo::Result<Vec<String>> {
|
||||
fn get_all_devices(&self) -> fdo::Result<Vec<OwnedObjectPath>> {
|
||||
self.get_devices()
|
||||
}
|
||||
|
||||
@@ -232,15 +328,17 @@ mod nm_iface {
|
||||
true
|
||||
}
|
||||
|
||||
/// Overall NetworkManager state (mirrors the active device state).
|
||||
/// Overall NetworkManager state (NMState enum, range 0..=70). Derived
|
||||
/// from the active device state via [`NmState::from_device_state`].
|
||||
#[zbus(property)]
|
||||
fn state(&self) -> u32 {
|
||||
self.nm_state()
|
||||
}
|
||||
|
||||
/// ActiveConnections property (paths); empty until a connection is up.
|
||||
/// ActiveConnections property (object paths); empty until a connection
|
||||
/// is up.
|
||||
#[zbus(property)]
|
||||
fn active_connections(&self) -> Vec<String> {
|
||||
fn active_connections(&self) -> Vec<OwnedObjectPath> {
|
||||
Vec::new()
|
||||
}
|
||||
|
||||
@@ -270,13 +368,13 @@ mod nm_iface {
|
||||
}
|
||||
|
||||
/// Object paths for every known access point.
|
||||
pub(crate) fn access_point_paths(&self) -> Vec<String> {
|
||||
pub(crate) fn access_point_paths(&self) -> fdo::Result<Vec<OwnedObjectPath>> {
|
||||
(0..self.snap().access_points.len())
|
||||
.map(access_point_path)
|
||||
.map(|i| try_path(&access_point_path(i)))
|
||||
.collect()
|
||||
}
|
||||
|
||||
pub(crate) fn active_access_point_inner(&self) -> String {
|
||||
pub(crate) fn active_access_point_inner(&self) -> fdo::Result<OwnedObjectPath> {
|
||||
let snap = self.snap();
|
||||
let has_active = !snap.active_ssid.is_empty()
|
||||
&& snap
|
||||
@@ -284,9 +382,11 @@ mod nm_iface {
|
||||
.iter()
|
||||
.any(|ap| ap.ssid == snap.active_ssid);
|
||||
if has_active {
|
||||
access_point_path(0)
|
||||
try_path(&access_point_path(0))
|
||||
} else {
|
||||
"/".to_string()
|
||||
// Per the NetworkManager spec, `ActiveAccessPoint` is the root
|
||||
// path "/" when no access point is associated.
|
||||
try_path("/")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -318,12 +418,12 @@ mod nm_iface {
|
||||
#[interface(name = "org.freedesktop.NetworkManager.Device.Wireless")]
|
||||
impl NmWirelessDevice {
|
||||
/// GetAccessPoints — object paths of all known access points.
|
||||
fn get_access_points(&self) -> fdo::Result<Vec<String>> {
|
||||
Ok(self.access_point_paths())
|
||||
fn get_access_points(&self) -> fdo::Result<Vec<OwnedObjectPath>> {
|
||||
self.access_point_paths()
|
||||
}
|
||||
|
||||
/// GetAllAccessPoints — includes hidden networks (same set here).
|
||||
fn get_all_access_points(&self) -> fdo::Result<Vec<String>> {
|
||||
fn get_all_access_points(&self) -> fdo::Result<Vec<OwnedObjectPath>> {
|
||||
self.get_access_points()
|
||||
}
|
||||
|
||||
@@ -377,8 +477,8 @@ mod nm_iface {
|
||||
|
||||
/// ActiveAccessPoint object path.
|
||||
#[zbus(property)]
|
||||
fn active_access_point(&self) -> fdo::Result<String> {
|
||||
Ok(self.active_access_point_inner())
|
||||
fn active_access_point(&self) -> fdo::Result<OwnedObjectPath> {
|
||||
self.active_access_point_inner()
|
||||
}
|
||||
|
||||
/// Interface property (device name, e.g. "wlan0").
|
||||
@@ -509,6 +609,90 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn nm_state_default_is_unknown() {
|
||||
assert_eq!(NmState::default(), NmState::Unknown);
|
||||
assert_eq!(NmState::default().as_u32(), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn nm_state_as_u32_matches_networkmanager_spec() {
|
||||
assert_eq!(NmState::Unknown.as_u32(), 0);
|
||||
assert_eq!(NmState::Asleep.as_u32(), 10);
|
||||
assert_eq!(NmState::Disconnected.as_u32(), 20);
|
||||
assert_eq!(NmState::Disconnecting.as_u32(), 30);
|
||||
assert_eq!(NmState::Connecting.as_u32(), 40);
|
||||
assert_eq!(NmState::ConnectedLocal.as_u32(), 50);
|
||||
assert_eq!(NmState::ConnectedSite.as_u32(), 60);
|
||||
assert_eq!(NmState::ConnectedGlobal.as_u32(), 70);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn nm_state_from_device_state_maps_activated_to_connected_global() {
|
||||
assert_eq!(
|
||||
NmState::from_device_state(NmDeviceState::Activated),
|
||||
NmState::ConnectedGlobal,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn nm_state_from_device_state_maps_connecting_states() {
|
||||
for connecting in [
|
||||
NmDeviceState::Prepare,
|
||||
NmDeviceState::Config,
|
||||
NmDeviceState::NeedAuth,
|
||||
NmDeviceState::IpConfig,
|
||||
NmDeviceState::IpCheck,
|
||||
] {
|
||||
assert_eq!(
|
||||
NmState::from_device_state(connecting),
|
||||
NmState::Connecting,
|
||||
"{connecting:?} should map to Connecting",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn nm_state_from_device_state_maps_disconnected_states() {
|
||||
for disconnected in [
|
||||
NmDeviceState::Unmanaged,
|
||||
NmDeviceState::Unavailable,
|
||||
NmDeviceState::Disconnected,
|
||||
NmDeviceState::Failed,
|
||||
] {
|
||||
assert_eq!(
|
||||
NmState::from_device_state(disconnected),
|
||||
NmState::Disconnected,
|
||||
"{disconnected:?} should map to Disconnected",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn nm_state_never_exceeds_70_for_any_device_state() {
|
||||
// The root State property must stay within the NMState contract
|
||||
// (0..=70) and never leak a NmDeviceState value (which reaches 120).
|
||||
for ds in [
|
||||
NmDeviceState::Unknown,
|
||||
NmDeviceState::Unmanaged,
|
||||
NmDeviceState::Unavailable,
|
||||
NmDeviceState::Disconnected,
|
||||
NmDeviceState::Prepare,
|
||||
NmDeviceState::Config,
|
||||
NmDeviceState::NeedAuth,
|
||||
NmDeviceState::IpConfig,
|
||||
NmDeviceState::IpCheck,
|
||||
NmDeviceState::Activated,
|
||||
NmDeviceState::Failed,
|
||||
] {
|
||||
let v = NmState::from_device_state(ds).as_u32();
|
||||
assert!(
|
||||
v <= 70,
|
||||
"NmState for {ds:?} is {v}, exceeds the 0..=70 NMState contract",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn nm_wifi_device_default_is_empty() {
|
||||
let dev = NmWifiDevice::default();
|
||||
@@ -573,7 +757,7 @@ mod tests {
|
||||
use super::super::nm_iface::{
|
||||
DEVICE_PATH, NmRoot, NmWirelessDevice, access_point_path, shared_device,
|
||||
};
|
||||
use super::{NmAccessPoint, NmDeviceState, NmWifiDevice};
|
||||
use super::{NmAccessPoint, NmDeviceState, NmState, NmWifiDevice};
|
||||
|
||||
fn device_with(state: NmDeviceState) -> NmWifiDevice {
|
||||
NmWifiDevice {
|
||||
@@ -595,15 +779,15 @@ mod tests {
|
||||
let root = NmRoot::new(shared_device(&device_with(
|
||||
NmDeviceState::Activated,
|
||||
)));
|
||||
assert_eq!(root.device_path(), DEVICE_PATH);
|
||||
assert_eq!(root.nm_state(), NmDeviceState::Activated.as_u32());
|
||||
assert_eq!(root.device_path().unwrap().as_str(), DEVICE_PATH);
|
||||
assert_eq!(root.nm_state(), NmState::ConnectedGlobal.as_u32());
|
||||
assert!(root.wireless_enabled_flag());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn root_defaults_to_unknown_state() {
|
||||
let root = NmRoot::new(shared_device(&NmWifiDevice::default()));
|
||||
assert_eq!(root.nm_state(), 0);
|
||||
assert_eq!(root.nm_state(), NmState::Unknown.as_u32());
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -623,11 +807,11 @@ mod tests {
|
||||
let dev = NmWirelessDevice::new(shared_device(&device_with(
|
||||
NmDeviceState::Disconnected,
|
||||
)));
|
||||
let paths = dev.access_point_paths();
|
||||
let paths = dev.access_point_paths().expect("valid access-point paths");
|
||||
assert_eq!(paths.len(), 2);
|
||||
assert_eq!(paths[0], access_point_path(0));
|
||||
assert_eq!(paths[1], access_point_path(1));
|
||||
assert!(paths[0].starts_with(DEVICE_PATH));
|
||||
assert_eq!(paths[0].as_str(), access_point_path(0));
|
||||
assert_eq!(paths[1].as_str(), access_point_path(1));
|
||||
assert!(paths[0].as_str().starts_with(DEVICE_PATH));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -635,7 +819,10 @@ mod tests {
|
||||
let dev = NmWirelessDevice::new(shared_device(&device_with(
|
||||
NmDeviceState::Activated,
|
||||
)));
|
||||
assert_eq!(dev.active_access_point_inner(), access_point_path(0));
|
||||
assert_eq!(
|
||||
dev.active_access_point_inner().unwrap().as_str(),
|
||||
access_point_path(0),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -644,7 +831,7 @@ mod tests {
|
||||
device.active_ssid = "Nonexistent".to_string();
|
||||
device.active_strength = 0;
|
||||
let dev = NmWirelessDevice::new(shared_device(&device));
|
||||
assert_eq!(dev.active_access_point_inner(), "/");
|
||||
assert_eq!(dev.active_access_point_inner().unwrap().as_str(), "/");
|
||||
assert_eq!(dev.strength_inner(), 0);
|
||||
}
|
||||
|
||||
@@ -652,8 +839,8 @@ mod tests {
|
||||
fn wireless_device_empty_device_has_no_access_points() {
|
||||
let dev =
|
||||
NmWirelessDevice::new(shared_device(&NmWifiDevice::default()));
|
||||
assert!(dev.access_point_paths().is_empty());
|
||||
assert_eq!(dev.active_access_point_inner(), "/");
|
||||
assert!(dev.access_point_paths().unwrap().is_empty());
|
||||
assert_eq!(dev.active_access_point_inner().unwrap().as_str(), "/");
|
||||
assert_eq!(dev.strength_inner(), 0);
|
||||
assert_eq!(dev.last_scan_inner(), 0);
|
||||
}
|
||||
@@ -664,8 +851,8 @@ mod tests {
|
||||
let root = NmRoot::new(Arc::clone(&shared));
|
||||
let wireless = NmWirelessDevice::new(Arc::clone(&shared));
|
||||
|
||||
assert_eq!(root.nm_state(), NmDeviceState::Unknown.as_u32());
|
||||
assert!(wireless.access_point_paths().is_empty());
|
||||
assert_eq!(root.nm_state(), NmState::Unknown.as_u32());
|
||||
assert!(wireless.access_point_paths().unwrap().is_empty());
|
||||
|
||||
{
|
||||
let mut guard = shared.lock().expect("lock not poisoned");
|
||||
@@ -679,9 +866,9 @@ mod tests {
|
||||
guard.active_strength = 90;
|
||||
}
|
||||
|
||||
assert_eq!(root.nm_state(), NmDeviceState::Activated.as_u32());
|
||||
assert_eq!(root.nm_state(), NmState::ConnectedGlobal.as_u32());
|
||||
assert_eq!(wireless.strength_inner(), 90);
|
||||
assert_eq!(wireless.access_point_paths().len(), 1);
|
||||
assert_eq!(wireless.access_point_paths().unwrap().len(), 1);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user