[asterisk-commits] russell: trunk r79027 - in /trunk: apps/ funcs/ include/asterisk/ main/ res/

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri Aug 10 11:24:11 CDT 2007


Author: russell
Date: Fri Aug 10 11:24:11 2007
New Revision: 79027

URL: http://svn.digium.com/view/asterisk?view=rev&rev=79027
Log:
Merge a set of device state improvements from team/russell/events.

The way a device state change propagates is kind of silly, in my opinion.  A
device state provider calls a function that indicates that the state of a
device has changed.  Then, another thread goes back and calls a callback for
the device state provider to find out what the new state is before it can go
send it off to whoever cares.

I have changed it so that you can include the state that the device has changed
to in the first function call from the device state provider.  This removes the
need to have to call the callback, which locks up critical containers to go find
out what the state changed to.

This change set changes the "simple" device state providers to use the new method.
This includes parking, meetme, and SLA.

I have also mostly converted chan_agent in my branch, but still have some more
things to think through before presenting the plan for converting channel drivers
to ensure all of the right events get generated ...

Modified:
    trunk/apps/app_meetme.c
    trunk/funcs/func_devstate.c
    trunk/include/asterisk/devicestate.h
    trunk/main/devicestate.c
    trunk/main/event.c
    trunk/res/res_features.c

Modified: trunk/apps/app_meetme.c
URL: http://svn.digium.com/view/asterisk/trunk/apps/app_meetme.c?view=diff&rev=79027&r1=79026&r2=79027
==============================================================================
--- trunk/apps/app_meetme.c (original)
+++ trunk/apps/app_meetme.c Fri Aug 10 11:24:11 2007
@@ -1513,7 +1513,7 @@
 
 	/* This device changed state now - if this is the first user */
 	if (conf->users == 1)
-		ast_device_state_changed("meetme:%s", conf->confno);
+		ast_devstate_changed(AST_DEVICE_INUSE, "meetme:%s", conf->confno);
 
 	ast_mutex_unlock(&conf->playlock);
 
@@ -2333,7 +2333,7 @@
 
 		/* Change any states */
 		if (!conf->users)
-			ast_device_state_changed("meetme:%s", conf->confno);
+			ast_devstate_changed(AST_DEVICE_NOT_INUSE, "meetme:%s", conf->confno);
 		
 		/* Return the number of seconds the user was in the conf */
 		snprintf(meetmesecs, sizeof(meetmesecs), "%d", (int) (time(NULL) - user->jointime));
@@ -3305,6 +3305,23 @@
 	return ringing_station;
 }
 
+static enum ast_device_state sla_state_to_devstate(enum sla_trunk_state state)
+{
+	switch (state) {
+	case SLA_TRUNK_STATE_IDLE:
+		return AST_DEVICE_NOT_INUSE;
+	case SLA_TRUNK_STATE_RINGING:
+		return AST_DEVICE_RINGING;
+	case SLA_TRUNK_STATE_UP:
+		return AST_DEVICE_INUSE;
+	case SLA_TRUNK_STATE_ONHOLD:
+	case SLA_TRUNK_STATE_ONHOLD_BYME:
+		return AST_DEVICE_ONHOLD;
+	}
+
+	return AST_DEVICE_UNKNOWN;
+}
+
 static void sla_change_trunk_state(const struct sla_trunk *trunk, enum sla_trunk_state state, 
 	enum sla_which_trunk_refs inactive_only, const struct sla_trunk_ref *exclude)
 {
@@ -3317,7 +3334,8 @@
 				|| trunk_ref == exclude)
 				continue;
 			trunk_ref->state = state;
-			ast_device_state_changed("SLA:%s_%s", station->name, trunk->name);
+			ast_devstate_changed(sla_state_to_devstate(state), 
+				"SLA:%s_%s", station->name, trunk->name);
 			break;
 		}
 	}
