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 ...


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@79027 65c4cc65-6c06-0410-ace0-fbb531ad65f3

6 files changed, 211 insertions, 70 deletions

diff --git a/apps/app_meetme.c b/apps/app_meetme.c
index c953d6603..5a0745311 100644
--- a/apps/app_meetme.c
+++ b/apps/app_meetme.c
@@ -1513,7 +1513,7 @@ static int conf_run(struct ast_channel *chan, struct ast_conference *conf, int c
 
 	/* 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 @@ bailoutandtrynormal:
 
 		/* 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 @@ static struct sla_ringing_station *sla_create_ringing_station(struct sla_station
 	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 @@ static void sla_change_trunk_state(const struct sla_trunk *trunk, enum sla_trunk
 				|| 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 @@ static void sla_handle_hold_event(struct sla_event *event)
 {
 	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 @@ static int sla_station_exec(struct ast_channel *chan, void *data)
 			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 @@ static enum ast_device_state sla_state(const char *data)
 			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);
diff --git a/funcs/func_devstate.c b/funcs/func_devstate.c
index 6c3d8e932..f1fad0ae5 100644
--- a/funcs/func_devstate.c
+++ b/funcs/func_devstate.c
@@ -86,7 +86,7 @@ static int devstate_write(struct ast_channel *chan, const char *cmd, char *data,
 		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;
diff --git a/include/asterisk/devicestate.h b/include/asterisk/devicestate.h
index 08c547649..8ef0b24f6 100644
--- a/include/asterisk/devicestate.h
+++ b/include/asterisk/devicestate.h
@@ -58,73 +58,133 @@ enum ast_device_state {
 	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 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 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 @@ int ast_devstate_prov_add(const char *label, ast_devstate_prov_cb_type callback)
 
 /*! 
  * \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);
 
diff --git a/main/devicestate.c b/main/devicestate.c
index 05a775227..aef21f70a 100644
--- a/main/devicestate.c
+++ b/main/devicestate.c
@@ -25,6 +25,7 @@
  *
  *	\arg \ref AstExtState
  */
+
 /*! \page AstExtState Extension and device states in Asterisk
  *
  *	Asterisk has an internal system that reports states
@@ -166,6 +167,17 @@ static pthread_t change_thread = AST_PTHREADT_NULL;
 /*! \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 @@ enum ast_device_state ast_parse_device_state(const char *device)
 	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,17 +404,10 @@ static int getproviderstate(const char *provider, const char *address)
 	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)
+static void devstate_event(const char *device, enum ast_device_state state, enum devstate_cache cache)
 {
-	enum ast_device_state state;
 	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,
 			AST_EVENT_IE_STATE, AST_EVENT_IE_PLTYPE_UINT, state,
@@ -386,12 +415,31 @@ static void do_state_change(const char *device)
 		return;
 	}
 
-	ast_event_queue_and_cache(event,
-		AST_EVENT_IE_DEVICE, AST_EVENT_IE_PLTYPE_STR,
-		AST_EVENT_IE_END);
+	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);
+	}
 }
 
-static int __ast_device_state_changed_literal(char *buf)
+/*! 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 @@ static int __ast_device_state_changed_literal(char *buf)
 	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,11 +471,34 @@ static int __ast_device_state_changed_literal(char *buf)
 	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);
+
+	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;
+
+	va_start(ap, fmt);
+	vsnprintf(buf, sizeof(buf), fmt, ap);
+	va_end(ap);
+
+	return __ast_devstate_changed_literal(state, buf);
 }
 
 /*! \brief Accept change notification, add it to change queue */
@@ -437,7 +510,8 @@ int ast_device_state_changed(const char *fmt, ...)
 	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(AST_DEVICE_UNKNOWN, buf);
 }
 
 /*! \brief Go through the dev state change queue and update changes in the dev state thread */
diff --git a/main/event.c b/main/event.c
index 905790614..58515e007 100644
--- a/main/event.c
+++ b/main/event.c
@@ -35,7 +35,8 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 #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;
diff --git a/res/res_features.c b/res/res_features.c
index 1083948a5..af5ee4865 100644
--- a/res/res_features.c
+++ b/res/res_features.c
@@ -349,13 +349,12 @@ static int adsi_announce_park(struct ast_channel *chan, char *parkingexten)
 }
 
 /*! \brief Notify metermaids that we've changed an extension */
-static void notify_metermaids(const char *exten, char *context)
+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", exten, context);
+	ast_debug(4, "Notification of state change to metermaids %s@%s\n to state '%s'", 
+		exten, context, devstate2str(state));
 
-	/* Send notification to devicestate subsystem */
-	ast_device_state_changed("park:%s@%s", exten, context);
-	return;
+	ast_devstate_changed(state, "park:%s@%s", exten, context);
 }
 
 /*! \brief metermaids callback from devicestate.c */
@@ -493,7 +492,7 @@ int ast_park_call(struct ast_channel *chan, struct ast_channel *peer, int timeou
 		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 @@ static void *do_parking_thread(void *ignore)
 					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 @@ static void *do_parking_thread(void *ignore)
 							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 @@ static int park_exec(struct ast_channel *chan, void *data)
 			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 @@ static int load_config(void)
 	/* 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 @@ static int load_config(void)
 	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;
 
 }