-
-
Notifications
You must be signed in to change notification settings - Fork 15
Description
Overview:
The current connect() implementation uses polling with futures_timer::Delay to wait for device state changes and connection activation. This works but is inefficient and not idiomatic. NetworkManager emits D-Bus signals for state changes andwe should subscribe to these signals and react immediately rather than polling every 300ms.
Current Behavior:
- Polls device state in a loop with 300ms delays to detect disconnection
- Sleeps for 3 seconds after requesting a scan before proceeding
- Uses arbitrary timeouts and delays that may be too short or too long
Desired Behavior:
- Subscribe to
NMDevice.StateChangedsignal to detect disconnection immediately - Subscribe to
ActiveConnection.StateandStateReasonproperty changes to detect connection success/failure - Use
tokio::select!to wait on multiple signal streams simultaneously - Provide detailed error messages when connections fail (wrong password, SSID not found, timeout, etc.)
References:
- NetworkManager D-Bus API - Device Interface
- NetworkManager D-Bus API - ActiveConnection Interface
- zbus Property Streams Documentation
Tasks:
- Add
ActiveConnectionStateandConnectionStateReasonenums tomodels.rs - Implement
wait_for_device_state()helper usingreceive_state_changed()signal - Implement
wait_for_connection_activation()helper using property change streams - Replace polling loops in
connect()with signal-based waiting - Add proper timeout handling with
tokio::time::timeout - Test that connection failures provide meaningful error messages
- Remove all
futures_timer::Delayusage fromconnect()method
This should theoretically give us faster response times, and definitely give us more robust error handling with specific failure reasons. This could also lower CPU usage (seemingly a given, since its just event-driven vs polling)
This follows NetworkManager best practices used by nmcli and other official clients. Definitely seems to be a non-starter if this should scale.