[PATCH v3 0/7] ubloxmodem add netmon interface

[PATCH v3 0/7] ubloxmodem add netmon interface

[Djalal Harouni]

[-- Attachment #1: Type: text/plain, Size: 2349 bytes --]

Hi,

These patches add netmon interface to ubloxmodem. This allows to query
and get data returned by +CESQ command through D-Bus.

The following types will be exposed as Network Monitor Property Types.
RSCP: Received Signal Code Power
ECN0: Received Energy Ratio
RSRQ: Reference Signal Received Quality
RSRP: Reference Signal Received Power


Thank you!

Changes since v2
  * Fixed subject of header message: we have 0/7 patches and not 0/5
  * Tag all subject of patches with v3, easy to read.
  * Fixed a bug we have to return when finishing please see:
    https://lists.ofono.org/pipermail/ofono/2016-November/016693.html


Changes since v1:
  * Re-worked patches to follow 'Submitting patches' section.
    Each top-level directory change should be in a separate patch
  * Removed OFONO_NETMON_INFO_OPERATOR
  * Documented all added fields in networkmonitor-api.txt
  * We only use one channel for communication, dropped the other one.
  * Fixed a cbd memory leak, now we try to be conservative and always
    report what we have collected so far. We only fail hard in case
    +COPS failed or sending +CESQ failed. Otherwise we report the result
    and make sure to free cbd.
  * Various fixes to follow the coding style.


v2 is here:
https://lists.ofono.org/pipermail/ofono/2016-November/016685.html

V1 is here:
https://lists.ofono.org/pipermail/ofono/2016-November/016671.html


Djalal Harouni (7):
  netmon: add OFONO_NETMON_INFO_{RSCP|ECN0|RSRQ|RSRP}
  netmon: handle OFONO_NETMON_INFO_{RSCP|ECN0|RSRQ|RSRP} in D-Bus
  doc: documentation for OFONO_NETMON_INFO_{RSCP|ECN0|RSRQ|RSRP}
  ubloxmodem: add the netmon driver
  ubloxmodem: register and initialize the netmon driver
  build: build the ublox netmon driver
  test: support OFONO_NETMON_INFO_{RXLEV|RSCP|ECN0|RSRQ|RSRP}

 Makefile.am                     |   1 +
 doc/networkmonitor-api.txt      |  22 +++
 drivers/ubloxmodem/netmon.c     | 335 ++++++++++++++++++++++++++++++++++++++++
 drivers/ubloxmodem/ubloxmodem.c |   2 +
 drivers/ubloxmodem/ubloxmodem.h |   3 +
 include/netmon.h                |   4 +
 plugins/ublox.c                 |   3 +
 src/netmon.c                    |  28 ++++
 test/get-serving-cell-info      |  20 +++
 9 files changed, 418 insertions(+)
 create mode 100644 drivers/ubloxmodem/netmon.c

[PATCH v3 1/7] netmon: add OFONO_NETMON_INFO_{RSCP|ECN0|RSRQ|RSRP}

[Djalal Harouni]

[-- Attachment #1: Type: text/plain, Size: 884 bytes --]

Add more ofono netmon info types that will be served through the netmon
interface. The main user of this now will be the ublox modem.

RSCP: Received Signal Code Power
ECN0: Received Energy Ratio
RSRQ: Reference Signal Received Quality
RSRP: Reference Signal Received Power

Patches using these types to follow.
---
 include/netmon.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/netmon.h b/include/netmon.h
index ec8a2e1..47d7a9c 100644
--- a/include/netmon.h
+++ b/include/netmon.h
@@ -59,6 +59,10 @@ enum ofono_netmon_info {
 	OFONO_NETMON_INFO_RSSI, /* int */
 	OFONO_NETMON_INFO_TIMING_ADVANCE, /* int */
 	OFONO_NETMON_INFO_PSC, /* int */
+	OFONO_NETMON_INFO_RSCP, /* int */
+	OFONO_NETMON_INFO_ECN0, /* int */
+	OFONO_NETMON_INFO_RSRQ, /* int */
+	OFONO_NETMON_INFO_RSRP, /* int */
 	OFONO_NETMON_INFO_INVALID,
 };
 
-- 
2.5.5

Re: [PATCH v3 1/7] netmon: add OFONO_NETMON_INFO_{RSCP|ECN0|RSRQ|RSRP}

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 562 bytes --]

Hi Djalal,

On 11/30/2016 06:31 AM, Djalal Harouni wrote:
> Add more ofono netmon info types that will be served through the netmon
> interface. The main user of this now will be the ublox modem.
>
> RSCP: Received Signal Code Power
> ECN0: Received Energy Ratio
> RSRQ: Reference Signal Received Quality
> RSRP: Reference Signal Received Power
>
> Patches using these types to follow.
> ---
>   include/netmon.h | 4 ++++
>   1 file changed, 4 insertions(+)
>

I tweaked the commit header and applied this patch.  Thanks!

Regards,
-Denis

[PATCH v3 2/7] netmon: handle OFONO_NETMON_INFO_{RSCP|ECN0|RSRQ|RSRP} in D-Bus

[Djalal Harouni]

[-- Attachment #1: Type: text/plain, Size: 1278 bytes --]

Handle the previously added types in D-Bus.

Documentation for these values is in next patch.
---
 src/netmon.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/src/netmon.c b/src/netmon.c
index eb18b9c..2a8c524 100644
--- a/src/netmon.c
+++ b/src/netmon.c
@@ -180,6 +180,34 @@ void ofono_netmon_serving_cell_notify(struct ofono_netmon *netmon,
 					intval, uint8_t, DBUS_TYPE_BYTE);
 			break;
 
+		case OFONO_NETMON_INFO_RSCP:
+			intval = va_arg(arglist, int);
+
+			CELL_INFO_DICT_APPEND(&dict, "ReceivedSignalCodePower",
+					intval, uint8_t, DBUS_TYPE_BYTE);
+			break;
+
+		case OFONO_NETMON_INFO_ECN0:
+			intval = va_arg(arglist, int);
+
+			CELL_INFO_DICT_APPEND(&dict, "ReceivedEnergyRatio",
+					intval, uint8_t, DBUS_TYPE_BYTE);
+			break;
+
+		case OFONO_NETMON_INFO_RSRQ:
+			intval = va_arg(arglist, int);
+
+			CELL_INFO_DICT_APPEND(&dict, "ReferenceSignalReceivedQuality",
+					intval, uint8_t, DBUS_TYPE_BYTE);
+			break;
+
+		case OFONO_NETMON_INFO_RSRP:
+			intval = va_arg(arglist, int);
+
+			CELL_INFO_DICT_APPEND(&dict, "ReferenceSignalReceivedPower",
+					intval, uint8_t, DBUS_TYPE_BYTE);
+			break;
+
 		case OFONO_NETMON_INFO_INVALID:
 			break;
 		}
-- 
2.5.5

Re: [PATCH v3 2/7] netmon: handle OFONO_NETMON_INFO_{RSCP|ECN0|RSRQ|RSRP} in D-Bus

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1613 bytes --]

Hi Djalal,

On 11/30/2016 06:31 AM, Djalal Harouni wrote:
> Handle the previously added types in D-Bus.
>
> Documentation for these values is in next patch.
> ---
>   src/netmon.c | 28 ++++++++++++++++++++++++++++
>   1 file changed, 28 insertions(+)
>

Tweaked the commit description and ...

> diff --git a/src/netmon.c b/src/netmon.c
> index eb18b9c..2a8c524 100644
> --- a/src/netmon.c
> +++ b/src/netmon.c
> @@ -180,6 +180,34 @@ void ofono_netmon_serving_cell_notify(struct ofono_netmon *netmon,
>   					intval, uint8_t, DBUS_TYPE_BYTE);
>   			break;
>
> +		case OFONO_NETMON_INFO_RSCP:
> +			intval = va_arg(arglist, int);
> +
> +			CELL_INFO_DICT_APPEND(&dict, "ReceivedSignalCodePower",
> +					intval, uint8_t, DBUS_TYPE_BYTE);
> +			break;
> +
> +		case OFONO_NETMON_INFO_ECN0:
> +			intval = va_arg(arglist, int);
> +
> +			CELL_INFO_DICT_APPEND(&dict, "ReceivedEnergyRatio",
> +					intval, uint8_t, DBUS_TYPE_BYTE);
> +			break;
> +
> +		case OFONO_NETMON_INFO_RSRQ:
> +			intval = va_arg(arglist, int);
> +
> +			CELL_INFO_DICT_APPEND(&dict, "ReferenceSignalReceivedQuality",
> +					intval, uint8_t, DBUS_TYPE_BYTE);
> +			break;
> +
> +		case OFONO_NETMON_INFO_RSRP:
> +			intval = va_arg(arglist, int);
> +
> +			CELL_INFO_DICT_APPEND(&dict, "ReferenceSignalReceivedPower",
> +					intval, uint8_t, DBUS_TYPE_BYTE);
> +			break;
> +

The above 2 CELL_INFO_DICT_APPEND lines were > 80 characters.  So I 
amended the patch to fix that.

>   		case OFONO_NETMON_INFO_INVALID:
>   			break;
>   		}
>

Applied, thanks!

Regards,
-Denis

[PATCH v3 3/7] doc: documentation for OFONO_NETMON_INFO_{RSCP|ECN0|RSRQ|RSRP}

[Djalal Harouni]

[-- Attachment #1: Type: text/plain, Size: 1577 bytes --]

This adds documentation for the following fields in
networkmonitor-api.txt

RSCP: Received Signal Code Power
ECN0: Received Energy Ratio
RSRQ: Reference Signal Received Quality
RSRP: Reference Signal Received Power
---
 doc/networkmonitor-api.txt | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/doc/networkmonitor-api.txt b/doc/networkmonitor-api.txt
index 703f19b..0d2fe3f 100644
--- a/doc/networkmonitor-api.txt
+++ b/doc/networkmonitor-api.txt
@@ -81,3 +81,25 @@ byte Strength [optional, gsm, umts]
 
 	Contains the signal strength.  Valid values are 0-31.  Refer to <rssi>
 	in 27.007, Section 8.5.
+
+byte ReceivedSignalCodePower [optional, umts]
+
+        Contains the Received Signal Code Power.  Valid range of values
+        is 0-96. Refer to <rscp> in 27.007, Section 8.69 for more details.
+
+byte ReceivedEnergyRatio [optional, umts]
+
+        Contains the Ratio of received energy per PN chip to the total
+        received power spectral density.  Valid range of values is 0-49.
+        Refer to <ecno> in 27.007, Section 8.69 for more details.
+
+byte ReferenceSignalReceivedQuality [optional, lte]
+
+        Contains the Reference Signal Received Quality.  Valid range of
+        values is 0-34. Refer to <rsrq> in 27.007, Section 8.69 for more
+        details.
+
+byte ReferenceSignalReceivedPower [optional, lte]
+
+        Contains the Reference Signal Received Power.  Valid range of values
+        is 0-97. Refer to <rsrp> in 27.007, Section 8.69 for more details.
-- 
2.5.5

Re: [PATCH v3 3/7] doc: documentation for OFONO_NETMON_INFO_{RSCP|ECN0|RSRQ|RSRP}

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 491 bytes --]

