[PATCHv3 1/4] quectel: add dbus hardware interface

[PATCHv3 1/4] quectel: add dbus hardware interface

[Martin =?unknown-8bit?q?Hundeb=C3=B8ll?=]

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

For now the interface only exposes the modem supply voltage, but is
added as a preparation for signaling power events.
---

Changes since v2:
 * updated commit message

Changes since v1:
 * use and export int32_t for all three properties
 * place dbus functions below close_serial()
 * remove doc/ changes
 * update comments for charge status and level
 * unregister dbus api after canceling/unregister at chats

 plugins/quectel.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 158 insertions(+)

diff --git a/plugins/quectel.c b/plugins/quectel.c
index 9cac92fa..efdb7617 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -37,6 +37,7 @@
 #include <gattty.h>
 
 #define OFONO_API_SUBJECT_TO_CHANGE
+#include <ofono.h>
 #include <ofono/plugin.h>
 #include <ofono/modem.h>
 #include <ofono/devinfo.h>
@@ -49,12 +50,16 @@
 #include <ofono/gprs.h>
 #include <ofono/gprs-context.h>
 #include <ofono/log.h>
+#include <ofono/dbus.h>
+
+#include <gdbus/gdbus.h>
 
 #include <drivers/atmodem/atutil.h>
 #include <drivers/atmodem/vendor.h>
 
 static const char *cfun_prefix[] = { "+CFUN:", NULL };
 static const char *cpin_prefix[] = { "+CPIN:", NULL };
+static const char *cbc_prefix[] = { "+CBC:", NULL };
 static const char *qinistat_prefix[] = { "+QINISTAT:", NULL };
 static const char *cgmm_prefix[] = { "UC15", "Quectel_M95", "Quectel_MC60",
 					NULL };
@@ -95,6 +100,16 @@ struct quectel_data {
 	struct l_gpio_writer *gpio;
 };
 
+struct dbus_hw {
+	DBusMessage *msg;
+	struct ofono_modem *modem;
+	int32_t charge_status;
+	int32_t charge_level;
+	int32_t voltage;
+};
+
+static const char dbus_hw_interface[] = OFONO_SERVICE ".quectel.Hardware";
+
 static void quectel_debug(const char *str, void *user_data)
 {
 	const char *prefix = user_data;
@@ -227,6 +242,142 @@ static void close_serial(struct ofono_modem *modem)
 		ofono_modem_set_powered(modem, false);
 }
 
