GNOME.org
 

Re: [PATCH] Add hooks to wifi devices to prevent scanning if needed

On Mon, 2008-08-04 at 12:28 +0100, Sjoerd Simons wrote:
> Signal wifi device scanning end and start and send out a signal to check before
> starting to scan to enable other objects to disable the scan if necessary.
> 
> Signed-off-by: Sjoerd Simons <sjoerd simons collabora co uk>
> ---
>  marshallers/nm-marshal.list |    1 +
>  src/nm-device-wifi.c        |   91 ++++++++++++++++++++++++++++++++++++++++++-
>  src/nm-device-wifi.h        |    3 +
>  3 files changed, 93 insertions(+), 2 deletions(-)
> 
> diff --git a/marshallers/nm-marshal.list b/marshallers/nm-marshal.list
> index 577fbe4..12a203c 100644
> --- a/marshallers/nm-marshal.list
> +++ b/marshallers/nm-marshal.list
> @@ -17,3 +17,4 @@ VOID:STRING,INT
>  VOID:STRING,UINT
>  VOID:OBJECT,OBJECT,ENUM
>  VOID:POINTER,STRING
> +BOOLEAN:VOID
> diff --git a/src/nm-device-wifi.c b/src/nm-device-wifi.c
> index b8313a7..8b74250 100644
> --- a/src/nm-device-wifi.c
> +++ b/src/nm-device-wifi.c
> @@ -37,6 +37,7 @@
>  #include "nm-device-interface.h"
>  #include "nm-device-private.h"
>  #include "nm-utils.h"
> +#include "nm-marshal.h"
>  #include "NetworkManagerUtils.h"
>  #include "NetworkManagerPolicy.h"
>  #include "nm-activation-request.h"
> @@ -91,6 +92,10 @@ enum {
>  	HIDDEN_AP_FOUND,
>  	PROPERTIES_CHANGED,
>  
> +	SCANNING_STARTED,
> +	SCANNING_STOPPED,
> +	SCANNING_ALLOWED,

Lets do:

SCAN_STARTED
SCAN_DONE
SCAN_ALLOWED

or better yet, I think a 'scanning' property on the NMDeviceWifi class
would be better, which the mesh device could attach a notification
handler to.  That way we only need SCAN_ALLOWED.  Let's do that instead.
The get_property() handler for 'scanning' would return TRUE if
(priv->scanning || priv->scan_requested).

>  	LAST_SIGNAL
>  };
>  
> @@ -141,8 +146,9 @@ struct _NMDeviceWifiPrivate
>  	guint32			rate;
>  	gboolean		enabled; /* rfkilled or not */
>  	guint			state_to_disconnected_id;
> -	
> +
>  	gboolean			scanning;
> +	gboolean			scan_requested;

Hmm; maybe a scanning state enum would work better here than two
separate variables.  Because there will never be this case:

scanning == TRUE && scan_requested == FALSE