Hi Djalal,

On 11/30/2016 06:31 AM, Djalal Harouni wrote:
> This adds documentation for the following fields in
> networkmonitor-api.txt
>
> RSCP: Received Signal Code Power
> ECN0: Received Energy Ratio
> RSRQ: Reference Signal Received Quality
> RSRP: Reference Signal Received Power
> ---
>   doc/networkmonitor-api.txt | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
>

Tweaked the commit description slightly and applied.  Thanks!

Regards,
-Denis

[PATCH v3 4/7] ubloxmodem: add the netmon driver

[Djalal Harouni]

[-- Attachment #1: Type: text/plain, Size: 9966 bytes --]

This adds a netmon driver for ublox. The driver support both +COPS and
+CESQ commands to return the previously added ofono netmon types:

RSCP: Received Signal Code Power
ECN0: Received Energy Ratio
RSRQ: Reference Signal Received Quality
RSRP: Reference Signal Received Power
---
 drivers/ubloxmodem/netmon.c     | 336 ++++++++++++++++++++++++++++++++++++++++
 drivers/ubloxmodem/ubloxmodem.c |   2 +
 drivers/ubloxmodem/ubloxmodem.h |   3 +
 3 files changed, 341 insertions(+)
 create mode 100644 drivers/ubloxmodem/netmon.c

diff --git a/drivers/ubloxmodem/netmon.c b/drivers/ubloxmodem/netmon.c
new file mode 100644
index 0000000..e5adf59
--- /dev/null
+++ b/drivers/ubloxmodem/netmon.c
@@ -0,0 +1,336 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2016  EndoCode AG. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <string.h>
+#include <errno.h>
+
+#include <glib.h>
+
+#include <ofono/log.h>
+#include <ofono/modem.h>
+#include <ofono/netmon.h>
+
+#include "gatchat.h"
+#include "gatresult.h"
+
+#include "common.h"
+#include "netreg.h"
+#include "ubloxmodem.h"
+#include "drivers/atmodem/vendor.h"
+
+static const char *cops_prefix[] = { "+COPS:", NULL };
+static const char *cesq_prefix[] = { "+CESQ:", NULL };
+
+struct netmon_driver_data {
+	GAtChat *chat;
+};
+
+struct req_cb_data {
+	struct ofono_netmon *netmon;
+
+	ofono_netmon_cb_t cb;
+	void *data;
+
+	struct ofono_network_operator op;
+
+	int rxlev;	/* CESQ: Received Signal Strength Indication */
+	int ber;	/* CESQ: Bit Error Rate */
+	int rscp;	/* CESQ: Received Signal Code Powe */
+	int rsrp;	/* CESQ: Reference Signal Received Power */
+	int ecn0;	/* CESQ: Received Energy Ratio */
+	int rsrq;	/* CESQ: Reference Signal Received Quality */
+};
+
+/*
+ * Returns the appropriate radio access technology.
+ *
+ * If we can not resolve to a specific radio access technolgy
+ * we return OFONO_NETMON_CELL_TYPE_GSM by default.
+ */
+static int ublox_map_radio_access_technology(int tech)
+{
+	switch (tech) {
+	case ACCESS_TECHNOLOGY_GSM:
+	case ACCESS_TECHNOLOGY_GSM_COMPACT:
+		return OFONO_NETMON_CELL_TYPE_GSM;
+	case ACCESS_TECHNOLOGY_UTRAN:
+	case ACCESS_TECHNOLOGY_UTRAN_HSDPA:
+	case ACCESS_TECHNOLOGY_UTRAN_HSUPA:
+	case ACCESS_TECHNOLOGY_UTRAN_HSDPA_HSUPA:
+		return OFONO_NETMON_CELL_TYPE_UMTS;
+	case ACCESS_TECHNOLOGY_EUTRAN:
+		return OFONO_NETMON_CELL_TYPE_LTE;
+	}
+
+	return OFONO_NETMON_CELL_TYPE_GSM;
+}
+
+static inline struct req_cb_data *req_cb_data_new0(void *cb, void *data, void *user)
+{
+	struct req_cb_data *ret = g_new0(struct req_cb_data, 1);
+	if (ret == NULL)
+		return NULL;
+
+	ret->cb = cb;
+	ret->data = data;
+	ret->netmon = user;
+	ret->rxlev = -1;
+	ret->ber = -1;
+	ret->rscp = -1;
+	ret->rsrp = -1;
+	ret->ecn0 = -1;
+	ret->rsrq = -1;
+
+	return ret;
+}
+
+static gboolean ublox_delayed_register(gpointer user_data)
+{
+	struct ofono_netmon *netmon = user_data;
+
+	ofono_netmon_register(netmon);
+
+	return FALSE;
+}
+
+static void ublox_netmon_finish_success(struct req_cb_data *cbd)
+{
+	struct ofono_netmon *nm = cbd->netmon;
+
+	ofono_netmon_serving_cell_notify(nm,
+					 cbd->op.tech,
+					 OFONO_NETMON_INFO_RXLEV, cbd->rxlev,
+					 OFONO_NETMON_INFO_BER, cbd->ber,
+					 OFONO_NETMON_INFO_RSCP, cbd->rscp,
+					 OFONO_NETMON_INFO_ECN0, cbd->ecn0,
+					 OFONO_NETMON_INFO_RSRQ, cbd->rsrq,
+					 OFONO_NETMON_INFO_RSRP, cbd->rsrp,
+					 OFONO_NETMON_INFO_INVALID);
+
+	CALLBACK_WITH_SUCCESS(cbd->cb, cbd->data);
+	g_free(cbd);
+}
+
+static void cesq_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	enum cesq_ofono_netmon_info {
+		CESQ_RXLEV,
+		CESQ_BER,
+		CESQ_RSCP,
+		CESQ_ECN0,
+		CESQ_RSRQ,
+		CESQ_RSRP,
+		_MAX,
+	};
+
+	struct req_cb_data *cbd = user_data;
+	struct ofono_error error;
+	GAtResultIter iter;
+	int idx, number;
+
+	DBG("ok %d", ok);
+
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	if (!ok)
+		goto error;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+CESQ:")) {
+		DBG(" CESQ: no result ");
+		goto out;
+	}
+
+	for (idx = 0; idx < _MAX; idx++) {
+
+		ok = g_at_result_iter_next_number(&iter, &number);
+		if (!ok) {
+			/* Ignore and do not fail */
+			DBG(" CESQ: error parsing idx: %d ", idx);
+			goto out;
+		}
+
+		switch (idx) {
+		case CESQ_RXLEV:
+			cbd->rxlev = number != 99 ? number:cbd->rxlev;
+			break;
+		case CESQ_BER:
+			cbd->ber = number != 99 ? number:cbd->ber;
+			break;
+		case CESQ_RSCP:
+			cbd->rscp = number != 255 ? number:cbd->rscp;
+			break;
+		case CESQ_ECN0:
+			cbd->ecn0 = number != 255 ? number:cbd->ecn0;
+			break;
+		case CESQ_RSRQ:
+			cbd->rsrq = number != 255 ? number:cbd->rsrq;
+			break;
+		case CESQ_RSRP:
+			cbd->rsrp = number != 255 ? number:cbd->rsrp;
+			break;
+		}
+	}
+
+	DBG(" RXLEV	%d ", cbd->rxlev);
+	DBG(" BER	%d ", cbd->ber);
+	DBG(" RSCP	%d ", cbd->rscp);
+	DBG(" ECN0	%d ", cbd->ecn0);
+	DBG(" RSRQ	%d ", cbd->rsrq);
+	DBG(" RSRP	%d ", cbd->rsrp);
+
+	/* We never fail at this point we always send what we collected so far */
+out:
+	ublox_netmon_finish_success(cbd);
+	return;
+
+error:
+	CALLBACK_WITH_FAILURE(cbd->cb, cbd->data);
+	g_free(cbd);
+}
+
+static void cops_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 tech;
+
+	DBG("ok %d", ok);
+
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	if (!ok)
+		goto error;
+
+	g_at_result_iter_init(&iter, result);
+
+	/* Do not fail */
+	if (!g_at_result_iter_next(&iter, "+COPS:"))
+		goto out;
+
+	g_at_result_iter_skip_next(&iter);
+	g_at_result_iter_skip_next(&iter);
+	g_at_result_iter_skip_next(&iter);
+
+	/* Default to GSM */
+	if (g_at_result_iter_next_number(&iter, &tech) == FALSE)
+		cbd->op.tech = ublox_map_radio_access_technology(ACCESS_TECHNOLOGY_GSM);
+	else
+		cbd->op.tech = ublox_map_radio_access_technology(tech);
+
+	if (g_at_chat_send(nmd->chat, "AT+CESQ", cesq_prefix,
+			   cesq_cb, cbd, NULL)) {
+		return;
+	}
+
+out:
+	CALLBACK_WITH_SUCCESS(cbd->cb, cbd->data);
+	g_free(cbd);
+	return;
+
+error:
+	CALLBACK_WITH_FAILURE(cbd->cb, cbd->data);
+	g_free(cbd);
+}
+
+static void ublox_netmon_request_update(struct ofono_netmon *netmon,
+					ofono_netmon_cb_t cb, void *data)
+{
+	struct netmon_driver_data *nmd = ofono_netmon_get_data(netmon);
+	struct req_cb_data *cbd;
+
+	DBG("ublox netmon request update");
+
+	cbd = req_cb_data_new0(cb, data, netmon);
+	if (!cbd) {
+		CALLBACK_WITH_FAILURE(cb, data);
+		return;
+	}
+
+	if (g_at_chat_send(nmd->chat, "AT+COPS?", cops_prefix,
+			   cops_cb, cbd, NULL) == 0) {
+		CALLBACK_WITH_FAILURE(cb, data);
+		g_free(cbd);
+	}
+}
+
+static int ublox_netmon_probe(struct ofono_netmon *netmon,
+			      unsigned int vendor, void *user)
+{
+	GAtChat *chat = user;
+	struct netmon_driver_data *nmd;
+
+	DBG("ublox netmon probe");
+
+	nmd = g_try_new0(struct netmon_driver_data, 1);
+	if (nmd == NULL)
+		return -ENOMEM;
+
+	nmd->chat = g_at_chat_clone(chat);
+
+	ofono_netmon_set_data(netmon, nmd);
+
+	g_idle_add(ublox_delayed_register, netmon);
+
+	return 0;
+}
+
+static void ublox_netmon_remove(struct ofono_netmon *netmon)
+{
+	struct netmon_driver_data *nmd = ofono_netmon_get_data(netmon);
+
+	DBG("ublox netmon remove");
+
+	g_at_chat_unref(nmd->chat);
+
+	ofono_netmon_set_data(netmon, NULL);
+
+	g_free(nmd);
+}
+
+static struct ofono_netmon_driver driver = {
+	.name			= UBLOXMODEM,
+	.probe			= ublox_netmon_probe,
+	.remove			= ublox_netmon_remove,
+	.request_update		= ublox_netmon_request_update,
+};
+
+void ublox_netmon_init(void)
+{
+	ofono_netmon_driver_register(&driver);
+}
+
+void ublox_netmon_exit(void)
+{
+	ofono_netmon_driver_unregister(&driver);
+}
diff --git a/drivers/ubloxmodem/ubloxmodem.c b/drivers/ubloxmodem/ubloxmodem.c
index 93cb928..a325b1f 100644
--- a/drivers/ubloxmodem/ubloxmodem.c
+++ b/drivers/ubloxmodem/ubloxmodem.c
@@ -36,6 +36,7 @@
 static int ubloxmodem_init(void)
 {
 	ublox_gprs_context_init();
+	ublox_netmon_init();
 	ublox_lte_init();
 
 	return 0;
@@ -44,6 +45,7 @@ static int ubloxmodem_init(void)
 static void ubloxmodem_exit(void)
 {
 	ublox_gprs_context_exit();
+	ublox_netmon_exit();
 	ublox_lte_exit();
 }
 
diff --git a/drivers/ubloxmodem/ubloxmodem.h b/drivers/ubloxmodem/ubloxmodem.h
index cf66412..bfb0106 100644
--- a/drivers/ubloxmodem/ubloxmodem.h
+++ b/drivers/ubloxmodem/ubloxmodem.h
@@ -26,5 +26,8 @@
 extern void ublox_gprs_context_init(void);
 extern void ublox_gprs_context_exit(void);
 
+extern void ublox_netmon_init(void);
+extern void ublox_netmon_exit(void);
+
 extern void ublox_lte_init(void);
 extern void ublox_lte_exit(void);
-- 
2.5.5

Re: [PATCH v3 4/7] ubloxmodem: add the netmon driver

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 12571 bytes --]