+static void dbus_hw_reply_properties(struct dbus_hw *hw)
+{
+	struct quectel_data *data = ofono_modem_get_data(hw->modem);
+	DBusMessage *reply;
+	DBusMessageIter dbus_iter;
+	DBusMessageIter dbus_dict;
+
+	DBG("%p", hw->modem);
+
+	reply = dbus_message_new_method_return(hw->msg);
+	dbus_message_iter_init_append(reply, &dbus_iter);
+	dbus_message_iter_open_container(&dbus_iter, DBUS_TYPE_ARRAY,
+			OFONO_PROPERTIES_ARRAY_SIGNATURE,
+			&dbus_dict);
+
+	/*
+	 * the charge status/level received from m95 and mc60 are invalid so
+	 * only return those for the UC15 modem.
+	 */
+	if (data->model == QUECTEL_UC15) {
+		ofono_dbus_dict_append(&dbus_dict, "ChargeStatus",
+					DBUS_TYPE_INT32, &hw->charge_status);
+
+		ofono_dbus_dict_append(&dbus_dict, "ChargeLevel",
+					DBUS_TYPE_INT32, &hw->charge_level);
+	}
+
+	ofono_dbus_dict_append(&dbus_dict, "Voltage", DBUS_TYPE_INT32,
+				&hw->voltage);
+
+	dbus_message_iter_close_container(&dbus_iter, &dbus_dict);
+
+	__ofono_dbus_pending_reply(&hw->msg, reply);
+}
+
+static void cbc_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct dbus_hw *hw = user_data;
+	GAtResultIter iter;
+
+	DBG("%p", hw->modem);
+
+	if (!hw->msg)
+		return;
+
+	if (!ok)
+		goto error;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+CBC:"))
+		goto error;
+
+	/* the returned charge status is valid only for uc15 */
+	if (!g_at_result_iter_next_number(&iter, &hw->charge_status))
+		goto error;
+
+	/* the returned charge level is valid only for uc15 */
+	if (!g_at_result_iter_next_number(&iter, &hw->charge_level))
+		goto error;
+
+	/* now comes the millivolts */
+	if (!g_at_result_iter_next_number(&iter, &hw->voltage))
+		goto error;
+
+	dbus_hw_reply_properties(hw);
+
+	return;
+
+error:
+	__ofono_dbus_pending_reply(&hw->msg, __ofono_error_failed(hw->msg));
+}
+
+static DBusMessage *dbus_hw_get_properties(DBusConnection *conn,
+						DBusMessage *msg,
+						void *user_data)
+{
+	struct dbus_hw *hw = user_data;
+	struct quectel_data *data = ofono_modem_get_data(hw->modem);
+
+	DBG("%p", hw->modem);
+
+	if (hw->msg != NULL)
+		return __ofono_error_busy(msg);
+
+	if (!g_at_chat_send(data->aux, "AT+CBC", cbc_prefix, cbc_cb, hw, NULL))
+		return __ofono_error_failed(msg);
+
+	hw->msg = dbus_message_ref(msg);
+
+	return NULL;
+}
+
+static const GDBusMethodTable dbus_hw_methods[] = {
+	{ GDBUS_ASYNC_METHOD("GetProperties",
+				NULL, GDBUS_ARGS({ "properties", "a{sv}" }),
+				dbus_hw_get_properties) },
+	{}
+};
+
+static void dbus_hw_cleanup(void *data)
+{
+	struct dbus_hw *hw = data;
+
+	DBG("%p", hw->modem);
+
+	if (hw->msg)
+		__ofono_dbus_pending_reply(&hw->msg,
+					__ofono_error_canceled(hw->msg));
+
+	l_free(hw);
+}
+
+static void dbus_hw_enable(struct ofono_modem *modem)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = ofono_modem_get_path(modem);
+	struct dbus_hw *hw;
+
+	DBG("%p", modem);
+
+	hw = l_new(struct dbus_hw, 1);
+	hw->modem = modem;
+
+	if (!g_dbus_register_interface(conn, path, dbus_hw_interface,
+					dbus_hw_methods, NULL, NULL,
+					hw, dbus_hw_cleanup)) {
+		ofono_error("Could not register %s interface under %s",
+				dbus_hw_interface, path);
+		l_free(hw);
+		return;
+	}
+
+	ofono_modem_add_interface(modem, dbus_hw_interface);
+}
+
 static void cpin_notify(GAtResult *result, gpointer user_data)
 {
 	struct ofono_modem *modem = user_data;
@@ -253,6 +404,8 @@ static void cpin_notify(GAtResult *result, gpointer user_data)
 
 	g_at_chat_unregister(data->aux, data->cpin_ready);
 	data->cpin_ready = 0;
+
+	dbus_hw_enable(modem);
 }
 
 static void cpin_query(gboolean ok, GAtResult *result, gpointer user_data)
