[PATCH] dbus: Replace StateChanged with PropertiesChanged signal

Marcel Holtmann marcel at holtmann.org
Sun Jan 3 20:53:53 EST 2010


The actual supplicant state is exposed via a property on the interface
object. So having a separate signal StateChanged for notifying about
changes is a bad idea. The standard PropertiesChanged signal should be
used for this.

The advantage of StateChanged signal was that it includes the previous
state, but not even NetworkManager is making use of this. And tracking
the old state via the property and this signal is easily possible anyway.
---
 doc/dbus.doxygen                |   14 +------
 wpa_supplicant/dbus/dbus_new.c  |   80 ++-------------------------------------
 wpa_supplicant/dbus/dbus_new.h  |    4 +-
 wpa_supplicant/notify.c         |    3 +-
 wpa_supplicant/wpa_supplicant.c |   12 ++++-
 5 files changed, 17 insertions(+), 96 deletions(-)

diff --git a/doc/dbus.doxygen b/doc/dbus.doxygen
index 9deb28d..9b7dff2 100644
--- a/doc/dbus.doxygen
+++ b/doc/dbus.doxygen
@@ -406,18 +406,6 @@ fi.w1.wpa_supplicant1.CreateInterface.
       </li>
 
       <li>
-	<h3>StateChanged ( s : newState, s : oldState )</h3>
-	<p>Interface state has changed.</p>
-	<h4>Arguments</h4>
-	<dl>
-	  <dt>s : newState</dt>
-	  <dd>A state which the interface goes to</dd>
-	  <dt>s : oldState</dt>
-	  <dd>A state which the interface goes from</dd>
-	</dl>
-      </li>
-
-      <li>
 	<h3>BSSAdded ( o : BSS, a{sv} : properties )</h3>
 	<p>Interface became aware of a new BSS.</p>
 	<h4>Arguments</h4>
@@ -501,7 +489,7 @@ fi.w1.wpa_supplicant1.CreateInterface.
 	<h4>Arguments</h4>
 	<dl>
 	  <dt>a{sv} : properties</dt>
-	  <dd>A dictionary with pairs of properties names which have changed and theirs new values. Possible dictionary keys are: "ApScan", "Scanning", "CurrentBSS", "CurrentNetwork"</dd>
+	  <dd>A dictionary with pairs of properties names which have changed and theirs new values. Possible dictionary keys are: "ApScan", "Scanning", "State", "CurrentBSS", "CurrentNetwork"</dd>
 	</dl>
       </li>
     </ul>
diff --git a/wpa_supplicant/dbus/dbus_new.c b/wpa_supplicant/dbus/dbus_new.c
index 1633b96..ce63325 100644
--- a/wpa_supplicant/dbus/dbus_new.c
+++ b/wpa_supplicant/dbus/dbus_new.c
@@ -393,75 +393,6 @@ void wpas_dbus_signal_network_selected(struct wpa_supplicant *wpa_s, int id)
 }
 
 
-static void str_to_lower(char *s)
-{
-	while (*s) {
-		*s = tolower(*s);
-		s++;
-	}
-}
-
-
-/**
- * wpas_dbus_signal_state_changed - Send a state changed signal
- * @wpa_s: %wpa_supplicant network interface data
- * @new_state: new state wpa_supplicant is entering
- * @old_state: old state wpa_supplicant is leaving
- *
- * Notify listeners that wpa_supplicant has changed state
- */
-void wpas_dbus_signal_state_changed(struct wpa_supplicant *wpa_s,
-				    enum wpa_states new_state,
-				    enum wpa_states old_state)
-{
-	struct wpas_dbus_priv *iface;
-	DBusMessage *msg;
-	char *new_state_str, *old_state_str;
-
-	/* Do nothing if the control interface is not turned on */
-	if (wpa_s->global == NULL)
-		return;
-	iface = wpa_s->global->dbus;
-	if (iface == NULL)
-		return;
-
-	/* Only send signal if state really changed */
-	if (new_state == old_state)
-		return;
-
-	msg = dbus_message_new_signal(wpa_s->dbus_new_path,
-				      WPAS_DBUS_NEW_IFACE_INTERFACE,
-				      "StateChanged");
-	if (msg == NULL)
-		return;
-
-	new_state_str = os_strdup(wpa_supplicant_state_txt(new_state));
-	old_state_str = os_strdup(wpa_supplicant_state_txt(old_state));
-	if (new_state_str == NULL || old_state_str == NULL)
-		goto out;
-
-	/* make state string lowercase to fit new DBus API convention */
-	str_to_lower(new_state_str);
-	str_to_lower(old_state_str);
-
-	if (!dbus_message_append_args(msg,
-	                              DBUS_TYPE_STRING, &new_state_str,
-	                              DBUS_TYPE_STRING, &old_state_str,
-	                              DBUS_TYPE_INVALID)) {
-		wpa_printf(MSG_ERROR, "dbus: Failed to construct state change "
-		           "signal");
-		goto out;
-	}
-
-	dbus_connection_send(iface->con, msg, NULL);
-
-out:
-	dbus_message_unref(msg);
-	os_free(new_state_str);
-	os_free(old_state_str);
-}
-
-
 /**
  * wpas_dbus_signal_network_enabled_changed - Signals Enabled property changes
  * @wpa_s: %wpa_supplicant network interface data
@@ -747,6 +678,10 @@ void wpas_dbus_signal_prop_changed(struct wpa_supplicant *wpa_s,
 		getter = (WPADBusPropertyAccessor) wpas_dbus_getter_scanning;
 		prop = "Scanning";
 		break;
+	case WPAS_DBUS_PROP_STATE:
+		getter = (WPADBusPropertyAccessor) wpas_dbus_getter_state;
+		prop = "State";
+		break;
 	case WPAS_DBUS_PROP_CURRENT_BSS:
 		getter = (WPADBusPropertyAccessor)
 			wpas_dbus_getter_current_bss;
@@ -1419,13 +1354,6 @@ static const struct wpa_dbus_signal_desc wpas_dbus_interface_signals[] = {
 		  END_ARGS
 	  }
 	},
-	{ "StateChanged", WPAS_DBUS_NEW_IFACE_INTERFACE,
-	  {
-		  { "newState", "s", ARG_OUT },
-		  { "oldState", "s", ARG_OUT },
-		  END_ARGS
-	  }
-	},
 	{ "BSSAdded", WPAS_DBUS_NEW_IFACE_INTERFACE,
 	  {
 		  { "path", "o", ARG_OUT },
diff --git a/wpa_supplicant/dbus/dbus_new.h b/wpa_supplicant/dbus/dbus_new.h
index 896e8f0..e20ab76 100644
--- a/wpa_supplicant/dbus/dbus_new.h
+++ b/wpa_supplicant/dbus/dbus_new.h
@@ -27,6 +27,7 @@ enum wpa_states;
 enum wpas_dbus_prop {
 	WPAS_DBUS_PROP_AP_SCAN,
 	WPAS_DBUS_PROP_SCANNING,
+	WPAS_DBUS_PROP_STATE,
 	WPAS_DBUS_PROP_CURRENT_BSS,
 	WPAS_DBUS_PROP_CURRENT_NETWORK,
 };
@@ -77,9 +78,6 @@ void wpas_dbus_ctrl_iface_deinit(struct wpas_dbus_priv *iface);
 
 int wpas_dbus_register_interface(struct wpa_supplicant *wpa_s);
 int wpas_dbus_unregister_interface(struct wpa_supplicant *wpa_s);
-void wpas_dbus_signal_state_changed(struct wpa_supplicant *wpa_s,
-				    enum wpa_states new_state,
-				    enum wpa_states old_state);
 void wpas_dbus_signal_prop_changed(struct wpa_supplicant *wpa_s,
 				   enum wpas_dbus_prop property);
 void wpas_dbus_signal_network_enabled_changed(struct wpa_supplicant *wpa_s,
diff --git a/wpa_supplicant/notify.c b/wpa_supplicant/notify.c
index 5e6aa19..970ba9b 100644
--- a/wpa_supplicant/notify.c
+++ b/wpa_supplicant/notify.c
@@ -78,7 +78,7 @@ void wpas_notify_state_changed(struct wpa_supplicant *wpa_s,
 						old_state);
 
 	/* notify the new DBus API */