Hi Djalal,

On 11/30/2016 06:31 AM, Djalal Harouni wrote:
> This adds a netmon driver for ublox. The driver support both +COPS and
> +CESQ commands to return the previously added ofono netmon types:
>
> RSCP: Received Signal Code Power
> ECN0: Received Energy Ratio
> RSRQ: Reference Signal Received Quality
> RSRP: Reference Signal Received Power
> ---
>   drivers/ubloxmodem/netmon.c     | 336 ++++++++++++++++++++++++++++++++++++++++
>   drivers/ubloxmodem/ubloxmodem.c |   2 +
>   drivers/ubloxmodem/ubloxmodem.h |   3 +
>   3 files changed, 341 insertions(+)
>   create mode 100644 drivers/ubloxmodem/netmon.c
>
> diff --git a/drivers/ubloxmodem/netmon.c b/drivers/ubloxmodem/netmon.c
> new file mode 100644
> index 0000000..e5adf59
> --- /dev/null
> +++ b/drivers/ubloxmodem/netmon.c
> @@ -0,0 +1,336 @@
> +/*
> + *
> + *  oFono - Open Source Telephony
> + *
> + *  Copyright (C) 2016  EndoCode AG. All rights reserved.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <string.h>
> +#include <errno.h>
> +
> +#include <glib.h>
> +
> +#include <ofono/log.h>
> +#include <ofono/modem.h>
> +#include <ofono/netmon.h>
> +
> +#include "gatchat.h"
> +#include "gatresult.h"
> +
> +#include "common.h"
> +#include "netreg.h"

This should probably be <ofono/netreg.h> and moved up above.

> +#include "ubloxmodem.h"
> +#include "drivers/atmodem/vendor.h"
> +
> +static const char *cops_prefix[] = { "+COPS:", NULL };
> +static const char *cesq_prefix[] = { "+CESQ:", NULL };
> +
> +struct netmon_driver_data {
> +	GAtChat *chat;
> +};
> +
> +struct req_cb_data {
> +	struct ofono_netmon *netmon;
> +
> +	ofono_netmon_cb_t cb;
> +	void *data;
> +
> +	struct ofono_network_operator op;
> +
> +	int rxlev;	/* CESQ: Received Signal Strength Indication */
> +	int ber;	/* CESQ: Bit Error Rate */
> +	int rscp;	/* CESQ: Received Signal Code Powe */
> +	int rsrp;	/* CESQ: Reference Signal Received Power */
> +	int ecn0;	/* CESQ: Received Energy Ratio */
> +	int rsrq;	/* CESQ: Reference Signal Received Quality */
> +};
> +
> +/*
> + * Returns the appropriate radio access technology.
> + *
> + * If we can not resolve to a specific radio access technolgy
> + * we return OFONO_NETMON_CELL_TYPE_GSM by default.
> + */
> +static int ublox_map_radio_access_technology(int tech)
> +{
> +	switch (tech) {
> +	case ACCESS_TECHNOLOGY_GSM:
> +	case ACCESS_TECHNOLOGY_GSM_COMPACT:
> +		return OFONO_NETMON_CELL_TYPE_GSM;
> +	case ACCESS_TECHNOLOGY_UTRAN:
> +	case ACCESS_TECHNOLOGY_UTRAN_HSDPA:
> +	case ACCESS_TECHNOLOGY_UTRAN_HSUPA:
> +	case ACCESS_TECHNOLOGY_UTRAN_HSDPA_HSUPA:
> +		return OFONO_NETMON_CELL_TYPE_UMTS;
> +	case ACCESS_TECHNOLOGY_EUTRAN:
> +		return OFONO_NETMON_CELL_TYPE_LTE;
> +	}
> +
> +	return OFONO_NETMON_CELL_TYPE_GSM;
> +}
> +
> +static inline struct req_cb_data *req_cb_data_new0(void *cb, void *data, void *user)

This line > 80 chars