@@ -3809,7 +3827,7 @@
 {
 	ast_atomic_fetchadd_int((int *) &event->trunk_ref->trunk->hold_stations, 1);
 	event->trunk_ref->state = SLA_TRUNK_STATE_ONHOLD_BYME;
-	ast_device_state_changed("SLA:%s_%s", 
+	ast_devstate_changed(AST_DEVICE_ONHOLD, "SLA:%s_%s", 
 		event->station->name, event->trunk_ref->trunk->name);
 	sla_change_trunk_state(event->trunk_ref->trunk, SLA_TRUNK_STATE_ONHOLD, 
 		INACTIVE_TRUNK_REFS, event->trunk_ref);
@@ -4319,7 +4337,8 @@
 			sla_change_trunk_state(trunk_ref->trunk, SLA_TRUNK_STATE_UP, ALL_TRUNK_REFS, NULL);
 		else {
 			trunk_ref->state = SLA_TRUNK_STATE_UP;
-			ast_device_state_changed("SLA:%s_%s", station->name, trunk_ref->trunk->name);
+			ast_devstate_changed(AST_DEVICE_INUSE, 
+				"SLA:%s_%s", station->name, trunk_ref->trunk->name);
 		}
 	}
 
@@ -4536,21 +4555,7 @@
 			AST_RWLIST_UNLOCK(&sla_trunks);
 			break;
 		}
-		switch (trunk_ref->state) {
-		case SLA_TRUNK_STATE_IDLE:
-			res = AST_DEVICE_NOT_INUSE;
-			break;
-		case SLA_TRUNK_STATE_RINGING:
-			res = AST_DEVICE_RINGING;
-			break;
-		case SLA_TRUNK_STATE_UP:
-			res = AST_DEVICE_INUSE;
-			break;
-		case SLA_TRUNK_STATE_ONHOLD:
-		case SLA_TRUNK_STATE_ONHOLD_BYME:
-			res = AST_DEVICE_ONHOLD;
-			break;
-		}
+		res = sla_state_to_devstate(trunk_ref->state);
 		AST_RWLIST_UNLOCK(&sla_trunks);
 	}
 	AST_RWLIST_UNLOCK(&sla_stations);

Modified: trunk/funcs/func_devstate.c
URL: http://svn.digium.com/view/asterisk/trunk/funcs/func_devstate.c?view=diff&rev=79027&r1=79026&r2=79027
==============================================================================
--- trunk/funcs/func_devstate.c (original)
+++ trunk/funcs/func_devstate.c Fri Aug 10 11:24:11 2007
@@ -86,7 +86,7 @@
 		AST_RWLIST_INSERT_HEAD(&custom_devices, dev, entry);
 	}
 	dev->state = ast_devstate_val(value);
-	ast_device_state_changed("Custom:%s", dev->name);
+	ast_devstate_changed(dev->state, "Custom:%s", dev->name);
 	AST_RWLIST_UNLOCK(&custom_devices);
 
 	return 0;

Modified: trunk/include/asterisk/devicestate.h
URL: http://svn.digium.com/view/asterisk/trunk/include/asterisk/devicestate.h?view=diff&rev=79027&r1=79026&r2=79027
==============================================================================
--- trunk/include/asterisk/devicestate.h (original)
+++ trunk/include/asterisk/devicestate.h Fri Aug 10 11:24:11 2007
@@ -58,73 +58,133 @@
 	AST_DEVICE_ONHOLD,       /*!< Device is on hold */
 };
 
-/*!  \brief Devicestate provider call back */
+/*! \brief Devicestate provider call back */
 typedef enum ast_device_state (*ast_devstate_prov_cb_type)(const char *data);
 
-/*! \brief Convert device state to text string for output 
+/*! 
+ * \brief Convert device state to text string for output 
+ *
  * \param devstate Current device state 
  */
 const char *devstate2str(enum ast_device_state devstate);
 
-/*! \brief Convert device state to text string that is easier to parse 
+/*! 
+ * \brief Convert device state to text string that is easier to parse 
+ *
  * \param devstate Current device state 
  */
 const char *ast_devstate_str(enum ast_device_state devstate);
 