-	wpas_dbus_signal_state_changed(wpa_s, new_state, old_state);
+	wpas_dbus_signal_prop_changed(wpa_s, WPAS_DBUS_PROP_STATE);
 }
 
 
@@ -118,6 +118,7 @@ void wpas_notify_scanning(struct wpa_supplicant *wpa_s)
 {
 	/* notify the old DBus API */
 	wpa_supplicant_dbus_notify_scanning(wpa_s);
+
 	/* notify the new DBus API */
 	wpas_dbus_signal_prop_changed(wpa_s, WPAS_DBUS_PROP_SCANNING);
 }
diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
index dff8a10..ee7acdd 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -520,6 +520,8 @@ const char * wpa_supplicant_state_txt(enum wpa_states state)
 void wpa_supplicant_set_state(struct wpa_supplicant *wpa_s,
 			      enum wpa_states state)
 {
+	enum wpa_states old_state = wpa_s->wpa_state;
+
 	wpa_printf(MSG_DEBUG, "State: %s -> %s",
 		   wpa_supplicant_state_txt(wpa_s->wpa_state),
 		   wpa_supplicant_state_txt(state));
@@ -527,8 +529,6 @@ void wpa_supplicant_set_state(struct wpa_supplicant *wpa_s,
 	if (state != WPA_SCANNING)
 		wpa_supplicant_notify_scanning(wpa_s, 0);
 
-	wpas_notify_state_changed(wpa_s, state, wpa_s->wpa_state);
-
 	if (state == WPA_COMPLETED && wpa_s->new_connection) {
 #if defined(CONFIG_CTRL_IFACE) || !defined(CONFIG_NO_STDOUT_DEBUG)
 		struct wpa_ssid *ssid = wpa_s->current_ssid;
@@ -548,6 +548,9 @@ void wpa_supplicant_set_state(struct wpa_supplicant *wpa_s,
 		wpa_drv_set_operstate(wpa_s, 0);
 	}
 	wpa_s->wpa_state = state;
+
+	if (wpa_s->wpa_state != old_state)
+		wpas_notify_state_changed(wpa_s, wpa_s->wpa_state, old_state);
 }
 
 
@@ -583,12 +586,15 @@ static void wpa_supplicant_terminate(int sig, void *signal_ctx)
 static void wpa_supplicant_clear_status(struct wpa_supplicant *wpa_s)
 {
 	enum wpa_states old_state = wpa_s->wpa_state;
+
 	wpa_s->pairwise_cipher = 0;
 	wpa_s->group_cipher = 0;
 	wpa_s->mgmt_group_cipher = 0;
 	wpa_s->key_mgmt = 0;
 	wpa_s->wpa_state = WPA_DISCONNECTED;
-	wpas_notify_state_changed(wpa_s, wpa_s->wpa_state, old_state);
+
+	if (wpa_s->wpa_state != old_state)
+		wpas_notify_state_changed(wpa_s, wpa_s->wpa_state, old_state);
 }
 
 
-- 
1.6.5.2



More information about the HostAP mailing list