> +{
> +	struct req_cb_data *ret = g_new0(struct req_cb_data, 1);

g_new0 cannot fail.  g_try_new0 can, but its not useful in this case. 
I'd just keep g_new0 and skip the ret checking below, as well as inside 
request_update()

> +	if (ret == NULL)
> +		return NULL;
> +
> +	ret->cb = cb;
> +	ret->data = data;
> +	ret->netmon = user;
> +	ret->rxlev = -1;
> +	ret->ber = -1;
> +	ret->rscp = -1;
> +	ret->rsrp = -1;
> +	ret->ecn0 = -1;
> +	ret->rsrq = -1;
> +
> +	return ret;
> +}
> +
> +static gboolean ublox_delayed_register(gpointer user_data)
> +{
> +	struct ofono_netmon *netmon = user_data;
> +
> +	ofono_netmon_register(netmon);
> +
> +	return FALSE;
> +}
> +
> +static void ublox_netmon_finish_success(struct req_cb_data *cbd)
> +{
> +	struct ofono_netmon *nm = cbd->netmon;
> +
> +	ofono_netmon_serving_cell_notify(nm,
> +					 cbd->op.tech,
> +					 OFONO_NETMON_INFO_RXLEV, cbd->rxlev,
> +					 OFONO_NETMON_INFO_BER, cbd->ber,
> +					 OFONO_NETMON_INFO_RSCP, cbd->rscp,
> +					 OFONO_NETMON_INFO_ECN0, cbd->ecn0,
> +					 OFONO_NETMON_INFO_RSRQ, cbd->rsrq,
> +					 OFONO_NETMON_INFO_RSRP, cbd->rsrp,
> +					 OFONO_NETMON_INFO_INVALID);

Mixing tabs & spaces here for indentation.  Only tabs please.

> +
> +	CALLBACK_WITH_SUCCESS(cbd->cb, cbd->data);
> +	g_free(cbd);
> +}
> +
> +static void cesq_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	enum cesq_ofono_netmon_info {
> +		CESQ_RXLEV,
> +		CESQ_BER,
> +		CESQ_RSCP,
> +		CESQ_ECN0,
> +		CESQ_RSRQ,
> +		CESQ_RSRP,
> +		_MAX,
> +	};
> +
> +	struct req_cb_data *cbd = user_data;
> +	struct ofono_error error;
> +	GAtResultIter iter;
> +	int idx, number;
> +
> +	DBG("ok %d", ok);
> +
> +	decode_at_error(&error, g_at_result_final_response(result));
> +
> +	if (!ok)
> +		goto error;
> +
> +	g_at_result_iter_init(&iter, result);
> +
> +	if (!g_at_result_iter_next(&iter, "+CESQ:")) {
> +		DBG(" CESQ: no result ");
> +		goto out;
> +	}
> +
> +	for (idx = 0; idx < _MAX; idx++) {
> +
> +		ok = g_at_result_iter_next_number(&iter, &number);
> +		if (!ok) {
> +			/* Ignore and do not fail */
> +			DBG(" CESQ: error parsing idx: %d ", idx);
> +			goto out;
> +		}
> +
> +		switch (idx) {
> +		case CESQ_RXLEV:
> +			cbd->rxlev = number != 99 ? number:cbd->rxlev;
> +			break;
> +		case CESQ_BER:
> +			cbd->ber = number != 99 ? number:cbd->ber;
> +			break;
> +		case CESQ_RSCP:
> +			cbd->rscp = number != 255 ? number:cbd->rscp;
> +			break;
> +		case CESQ_ECN0:
> +			cbd->ecn0 = number != 255 ? number:cbd->ecn0;
> +			break;
> +		case CESQ_RSRQ:
> +			cbd->rsrq = number != 255 ? number:cbd->rsrq;
> +			break;
> +		case CESQ_RSRP:
> +			cbd->rsrp = number != 255 ? number:cbd->rsrp;
> +			break;
> +		}
> +	}
> +
> +	DBG(" RXLEV	%d ", cbd->rxlev);
> +	DBG(" BER	%d ", cbd->ber);
> +	DBG(" RSCP	%d ", cbd->rscp);
> +	DBG(" ECN0	%d ", cbd->ecn0);
> +	DBG(" RSRQ	%d ", cbd->rsrq);
> +	DBG(" RSRP	%d ", cbd->rsrp);
> +
> +	/* We never fail at this point we always send what we collected so far */
> +out:
> +	ublox_netmon_finish_success(cbd);
> +	return;
> +
> +error:
> +	CALLBACK_WITH_FAILURE(cbd->cb, cbd->data);
> +	g_free(cbd);
> +}
> +
> +static void cops_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 tech;
> +
> +	DBG("ok %d", ok);
> +
> +	decode_at_error(&error, g_at_result_final_response(result));
> +
> +	if (!ok)
> +		goto error;
> +
> +	g_at_result_iter_init(&iter, result);
> +
> +	/* Do not fail */
> +	if (!g_at_result_iter_next(&iter, "+COPS:"))
> +		goto out;
> +
> +	g_at_result_iter_skip_next(&iter);
> +	g_at_result_iter_skip_next(&iter);
> +	g_at_result_iter_skip_next(&iter);
> +
> +	/* Default to GSM */
> +	if (g_at_result_iter_next_number(&iter, &tech) == FALSE)
> +		cbd->op.tech = ublox_map_radio_access_technology(ACCESS_TECHNOLOGY_GSM);
> +	else
> +		cbd->op.tech = ublox_map_radio_access_technology(tech);
> +
> +	if (g_at_chat_send(nmd->chat, "AT+CESQ", cesq_prefix,
> +			   cesq_cb, cbd, NULL)) {
> +		return;
> +	}
> +
> +out:
> +	CALLBACK_WITH_SUCCESS(cbd->cb, cbd->data);
> +	g_free(cbd);
> +	return;
> +
> +error:
> +	CALLBACK_WITH_FAILURE(cbd->cb, cbd->data);
> +	g_free(cbd);
> +}
> +
> +static void ublox_netmon_request_update(struct ofono_netmon *netmon,
> +					ofono_netmon_cb_t cb, void *data)
> +{
> +	struct netmon_driver_data *nmd = ofono_netmon_get_data(netmon);
> +	struct req_cb_data *cbd;
> +
> +	DBG("ublox netmon request update");
> +
> +	cbd = req_cb_data_new0(cb, data, netmon);
> +	if (!cbd) {
> +		CALLBACK_WITH_FAILURE(cb, data);
> +		return;
> +	}

This if can in theory be removed, see my comments in req_cb_data_new0.

> +
> +	if (g_at_chat_send(nmd->chat, "AT+COPS?", cops_prefix,
> +			   cops_cb, cbd, NULL) == 0) {

So this is still not right with the missing GDestroyNotify parameter. 
GAtChat is pretty flexible & neat, so it makes things like this easy to 
solve.  Here are a couple of ways you can handle this:

1. Send all commands right away.  Provide cbd as userdata for all of 
them, and provide GDestroyNotify callback (e.g. g_free) only to the last 
command sent. GAtChat will queue them up internally, and if this atom 
gets destroyed prematurely (e.g. due to sim removal conditions, hardware 
removal, etc) the GDestroyNotify callback will be called in all cases.

So roughly:
g_at_chat_send(..., "AT+COPS?", ..., cbd, NULL);
g_at_chat_send(..., "AT+CESQ?", ..., cbd, g_free);

2. Use reference counting on the cbd object.  req_cb_data_new0 would 
initialize reference count to 1.  req_cb_data_ref / unref would increase 
/ decrease it.  req_cb_data_unref would use g_free if reference count 
reaches 0.

This way you can do something like:

g_at_chat_send(..., "AT+COPS?", ..., cbd, req_cb_data_unref);

Then inside the cops callback, if the command succeeds and you want to 
proceed with CESQ, you'd invoke req_cb_data_ref(), otherwise simply 
callback with failure & return.  In both cases req_cb_data_unref will be 
called after the cops callback is executed.

ping me on IRC if my explanation wasn't clear.

> +		CALLBACK_WITH_FAILURE(cb, data);
> +		g_free(cbd);
> +	}
> +}
> +
> +static int ublox_netmon_probe(struct ofono_netmon *netmon,
> +			      unsigned int vendor, void *user)
> +{
> +	GAtChat *chat = user;
> +	struct netmon_driver_data *nmd;
> +
> +	DBG("ublox netmon probe");
> +
> +	nmd = g_try_new0(struct netmon_driver_data, 1);
> +	if (nmd == NULL)
> +		return -ENOMEM;
> +
> +	nmd->chat = g_at_chat_clone(chat);
> +
> +	ofono_netmon_set_data(netmon, nmd);
> +
> +	g_idle_add(ublox_delayed_register, netmon);
> +
> +	return 0;
> +}
> +
> +static void ublox_netmon_remove(struct ofono_netmon *netmon)
> +{
> +	struct netmon_driver_data *nmd = ofono_netmon_get_data(netmon);
> +
> +	DBG("ublox netmon remove");
> +
> +	g_at_chat_unref(nmd->chat);
> +
> +	ofono_netmon_set_data(netmon, NULL);
> +
> +	g_free(nmd);
> +}
> +
> +static struct ofono_netmon_driver driver = {
> +	.name			= UBLOXMODEM,
> +	.probe			= ublox_netmon_probe,
> +	.remove			= ublox_netmon_remove,
> +	.request_update		= ublox_netmon_request_update,
> +};
> +
> +void ublox_netmon_init(void)
> +{
> +	ofono_netmon_driver_register(&driver);
> +}
> +
> +void ublox_netmon_exit(void)
> +{
> +	ofono_netmon_driver_unregister(&driver);
> +}
> diff --git a/drivers/ubloxmodem/ubloxmodem.c b/drivers/ubloxmodem/ubloxmodem.c
> index 93cb928..a325b1f 100644
> --- a/drivers/ubloxmodem/ubloxmodem.c
> +++ b/drivers/ubloxmodem/ubloxmodem.c
> @@ -36,6 +36,7 @@
>   static int ubloxmodem_init(void)
>   {
>   	ublox_gprs_context_init();
> +	ublox_netmon_init();
>   	ublox_lte_init();
>
>   	return 0;
> @@ -44,6 +45,7 @@ static int ubloxmodem_init(void)
>   static void ubloxmodem_exit(void)
>   {
>   	ublox_gprs_context_exit();
> +	ublox_netmon_exit();
>   	ublox_lte_exit();
>   }
>
> diff --git a/drivers/ubloxmodem/ubloxmodem.h b/drivers/ubloxmodem/ubloxmodem.h
> index cf66412..bfb0106 100644
> --- a/drivers/ubloxmodem/ubloxmodem.h
> +++ b/drivers/ubloxmodem/ubloxmodem.h
> @@ -26,5 +26,8 @@
>   extern void ublox_gprs_context_init(void);
>   extern void ublox_gprs_context_exit(void);
>
> +extern void ublox_netmon_init(void);
> +extern void ublox_netmon_exit(void);
> +
>   extern void ublox_lte_init(void);
>   extern void ublox_lte_exit(void);
>

Regards,
-Denis

[PATCH v4 4/7] ubloxmodem: add the netmon driver

[Djalal Harouni]

[-- Attachment #1: Type: text/plain, Size: 10517 bytes --]

This adds a netmon driver for ublox. The driver support both +COPS and
+CESQ commands to return the previously added ofono netmon types:

RSCP: Received Signal Code Power
ECN0: Received Energy Ratio
RSRQ: Reference Signal Received Quality
RSRP: Reference Signal Received Power

Current revision makes the driver use ref countig when chaining multiple
AT commands.
---
 drivers/ubloxmodem/netmon.c     | 352 ++++++++++++++++++++++++++++++++++++++++
 drivers/ubloxmodem/ubloxmodem.c |   2 +
 drivers/ubloxmodem/ubloxmodem.h |   3 +
 3 files changed, 357 insertions(+)
 create mode 100644 drivers/ubloxmodem/netmon.c

diff --git a/drivers/ubloxmodem/netmon.c b/drivers/ubloxmodem/netmon.c
new file mode 100644
index 0000000..c4c4fcd
--- /dev/null
+++ b/drivers/ubloxmodem/netmon.c
@@ -0,0 +1,352 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2016  EndoCode AG. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <string.h>
+#include <errno.h>
+
+#include <glib.h>
+
+#include <ofono/log.h>
+#include <ofono/modem.h>
+#include <ofono/netreg.h>
+#include <ofono/netmon.h>
+
+#include "gatchat.h"
+#include "gatresult.h"
+
+#include "common.h"
+#include "ubloxmodem.h"
+#include "drivers/atmodem/vendor.h"
+
+static const char *cops_prefix[] = { "+COPS:", NULL };
+static const char *cesq_prefix[] = { "+CESQ:", NULL };
+
+struct netmon_driver_data {
+	GAtChat *chat;
+};
+
+struct req_cb_data {
+	gint ref_count; /* Ref count */
+
+	struct ofono_netmon *netmon;
+
+	ofono_netmon_cb_t cb;
+	void *data;
+
+	struct ofono_network_operator op;
+
+	int rxlev;	/* CESQ: Received Signal Strength Indication */
+	int ber;	/* CESQ: Bit Error Rate */
+	int rscp;	/* CESQ: Received Signal Code Powe */
+	int rsrp;	/* CESQ: Reference Signal Received Power */
+	int ecn0;	/* CESQ: Received Energy Ratio */
+	int rsrq;	/* CESQ: Reference Signal Received Quality */
+};
+
+/*
+ * Returns the appropriate radio access technology.
+ *
+ * If we can not resolve to a specific radio access technolgy
+ * we return OFONO_NETMON_CELL_TYPE_GSM by default.
+ */
+static int ublox_map_radio_access_technology(int tech)
+{
+	switch (tech) {
+	case ACCESS_TECHNOLOGY_GSM:
+	case ACCESS_TECHNOLOGY_GSM_COMPACT:
+		return OFONO_NETMON_CELL_TYPE_GSM;
+	case ACCESS_TECHNOLOGY_UTRAN:
+	case ACCESS_TECHNOLOGY_UTRAN_HSDPA:
+	case ACCESS_TECHNOLOGY_UTRAN_HSUPA:
+	case ACCESS_TECHNOLOGY_UTRAN_HSDPA_HSUPA:
+		return OFONO_NETMON_CELL_TYPE_UMTS;
+	case ACCESS_TECHNOLOGY_EUTRAN:
+		return OFONO_NETMON_CELL_TYPE_LTE;
+	}
+
+	return OFONO_NETMON_CELL_TYPE_GSM;
+}
+
+static inline struct req_cb_data *req_cb_data_new0(void *cb, void *data,
+							void *user)
+{
+	struct req_cb_data *ret = g_new0(struct req_cb_data, 1);
+
+	ret->ref_count = 1;
+	ret->cb = cb;
+	ret->data = data;
+	ret->netmon = user;
+	ret->rxlev = -1;
+	ret->ber = -1;
+	ret->rscp = -1;
+	ret->rsrp = -1;
+	ret->ecn0 = -1;
+	ret->rsrq = -1;
+
+	return ret;
+}
+
+static inline struct req_cb_data *req_cb_data_ref(struct req_cb_data *cbd)
+{
+	if (cbd == NULL)
+		return NULL;
+
+	g_atomic_int_inc(&cbd->ref_count);
+
+	return cbd;
+}
+
+static void req_cb_data_unref(gpointer user_data)
+{
+	gboolean is_zero;
+	struct req_cb_data *cbd = user_data;
+
+	if (cbd == NULL)
+		return NULL;
+
+	is_zero = g_atomic_int_dec_and_test(&cbd->ref_count);
+
+	if (is_zero == TRUE)
+		g_free(cbd);
+}
+
+static gboolean ublox_delayed_register(gpointer user_data)
+{
+	struct ofono_netmon *netmon = user_data;
+
+	ofono_netmon_register(netmon);
+
+	return FALSE;
+}
+
+static void ublox_netmon_finish_success(struct req_cb_data *cbd)
+{
+	struct ofono_netmon *nm = cbd->netmon;
+
+	ofono_netmon_serving_cell_notify(nm,
+					cbd->op.tech,
+					OFONO_NETMON_INFO_RXLEV, cbd->rxlev,
+					OFONO_NETMON_INFO_BER, cbd->ber,
+					OFONO_NETMON_INFO_RSCP, cbd->rscp,
+					OFONO_NETMON_INFO_ECN0, cbd->ecn0,
+					OFONO_NETMON_INFO_RSRQ, cbd->rsrq,
+					OFONO_NETMON_INFO_RSRP, cbd->rsrp,
+					OFONO_NETMON_INFO_INVALID);
+
+	CALLBACK_WITH_SUCCESS(cbd->cb, cbd->data);
+}
+
+static void cesq_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	enum cesq_ofono_netmon_info {
+		CESQ_RXLEV,
+		CESQ_BER,
+		CESQ_RSCP,
+		CESQ_ECN0,
+		CESQ_RSRQ,
+		CESQ_RSRP,
+		_MAX,
+	};
+
+	struct req_cb_data *cbd = user_data;
+	struct ofono_error error;
+	GAtResultIter iter;
+	int idx, number;
+
+	DBG("ok %d", ok);
+
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	if (!ok) {
+		CALLBACK_WITH_FAILURE(cbd->cb, cbd->data);
+		return;
+	}
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+CESQ:")) {
+		DBG(" CESQ: no result ");
+		goto out;
+	}
+
+	for (idx = 0; idx < _MAX; idx++) {
+
+		ok = g_at_result_iter_next_number(&iter, &number);
+		if (!ok) {
+			/* Ignore and do not fail */
+			DBG(" CESQ: error parsing idx: %d ", idx);
+			goto out;
+		}
+
+		switch (idx) {
+		case CESQ_RXLEV:
+			cbd->rxlev = number != 99 ? number:cbd->rxlev;
+			break;
+		case CESQ_BER:
+			cbd->ber = number != 99 ? number:cbd->ber;
+			break;
+		case CESQ_RSCP:
+			cbd->rscp = number != 255 ? number:cbd->rscp;
+			break;
+		case CESQ_ECN0:
+			cbd->ecn0 = number != 255 ? number:cbd->ecn0;
+			break;
+		case CESQ_RSRQ:
+			cbd->rsrq = number != 255 ? number:cbd->rsrq;
+			break;
+		case CESQ_RSRP:
+			cbd->rsrp = number != 255 ? number:cbd->rsrp;
+			break;
+		}
+	}
+
+	DBG(" RXLEV	%d ", cbd->rxlev);
+	DBG(" BER	%d ", cbd->ber);
+	DBG(" RSCP	%d ", cbd->rscp);
+	DBG(" ECN0	%d ", cbd->ecn0);
+	DBG(" RSRQ	%d ", cbd->rsrq);
+	DBG(" RSRP	%d ", cbd->rsrp);
+
+	/* We never fail at this point we always send what we collected so far */
+out:
+	ublox_netmon_finish_success(cbd);
+	return;
+}
+
+static void cops_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 tech;
+
+	DBG("ok %d", ok);
+
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	if (!ok) {
+		CALLBACK_WITH_FAILURE(cbd->cb, cbd->data);
+		return;
+	}
+
+	g_at_result_iter_init(&iter, result);
+
+	/* Do not fail */
+	if (!g_at_result_iter_next(&iter, "+COPS:")) {
+		CALLBACK_WITH_SUCCESS(cbd->cb, cbd->data);
+		return;
+	}
+
+	g_at_result_iter_skip_next(&iter);
+	g_at_result_iter_skip_next(&iter);
+	g_at_result_iter_skip_next(&iter);
+
+	/* Default to GSM */
+	if (g_at_result_iter_next_number(&iter, &tech) == FALSE)
+		cbd->op.tech = ublox_map_radio_access_technology(ACCESS_TECHNOLOGY_GSM);
+	else
+		cbd->op.tech = ublox_map_radio_access_technology(tech);
+
+	cbd = req_cb_data_ref(cbd);
+	if (g_at_chat_send(nmd->chat, "AT+CESQ", cesq_prefix,
+			   cesq_cb, cbd, req_cb_data_unref) == 0) {
+		CALLBACK_WITH_FAILURE(cbd->cb, cbd->data);
+		req_cb_data_unref(cbd);
+	}
+}
+
+static void ublox_netmon_request_update(struct ofono_netmon *netmon,
+					ofono_netmon_cb_t cb, void *data)
+{
+	struct netmon_driver_data *nmd = ofono_netmon_get_data(netmon);
+	struct req_cb_data *cbd;
+
+	DBG("ublox netmon request update");
+
+	cbd = req_cb_data_new0(cb, data, netmon);
+
+	if (g_at_chat_send(nmd->chat, "AT+COPS?", cops_prefix,
+			   cops_cb, cbd, req_cb_data_unref) == 0) {
+		CALLBACK_WITH_FAILURE(cbd(>cb, cbd->data);
+		req_cb_data_unref(cbd);
+	}
+}
+
+static int ublox_netmon_probe(struct ofono_netmon *netmon,
+					unsigned int vendor, void *user)
+{
+	GAtChat *chat = user;
+	struct netmon_driver_data *nmd;
+
+	DBG("ublox netmon probe");
+
+	nmd = g_try_new0(struct netmon_driver_data, 1);
+	if (nmd == NULL)
+		return -ENOMEM;
+
+	nmd->chat = g_at_chat_clone(chat);
+
+	ofono_netmon_set_data(netmon, nmd);
+
+	g_idle_add(ublox_delayed_register, netmon);
+
+	return 0;
+}
+
+static void ublox_netmon_remove(struct ofono_netmon *netmon)
+{
+	struct netmon_driver_data *nmd = ofono_netmon_get_data(netmon);
+
+	DBG("ublox netmon remove");
+
+	g_at_chat_unref(nmd->chat);
+
+	ofono_netmon_set_data(netmon, NULL);
+
+	g_free(nmd);
+}
+
+static struct ofono_netmon_driver driver = {
+	.name			= UBLOXMODEM,
+	.probe			= ublox_netmon_probe,
+	.remove			= ublox_netmon_remove,
+	.request_update		= ublox_netmon_request_update,
+};
+
+void ublox_netmon_init(void)
+{
+	ofono_netmon_driver_register(&driver);
+}
+
+void ublox_netmon_exit(void)
+{
+	ofono_netmon_driver_unregister(&driver);
+}
diff --git a/drivers/ubloxmodem/ubloxmodem.c b/drivers/ubloxmodem/ubloxmodem.c
index 93cb928..a325b1f 100644
--- a/drivers/ubloxmodem/ubloxmodem.c
+++ b/drivers/ubloxmodem/ubloxmodem.c
@@ -36,6 +36,7 @@
 static int ubloxmodem_init(void)
 {
 	ublox_gprs_context_init();
+	ublox_netmon_init();
 	ublox_lte_init();
 
 	return 0;
@@ -44,6 +45,7 @@ static int ubloxmodem_init(void)
 static void ubloxmodem_exit(void)
 {
 	ublox_gprs_context_exit();
+	ublox_netmon_exit();
 	ublox_lte_exit();
 }
 
diff --git a/drivers/ubloxmodem/ubloxmodem.h b/drivers/ubloxmodem/ubloxmodem.h
index cf66412..bfb0106 100644
--- a/drivers/ubloxmodem/ubloxmodem.h
+++ b/drivers/ubloxmodem/ubloxmodem.h
@@ -26,5 +26,8 @@
 extern void ublox_gprs_context_init(void);
 extern void ublox_gprs_context_exit(void);
 
+extern void ublox_netmon_init(void);
+extern void ublox_netmon_exit(void);
+
 extern void ublox_lte_init(void);
 extern void ublox_lte_exit(void);
-- 
2.5.5

Re: [PATCH v3 4/7] ubloxmodem: add the netmon driver

[Djalal Harouni]

[-- Attachment #1: Type: text/plain, Size: 7451 bytes --]

Hi Denis,

On 30 November 2016 at 16:55, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Djalal,
>
>
> On 11/30/2016 06:31 AM, Djalal Harouni wrote:
>>
>> This adds a netmon driver for ublox. The driver support both +COPS and
>> +CESQ commands to return the previously added ofono netmon types:
>>
>> RSCP: Received Signal Code Power
>> ECN0: Received Energy Ratio
>> RSRQ: Reference Signal Received Quality
>> RSRP: Reference Signal Received Power
>> ---
>>   drivers/ubloxmodem/netmon.c     | 336
>> ++++++++++++++++++++++++++++++++++++++++
>>   drivers/ubloxmodem/ubloxmodem.c |   2 +
>>   drivers/ubloxmodem/ubloxmodem.h |   3 +
>>   3 files changed, 341 insertions(+)
>>   create mode 100644 drivers/ubloxmodem/netmon.c
>>
>> diff --git a/drivers/ubloxmodem/netmon.c b/drivers/ubloxmodem/netmon.c
>> new file mode 100644
>> index 0000000..e5adf59
>> --- /dev/null
>> +++ b/drivers/ubloxmodem/netmon.c
>> @@ -0,0 +1,336 @@
>> +/*
>> + *
>> + *  oFono - Open Source Telephony
>> + *
>> + *  Copyright (C) 2016  EndoCode AG. All rights reserved.
>> + *
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License version 2 as
>> + *  published by the Free Software Foundation.
>> + *
>> + *  This program is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License
>> + *  along with this program; if not, write to the Free Software
>> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301
>> USA
>> + *
>> + */
>> +
>> +#ifdef HAVE_CONFIG_H
>> +#include <config.h>
>> +#endif
>> +
>> +#define _GNU_SOURCE
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <stdint.h>
>> +#include <string.h>
>> +#include <errno.h>
>> +
>> +#include <glib.h>
>> +
>> +#include <ofono/log.h>
>> +#include <ofono/modem.h>
>> +#include <ofono/netmon.h>
>> +
>> +#include "gatchat.h"
>> +#include "gatresult.h"
>> +
>> +#include "common.h"
>> +#include "netreg.h"
>
>
> This should probably be <ofono/netreg.h> and moved up above.
>

Fixed.

>> +#include "ubloxmodem.h"
[...]

> This if can in theory be removed, see my comments in req_cb_data_new0.
>
>> +
>> +       if (g_at_chat_send(nmd->chat, "AT+COPS?", cops_prefix,
>> +                          cops_cb, cbd, NULL) == 0) {
>
>
> So this is still not right with the missing GDestroyNotify parameter.
> GAtChat is pretty flexible & neat, so it makes things like this easy to
> solve.  Here are a couple of ways you can handle this:
>
> 1. Send all commands right away.  Provide cbd as userdata for all of them,
> and provide GDestroyNotify callback (e.g. g_free) only to the last command
> sent. GAtChat will queue them up internally, and if this atom gets destroyed
> prematurely (e.g. due to sim removal conditions, hardware removal, etc) the
> GDestroyNotify callback will be called in all cases.
>
> So roughly:
> g_at_chat_send(..., "AT+COPS?", ..., cbd, NULL);
> g_at_chat_send(..., "AT+CESQ?", ..., cbd, g_free);
>
> 2. Use reference counting on the cbd object.  req_cb_data_new0 would
> initialize reference count to 1.  req_cb_data_ref / unref would increase /
> decrease it.  req_cb_data_unref would use g_free if reference count reaches
> 0.
>
> This way you can do something like:
>
> g_at_chat_send(..., "AT+COPS?", ..., cbd, req_cb_data_unref);
>
> Then inside the cops callback, if the command succeeds and you want to
> proceed with CESQ, you'd invoke req_cb_data_ref(), otherwise simply callback
> with failure & return.  In both cases req_cb_data_unref will be called after
> the cops callback is executed.
>

OK thank you for the explanation. I did go with ref counting since
they are easy to use and I will follow up later with +UCGED which has
different behaviour depending on firmware version...

I take a ref just before doing the g_at_chat_send() , however I call
unref in case g_at_chat_send() returns 0 and fails since in that case
the GDestroyNotify is still not registered and the command was not
queued...

Hmm so now maybe the leak may happen in this small window between:
cbd = req_cb_data_ref(cbd);
and
g_at_chat_send() and before registering the GDestroyNotify
parameter... in case hardware removal happens or anything... I'm not
sure and I also don't know how to fix it.

Otherwise all comments for this patch were fixed.

Thank you for the review ;-)



>
>> +               CALLBACK_WITH_FAILURE(cb, data);
>> +               g_free(cbd);
>> +       }
>> +}
>> +
>> +static int ublox_netmon_probe(struct ofono_netmon *netmon,
>> +                             unsigned int vendor, void *user)
>> +{
>> +       GAtChat *chat = user;
>> +       struct netmon_driver_data *nmd;
>> +
>> +       DBG("ublox netmon probe");
>> +
>> +       nmd = g_try_new0(struct netmon_driver_data, 1);
>> +       if (nmd == NULL)
>> +               return -ENOMEM;
>> +
>> +       nmd->chat = g_at_chat_clone(chat);
>> +
>> +       ofono_netmon_set_data(netmon, nmd);
>> +
>> +       g_idle_add(ublox_delayed_register, netmon);
>> +
>> +       return 0;
>> +}
>> +
>> +static void ublox_netmon_remove(struct ofono_netmon *netmon)
>> +{
>> +       struct netmon_driver_data *nmd = ofono_netmon_get_data(netmon);
>> +
>> +       DBG("ublox netmon remove");
>> +
>> +       g_at_chat_unref(nmd->chat);
>> +
>> +       ofono_netmon_set_data(netmon, NULL);
>> +
>> +       g_free(nmd);
>> +}
>> +
>> +static struct ofono_netmon_driver driver = {
>> +       .name                   = UBLOXMODEM,
>> +       .probe                  = ublox_netmon_probe,
>> +       .remove                 = ublox_netmon_remove,
>> +       .request_update         = ublox_netmon_request_update,
>> +};
>> +
>> +void ublox_netmon_init(void)
>> +{
>> +       ofono_netmon_driver_register(&driver);
>> +}
>> +
>> +void ublox_netmon_exit(void)
>> +{
>> +       ofono_netmon_driver_unregister(&driver);
>> +}
>> diff --git a/drivers/ubloxmodem/ubloxmodem.c
>> b/drivers/ubloxmodem/ubloxmodem.c
>> index 93cb928..a325b1f 100644
>> --- a/drivers/ubloxmodem/ubloxmodem.c
>> +++ b/drivers/ubloxmodem/ubloxmodem.c
>> @@ -36,6 +36,7 @@
>>   static int ubloxmodem_init(void)
>>   {
>>         ublox_gprs_context_init();
>> +       ublox_netmon_init();
>>         ublox_lte_init();
>>
>>         return 0;
>> @@ -44,6 +45,7 @@ static int ubloxmodem_init(void)
>>   static void ubloxmodem_exit(void)
>>   {
>>         ublox_gprs_context_exit();
>> +       ublox_netmon_exit();
>>         ublox_lte_exit();
>>   }
>>
>> diff --git a/drivers/ubloxmodem/ubloxmodem.h
>> b/drivers/ubloxmodem/ubloxmodem.h
>> index cf66412..bfb0106 100644
>> --- a/drivers/ubloxmodem/ubloxmodem.h
>> +++ b/drivers/ubloxmodem/ubloxmodem.h
>> @@ -26,5 +26,8 @@
>>   extern void ublox_gprs_context_init(void);
>>   extern void ublox_gprs_context_exit(void);
>>
>> +extern void ublox_netmon_init(void);
>> +extern void ublox_netmon_exit(void);
>> +
>>   extern void ublox_lte_init(void);
>>   extern void ublox_lte_exit(void);
>>
>
> Regards,
> -Denis
>

Re: [PATCH v3 4/7] ubloxmodem: add the netmon driver

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1285 bytes --]

Hi Djalal,

 >
> OK thank you for the explanation. I did go with ref counting since
> they are easy to use and I will follow up later with +UCGED which has
> different behaviour depending on firmware version...

Sounds good.

>
> I take a ref just before doing the g_at_chat_send() , however I call
> unref in case g_at_chat_send() returns 0 and fails since in that case
> the GDestroyNotify is still not registered and the command was not
> queued...

That is fine.  In general it might be simpler to have req_cb_data_ref 
initialize the ref count to 1.  Saves you a call to ref()

>
> Hmm so now maybe the leak may happen in this small window between:
> cbd = req_cb_data_ref(cbd);
> and
> g_at_chat_send() and before registering the GDestroyNotify
> parameter... in case hardware removal happens or anything... I'm not
> sure and I also don't know how to fix it.

This is not possible.  The hardware removal notification still comes 
over a socket, so regular event loop rules apply.  The function 
invocation won't be interrupted mid-stream.

What we're worried about is us allocating memory, queuing the command 
into GAtChat, but at some point later, the GAtChat object is destroyed 
before the command callback was executed.

Regards,
-Denis

Re: [PATCH v3 4/7] ubloxmodem: add the netmon driver

[Djalal Harouni]

[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]

Hi Denis,

On 1 December 2016 at 18:18, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Djalal,
>
>>
>>
>> OK thank you for the explanation. I did go with ref counting since
>> they are easy to use and I will follow up later with +UCGED which has
>> different behaviour depending on firmware version...
>
>
> Sounds good.
[...]
>> Hmm so now maybe the leak may happen in this small window between:
>> cbd = req_cb_data_ref(cbd);
>> and
>> g_at_chat_send() and before registering the GDestroyNotify
>> parameter... in case hardware removal happens or anything... I'm not
>> sure and I also don't know how to fix it.
>
>
> This is not possible.  The hardware removal notification still comes over a
> socket, so regular event loop rules apply.  The function invocation won't be
> interrupted mid-stream.
I see, thanks for the explanation.

> What we're worried about is us allocating memory, queuing the command into
> GAtChat, but at some point later, the GAtChat object is destroyed before the
> command callback was executed.
Ok will follow same rules for next patches.


> Regards,
> -Denis

[PATCH v5 4/7] ubloxmodem: add the netmon driver

[Djalal Harouni]

[-- Attachment #1: Type: text/plain, Size: 10517 bytes --]

This adds a netmon driver for ublox. The driver support both +COPS and
+CESQ commands to return the previously added ofono netmon types:

RSCP: Received Signal Code Power
ECN0: Received Energy Ratio
RSRQ: Reference Signal Received Quality
RSRP: Reference Signal Received Power

Current revision makes the driver use ref countig when chaining multiple
AT commands.
---
 drivers/ubloxmodem/netmon.c     | 352 ++++++++++++++++++++++++++++++++++++++++
 drivers/ubloxmodem/ubloxmodem.c |   2 +
 drivers/ubloxmodem/ubloxmodem.h |   3 +
 3 files changed, 357 insertions(+)
 create mode 100644 drivers/ubloxmodem/netmon.c

diff --git a/drivers/ubloxmodem/netmon.c b/drivers/ubloxmodem/netmon.c
new file mode 100644
index 0000000..c4c4fcd
--- /dev/null
+++ b/drivers/ubloxmodem/netmon.c
@@ -0,0 +1,352 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2016  EndoCode AG. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <string.h>
+#include <errno.h>
+
+#include <glib.h>
+
+#include <ofono/log.h>
+#include <ofono/modem.h>
+#include <ofono/netreg.h>
+#include <ofono/netmon.h>
+
+#include "gatchat.h"
+#include "gatresult.h"
+
+#include "common.h"
+#include "ubloxmodem.h"
+#include "drivers/atmodem/vendor.h"
+
+static const char *cops_prefix[] = { "+COPS:", NULL };
+static const char *cesq_prefix[] = { "+CESQ:", NULL };
+
+struct netmon_driver_data {
+	GAtChat *chat;
+};
+
+struct req_cb_data {
+	gint ref_count; /* Ref count */
+
+	struct ofono_netmon *netmon;
+
+	ofono_netmon_cb_t cb;
+	void *data;
+
+	struct ofono_network_operator op;
+
+	int rxlev;	/* CESQ: Received Signal Strength Indication */
+	int ber;	/* CESQ: Bit Error Rate */
+	int rscp;	/* CESQ: Received Signal Code Powe */
+	int rsrp;	/* CESQ: Reference Signal Received Power */
+	int ecn0;	/* CESQ: Received Energy Ratio */
+	int rsrq;	/* CESQ: Reference Signal Received Quality */
+};
+
+/*
+ * Returns the appropriate radio access technology.
+ *
+ * If we can not resolve to a specific radio access technolgy
+ * we return OFONO_NETMON_CELL_TYPE_GSM by default.
+ */
+static int ublox_map_radio_access_technology(int tech)
+{
+	switch (tech) {
+	case ACCESS_TECHNOLOGY_GSM:
+	case ACCESS_TECHNOLOGY_GSM_COMPACT:
+		return OFONO_NETMON_CELL_TYPE_GSM;
+	case ACCESS_TECHNOLOGY_UTRAN:
+	case ACCESS_TECHNOLOGY_UTRAN_HSDPA:
+	case ACCESS_TECHNOLOGY_UTRAN_HSUPA:
+	case ACCESS_TECHNOLOGY_UTRAN_HSDPA_HSUPA:
+		return OFONO_NETMON_CELL_TYPE_UMTS;
+	case ACCESS_TECHNOLOGY_EUTRAN:
+		return OFONO_NETMON_CELL_TYPE_LTE;
+	}
+
+	return OFONO_NETMON_CELL_TYPE_GSM;
+}
+
+static inline struct req_cb_data *req_cb_data_new0(void *cb, void *data,
+							void *user)
+{
+	struct req_cb_data *ret = g_new0(struct req_cb_data, 1);
+
+	ret->ref_count = 1;
+	ret->cb = cb;
+	ret->data = data;
+	ret->netmon = user;
+	ret->rxlev = -1;
+	ret->ber = -1;
+	ret->rscp = -1;
+	ret->rsrp = -1;
+	ret->ecn0 = -1;
+	ret->rsrq = -1;
+
+	return ret;
+}
+
+static inline struct req_cb_data *req_cb_data_ref(struct req_cb_data *cbd)
+{
+	if (cbd == NULL)
+		return NULL;
+
+	g_atomic_int_inc(&cbd->ref_count);
+
+	return cbd;
+}
+
+static void req_cb_data_unref(gpointer user_data)
+{
+	gboolean is_zero;
+	struct req_cb_data *cbd = user_data;
+
+	if (cbd == NULL)
+		return NULL;
+
+	is_zero = g_atomic_int_dec_and_test(&cbd->ref_count);
+
+	if (is_zero == TRUE)
+		g_free(cbd);
+}
+
+static gboolean ublox_delayed_register(gpointer user_data)
+{
+	struct ofono_netmon *netmon = user_data;
+
+	ofono_netmon_register(netmon);
+
+	return FALSE;
+}
+
+static void ublox_netmon_finish_success(struct req_cb_data *cbd)
+{
+	struct ofono_netmon *nm = cbd->netmon;
+
+	ofono_netmon_serving_cell_notify(nm,
+					cbd->op.tech,
+					OFONO_NETMON_INFO_RXLEV, cbd->rxlev,
+					OFONO_NETMON_INFO_BER, cbd->ber,
+					OFONO_NETMON_INFO_RSCP, cbd->rscp,
+					OFONO_NETMON_INFO_ECN0, cbd->ecn0,
+					OFONO_NETMON_INFO_RSRQ, cbd->rsrq,
+					OFONO_NETMON_INFO_RSRP, cbd->rsrp,
+					OFONO_NETMON_INFO_INVALID);
+
+	CALLBACK_WITH_SUCCESS(cbd->cb, cbd->data);
+}
+
+static void cesq_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	enum cesq_ofono_netmon_info {
+		CESQ_RXLEV,
+		CESQ_BER,
+		CESQ_RSCP,
+		CESQ_ECN0,
+		CESQ_RSRQ,
+		CESQ_RSRP,
+		_MAX,
+	};
+
+	struct req_cb_data *cbd = user_data;
+	struct ofono_error error;
+	GAtResultIter iter;
+	int idx, number;
+
+	DBG("ok %d", ok);
+
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	if (!ok) {
+		CALLBACK_WITH_FAILURE(cbd->cb, cbd->data);
+		return;
+	}
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+CESQ:")) {
+		DBG(" CESQ: no result ");
+		goto out;
+	}
+
+	for (idx = 0; idx < _MAX; idx++) {
+
+		ok = g_at_result_iter_next_number(&iter, &number);
+		if (!ok) {
+			/* Ignore and do not fail */
+			DBG(" CESQ: error parsing idx: %d ", idx);
+			goto out;
+		}
+
+		switch (idx) {
+		case CESQ_RXLEV:
+			cbd->rxlev = number != 99 ? number:cbd->rxlev;
+			break;
+		case CESQ_BER:
+			cbd->ber = number != 99 ? number:cbd->ber;
+			break;
+		case CESQ_RSCP:
+			cbd->rscp = number != 255 ? number:cbd->rscp;
+			break;
+		case CESQ_ECN0:
+			cbd->ecn0 = number != 255 ? number:cbd->ecn0;
+			break;
+		case CESQ_RSRQ:
+			cbd->rsrq = number != 255 ? number:cbd->rsrq;
+			break;
+		case CESQ_RSRP:
+			cbd->rsrp = number != 255 ? number:cbd->rsrp;
+			break;
+		}
+	}
+
+	DBG(" RXLEV	%d ", cbd->rxlev);
+	DBG(" BER	%d ", cbd->ber);
+	DBG(" RSCP	%d ", cbd->rscp);
+	DBG(" ECN0	%d ", cbd->ecn0);
+	DBG(" RSRQ	%d ", cbd->rsrq);
+	DBG(" RSRP	%d ", cbd->rsrp);
+
+	/* We never fail at this point we always send what we collected so far */
+out:
+	ublox_netmon_finish_success(cbd);
+	return;
+}
+
+static void cops_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 tech;
+
+	DBG("ok %d", ok);
+
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	if (!ok) {
+		CALLBACK_WITH_FAILURE(cbd->cb, cbd->data);
+		return;
+	}
+
+	g_at_result_iter_init(&iter, result);
+
+	/* Do not fail */
+	if (!g_at_result_iter_next(&iter, "+COPS:")) {
+		CALLBACK_WITH_SUCCESS(cbd->cb, cbd->data);
+		return;
+	}
+
+	g_at_result_iter_skip_next(&iter);
+	g_at_result_iter_skip_next(&iter);
+	g_at_result_iter_skip_next(&iter);
+
+	/* Default to GSM */
+	if (g_at_result_iter_next_number(&iter, &tech) == FALSE)
+		cbd->op.tech = ublox_map_radio_access_technology(ACCESS_TECHNOLOGY_GSM);
+	else
+		cbd->op.tech = ublox_map_radio_access_technology(tech);
+
+	cbd = req_cb_data_ref(cbd);
+	if (g_at_chat_send(nmd->chat, "AT+CESQ", cesq_prefix,
+			   cesq_cb, cbd, req_cb_data_unref) == 0) {
+		CALLBACK_WITH_FAILURE(cbd->cb, cbd->data);
+		req_cb_data_unref(cbd);
+	}
+}
+
+static void ublox_netmon_request_update(struct ofono_netmon *netmon,
+					ofono_netmon_cb_t cb, void *data)
+{
+	struct netmon_driver_data *nmd = ofono_netmon_get_data(netmon);
+	struct req_cb_data *cbd;
+
+	DBG("ublox netmon request update");
+
+	cbd = req_cb_data_new0(cb, data, netmon);
+
+	if (g_at_chat_send(nmd->chat, "AT+COPS?", cops_prefix,
+			   cops_cb, cbd, req_cb_data_unref) == 0) {
+		CALLBACK_WITH_FAILURE(cbd->cb, cbd->data);
+		req_cb_data_unref(cbd);
+	}
+}
+
+static int ublox_netmon_probe(struct ofono_netmon *netmon,
+					unsigned int vendor, void *user)
+{
+	GAtChat *chat = user;
+	struct netmon_driver_data *nmd;
+
+	DBG("ublox netmon probe");
+
+	nmd = g_try_new0(struct netmon_driver_data, 1);
+	if (nmd == NULL)
+		return -ENOMEM;
+
+	nmd->chat = g_at_chat_clone(chat);
+
+	ofono_netmon_set_data(netmon, nmd);
+
+	g_idle_add(ublox_delayed_register, netmon);
+
+	return 0;
+}
+
+static void ublox_netmon_remove(struct ofono_netmon *netmon)
+{
+	struct netmon_driver_data *nmd = ofono_netmon_get_data(netmon);
+
+	DBG("ublox netmon remove");
+
+	g_at_chat_unref(nmd->chat);
+
+	ofono_netmon_set_data(netmon, NULL);
+
+	g_free(nmd);
+}
+
+static struct ofono_netmon_driver driver = {
+	.name			= UBLOXMODEM,
+	.probe			= ublox_netmon_probe,
+	.remove			= ublox_netmon_remove,
+	.request_update		= ublox_netmon_request_update,
+};
+
+void ublox_netmon_init(void)
+{
+	ofono_netmon_driver_register(&driver);
+}
+
+void ublox_netmon_exit(void)
+{
+	ofono_netmon_driver_unregister(&driver);
+}
diff --git a/drivers/ubloxmodem/ubloxmodem.c b/drivers/ubloxmodem/ubloxmodem.c
index 93cb928..a325b1f 100644
--- a/drivers/ubloxmodem/ubloxmodem.c
+++ b/drivers/ubloxmodem/ubloxmodem.c
@@ -36,6 +36,7 @@
 static int ubloxmodem_init(void)
 {
 	ublox_gprs_context_init();
+	ublox_netmon_init();
 	ublox_lte_init();
 
 	return 0;
@@ -44,6 +45,7 @@ static int ubloxmodem_init(void)
 static void ubloxmodem_exit(void)
 {
 	ublox_gprs_context_exit();
+	ublox_netmon_exit();
 	ublox_lte_exit();
 }
 
diff --git a/drivers/ubloxmodem/ubloxmodem.h b/drivers/ubloxmodem/ubloxmodem.h
index cf66412..bfb0106 100644
--- a/drivers/ubloxmodem/ubloxmodem.h
+++ b/drivers/ubloxmodem/ubloxmodem.h
@@ -26,5 +26,8 @@
 extern void ublox_gprs_context_init(void);
 extern void ublox_gprs_context_exit(void);
 
+extern void ublox_netmon_init(void);
+extern void ublox_netmon_exit(void);
+
 extern void ublox_lte_init(void);
 extern void ublox_lte_exit(void);
-- 
2.5.5

Re: [PATCH v5 4/7] ubloxmodem: add the netmon driver

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 935 bytes --]