-/*! \brief Convert device state from text to integer value
+/*! 
+ * \brief Convert device state from text to integer value
+ *
  * \param val The text representing the device state.  Valid values are anything
  *        that comes after AST_DEVICE_ in one of the defined values.
+ *
  * \return The AST_DEVICE_ integer value
  */
 enum ast_device_state ast_devstate_val(const char *val);
 
 /*! 
  * \brief Search the Channels by Name
- * \param device like a dialstring
- * Search the Device in active channels by compare the channelname against 
- * the devicename. Compared are only the first chars to the first '-' char.
- * \retval an AST_DEVICE_UNKNOWN if no channel found
+ *
+ * \param device like a dial string
+ *
+ * Search the Device in active channels by compare the channel name against 
+ * the device name. Compared are only the first chars to the first '-' char.
+ *
+ * \retval AST_DEVICE_UNKNOWN if no channel found
  * \retval AST_DEVICE_INUSE if a channel is found
  */
 enum ast_device_state ast_parse_device_state(const char *device);
 
 /*! 
  * \brief Asks a channel for device state
- * \param device like a dialstring
- * Asks a channel for device state, data is  normaly a number from dialstring
+ *
+ * \param device like a dial string
+ *
+ * Asks a channel for device state, data is normally a number from a dial string
  * used by the low level module
- * Trys the channel devicestate callback if not supported search in the
+ * Tries the channel device state callback if not supported search in the
  * active channels list for the device.
- * \retval an AST_DEVICE_??? state
+ *
+ * \retval an AST_DEVICE_??? state 
  * \retval -1 on failure
  */
 enum ast_device_state ast_device_state(const char *device);
 
 /*! 
  * \brief Tells Asterisk the State for Device is changed
- * \param fmt devicename like a dialstring with format parameters
- * Asterisk polls the new extensionstates and calls the registered
+ *
+ * \param state the new state of the device
+ * \param fmt device name like a dial string with format parameters
+ *
+ * The new state of the device will be sent off to any subscribers
+ * of device states.  It will also be stored in the internal event
+ * cache.
+ *
+ * \retval 0 on success 
+ * \retval -1 on failure
+ *
+ * \note This is deprecated in favor of ast_devstate_changed()
+ */
+int ast_devstate_changed(enum ast_device_state state, const char *fmt, ...)
+	__attribute__ ((format (printf, 2, 3)));
+
+/*! 
+ * \brief Tells Asterisk the State for Device is changed
+ *
+ * \param state the new state of the device
+ * \param fmt device name like a dial string with format parameters
+ *
+ * The new state of the device will be sent off to any subscribers
+ * of device states.  It will also be stored in the internal event
+ * cache.
+ *
+ * \retval 0 on success 
+ * \retval -1 on failure
+ *
+ * \note This is deprecated in favor of ast_devstate_changed()
+ */
+int ast_devstate_changed_literal(enum ast_device_state state, const char *device);
+
+/*! 
+ * \brief Tells Asterisk the State for Device is changed
+ *
+ * \param fmt device name like a dial string with format parameters
+ *
+ * Asterisk polls the new extension states and calls the registered
  * callbacks for the changed extensions
- * \retval 0 on success
- * \retval -1 on failure
+ *
+ * \retval 0 on success 
+ * \retval -1 on failure
+ *
+ * \note This is deprecated in favor of ast_devstate_changed()
  */
 int ast_device_state_changed(const char *fmt, ...)
 	__attribute__ ((format (printf, 1, 2)));
 
 /*! 
  * \brief Tells Asterisk the State for Device is changed 
- * \param device devicename like a dialstring
- * Asterisk polls the new extensionstates and calls the registered
+ *
+ * \param device device name like a dial string
+ *
+ * Asterisk polls the new extension states and calls the registered
  * callbacks for the changed extensions
- * \retval 0 on success
- * \retval -1 on failure
+ *
+ * \retval 0 on success 
+ * \retval -1 on failure
+ *
+ * \note This is deprecated in favor of ast_devstate_changed_literal()
  */
 int ast_device_state_changed_literal(const char *device);
 
 /*! 
  * \brief Add device state provider 
+ *
  * \param label to use in hint, like label:object
  * \param callback Callback
+ *
  * \retval 0 success
  * \retval -1 failure
  */ 
