Hi Djalal,
On 12/16/2016 04:05 AM, Djalal Harouni wrote:
This adds support for +UCGED mode 2 and for now we collect only
EARFCN, EBAND and CQI. Later we will follow up with +UCGED mode 3.
Some ublox modems supports +UCGED mode 2 and others only +UCGED mode 3
depending on the firmware.
---
drivers/ubloxmodem/netmon.c | 210 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 206 insertions(+), 4 deletions(-)
This looks good. Just a couple of small nitpicks:
diff --git a/drivers/ubloxmodem/netmon.c
b/drivers/ubloxmodem/netmon.c
index 0749078..6fc3bad 100644
--- a/drivers/ubloxmodem/netmon.c
+++ b/drivers/ubloxmodem/netmon.c
@@ -46,6 +46,20 @@
static const char *cops_prefix[] = { "+COPS:", NULL };
static const char *cesq_prefix[] = { "+CESQ:", NULL };
+static const char *ucged_prefix[] = { "+UCGED:", NULL };
+
+/*
+ * Generic function that should point to number or hexstring result
+ * parsers. Result should be returned as an int.
+ */
+typedef gboolean (*ucged_g_at_result_iter_next)(GAtResultIter *iter,
+ int *result);
+
Maybe ucged_parse_field_t? However, I'm not sure about the function
signature here. g_at_result_iter_next_hexstring returns a guint8 * and
gsize value.
+/* Field from +UCGED response */
+typedef struct ublox_ucged_value {
+ int offset; /* Offset at response */
+ ucged_g_at_result_iter_next ucged_g_at_result_iter_next_field;
The name is unnecessarily long. Maybe parse_field?
+} ublox_ucged_value;
struct netmon_driver_data {
GAtChat *chat;
@@ -67,6 +81,10 @@ struct req_cb_data {
int rsrp; /* CESQ: Reference Signal Received Power */
int ecn0; /* CESQ: Received Energy Ratio */
int rsrq; /* CESQ: Reference Signal Received Quality */
+
+ int earfcn; /* UCGED: E-UTRA ARFCN */
+ int eband; /* UCGED: E-UTRA Band */
+ int cqi; /* UCGED: Channel Quality Indicator */
};
/*
@@ -102,6 +120,7 @@ static inline struct req_cb_data *req_cb_data_new0(void *cb, void
*data,
ret->cb = cb;
ret->data = data;
ret->netmon = user;
+
ret->rxlev = -1;
ret->ber = -1;
ret->rscp = -1;
@@ -109,6 +128,10 @@ static inline struct req_cb_data *req_cb_data_new0(void *cb, void
*data,
ret->ecn0 = -1;
ret->rsrq = -1;
+ ret->earfcn = -1;
+ ret->eband = -1;
+ ret->cqi = -1;
+
return ret;
}
@@ -157,11 +180,165 @@ static void ublox_netmon_finish_success(struct req_cb_data *cbd)
OFONO_NETMON_INFO_ECN0, cbd->ecn0,
OFONO_NETMON_INFO_RSRQ, cbd->rsrq,
OFONO_NETMON_INFO_RSRP, cbd->rsrp,
+ OFONO_NETMON_INFO_EARFCN, cbd->earfcn,
+ OFONO_NETMON_INFO_EBAND, cbd->eband,
+ OFONO_NETMON_INFO_CQI, cbd->cqi,
OFONO_NETMON_INFO_INVALID);
CALLBACK_WITH_SUCCESS(cbd->cb, cbd->data);
}
+static void ucged_parse_4g(struct req_cb_data *cbd, GAtResultIter *iter)
+{
+ /* Offset of values in returned response */
+ enum ucged_mode2_rat4_offsets {
+ UCGED_EARFCN = 0,
+ UCGED_EBAND = 1,
+ UCGED_CQI = 15,
+ };
+
+ ublox_ucged_value ucged_values[] = {
+ { UCGED_EARFCN, g_at_result_iter_next_number },
+ { UCGED_EBAND, g_at_result_iter_next_number },
+ { UCGED_CQI, g_at_result_iter_next_number },
+ { -1, NULL }, /* End of Array */
+ };
+
+ gboolean ok;
+ unsigned i;
+ ublox_ucged_value *cur;
+
+ /* Skip to next */
+ ok = g_at_result_iter_next(iter, NULL);
+ if (!ok) {
+ DBG(" UCGED:response: failed to go to next line ");
+ return;
+ }
+
+ i = 0;
+ cur = &ucged_values[0];
+ while (cur->offset >= 0) {
+ int number;
+
If you declare number as unsigned int you won't need the ugly cast below.
+ /* Skip values that we do not want */
+ while (i != (unsigned) cur->offset) {
+ ok = g_at_result_iter_skip_next(iter);
+ if (!ok) {
+ DBG(" UCGED: error skip idx: %d ", i);
+ return;
+ }
+ i++;
+ }
+
+ ok = cur->ucged_g_at_result_iter_next_field(iter, &number);
+ if (!ok) {
+ DBG(" UCGED: error parsing at idx: %d ", i);
+ return;
+ }
+
+ switch (i) {
+ case UCGED_EARFCN:
+ cbd->earfcn = number != 65535 ? number : cbd->earfcn;
+ break;
+ case UCGED_EBAND:
+ cbd->eband = number != 255 ? number : cbd->eband;
+ break;
+ case UCGED_CQI:
+ cbd->cqi = number != 255 ? number : cbd->cqi;
+ break;
+ }
+
+ i++;
+ cur++;
+ }
+
+ /*
+ * Sample output for +UCGED MODE 2 will be:
+ *
+ * UCGED: MODE 2 RAT = 4G EARFCN = 1300
+ * UCGED: MODE 2 RAT = 4G EBAND = 3
+ * UCGED: MODE 2 RAT = 4G CQI = -1
When I asked for sample output, I meant from the modem, not from this
function
+ *
+ * Check the networkmonitor-api documentation and the related
+ * references for mode details.
+ */
+ DBG(" UCGED: MODE 2 RAT = 4G EARFCN = %d ", cbd->earfcn);
+ DBG(" UCGED: MODE 2 RAT = 4G EBAND = %d ", cbd->eband);
+ DBG(" UCGED: MODE 2 RAT = 4G CQI = %d ", cbd->cqi);
+}
+
+static void ucged_collect_mode2_data(struct req_cb_data *cbd,
+ GAtResultIter *iter)
+{
+ int rat;
+ gboolean ok;
+
+ ok = g_at_result_iter_next(iter, NULL);
+ if (!ok) {
+ DBG(" UCGED:response: failed to go to next line ");
+ return;
+ }
+
+ ok = g_at_result_iter_next_number(iter, &rat);
+ if (!ok) {
+ DBG(" UCGED: error parsing 'RAT' ");
+ return;
+ }
+
+ /* For now we support only RAT 4 */
+ if (rat != 4) {
+ DBG(" UCGED: 'RAT' is %d, nothing to do. ", rat);
+ return;
+ }
+
+ DBG(" UCGED: 'RAT' is %d ", rat);
+ ucged_parse_4g(cbd, iter);
+}
+
+static void ucged_query_mode2_cb(gboolean ok, GAtResult *result,
+ gpointer user_data)
+{
+ struct req_cb_data *cbd = user_data;
+ struct ofono_error error;
+ GAtResultIter iter;
+ int mode;
+
+ DBG("ok %d", ok);
+
+ decode_at_error(&error, g_at_result_final_response(result));
+
+ if (!ok) {
+ DBG(" UCGED: failed ");
+ goto out;
+ }
+
+ g_at_result_iter_init(&iter, result);
+
+ if (!g_at_result_iter_next(&iter, "+UCGED:")) {
+ DBG(" UCGED: no result ");
+ goto out;
+ }
+
+ ok = g_at_result_iter_next_number(&iter, &mode);
+ if (!ok) {
+ DBG(" UCGED: error parsing 'mode' ");
+ goto out;
+ }
+
+ DBG(" UCGED: report mode is %d ", mode);
+
+ /* mode 2 collect +UCGED data */
+ if (mode == 2)
+ ucged_collect_mode2_data(cbd, &iter);
+
+ /*
+ * We never fail at this point we always send what we collected so
+ * far
+ */
+out:
+ ublox_netmon_finish_success(cbd);
+}
+
static void cesq_cb(gboolean ok, GAtResult *result, gpointer user_data)
{
enum cesq_ofono_netmon_info {
@@ -175,6 +352,8 @@ static void cesq_cb(gboolean ok, GAtResult *result, gpointer
user_data)
};
struct req_cb_data *cbd = user_data;
+ struct ofono_netmon *nm = cbd->netmon;
+ struct netmon_driver_data *nmd = ofono_netmon_get_data(nm);
struct ofono_error error;
GAtResultIter iter;
int idx, number;
@@ -233,10 +412,17 @@ static void cesq_cb(gboolean ok, GAtResult *result, gpointer
user_data)
DBG(" RSRQ %d ", cbd->rsrq);
DBG(" RSRP %d ", cbd->rsrp);
- /*
- * We never fail at this point we always send what we collected so
- * far
- */
+ cbd = req_cb_data_ref(cbd);
+ if (g_at_chat_send(nmd->chat, "AT+UCGED?", NULL,
+ ucged_query_mode2_cb, cbd, req_cb_data_unref) == 0) {
+ /* We never fail, we always send what we collected so far */
+ ofono_error("%s: send +UCGED failed", __func__);
+ req_cb_data_unref(cbd);
+ goto out;
+ }
+
+ return;
+
out:
ublox_netmon_finish_success(cbd);
}
@@ -320,6 +506,22 @@ static int ublox_netmon_probe(struct ofono_netmon *netmon,
g_idle_add(ublox_delayed_register, netmon);
I still think we should remove ublox_delayed_register and move the
ofono_netmon_register call into the UCGED=2 callback below.
+ /*
+ * We set +UCGED=2 mode as early as possible so we are able to handle
+ * the following:
+ * 1) In case the modem supports only +UCGED mode 2 then we have
+ * already set the appropriate mode and we can query the information
+ * later.
+ * 2) In case the modem supports only +UCGED mode 3, then to set mode 3
+ * the modem has to be first in +UCGED mode 2. Setting the mode here
+ * allows later to query the information and on errors we fallback
+ * to mode 3.
+ *
+ * This should handle current firmware versions of ublox modems.
+ */
+ g_at_chat_send(nmd->chat, "AT+UCGED=2", ucged_prefix,
+ NULL, NULL, NULL);
+
return 0;
}
Regards,
-Denis