[Asterisk-code-review] pjsip options: Fix non-qualified contacts showing as unavai... (asterisk[13])

Joshua Colp asteriskteam at digium.com
Mon Apr 20 17:23:56 CDT 2015


Joshua Colp has submitted this change and it was merged.

Change subject: pjsip_options:  Fix non-qualified contacts showing as unavailable
......................................................................


pjsip_options:  Fix non-qualified contacts showing as unavailable

The "Add qualify_timeout processing and eventing" patch introduced
an issue where contacts that had qualify_frequency set to 0 were
showing Unavailable instead Unknown.  This patch checks for
qualify_frequency=0 and create an "Unknown"  contact_status
with an RTT = 0.

Previously, the lack of contact_status implied Unknown but since
we're now changing endpoint state based on contact_status, I've
had to add new UNKNOWN status so that changes could trigger the
appropriate contact_status observers.

ASTERISK-24977: #close

Change-Id: Ifcbc01533ce57f0e4e584b89a395326e098b8fe7
---
M funcs/func_pjsip_contact.c
M include/asterisk/res_pjsip.h
M res/res_pjsip/location.c
M res/res_pjsip/pjsip_configuration.c
M res/res_pjsip/pjsip_options.c
5 files changed, 51 insertions(+), 41 deletions(-)

Approvals:
  Matt Jordan: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Verified



diff --git a/funcs/func_pjsip_contact.c b/funcs/func_pjsip_contact.c
index 0f5a204..005f29b 100644
--- a/funcs/func_pjsip_contact.c
+++ b/funcs/func_pjsip_contact.c
@@ -147,15 +147,9 @@
 	contact_status = ast_sorcery_retrieve_by_id(pjsip_sorcery, CONTACT_STATUS, ast_sorcery_object_get_id(contact_obj));
 
 	if (!strcmp(args.field_name, "status")) {
-		if (!contact_status) {
-			ast_str_set(buf, len, "%s", "Unknown");
-		} else if (contact_status->status == UNAVAILABLE) {
-			ast_str_set(buf, len, "%s", "Unreachable");
-		} else if (contact_status->status == AVAILABLE) {
-			ast_str_set(buf, len, "%s", "Reachable");
-		}
+		ast_str_set(buf, len, "%s", ast_sip_get_contact_status_label(contact_status->status));
 	} else if (!strcmp(args.field_name, "rtt")) {
-		if (!contact_status) {
+		if (contact_status->status == UNKNOWN) {
 			ast_str_set(buf, len, "%s", "N/A");
 		} else {
 			ast_str_set(buf, len, "%" PRId64, contact_status->rtt);
diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index e680950..a15e967 100644
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -177,7 +177,8 @@
  */
 enum ast_sip_contact_status_type {
 	UNAVAILABLE,
-	AVAILABLE
+	AVAILABLE,
+	UNKNOWN
 };
 
 /*!
@@ -2000,5 +2001,13 @@
  */
 unsigned int ast_sip_get_max_initial_qualify_time(void);
 
+/*!
+ * \brief translate ast_sip_contact_status_type to character string.
+ *
+ * \retval the character string equivalent.
+ */
+
+const char *ast_sip_get_contact_status_label(const enum ast_sip_contact_status_type status);
+const char *ast_sip_get_contact_short_status_label(const enum ast_sip_contact_status_type status);
 
 #endif /* _RES_PJSIP_H */
diff --git a/res/res_pjsip/location.c b/res/res_pjsip/location.c
index f784cb4..2165041 100644
--- a/res/res_pjsip/location.c
+++ b/res/res_pjsip/location.c
@@ -747,8 +747,8 @@
 		"Contact",
 		flexwidth, flexwidth,
 		wrapper->contact_id,
-		(status ? (status->status == AVAILABLE ? "Avail" : "Unavail") : "Unknown"),
-		(status ? ((long long) status->rtt) / 1000.0 : NAN));
+		ast_sip_get_contact_short_status_label(status->status),
+		(status->status != UNKNOWN ? ((long long) status->rtt) / 1000.0 : NAN));
 
 	return 0;
 }
diff --git a/res/res_pjsip/pjsip_configuration.c b/res/res_pjsip/pjsip_configuration.c
index a69e1c4..42f16d2 100644
--- a/res/res_pjsip/pjsip_configuration.c
+++ b/res/res_pjsip/pjsip_configuration.c
@@ -86,7 +86,7 @@
 			contact_status = ast_sorcery_retrieve_by_id(ast_sip_get_sorcery(),
 				CONTACT_STATUS, contact_id);
 
-			if (contact_status && contact_status->status == AVAILABLE) {
+			if (contact_status && contact_status->status != UNAVAILABLE) {
 				state = AST_ENDPOINT_ONLINE;
 			}
 			ao2_cleanup(contact_status);
@@ -184,10 +184,10 @@
 		"Contact: %s\r\n"
 			"Status: %s",
 		ast_sorcery_object_get_id(contact_status),
-		(contact_status->status == AVAILABLE ? "Available" : "Unavailable"));
+		ast_sip_get_contact_status_label(contact_status->status));
 
 	ast_verb(1, "Contact %s/%s is now %s\n", aor, contact,
-		contact_status->status == AVAILABLE ? "Available" : "Unavailable");
+		ast_sip_get_contact_status_label(contact_status->status));
 
 	ao2_callback(persistent_endpoints, OBJ_NODATA, persistent_endpoint_update_state, aor);
 }