@@ -132,9 +192,11 @@
 
 /*! 
  * \brief Remove device state provider 
+ *
  * \param label to use in hint, like label:object
+ *
+ * \retval -1 on failure 
  * \retval 0 on success
- * \retval -1 on failure
  */ 
 int ast_devstate_prov_del(const char *label);
 

Modified: trunk/main/devicestate.c
URL: http://svn.digium.com/view/asterisk/trunk/main/devicestate.c?view=diff&rev=79027&r1=79026&r2=79027
==============================================================================
--- trunk/main/devicestate.c (original)
+++ trunk/main/devicestate.c Fri Aug 10 11:24:11 2007
@@ -25,6 +25,7 @@
  *
  *	\arg \ref AstExtState
  */
+
 /*! \page AstExtState Extension and device states in Asterisk
  *
  *	Asterisk has an internal system that reports states
@@ -165,6 +166,17 @@
 
 /*! \brief Flag for the queue */
 static ast_cond_t change_pending;
+
+/*! \brief Whether or not to cache this device state value */
+enum devstate_cache {
+	/*! Cache this value as it is coming from a device state provider which is
+	 *  pushing up state change events to us as they happen */
+	CACHE_ON,
+	/*! Don't cache this result, since it was pulled from the device state provider.
+	 *  We only want to cache results from device state providers that are being nice
+	 *  and pushing state change events up to us as they happen. */
+	CACHE_OFF,
+};
 
 /* Forward declarations */
 static int getproviderstate(const char *provider, const char *address);
@@ -261,18 +273,42 @@
 	return res;
 }
 