@@ -650,6 +803,8 @@ static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
 static int quectel_disable(struct ofono_modem *modem)
 {
 	struct quectel_data *data = ofono_modem_get_data(modem);
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = ofono_modem_get_path(modem);
 
 	DBG("%p", modem);
 
@@ -659,6 +814,9 @@ static int quectel_disable(struct ofono_modem *modem)
 	g_at_chat_cancel_all(data->aux);
 	g_at_chat_unregister_all(data->aux);
 
+	if (g_dbus_unregister_interface(conn, path, dbus_hw_interface))
+		ofono_modem_remove_interface(modem, dbus_hw_interface);
+
 	g_at_chat_send(data->aux, "AT+CFUN=0", cfun_prefix, cfun_disable, modem,
 			NULL);
 
-- 
2.22.0

[PATCHv3 2/4] doc: add documentaion for Quectel hardware API

[Martin =?unknown-8bit?q?Hundeb=C3=B8ll?=]

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

---

Changes since v2:
 * none

Changes since v1:
 * new patch split out from PATCHv1 6/7
 * added documentation of charge status and level

 doc/quectel-hardware-api.txt | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 doc/quectel-hardware-api.txt

diff --git a/doc/quectel-hardware-api.txt b/doc/quectel-hardware-api.txt
new file mode 100644
index 00000000..6aafbfd8
--- /dev/null
+++ b/doc/quectel-hardware-api.txt
@@ -0,0 +1,33 @@
+Hardware hierarchy
+==================
+
+Service		org.ofono
+Interface	org.ofono.quectel.Hardware
+Object path	/{device0,device1,...}
+
+Methods		array{string,variant} GetProperties
+
+			Returns hardware properties for the modem object. See
+			the properties section for available properties.
+
+Properties	int32 Voltage [readonly]
+
+			Integer with the modem supply voltage in mV.
+
+		int32 ChargeStatus [readonly,optional]
+
+			Integer with one of either:
+
+			0: Modem is not charging
+			1: Modem is charging
+			2: Charging is finished
+
+			This property is available on UC15
+
+		int32 ChargeLevel [readonly,optional]
+
+			Integer representing the battery charge level in
+			percent (from 0 to 100).
+
+			This property is available on UC15
+
-- 
2.22.0

Re: [PATCHv3 2/4] doc: add documentaion for Quectel hardware API

[Denis Kenzior]

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

Hi Martin,

On 7/19/19 7:39 AM, Martin Hundebøll wrote:
> ---
> 
> Changes since v2:
>   * none
> 
> Changes since v1:
>   * new patch split out from PATCHv1 6/7
>   * added documentation of charge status and level
> 
>   doc/quectel-hardware-api.txt | 33 +++++++++++++++++++++++++++++++++
>   1 file changed, 33 insertions(+)
>   create mode 100644 doc/quectel-hardware-api.txt
> 
> diff --git a/doc/quectel-hardware-api.txt b/doc/quectel-hardware-api.txt
> new file mode 100644
> index 00000000..6aafbfd8
> --- /dev/null
> +++ b/doc/quectel-hardware-api.txt
> @@ -0,0 +1,33 @@
> +Hardware hierarchy
> +==================
> +
> +Service		org.ofono
> +Interface	org.ofono.quectel.Hardware
> +Object path	/{device0,device1,...}
> +
> +Methods		array{string,variant} GetProperties
> +
> +			Returns hardware properties for the modem object. See
> +			the properties section for available properties.
> +
> +Properties	int32 Voltage [readonly]
> +
> +			Integer with the modem supply voltage in mV.
> +
> +		int32 ChargeStatus [readonly,optional]
> +
> +			Integer with one of either:
> +
> +			0: Modem is not charging
> +			1: Modem is charging
> +			2: Charging is finished
> +
> +			This property is available on UC15
> +
> +		int32 ChargeLevel [readonly,optional]
> +
> +			Integer representing the battery charge level in
> +			percent (from 0 to 100).
> +
> +			This property is available on UC15
> +
> 

So just a nitpick, but should these be unsigned?

Regards,
-Denis

Re: [PATCHv3 2/4] doc: add documentaion for Quectel hardware API

[Pavel Machek]

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

Hi!


> >+Hardware hierarchy
> >+==================
> >+
> >+Service		org.ofono
> >+Interface	org.ofono.quectel.Hardware
> >+Object path	/{device0,device1,...}
> >+
> >+Methods		array{string,variant} GetProperties
> >+
> >+			Returns hardware properties for the modem object. See
> >+			the properties section for available properties.
> >+
> >+Properties	int32 Voltage [readonly]
> >+
> >+			Integer with the modem supply voltage in mV.
> >+
> >+		int32 ChargeStatus [readonly,optional]
> >+
> >+			Integer with one of either:
> >+
> >+			0: Modem is not charging
> >+			1: Modem is charging
> >+			2: Charging is finished
> >+
> >+			This property is available on UC15
> >+
> >+		int32 ChargeLevel [readonly,optional]
> >+
> >+			Integer representing the battery charge level in
> >+			percent (from 0 to 100).
> >+
> >+			This property is available on UC15
> >+

Maybe more resolution than percent would be useful? From 0 to 1000?

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[PATCHv3 3/4] quectel: implement dbus signals for modem power notifications

[Martin =?unknown-8bit?q?Hundeb=C3=B8ll?=]

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

The Quectel modems issues unsolicited strings in case of power related
events. The UC15 uses +QIND: for the events, while M95 and MC60 uses
descriptive strings. (UC15 also uses a string for normal power down).

Register listeners for these strings/codes. The handler emits an
appropriate dbus signal, and closes down the modem if needed.
---

Changes since v2:
 * Use Voltage{High,Low} instead of {High,Low}Power

Changes since v1:
 * remove doc/ changes
 * place dbus functions below close_serial()

 plugins/quectel.c | 146 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 145 insertions(+), 1 deletion(-)

diff --git a/plugins/quectel.c b/plugins/quectel.c
index efdb7617..79316d45 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -108,6 +108,14 @@ struct dbus_hw {
 	int32_t voltage;
 };
 
+enum quectel_power_event {
+	LOW_POWER_DOWN    = -2,
+	LOW_WARNING       = -1,
+	NORMAL_POWER_DOWN =  0,
+	HIGH_WARNING      =  1,
+	HIGH_POWER_DOWN   =  2,
+};
+
 static const char dbus_hw_interface[] = OFONO_SERVICE ".quectel.Hardware";
 
 static void quectel_debug(const char *str, void *user_data)
@@ -335,6 +343,110 @@ static DBusMessage *dbus_hw_get_properties(DBusConnection *conn,
 	return NULL;
 }
 
+static void voltage_handle(struct ofono_modem *modem,
+				enum quectel_power_event event)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	DBusMessage *signal;
+	DBusMessageIter iter;
+	const char *path = ofono_modem_get_path(modem);
+	const char *name;
+	const char *reason;
+	bool close;
+
+	DBG("%p", modem);
+
+	switch (event) {
+	case LOW_POWER_DOWN:
+		close = true;
+		name = "PowerDown";
+		reason = "VoltageLow";
+		break;
+	case LOW_WARNING:
+		close = false;
+		name = "PowerWarning";
+		reason = "VoltageLow";
+		break;
+	case NORMAL_POWER_DOWN:
+		close = true;
+		name = "PowerDown";
+		reason = "Normal";
+		break;
+	case HIGH_WARNING:
+		close = false;
+		name = "PowerWarning";
+		reason = "VoltageHigh";
+		break;
+	case HIGH_POWER_DOWN:
+		close = true;
+		name = "PowerDown";
+		reason = "VoltageHigh";
+		break;
+	default:
+		return;
+	}
+
+	signal = dbus_message_new_signal(path, dbus_hw_interface, name);
+	if (signal) {
+		dbus_message_iter_init_append(signal, &iter);
+		dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING,
+						&reason);
+		g_dbus_send_message(conn, signal);
+	}
+
+	if (close)
+		close_serial(modem);
+}
+
+static void qind_notify(GAtResult *result, void *user_data)
+{
+	struct dbus_hw *hw = user_data;
+	GAtResultIter iter;
+	enum quectel_power_event event;
+	const char *type;
+
+	DBG("%p", hw->modem);
+
+	g_at_result_iter_init(&iter, result);
+	g_at_result_iter_next(&iter, "+QIND:");
+
+	if (!g_at_result_iter_next_string(&iter, &type))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &event))
+		return;
+
+	voltage_handle(hw->modem, event);
+}
+
+static void power_notify(GAtResult *result, void *user_data)
+{
+	struct dbus_hw *hw = user_data;
+	GAtResultIter iter;
+	const char *event;
+
+	DBG("%p", hw->modem);
+
+	g_at_result_iter_init(&iter, result);
+	g_at_result_iter_next(&iter, NULL);
+
+	if (!g_at_result_iter_next_unquoted_string(&iter, &event))
+		return;
+
+	DBG("event: %s", event);
+
+	if (g_strcmp0(event, "UNDER_VOLTAGE POWER DOWN") == 0)
+		voltage_handle(hw->modem, LOW_POWER_DOWN);
+	else if (g_strcmp0(event, "UNDER_VOLTAGE WARNING") == 0)
+		voltage_handle(hw->modem, LOW_WARNING);
+	else if (g_strcmp0(event, "NORMAL POWER DOWN") == 0)
+		voltage_handle(hw->modem, NORMAL_POWER_DOWN);
+	else if (g_strcmp0(event, "OVER_VOLTAGE WARNING") == 0)
+		voltage_handle(hw->modem, HIGH_WARNING);
+	else if (g_strcmp0(event, "OVER_VOLTAGE POWER DOWN") == 0)
+		voltage_handle(hw->modem, HIGH_POWER_DOWN);
+}
+
 static const GDBusMethodTable dbus_hw_methods[] = {
 	{ GDBUS_ASYNC_METHOD("GetProperties",
 				NULL, GDBUS_ARGS({ "properties", "a{sv}" }),
@@ -342,6 +454,14 @@ static const GDBusMethodTable dbus_hw_methods[] = {
 	{}
 };
 
+static const GDBusSignalTable dbus_hw_signals[] = {
+	{ GDBUS_SIGNAL("PowerDown",
+			GDBUS_ARGS({ "reason", "s" })) },
+	{ GDBUS_SIGNAL("PowerWarning",
+			GDBUS_ARGS({ "reason", "s" })) },
+	{ }
+};
+
 static void dbus_hw_cleanup(void *data)
 {
 	struct dbus_hw *hw = data;
@@ -358,6 +478,7 @@ static void dbus_hw_cleanup(void *data)
 static void dbus_hw_enable(struct ofono_modem *modem)
 {
 	DBusConnection *conn = ofono_dbus_get_connection();
+	struct quectel_data *data = ofono_modem_get_data(modem);
 	const char *path = ofono_modem_get_path(modem);
 	struct dbus_hw *hw;
 
@@ -367,7 +488,7 @@ static void dbus_hw_enable(struct ofono_modem *modem)
 	hw->modem = modem;
 
 	if (!g_dbus_register_interface(conn, path, dbus_hw_interface,
-					dbus_hw_methods, NULL, NULL,
+					dbus_hw_methods, dbus_hw_signals, NULL,
 					hw, dbus_hw_cleanup)) {
 		ofono_error("Could not register %s interface under %s",
 				dbus_hw_interface, path);
@@ -375,6 +496,29 @@ static void dbus_hw_enable(struct ofono_modem *modem)
 		return;
 	}
 
+	g_at_chat_register(data->aux, "NORMAL POWER DOWN", power_notify, FALSE,
+				hw, NULL);
+
+	switch (data->model) {
+	case QUECTEL_UC15:
+		g_at_chat_register(data->aux, "+QIND",  qind_notify, FALSE, hw,
+					NULL);
+		break;
+	case QUECTEL_M95:
+	case QUECTEL_MC60:
+		g_at_chat_register(data->aux, "OVER_VOLTAGE POWER DOWN",
+					power_notify, FALSE, hw, NULL);
+		g_at_chat_register(data->aux, "UNDER_VOLTAGE POWER DOWN",
+					power_notify, FALSE, hw, NULL);
+		g_at_chat_register(data->aux, "OVER_VOLTAGE WARNING",
+					power_notify, FALSE, hw, NULL);
+		g_at_chat_register(data->aux, "UNDER_VOLTAGE WARNING",
+					power_notify, FALSE, hw, NULL);
+		break;
+	case QUECTEL_UNKNOWN:
+		break;
+	}
+
 	ofono_modem_add_interface(modem, dbus_hw_interface);
 }
 
-- 
2.22.0

[PATCHv3 4/4] doc: document power-event signals for quectel Hardware API

[Martin =?unknown-8bit?q?Hundeb=C3=B8ll?=]

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

---

Changes since v2:
 * Use Voltage{High,Low} instead of {High,Low}Power

Changes since v1:
 * new patch split out from PATCHv1 7/7

 doc/quectel-hardware-api.txt | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/doc/quectel-hardware-api.txt b/doc/quectel-hardware-api.txt
index 6aafbfd8..8795ef40 100644
--- a/doc/quectel-hardware-api.txt
+++ b/doc/quectel-hardware-api.txt
@@ -10,6 +10,25 @@ Methods		array{string,variant} GetProperties
 			Returns hardware properties for the modem object. See
 			the properties section for available properties.
 
+Signals		PowerDown(string reason)
+
+			This signal is emitted on gracefull shutdowns initiated
+			by the modem.
+
+			Possible reasons:
+				"VoltageLow"	The supply voltage is too low
+				"Normal"	The PWRKEY pin was asserted
+				"VoltageHigh"	The supply voltage is too high
+
+		PowerWarning(string reason)
+
+			This signal is emitted when the modem detects its supply
+			voltage is close to its supported limits.
+
+			Possible reasons:
+				"VoltageLow"	The supply voltage is low
+				"VoltageHigh"	The supply voltage is high
+
 Properties	int32 Voltage [readonly]
 
 			Integer with the modem supply voltage in mV.
-- 
2.22.0

Re: [PATCHv3 4/4] doc: document power-event signals for quectel Hardware API

[Denis Kenzior]

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

Hi Martin,

On 7/19/19 7:39 AM, Martin Hundebøll wrote:
> ---
> 
> Changes since v2:
>   * Use Voltage{High,Low} instead of {High,Low}Power
> 
> Changes since v1:
>   * new patch split out from PATCHv1 7/7
> 
>   doc/quectel-hardware-api.txt | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
> 
> diff --git a/doc/quectel-hardware-api.txt b/doc/quectel-hardware-api.txt
> index 6aafbfd8..8795ef40 100644
> --- a/doc/quectel-hardware-api.txt
> +++ b/doc/quectel-hardware-api.txt
> @@ -10,6 +10,25 @@ Methods		array{string,variant} GetProperties
>   			Returns hardware properties for the modem object. See
>   			the properties section for available properties.
>   
> +Signals		PowerDown(string reason)
> +
> +			This signal is emitted on gracefull shutdowns initiated
> +			by the modem.
> +
> +			Possible reasons:
> +				"VoltageLow"	The supply voltage is too low
> +				"Normal"	The PWRKEY pin was asserted
> +				"VoltageHigh"	The supply voltage is too high
> +
> +		PowerWarning(string reason)
> +
> +			This signal is emitted when the modem detects its supply
> +			voltage is close to its supported limits.
> +
> +			Possible reasons:
> +				"VoltageLow"	The supply voltage is low
> +				"VoltageHigh"	The supply voltage is high

So just another nitpick, but our APIs use CamelCase for Property names 
and Member names (signals, method calls).  However, the values are 
always lower case.

> +
>   Properties	int32 Voltage [readonly]
>   
>   			Integer with the modem supply voltage in mV.
> 

Regards,
-Denis