>  	glong			scheduled_scan_time;
>  	guint8			scan_interval; /* seconds */
>  	guint               pending_scan_id;
> @@ -209,6 +215,10 @@ static void supplicant_mgr_state_cb (NMSupplicantInterface * iface,
>  
>  static guint32 nm_device_wifi_get_bitrate (NMDeviceWifi *self);
>  
> +static gboolean scan_allowed_accumulator (GSignalInvocationHint *ihint,
> +                                      GValue *return_accu,
> +                                      const GValue *handler_return,
> +                                      gpointer data);
>  
>  static GQuark
>  nm_wifi_error_quark (void)
> @@ -407,6 +417,7 @@ nm_device_wifi_init (NMDeviceWifi * self)
>  	priv->dispose_has_run = FALSE;
>  	priv->supplicant.iface_error_id = 0;
>  	priv->scanning = FALSE;
> +	priv->scan_requested = FALSE;
>  	priv->ap_list = NULL;
>  	priv->we_version = 0;
>  
> @@ -1591,15 +1602,47 @@ can_scan (NMDeviceWifi *self)
>  }
>  
>  static gboolean
> +scan_allowed_accumulator (GSignalInvocationHint *ihint,
> +							GValue *return_accu,
> +							const GValue *handler_return,
> +							gpointer data) {
> +
> +	if (!g_value_get_boolean (handler_return)) {
> +		g_value_set_boolean (return_accu, FALSE);
> +	}

Don't need the {} for a single line after an if.

> +	return TRUE;
> +}
> +
> +static gboolean
> +scan_allowed (NMDeviceWifi *self) {
> +	GValue instance = { 0, };
> +	GValue retval = { 0, };
> +
> +	g_value_init (&instance, G_TYPE_OBJECT);
> +	g_value_set_object (&instance, self);
> +
> +	g_value_init (&retval, G_TYPE_BOOLEAN);
> +	g_value_set_boolean (&retval, TRUE);
> +
> +	g_signal_emitv (&instance, signals[SCANNING_ALLOWED], 0, &retval);
> +	if (!g_value_get_boolean (&retval))
> +		return FALSE;

I assume we need to do g_signal_emitv() to ensure that the return value
isn't changed automagically like g_signal_emit() does, right?

Also, g_value_set_object() refs the object, so you'll need to
g_value_unset (&instance) to unref 'self' here before returning, I
think.

> +  return TRUE;
> +}
> +
> +static gboolean
>  request_wireless_scan (gpointer user_data)
>  {
>  	NMDeviceWifi *self = NM_DEVICE_WIFI (user_data);
>  	NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self);
>  	gboolean backoff = FALSE;
>  
> -	if (can_scan (self)) {
> +	if (can_scan (self) && scan_allowed(self)) {

Spacing issue; should be "scan_allowed (self)"

>  		if (nm_supplicant_interface_request_scan (priv->supplicant.iface)) {
>  			/* success */
> +			priv->scan_requested = TRUE;
> +			g_signal_emit (G_OBJECT(self), signals[SCANNING_STARTED], 0);

After converting to using a 'scanning' property, we can just do a
g_object_notify (G_OBJECT (self), "scanning") here.

>  			backoff = TRUE;
>  		}
>  	}
> @@ -1675,6 +1718,13 @@ supplicant_iface_scan_result_cb (NMSupplicantInterface * iface,
>  								 gboolean result,
>  								 NMDeviceWifi * self)
>  {
> +	NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self);
> +
> +	if (priv->scan_requested) {
> +		priv->scan_requested = FALSE;
> +		g_signal_emit (G_OBJECT(self), signals[SCANNING_STOPPED], 0);
> +	}
> +

While this does technically work, the more correct place to put the
'stopped scanning' signal would be in
supplicant_iface_connection_state_cb_handler() where priv->scanning gets
reset to FALSE when the supplicant state changes to !SCANNING.

>  	if (can_scan (self))
>  		schedule_scan (self, TRUE);
>  }
> @@ -3264,6 +3314,33 @@ nm_device_wifi_class_init (NMDeviceWifiClass *klass)
>  		nm_properties_changed_signal_new (object_class,
>  								    G_STRUCT_OFFSET (NMDeviceWifiClass, properties_changed));
>  
> +	signals[SCANNING_STARTED] =
> +		g_signal_new ("scanning-started",
> +					  G_OBJECT_CLASS_TYPE (object_class),
> +					  G_SIGNAL_RUN_FIRST,
> +					  0,
> +					  NULL, NULL,
> +					  g_cclosure_marshal_VOID__VOID,
> +					  G_TYPE_NONE, 0);
> +
> +	signals[SCANNING_STOPPED] =
> +		g_signal_new ("scanning-stopped",
> +					  G_OBJECT_CLASS_TYPE (object_class),
> +					  G_SIGNAL_RUN_FIRST,
> +					  0,
> +					  NULL, NULL,
> +					  g_cclosure_marshal_VOID__VOID,
> +					  G_TYPE_NONE, 0);
> +
> +	signals[SCANNING_ALLOWED] =
> +		g_signal_new ("scanning-allowed",
> +					  G_OBJECT_CLASS_TYPE (object_class),
> +					  G_SIGNAL_RUN_LAST,
> +					  0,
> +					  scan_allowed_accumulator, NULL,
> +					  nm_marshal_BOOLEAN__VOID,
> +					  G_TYPE_BOOLEAN, 0);
> +
>  	dbus_g_object_type_install_info (G_TYPE_FROM_CLASS (klass),
>  									 &dbus_glib_nm_device_wifi_object_info);
>  
> @@ -3440,3 +3517,13 @@ nm_device_wifi_set_enabled (NMDeviceWifi *self, gboolean enabled)
>  	}
>  }
>  
> +gboolean
> +nm_device_wifi_is_scanning (NMDeviceWifi *self) {
> +	NMDeviceWifiPrivate *priv;
> +
> +	g_return_val_if_fail (NM_IS_DEVICE_WIFI (self), FALSE);
> +
> +	priv = NM_DEVICE_WIFI_GET_PRIVATE (self);
> +
> +	return priv->scan_requested;
> +}

If using a 'scanning' property on the NMDeviceWifi object, this could
just return the result of g_object_get.

Thanks for working on this!
Dan

> diff --git a/src/nm-device-wifi.h b/src/nm-device-wifi.h
> index 3ee5641..521adef 100644
> --- a/src/nm-device-wifi.h
> +++ b/src/nm-device-wifi.h
> @@ -102,6 +102,9 @@ NMAccessPoint * nm_device_wifi_get_activation_ap (NMDeviceWifi *self);
>  
>  void nm_device_wifi_set_enabled (NMDeviceWifi *self, gboolean enabled);
>  
> +/* Wireless device _currently_ doing a scan */
> +gboolean nm_device_wifi_is_scanning (NMDeviceWifi *self);
> +
>  G_END_DECLS
>  
>  #endif	/* NM_DEVICE_WIFI_H */
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]