+static enum ast_device_state devstate_cached(const char *device)
+{
+	enum ast_device_state res = AST_DEVICE_UNKNOWN;
+	struct ast_event *event;
+
+	event = ast_event_get_cached(AST_EVENT_DEVICE_STATE,
+		AST_EVENT_IE_DEVICE, AST_EVENT_IE_PLTYPE_STR, device,
+		AST_EVENT_IE_END);
+
+	if (!event)
+		return res;
+
+	res = ast_event_get_ie_uint(event, AST_EVENT_IE_STATE);
+
+	ast_event_destroy(event);
+
+	return res;
+}
+
 /*! \brief Check device state through channel specific function or generic function */
 enum ast_device_state ast_device_state(const char *device)
 {
 	char *buf;
 	char *number;
 	const struct ast_channel_tech *chan_tech;
-	enum ast_device_state res = AST_DEVICE_UNKNOWN;
+	enum ast_device_state res;
 	/*! \brief Channel driver that provides device state */
 	char *tech;
 	/*! \brief Another provider of device state */
 	char *provider = NULL;
-	
+
+	/* If the last known state is cached, just return that */
+	res = devstate_cached(device);
+	if (res != AST_DEVICE_UNKNOWN)
+		return res;
+
 	buf = ast_strdupa(device);
 	tech = strsep(&buf, "/");
 	if (!(number = buf)) {
@@ -368,16 +404,9 @@
 	return res;
 }
 
-/*! \brief Notify callback watchers of change, and notify PBX core for hint updates
-	Normally executed within a separate thread
-*/
-static void do_state_change(const char *device)
-{
-	enum ast_device_state state;
+static void devstate_event(const char *device, enum ast_device_state state, enum devstate_cache cache)
+{
 	struct ast_event *event;
-
-	state = ast_device_state(device);
-	ast_debug(3, "Changing state for %s - state %d (%s)\n", device, state, devstate2str(state));
 
 	if (!(event = ast_event_new(AST_EVENT_DEVICE_STATE,
 			AST_EVENT_IE_DEVICE, AST_EVENT_IE_PLTYPE_STR, device,
@@ -386,12 +415,31 @@
 		return;
 	}
 
-	ast_event_queue_and_cache(event,
-		AST_EVENT_IE_DEVICE, AST_EVENT_IE_PLTYPE_STR,
-		AST_EVENT_IE_END);
-}
-
-static int __ast_device_state_changed_literal(char *buf)
+	if (cache == CACHE_ON) {
+		/* Cache this event, replacing an event in the cache with the same
+		 * device name if it exists. */
+		ast_event_queue_and_cache(event,
+			AST_EVENT_IE_DEVICE, AST_EVENT_IE_PLTYPE_STR,
+			AST_EVENT_IE_END);
+	} else {
+		ast_event_queue(event);
+	}
+}
+
+/*! Called by the state change thread to find out what the state is, and then
+ *  to queue up the state change event */
+static void do_state_change(const char *device)
+{
+	enum ast_device_state state;
+
+	state = ast_device_state(device);
+
+	ast_debug(3, "Changing state for %s - state %d (%s)\n", device, state, devstate2str(state));
+
+	devstate_event(device, state, CACHE_OFF);
+}
+
+static int __ast_devstate_changed_literal(enum ast_device_state state, char *buf)
 {
 	char *device;
 	struct state_change *change;
@@ -404,8 +452,10 @@
 	tmp = strrchr(device, '-');
 	if (tmp)
 		*tmp = '\0';
-	
-	if (change_thread == AST_PTHREADT_NULL || !(change = ast_calloc(1, sizeof(*change) + strlen(device)))) {
+
+	if (state != AST_DEVICE_UNKNOWN) {
+		devstate_event(device, state, CACHE_ON);
+	} else if (change_thread == AST_PTHREADT_NULL || !(change = ast_calloc(1, sizeof(*change) + strlen(device)))) {
 		/* we could not allocate a change struct, or */
 		/* there is no background thread, so process the change now */
 		do_state_change(device);
@@ -421,15 +471,25 @@
 	return 1;
 }
 
+int ast_devstate_changed_literal(enum ast_device_state state, const char *dev)
+{
+	char *buf;
+
+	buf = ast_strdupa(dev);
+
+	return __ast_devstate_changed_literal(state, buf);
+}
+
 int ast_device_state_changed_literal(const char *dev)
 {
 	char *buf;
+
 	buf = ast_strdupa(dev);
-	return __ast_device_state_changed_literal(buf);
-}
-
-/*! \brief Accept change notification, add it to change queue */
-int ast_device_state_changed(const char *fmt, ...) 
+
+	return __ast_devstate_changed_literal(AST_DEVICE_UNKNOWN, buf);
+}
+
+int ast_devstate_changed(enum ast_device_state state, const char *fmt, ...) 
 {
 	char buf[AST_MAX_EXTENSION];
 	va_list ap;
@@ -437,7 +497,21 @@
 	va_start(ap, fmt);
 	vsnprintf(buf, sizeof(buf), fmt, ap);
 	va_end(ap);
-	return __ast_device_state_changed_literal(buf);
+
+	return __ast_devstate_changed_literal(state, buf);
+}
+
+/*! \brief Accept change notification, add it to change queue */
+int ast_device_state_changed(const char *fmt, ...) 
+{
+	char buf[AST_MAX_EXTENSION];
+	va_list ap;
+
+	va_start(ap, fmt);
+	vsnprintf(buf, sizeof(buf), fmt, ap);
+	va_end(ap);
+
+	return __ast_devstate_changed_literal(AST_DEVICE_UNKNOWN, buf);
 }
 
 /*! \brief Go through the dev state change queue and update changes in the dev state thread */

Modified: trunk/main/event.c
URL: http://svn.digium.com/view/asterisk/trunk/main/event.c?view=diff&rev=79027&r1=79026&r2=79027
==============================================================================
--- trunk/main/event.c (original)
+++ trunk/main/event.c Fri Aug 10 11:24:11 2007
@@ -35,7 +35,8 @@
 #include "asterisk/lock.h"
 #include "asterisk/utils.h"
 
-#define NUM_EVENT_THREADS 5
+/* Only use one thread for now to ensure ordered delivery */
+#define NUM_EVENT_THREADS 1
 
 struct ast_event_ie {
 	enum ast_event_ie_type ie_type:16;

Modified: trunk/res/res_features.c
URL: http://svn.digium.com/view/asterisk/trunk/res/res_features.c?view=diff&rev=79027&r1=79026&r2=79027
==============================================================================
--- trunk/res/res_features.c (original)
+++ trunk/res/res_features.c Fri Aug 10 11:24:11 2007
@@ -349,13 +349,12 @@
 }
 
 /*! \brief Notify metermaids that we've changed an extension */
-static void notify_metermaids(const char *exten, char *context)
-{
-	ast_debug(4, "Notification of state change to metermaids %s@%s\n", exten, context);
-
-	/* Send notification to devicestate subsystem */
-	ast_device_state_changed("park:%s@%s", exten, context);
-	return;
+static void notify_metermaids(const char *exten, char *context, enum ast_device_state state)
+{
+	ast_debug(4, "Notification of state change to metermaids %s@%s\n to state '%s'", 
+		exten, context, devstate2str(state));
+
+	ast_devstate_changed(state, "park:%s@%s", exten, context);
 }
 
 /*! \brief metermaids callback from devicestate.c */
@@ -493,7 +492,7 @@
 		ast_say_digits(peer, pu->parkingnum, "", peer->language);
 	if (con) {
 		if (!ast_add_extension2(con, 1, pu->parkingexten, 1, NULL, NULL, parkedcall, ast_strdup(pu->parkingexten), ast_free, registrar))
-			notify_metermaids(pu->parkingexten, parking_con);
+			notify_metermaids(pu->parkingexten, parking_con, AST_DEVICE_INUSE);
 	}
 	if (pu->notquiteyet) {
 		/* Wake up parking thread if we're really done */
@@ -2097,7 +2096,7 @@
 					if (ast_context_remove_extension2(con, pu->parkingexten, 1, NULL))
 						ast_log(LOG_WARNING, "Whoa, failed to remove the extension!\n");
 					else
-						notify_metermaids(pu->parkingexten, parking_con);
+						notify_metermaids(pu->parkingexten, parking_con, AST_DEVICE_NOT_INUSE);
 				} else
 					ast_log(LOG_WARNING, "Whoa, no parking context?\n");
 				ast_free(pu);
@@ -2131,7 +2130,7 @@
 							if (ast_context_remove_extension2(con, pu->parkingexten, 1, NULL))
 								ast_log(LOG_WARNING, "Whoa, failed to remove the extension!\n");
 							else
-								notify_metermaids(pu->parkingexten, parking_con);
+								notify_metermaids(pu->parkingexten, parking_con, AST_DEVICE_NOT_INUSE);
 						} else
 							ast_log(LOG_WARNING, "Whoa, no parking context?\n");
 						ast_free(pu);
@@ -2246,7 +2245,7 @@
 			if (ast_context_remove_extension2(con, pu->parkingexten, 1, NULL))
 				ast_log(LOG_WARNING, "Whoa, failed to remove the extension!\n");
 			else
-				notify_metermaids(pu->parkingexten, parking_con);
+				notify_metermaids(pu->parkingexten, parking_con, AST_DEVICE_NOT_INUSE);
 		} else
 			ast_log(LOG_WARNING, "Whoa, no parking context?\n");
 
@@ -3030,7 +3029,7 @@
 	/* Remove the old parking extension */
 	if (!ast_strlen_zero(old_parking_con) && (con = ast_context_find(old_parking_con)))	{
 		if(ast_context_remove_extension2(con, old_parking_ext, 1, registrar))
-				notify_metermaids(old_parking_ext, old_parking_con);
+				notify_metermaids(old_parking_ext, old_parking_con, AST_DEVICE_NOT_INUSE);
 		ast_debug(1, "Removed old parking extension %s@%s\n", old_parking_ext, old_parking_con);
 	}
 	
@@ -3042,7 +3041,7 @@
 	if (parkaddhints)
 		park_add_hints(parking_con, parking_start, parking_stop);
 	if (!res)
-		notify_metermaids(ast_parking_ext(), parking_con);
+		notify_metermaids(ast_parking_ext(), parking_con, AST_DEVICE_INUSE);
 	return res;
 
 }




More information about the asterisk-commits mailing list