Hi Djalal,

On 12/01/2016 07:59 AM, Djalal Harouni wrote:
> This adds a netmon driver for ublox. The driver support both +COPS and
> +CESQ commands to return the previously added ofono netmon types:
>
> RSCP: Received Signal Code Power
> ECN0: Received Energy Ratio
> RSRQ: Reference Signal Received Quality
> RSRP: Reference Signal Received Power
>
> Current revision makes the driver use ref countig when chaining multiple
> AT commands.
> ---
>   drivers/ubloxmodem/netmon.c     | 352 ++++++++++++++++++++++++++++++++++++++++
>   drivers/ubloxmodem/ubloxmodem.c |   2 +
>   drivers/ubloxmodem/ubloxmodemn.h |   3 +
>   3 files changed, 357 insertions(+)
>   create mode 100644 drivers/ubloxmodem/netmon.c
>

Applied.  Also applied v3 patch 5 & 6.

I pushed out a couple of small patches afterwards to clean up this 
version.  Please have a look and make sure I didn't screw anything up.

Regards,
-Denis

Re: [PATCH v5 4/7] ubloxmodem: add the netmon driver

[Djalal Harouni]

[-- Attachment #1: Type: text/plain, Size: 1094 bytes --]

On 1 December 2016 at 18:29, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Djalal,
>
> On 12/01/2016 07:59 AM, Djalal Harouni wrote:
>>
>> This adds a netmon driver for ublox. The driver support both +COPS and
>> +CESQ commands to return the previously added ofono netmon types:
>>
>> RSCP: Received Signal Code Power
>> ECN0: Received Energy Ratio
>> RSRQ: Reference Signal Received Quality
>> RSRP: Reference Signal Received Power
>>
>> Current revision makes the driver use ref countig when chaining multiple
>> AT commands.
>> ---
>>   drivers/ubloxmodem/netmon.c     | 352
>> ++++++++++++++++++++++++++++++++++++++++
>>   drivers/ubloxmodem/ubloxmodem.c |   2 +
>>   drivers/ubloxmodem/ubloxmodemn.h |   3 +
>>   3 files changed, 357 insertions(+)
>>   create mode 100644 drivers/ubloxmodem/netmon.c
>>
>
> Applied.  Also applied v3 patch 5 & 6.
>
> I pushed out a couple of small patches afterwards to clean up this version.
> Please have a look and make sure I didn't screw anything up.
Thanks for cleaning it. Tested, all good!


>
> Regards,
> -Denis

[PATCH v3 5/7] ubloxmodem: register and initialize the netmon driver

[Djalal Harouni]

[-- Attachment #1: Type: text/plain, Size: 756 bytes --]

---
 plugins/ublox.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/plugins/ublox.c b/plugins/ublox.c
index 6d77df8..2ced577 100644
--- a/plugins/ublox.c
+++ b/plugins/ublox.c
@@ -38,6 +38,7 @@
 #include <ofono/sim.h>
 #include <ofono/gprs.h>
 #include <ofono/gprs-context.h>
+#include <ofono/netmon.h>
 #include <ofono/lte.h>
 
 #include <drivers/atmodem/atutil.h>
@@ -323,6 +324,8 @@ static void ublox_post_online(struct ofono_modem *modem)
 	struct ublox_data *data = ofono_modem_get_data(modem);
 
 	ofono_netreg_create(modem, data->vendor_family, "atmodem", data->aux);
+
+	ofono_netmon_create(modem, data->vendor_family, "ubloxmodem", data->aux);
 }
 
 static struct ofono_modem_driver ublox_driver = {
-- 
2.5.5

[PATCH v3 6/7] build: build the ublox netmon driver

[Djalal Harouni]

[-- Attachment #1: Type: text/plain, Size: 440 bytes --]

---
 Makefile.am | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile.am b/Makefile.am
index 3d7774b..07adeab 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -383,6 +383,7 @@ builtin_sources += drivers/atmodem/atutil.h \
 			drivers/ubloxmodem/ubloxmodem.h \
 			drivers/ubloxmodem/ubloxmodem.c \
 			drivers/ubloxmodem/gprs-context.c \
+			drivers/ubloxmodem/netmon.c \
 			drivers/ubloxmodem/lte.c
 
 
-- 
2.5.5

[PATCH v3 7/7] test: support OFONO_NETMON_INFO_{RXLEV|RSCP|ECN0|RSRQ|RSRP}

[Djalal Harouni]

[-- Attachment #1: Type: text/plain, Size: 1614 bytes --]

Display the following fields if they are returned.

RXLEV:  Received Signal Strength
RSCP:   Received Signal Code Power
ECN0:   Received Energy Ratio
RSRQ:   Reference Signal Received Quality
RSRP:   Reference Signal Received Power
---
 test/get-serving-cell-info | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/test/get-serving-cell-info b/test/get-serving-cell-info
index 05dc9fe..94e1c54 100755
--- a/test/get-serving-cell-info
+++ b/test/get-serving-cell-info
@@ -26,6 +26,11 @@ cid = 'CellId'
 psc = 'PrimaryScramblingCode'
 rssi = 'Strength'
 ber = 'BitErrorRate'
+rxlev = 'ReceivedSignalStrength'
+rscp = 'ReceivedSignalCodePower'
+ecn0 = 'ReceivedEnergyRatio'
+rsrq = 'ReferenceSignalReceivedQuality'
+rsrp = 'ReferenceSignalReceivedPower'
 
 print("Current serving cell information:")
 
@@ -50,7 +55,22 @@ if psc in servingcell:
 if rssi in servingcell:
 	print("    [ Signal Strength = %d]" % (servingcell[rssi]))
 
+if rxlev in servingcell:
+	print("    [ Received Signal Strength = %d]" % (servingcell[rxlev]))
+
 if ber in servingcell:
 	print("    [ Bit Error Rate = %d]" % (servingcell[ber]))
 
+if rscp in servingcell:
+	print("    [ Received Signal Code Power = %d]" % (servingcell[rscp]))
+
+if ecn0 in servingcell:
+	print("    [ Received Energy Ratio = %d]" % (servingcell[ecn0]))
+
+if rsrq in servingcell:
+	print("    [ Reference Signal Received Quality = %d]" % (servingcell[rsrq]))
+
+if rsrp in servingcell:
+	print("    [ Reference Signal Received Power = %d]" % (servingcell[rsrp]))
+
 print('')
-- 
2.5.5

Re: [PATCH v3 7/7] test: support OFONO_NETMON_INFO_{RXLEV|RSCP|ECN0|RSRQ|RSRP}

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 463 bytes --]

Hi Djalal,

On 11/30/2016 06:31 AM, Djalal Harouni wrote:
> Display the following fields if they are returned.
>
> RXLEV:  Received Signal Strength
> RSCP:   Received Signal Code Power
> ECN0:   Received Energy Ratio
> RSRQ:   Reference Signal Received Quality
> RSRP:   Reference Signal Received Power
> ---
>   test/get-serving-cell-info | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
>

Applied, thanks.

Regards,
-Denis

Re: [PATCH v3 7/7] test: support OFONO_NETMON_INFO_{RXLEV|RSCP|ECN0|RSRQ|RSRP}

[Djalal Harouni]

[-- Attachment #1: Type: text/plain, Size: 593 bytes --]

Hi Denis,


On 30 November 2016 at 16:56, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Djalal,
>
> On 11/30/2016 06:31 AM, Djalal Harouni wrote:
>>
>> Display the following fields if they are returned.
>>
>> RXLEV:  Received Signal Strength
>> RSCP:   Received Signal Code Power
>> ECN0:   Received Energy Ratio
>> RSRQ:   Reference Signal Received Quality
>> RSRP:   Reference Signal Received Power
>> ---
>>   test/get-serving-cell-info | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>
> Applied, thanks.

Thank you!

> Regards,
> -Denis
>
>