diff --git a/res/res_pjsip/pjsip_options.c b/res/res_pjsip/pjsip_options.c
index 9c0a137..692cbdb 100644
--- a/res/res_pjsip/pjsip_options.c
+++ b/res/res_pjsip/pjsip_options.c
@@ -35,6 +35,28 @@
 #define DEFAULT_ENCODING "text/plain"
 #define QUALIFIED_BUCKETS 211
 
+static const char *status_map [] = {
+	[UNAVAILABLE] = "Unreachable",
+	[AVAILABLE] = "Reachable",
+	[UNKNOWN] = "Unknown",
+};
+
+static const char *short_status_map [] = {
+	[UNAVAILABLE] = "Unavail",
+	[AVAILABLE] = "Avail",
+	[UNKNOWN] = "Unknown",
+};
+
+const char *ast_sip_get_contact_status_label(const enum ast_sip_contact_status_type status)
+{
+	return status_map[status];
+}
+
+const char *ast_sip_get_contact_short_status_label(const enum ast_sip_contact_status_type status)
+{
+	return short_status_map[status];
+}
+
 /*!
  * \internal
  * \brief Create a ast_sip_contact_status object.
@@ -48,7 +70,7 @@
 		return NULL;
 	}
 
-	status->status = UNAVAILABLE;
+	status->status = UNKNOWN;
 
 	return status;
 }
@@ -86,19 +108,6 @@
 	return status;
 }
 
-static void delete_contact_status(const struct ast_sip_contact *contact)
-{
-	struct ast_sip_contact_status *status = ast_sorcery_retrieve_by_id(
-		ast_sip_get_sorcery(), CONTACT_STATUS, ast_sorcery_object_get_id(contact));
-
-	if (!status) {
-		return;
-	}
-
-	ast_sorcery_delete(ast_sip_get_sorcery(), status);
-	ao2_ref(status, -1);
-}
-
 /*!
  * \internal
  * \brief Update an ast_sip_contact_status's elements.
@@ -129,17 +138,19 @@
 
 	/* if the contact is available calculate the rtt as
 	   the diff between the last start time and "now" */
-	update->rtt = update->status == AVAILABLE ?
+	update->rtt = update->status == AVAILABLE && status->rtt_start.tv_sec > 0 ?
 		ast_tvdiff_us(ast_tvnow(), status->rtt_start) : 0;
 
 	update->rtt_start = ast_tv(0, 0);
+
+
 
 	ast_test_suite_event_notify("AOR_CONTACT_QUALIFY_RESULT",
 		"Contact: %s\r\n"
 			"Status: %s\r\n"
 			"RTT: %ld",
 		ast_sorcery_object_get_id(update),
-		(update->status == AVAILABLE ? "Available" : "Unavailable"),
+		ast_sip_get_contact_status_label(update->status),
 		update->rtt);
 
 	if (ast_sorcery_update(ast_sip_get_sorcery(), update)) {
@@ -499,7 +510,7 @@
 
 		schedule_qualify(contact, contact->qualify_frequency * 1000);
 	} else {
-		delete_contact_status(contact);
+		update_contact_status(contact, UNKNOWN);
 	}
 }
 
@@ -1011,6 +1022,8 @@
 
 	if (contact->qualify_frequency) {
 		schedule_qualify(contact, initial_interval);
+	} else {
+		update_contact_status(contact, UNKNOWN);
 	}
 
 	return 0;
@@ -1082,11 +1095,6 @@
 	ao2_ref(endpoints, -1);
 }
 
-static const char *status_map [] = {
-	[UNAVAILABLE] = "Unreachable",
-	[AVAILABLE] = "Reachable",
-};
-
 static int format_contact_status(void *obj, void *arg, int flags)
 {
 	struct ast_sip_contact_wrapper *wrapper = obj;
@@ -1107,12 +1115,11 @@
 
 	ast_str_append(&buf, 0, "AOR: %s\r\n", wrapper->aor_id);
 	ast_str_append(&buf, 0, "URI: %s\r\n", contact->uri);
-	if (status) {
-		ast_str_append(&buf, 0, "Status: %s\r\n", status_map[status->status]);
-		ast_str_append(&buf, 0, "RoundtripUsec: %" PRId64 "\r\n", status->rtt);
-	} else {
-		ast_str_append(&buf, 0, "Status: Unknown\r\n");
+	ast_str_append(&buf, 0, "Status: %s\r\n", ast_sip_get_contact_status_label(status->status));
+	if (status->status == UNKNOWN) {
 		ast_str_append(&buf, 0, "RoundtripUsec: N/A\r\n");
+	} else {
+		ast_str_append(&buf, 0, "RoundtripUsec: %" PRId64 "\r\n", status->rtt);
 	}
 	ast_str_append(&buf, 0, "EndpointName: %s\r\n",
 			ast_sorcery_object_get_id(endpoint));

-- 
To view, visit https://gerrit.asterisk.org/147
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ifcbc01533ce57f0e4e584b89a395326e098b8fe7
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>



More information about the asterisk-code-review mailing list