* [PATCH 1/2] smsutil: sms_decode() allow to pass sms with unknown TPDU size
@ 2017-05-16 15:49 Alexander Couzens
2017-05-16 15:49 ` [PATCH 2/2] qmimodem: receive & delete sms from NV storage on 'Powered' Alexander Couzens
2017-05-16 17:49 ` [PATCH 1/2] smsutil: sms_decode() allow to pass sms with unknown TPDU size Denis Kenzior
0 siblings, 2 replies; 5+ messages in thread
From: Alexander Couzens @ 2017-05-16 15:49 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 2032 bytes --]
Certain modems (QMI based) only return a PDU length, but without
the length of the TPDU.
Without passing this information over ofono_sms_deliver_notify() down
to sms_decode(), sms_decode() will fail, as it try to interpret the
SC header as PDU header.
---
I'm not quite sure this is the best way to implement this.
a) adding a flag to ofono_sms_deliver_notify() would break the api
b) export sms_decode_address_field() which is part of smsutil.h would mean
using this function only to get the length of TPDU and throw away the
result (address), which is later called a second time.
c) add another function to cover this case is also bloated
include/sms.h | 2 ++
src/smsutil.c | 8 +++++++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/include/sms.h b/include/sms.h
index e9260561..602042d2 100644
--- a/include/sms.h
+++ b/include/sms.h
@@ -60,6 +60,8 @@ struct ofono_sms_driver {
ofono_sms_bearer_set_cb_t, void *data);
};
+/* unknown tpdu length, but contains a SC header */
+#define TPDU_UNKNOWN_WITH_SC -2
void ofono_sms_deliver_notify(struct ofono_sms *sms, const unsigned char *pdu,
int len, int tpdu_len);
void ofono_sms_status_notify(struct ofono_sms *sms, const unsigned char *pdu,
diff --git a/src/smsutil.c b/src/smsutil.c
index 24dcfaa7..d4b4a0d4 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -33,6 +33,7 @@
#include <unistd.h>
#include <glib.h>
+#include <ofono/sms.h>
#include "util.h"
#include "storage.h"
@@ -1545,7 +1546,12 @@ gboolean sms_decode(const unsigned char *pdu, int len, gboolean outgoing,
memset(out, 0, sizeof(*out));
- if (tpdu_len < len) {
+ if (tpdu_len == TPDU_UNKNOWN_WITH_SC) {
+ /* unknown TPDU length but prefixed with SC */
+ sms_decode_address_field(pdu, len, &offset, TRUE,
+ &out->sc_addr);
+ tpdu_len = len - offset;
+ } else if (tpdu_len < len) {
if (!sms_decode_address_field(pdu, len, &offset,
TRUE, &out->sc_addr))
return FALSE;
--
2.13.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] qmimodem: receive & delete sms from NV storage on 'Powered'
2017-05-16 15:49 [PATCH 1/2] smsutil: sms_decode() allow to pass sms with unknown TPDU size Alexander Couzens
@ 2017-05-16 15:49 ` Alexander Couzens
2017-05-16 18:06 ` Denis Kenzior
2017-05-16 17:49 ` [PATCH 1/2] smsutil: sms_decode() allow to pass sms with unknown TPDU size Denis Kenzior
1 sibling, 1 reply; 5+ messages in thread
From: Alexander Couzens @ 2017-05-16 15:49 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 8237 bytes --]
When the modem is powered download all message from the storage
and delete them afterwards. This is required as certain modems
(e.g. Gobi 2000) won't receive any further messages and generate
a SMPP RP-Error 'Protocol error, unspecific'. Even those messages
should not saved in the ME.
This should also the case for newer modems, like the EC20, but they
have a much bigger storage as it doesn't got noticed.
Storage capacity: EC20: 255 msg, Gobi2000: 23 msg.
---
drivers/qmimodem/sms.c | 169 ++++++++++++++++++++++++++++++++++++++++++++++++-
drivers/qmimodem/wms.h | 55 ++++++++++++++--
2 files changed, 219 insertions(+), 5 deletions(-)
diff --git a/drivers/qmimodem/sms.c b/drivers/qmimodem/sms.c
index 43bf22d1..4df65962 100644
--- a/drivers/qmimodem/sms.c
+++ b/drivers/qmimodem/sms.c
@@ -334,6 +334,170 @@ error:
g_free(cbd);
}
+static void delete_message(struct ofono_sms *sms, uint32_t message_id)
+{
+ struct qmi_param *param = qmi_param_new();
+ struct sms_data *data = ofono_sms_get_data(sms);
+
+ if (!param)
+ return;
+
+ DBG("remove message %08x", message_id);
+
+ qmi_param_append_uint8(param, QMI_WMS_PARAM_DELETE_STORAGE,
+ QMI_WMS_STORAGE_TYPE_NV);
+ qmi_param_append_uint32(param, QMI_WMS_PARAM_DELETE_MESSAGE_INDEX,
+ message_id);
+ qmi_param_append_uint8(param, QMI_WMS_PARAM_DELETE_MESSAGE_MODE,
+ QMI_WMS_MESSAGE_MODE_GSMWCDMA);
+
+ if (qmi_service_send(data->wms, QMI_WMS_DELETE_MESSAGE, param,
+ NULL, NULL, NULL) > 0)
+ return;
+}
+
+struct read_message_data {
+ struct ofono_sms *sms;
+ uint32_t message_id;
+};
+
+static void read_message_cb(struct qmi_result *result, void *user_data)
+{
+ struct read_message_data *read_data = user_data;
+ const struct qmi_wms_raw_message *message;
+ uint16_t len;
+ uint16_t plen;
+
+ message = qmi_result_get(result, QMI_WMS_PARAM_RAW_READ_MESSAGE, &len);
+ if (!message)
+ return;
+
+ /* -1 when the payload length is 0 */
+ if (sizeof(*message) - 1 > len)
+ return;
+
+ plen = GUINT16_FROM_LE(message->count);
+ if (plen + 4 > len)
+ return;
+
+ switch (message->message_tag) {
+ case QMI_WMS_MESSAGE_TAG_TYPE_MT_NOT_READ:
+ case QMI_WMS_MESSAGE_TAG_TYPE_MT_READ:
+ ofono_sms_deliver_notify(read_data->sms,
+ message->data,
+ plen,
+ TPDU_UNKNOWN_WITH_SC);
+ break;
+ default:
+ break;
+ }
+
+ delete_message(read_data->sms, read_data->message_id);
+ g_free(read_data);
+}
+
+static void read_delete_message(struct ofono_sms *sms, uint32_t message_id)
+{
+ struct sms_data *data = ofono_sms_get_data(sms);
+ struct qmi_wms_result_new_msg_notify read_message;
+ struct qmi_param *param = NULL;
+ struct read_message_data *read_data = NULL;
+
+ DBG("");
+
+ read_data = g_try_new0(struct read_message_data, 1);
+ if (!read_data)
+ return;
+ read_data->sms = sms;
+ read_data->message_id = message_id;
+
+ param = qmi_param_new();
+ if (!param)
+ goto err;
+
+ read_message.storage_index = GUINT32_TO_LE(message_id);
+ read_message.storage_type = QMI_WMS_STORAGE_TYPE_NV;
+
+ if (!qmi_param_append(param, QMI_WMS_PARAM_RAW_READ_MESSAGE_ID,
+ sizeof(read_message), &read_message))
+ goto err;
+ if (!qmi_param_append_uint8(param, QMI_WMS_PARAM_RAW_READ_MESSAGE_MODE,
+ QMI_WMS_MESSAGE_MODE_GSMWCDMA))
+ goto err;
+
+ if (qmi_service_send(data->wms, QMI_WMS_RAW_READ, param,
+ read_message_cb, read_data, NULL) > 0)
+ return;
+err:
+ qmi_param_free(param);
+ g_free(read_data);
+}
+
+static void get_msg_list_cb(struct qmi_result *result, void *user_data)
+{
+ struct ofono_sms *sms = user_data;
+ const struct qmi_wms_message_list *list;
+ uint16_t len;
+ uint32_t num;
+ int i;
+
+ DBG("");
+
+ if (qmi_result_set_error(result, NULL))
+ return;
+
+ list = qmi_result_get(result, QMI_WMS_RESULT_MESSAGE_LIST, &len);
+ if (!list)
+ return;
+
+ num = GUINT32_FROM_LE(list->count);
+ if (num * sizeof(*list->message) + 4 > len) {
+ DBG("invalid length of the message list");
+ return;
+ }
+
+ DBG("found %d messages", num);
+
+ for (i = 0; i < num; i++) {
+ uint32_t index = GUINT32_FROM_LE(list->message[i].memory_index);
+
+ DBG("message %d tags %d", index, list->message[i].message_tag);
+ /* read messages, delete the message in the callback */
+ read_delete_message(sms, index);
+ }
+}
+
+static void list_messages(struct ofono_sms *sms, enum qmi_wms_message_tag tag)
+{
+ struct sms_data *data = ofono_sms_get_data(sms);
+ struct qmi_param *param;
+
+ DBG("");
+
+ param = qmi_param_new();
+ if (!param)
+ return;
+
+ if (!qmi_param_append_uint8(param,
+ QMI_WMS_PARAM_LIST_MESSAGE_STORAGE_TYPE,
+ QMI_WMS_STORAGE_TYPE_NV))
+ goto err;
+
+ if (!qmi_param_append_uint8(param, QMI_WMS_PARAM_LIST_MESSAGE_MODE,
+ QMI_WMS_MESSAGE_MODE_GSMWCDMA))
+ goto err;
+
+ if (!qmi_param_append_uint8(param, QMI_WMS_PARAM_LIST_MESSAGE_TAG, tag))
+ goto err;
+
+ if (qmi_service_send(data->wms, QMI_WMS_GET_MSG_LIST, param,
+ get_msg_list_cb, sms, NULL) > 0)
+ return;
+err:
+ qmi_param_free(param);
+}
+
+
static void event_notify(struct qmi_result *result, void *user_data)
{
struct ofono_sms *sms = user_data;
@@ -369,6 +533,10 @@ static void set_routes_cb(struct qmi_result *result, void *user_data)
DBG("");
+ list_messages(sms, QMI_WMS_MESSAGE_TAG_TYPE_MT_READ);
+ list_messages(sms, QMI_WMS_MESSAGE_TAG_TYPE_MT_NOT_READ);
+ list_messages(sms, QMI_WMS_MESSAGE_TAG_TYPE_MO_SENT);
+ list_messages(sms, QMI_WMS_MESSAGE_TAG_TYPE_MO_NOT_SENT);
ofono_sms_register(sms);
}
diff --git a/drivers/qmimodem/wms.h b/drivers/qmimodem/wms.h
index dae86c17..fde2cbc1 100644
--- a/drivers/qmimodem/wms.h
+++ b/drivers/qmimodem/wms.h
@@ -24,6 +24,8 @@
#define QMI_WMS_SET_EVENT 1 /* Set new message conditions */
#define QMI_WMS_RAW_SEND 32 /* Send a raw message */
+#define QMI_WMS_RAW_READ 34 /* Read a raw message */
+#define QMI_WMS_DELETE_MESSAGE 36 /* Delete a message */
#define QMI_WMS_GET_MSG_LIST 49 /* Get list of messages from the device */
#define QMI_WMS_SET_ROUTES 50 /* Set routes for message memory storage */
@@ -55,16 +57,61 @@ struct qmi_wms_param_message {
} __attribute__((__packed__));
#define QMI_WMS_RESULT_MESSAGE_ID 0x01 /* uint16 */
-/* Get list of messages from the device */
-#define QMI_WMS_PARAM_STORAGE_TYPE 0x01 /* uint8 */
-#define QMI_WMS_PARAM_MESSAGE_MODE 0x11 /* uint8 */
-
#define QMI_WMS_STORAGE_TYPE_UIM 0
#define QMI_WMS_STORAGE_TYPE_NV 1
#define QMI_WMS_STORAGE_TYPE_UNKNOWN 2
#define QMI_WMS_MESSAGE_MODE_GSMWCDMA 1
+#define QMI_WMS_RESULT_MESSAGE_LIST 0x01
+struct qmi_wms_message_list {
+ uint32_t count;
+ struct {
+ uint32_t memory_index;
+ uint8_t message_tag;
+ } __attribute__((__packed__)) message[0];
+} __attribute__((__packed__));
+
+enum qmi_wms_message_tag {
+ QMI_WMS_MESSAGE_TAG_TYPE_MT_READ = 0x00,
+ QMI_WMS_MESSAGE_TAG_TYPE_MT_NOT_READ = 0x01,
+ QMI_WMS_MESSAGE_TAG_TYPE_MO_SENT = 0x02,
+ QMI_WMS_MESSAGE_TAG_TYPE_MO_NOT_SENT = 0x03
+};
+
+/* read a message */
+enum qmi_wms_param_read_message_req {
+ QMI_WMS_PARAM_RAW_READ_MESSAGE_ID = 0x01,
+ QMI_WMS_PARAM_RAW_READ_MESSAGE_MODE = 0x10,
+};
+enum qmi_wms_param_read_message_res {
+ QMI_WMS_PARAM_RAW_READ_MESSAGE = 0x01,
+};
+struct qmi_wms_raw_message {
+ uint8_t message_tag;
+ uint8_t message_format;
+ uint16_t count;
+ uint8_t data[0];
+} __attribute__((__packed__));
+
+/* delete a message */
+enum qmi_wms_params_delete_req {
+ QMI_WMS_PARAM_DELETE_STORAGE = 0x01,
+ QMI_WMS_PARAM_DELETE_MESSAGE_INDEX = 0x10,
+ QMI_WMS_PARAM_DELETE_MESSAGE_MODE = 0x12,
+};
+
+/* Get list of messages from the device response */
+enum qmi_wms_params_list_message_req {
+ QMI_WMS_PARAM_LIST_MESSAGE_STORAGE_TYPE = 0x01,
+ QMI_WMS_PARAM_LIST_MESSAGE_TAG = 0x11,
+ QMI_WMS_PARAM_LIST_MESSAGE_MODE = 0x12,
+};
+
+enum qmi_wms_params_list_message_res {
+ QMI_WMS_PARAM_LIST_MESSAGE_LIST = 0x01,
+};
+
/* Get routes for message memory storage */
#define QMI_WMS_RESULT_ROUTE_LIST 0x01
#define QMI_WMS_PARAM_ROUTE_LIST 0x01
--
2.13.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] smsutil: sms_decode() allow to pass sms with unknown TPDU size
2017-05-16 15:49 [PATCH 1/2] smsutil: sms_decode() allow to pass sms with unknown TPDU size Alexander Couzens
2017-05-16 15:49 ` [PATCH 2/2] qmimodem: receive & delete sms from NV storage on 'Powered' Alexander Couzens
@ 2017-05-16 17:49 ` Denis Kenzior
2017-05-16 23:01 ` Alexander Couzens
1 sibling, 1 reply; 5+ messages in thread
From: Denis Kenzior @ 2017-05-16 17:49 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 2963 bytes --]
Hi Alexander,
On 05/16/2017 10:49 AM, Alexander Couzens wrote:
> Certain modems (QMI based) only return a PDU length, but without
> the length of the TPDU.
It looks like the current QMI_WMS_RESULT_NEW_MSG_NOTIFY assumes that the
message doesn't have an SMSC preceeding it. Or is it that we are not
parsing some other parameters?
> Without passing this information over ofono_sms_deliver_notify() down
> to sms_decode(), sms_decode() will fail, as it try to interpret the
> SC header as PDU header.
How do we know when a PDU contains the SC address and when it doesn't?
> ---
>
> I'm not quite sure this is the best way to implement this.
>
> a) adding a flag to ofono_sms_deliver_notify() would break the api
No, this is not the way to do it
> b) export sms_decode_address_field() which is part of smsutil.h would mean
> using this function only to get the length of TPDU and throw away the
> result (address), which is later called a second time.
In-tree drivers can freely utilize core utilities such as smsutil and
stkutil. So this isn't really a problem. Since the QMI API is quite
dumb, then parsing the SC address field twice is probably the least evil
way to do this.
> c) add another function to cover this case is also bloated
>
>
> include/sms.h | 2 ++
> src/smsutil.c | 8 +++++++-
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/include/sms.h b/include/sms.h
> index e9260561..602042d2 100644
> --- a/include/sms.h
> +++ b/include/sms.h
> @@ -60,6 +60,8 @@ struct ofono_sms_driver {
> ofono_sms_bearer_set_cb_t, void *data);
> };
>
> +/* unknown tpdu length, but contains a SC header */
> +#define TPDU_UNKNOWN_WITH_SC -2
Nak. This couples together the core <-> driver API with the utility
functions. They were always independent of each other and should remain
this way.
> void ofono_sms_deliver_notify(struct ofono_sms *sms, const unsigned char *pdu,
> int len, int tpdu_len);
> void ofono_sms_status_notify(struct ofono_sms *sms, const unsigned char *pdu,
> diff --git a/src/smsutil.c b/src/smsutil.c
> index 24dcfaa7..d4b4a0d4 100644
> --- a/src/smsutil.c
> +++ b/src/smsutil.c
> @@ -33,6 +33,7 @@
> #include <unistd.h>
>
> #include <glib.h>
> +#include <ofono/sms.h>
>
> #include "util.h"
> #include "storage.h"
> @@ -1545,7 +1546,12 @@ gboolean sms_decode(const unsigned char *pdu, int len, gboolean outgoing,
>
> memset(out, 0, sizeof(*out));
>
> - if (tpdu_len < len) {
> + if (tpdu_len == TPDU_UNKNOWN_WITH_SC) {
> + /* unknown TPDU length but prefixed with SC */
> + sms_decode_address_field(pdu, len, &offset, TRUE,
> + &out->sc_addr);
Why don't you actually check the return value ?
> + tpdu_len = len - offset;
> + } else if (tpdu_len < len) {
> if (!sms_decode_address_field(pdu, len, &offset,
> TRUE, &out->sc_addr))
> return FALSE;
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] qmimodem: receive & delete sms from NV storage on 'Powered'
2017-05-16 15:49 ` [PATCH 2/2] qmimodem: receive & delete sms from NV storage on 'Powered' Alexander Couzens
@ 2017-05-16 18:06 ` Denis Kenzior
0 siblings, 0 replies; 5+ messages in thread
From: Denis Kenzior @ 2017-05-16 18:06 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 9447 bytes --]
Hi Alexander,
On 05/16/2017 10:49 AM, Alexander Couzens wrote:
> When the modem is powered download all message from the storage
> and delete them afterwards. This is required as certain modems
> (e.g. Gobi 2000) won't receive any further messages and generate
> a SMPP RP-Error 'Protocol error, unspecific'. Even those messages
> should not saved in the ME.
>
> This should also the case for newer modems, like the EC20, but they
> have a much bigger storage as it doesn't got noticed.
> Storage capacity: EC20: 255 msg, Gobi2000: 23 msg.
> ---
> drivers/qmimodem/sms.c | 169 ++++++++++++++++++++++++++++++++++++++++++++++++-
> drivers/qmimodem/wms.h | 55 ++++++++++++++--
> 2 files changed, 219 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/qmimodem/sms.c b/drivers/qmimodem/sms.c
> index 43bf22d1..4df65962 100644
> --- a/drivers/qmimodem/sms.c
> +++ b/drivers/qmimodem/sms.c
> @@ -334,6 +334,170 @@ error:
> g_free(cbd);
> }
>
> +static void delete_message(struct ofono_sms *sms, uint32_t message_id)
> +{
> + struct qmi_param *param = qmi_param_new();
> + struct sms_data *data = ofono_sms_get_data(sms);
> +
> + if (!param)
> + return;
> +
> + DBG("remove message %08x", message_id);
> +
> + qmi_param_append_uint8(param, QMI_WMS_PARAM_DELETE_STORAGE,
> + QMI_WMS_STORAGE_TYPE_NV);
> + qmi_param_append_uint32(param, QMI_WMS_PARAM_DELETE_MESSAGE_INDEX,
> + message_id);
> + qmi_param_append_uint8(param, QMI_WMS_PARAM_DELETE_MESSAGE_MODE,
> + QMI_WMS_MESSAGE_MODE_GSMWCDMA);
> +
> + if (qmi_service_send(data->wms, QMI_WMS_DELETE_MESSAGE, param,
> + NULL, NULL, NULL) > 0)
> + return;
Okay, but pointless. If you don't care about the return value might as
well just don't wrap it in the 'if'
> +}
> +
> +struct read_message_data {
> + struct ofono_sms *sms;
> + uint32_t message_id;
> +};
> +
> +static void read_message_cb(struct qmi_result *result, void *user_data)
> +{
> + struct read_message_data *read_data = user_data;
> + const struct qmi_wms_raw_message *message;
> + uint16_t len;
> + uint16_t plen;
> +
> + message = qmi_result_get(result, QMI_WMS_PARAM_RAW_READ_MESSAGE, &len);
> + if (!message)
> + return;
You're leaking read_data here and a few other places below. Please
learn to use the destroy callback, that is what it is there for.
> +
> + /* -1 when the payload length is 0 */
> + if (sizeof(*message) - 1 > len)
> + return;
> +
???
> + plen = GUINT16_FROM_LE(message->count);
> + if (plen + 4 > len)
> + return;
> +
> + switch (message->message_tag) {
> + case QMI_WMS_MESSAGE_TAG_TYPE_MT_NOT_READ:
> + case QMI_WMS_MESSAGE_TAG_TYPE_MT_READ:
> + ofono_sms_deliver_notify(read_data->sms,
> + message->data,
> + plen,
> + TPDU_UNKNOWN_WITH_SC);
> + break;
> + default:
> + break;
> + }
> +
> + delete_message(read_data->sms, read_data->message_id);
> + g_free(read_data);
> +}
> +
> +static void read_delete_message(struct ofono_sms *sms, uint32_t message_id)
> +{
> + struct sms_data *data = ofono_sms_get_data(sms);
> + struct qmi_wms_result_new_msg_notify read_message;
> + struct qmi_param *param = NULL;
> + struct read_message_data *read_data = NULL;
> +
> + DBG("");
> +
> + read_data = g_try_new0(struct read_message_data, 1);
> + if (!read_data)
> + return;
Not our style. See doc/coding-style.txt item M1. Also, we have deemed
it acceptable to use g_new0 for small allocations, so just make your
life simple.
> + read_data->sms = sms;
> + read_data->message_id = message_id;
> +
> + param = qmi_param_new();
> + if (!param)
> + goto err;
> +
> + read_message.storage_index = GUINT32_TO_LE(message_id);
> + read_message.storage_type = QMI_WMS_STORAGE_TYPE_NV;
> +
> + if (!qmi_param_append(param, QMI_WMS_PARAM_RAW_READ_MESSAGE_ID,
> + sizeof(read_message), &read_message))
> + goto err;
Not our style
> + if (!qmi_param_append_uint8(param, QMI_WMS_PARAM_RAW_READ_MESSAGE_MODE,
> + QMI_WMS_MESSAGE_MODE_GSMWCDMA))
> + goto err;
> +
> + if (qmi_service_send(data->wms, QMI_WMS_RAW_READ, param,
> + read_message_cb, read_data, NULL) > 0)
> + return;
As above
> +err:
> + qmi_param_free(param);
> + g_free(read_data);
> +}
> +
> +static void get_msg_list_cb(struct qmi_result *result, void *user_data)
> +{
> + struct ofono_sms *sms = user_data;
> + const struct qmi_wms_message_list *list;
> + uint16_t len;
> + uint32_t num;
> + int i;
> +
> + DBG("");
> +
> + if (qmi_result_set_error(result, NULL))
> + return;
> +
> + list = qmi_result_get(result, QMI_WMS_RESULT_MESSAGE_LIST, &len);
> + if (!list)
> + return;
> +
> + num = GUINT32_FROM_LE(list->count);
> + if (num * sizeof(*list->message) + 4 > len) {
> + DBG("invalid length of the message list");
> + return;
> + }
> +
> + DBG("found %d messages", num);
> +
> + for (i = 0; i < num; i++) {
> + uint32_t index = GUINT32_FROM_LE(list->message[i].memory_index);
> +
> + DBG("message %d tags %d", index, list->message[i].message_tag);
> + /* read messages, delete the message in the callback */
> + read_delete_message(sms, index);
> + }
> +}
> +
> +static void list_messages(struct ofono_sms *sms, enum qmi_wms_message_tag tag)
> +{
> + struct sms_data *data = ofono_sms_get_data(sms);
> + struct qmi_param *param;
> +
> + DBG("");
> +
> + param = qmi_param_new();
> + if (!param)
> + return;
> +
> + if (!qmi_param_append_uint8(param,
> + QMI_WMS_PARAM_LIST_MESSAGE_STORAGE_TYPE,
> + QMI_WMS_STORAGE_TYPE_NV))
> + goto err;
> +
> + if (!qmi_param_append_uint8(param, QMI_WMS_PARAM_LIST_MESSAGE_MODE,
> + QMI_WMS_MESSAGE_MODE_GSMWCDMA))
> + goto err;
> +
> + if (!qmi_param_append_uint8(param, QMI_WMS_PARAM_LIST_MESSAGE_TAG, tag))
> + goto err;
> +
> + if (qmi_service_send(data->wms, QMI_WMS_GET_MSG_LIST, param,
> + get_msg_list_cb, sms, NULL) > 0)
> + return;
> +err:
> + qmi_param_free(param);
> +}
> +
> +
> static void event_notify(struct qmi_result *result, void *user_data)
> {
> struct ofono_sms *sms = user_data;
> @@ -369,6 +533,10 @@ static void set_routes_cb(struct qmi_result *result, void *user_data)
>
> DBG("");
>
> + list_messages(sms, QMI_WMS_MESSAGE_TAG_TYPE_MT_READ);
> + list_messages(sms, QMI_WMS_MESSAGE_TAG_TYPE_MT_NOT_READ);
> + list_messages(sms, QMI_WMS_MESSAGE_TAG_TYPE_MO_SENT);
> + list_messages(sms, QMI_WMS_MESSAGE_TAG_TYPE_MO_NOT_SENT);
> ofono_sms_register(sms);
> }
>
> diff --git a/drivers/qmimodem/wms.h b/drivers/qmimodem/wms.h
> index dae86c17..fde2cbc1 100644
> --- a/drivers/qmimodem/wms.h
> +++ b/drivers/qmimodem/wms.h
> @@ -24,6 +24,8 @@
> #define QMI_WMS_SET_EVENT 1 /* Set new message conditions */
>
> #define QMI_WMS_RAW_SEND 32 /* Send a raw message */
> +#define QMI_WMS_RAW_READ 34 /* Read a raw message */
> +#define QMI_WMS_DELETE_MESSAGE 36 /* Delete a message */
>
> #define QMI_WMS_GET_MSG_LIST 49 /* Get list of messages from the device */
> #define QMI_WMS_SET_ROUTES 50 /* Set routes for message memory storage */
> @@ -55,16 +57,61 @@ struct qmi_wms_param_message {
> } __attribute__((__packed__));
> #define QMI_WMS_RESULT_MESSAGE_ID 0x01 /* uint16 */
>
> -/* Get list of messages from the device */
> -#define QMI_WMS_PARAM_STORAGE_TYPE 0x01 /* uint8 */
> -#define QMI_WMS_PARAM_MESSAGE_MODE 0x11 /* uint8 */
> -
> #define QMI_WMS_STORAGE_TYPE_UIM 0
> #define QMI_WMS_STORAGE_TYPE_NV 1
> #define QMI_WMS_STORAGE_TYPE_UNKNOWN 2
>
> #define QMI_WMS_MESSAGE_MODE_GSMWCDMA 1
>
> +#define QMI_WMS_RESULT_MESSAGE_LIST 0x01
> +struct qmi_wms_message_list {
> + uint32_t count;
> + struct {
> + uint32_t memory_index;
> + uint8_t message_tag;
> + } __attribute__((__packed__)) message[0];
> +} __attribute__((__packed__));
> +
> +enum qmi_wms_message_tag {
> + QMI_WMS_MESSAGE_TAG_TYPE_MT_READ = 0x00,
> + QMI_WMS_MESSAGE_TAG_TYPE_MT_NOT_READ = 0x01,
> + QMI_WMS_MESSAGE_TAG_TYPE_MO_SENT = 0x02,
> + QMI_WMS_MESSAGE_TAG_TYPE_MO_NOT_SENT = 0x03
> +};
> +
> +/* read a message */
> +enum qmi_wms_param_read_message_req {
> + QMI_WMS_PARAM_RAW_READ_MESSAGE_ID = 0x01,
> + QMI_WMS_PARAM_RAW_READ_MESSAGE_MODE = 0x10,
> +};
> +enum qmi_wms_param_read_message_res {
> + QMI_WMS_PARAM_RAW_READ_MESSAGE = 0x01,
> +};
> +struct qmi_wms_raw_message {
> + uint8_t message_tag;
msg_tag
> + uint8_t message_format;
msg_format
> + uint16_t count;
Should this not be msg_length to be consistent with
qmi_wms_param_message and qmi_wms_result_message
> + uint8_t data[0];
> +} __attribute__((__packed__));
> +
> +/* delete a message */
> +enum qmi_wms_params_delete_req {
> + QMI_WMS_PARAM_DELETE_STORAGE = 0x01,
> + QMI_WMS_PARAM_DELETE_MESSAGE_INDEX = 0x10,
> + QMI_WMS_PARAM_DELETE_MESSAGE_MODE = 0x12,
> +};
> +
> +/* Get list of messages from the device response */
> +enum qmi_wms_params_list_message_req {
> + QMI_WMS_PARAM_LIST_MESSAGE_STORAGE_TYPE = 0x01,
> + QMI_WMS_PARAM_LIST_MESSAGE_TAG = 0x11,
> + QMI_WMS_PARAM_LIST_MESSAGE_MODE = 0x12,
> +};
> +
> +enum qmi_wms_params_list_message_res {
> + QMI_WMS_PARAM_LIST_MESSAGE_LIST = 0x01,
> +};
> +
> /* Get routes for message memory storage */
> #define QMI_WMS_RESULT_ROUTE_LIST 0x01
> #define QMI_WMS_PARAM_ROUTE_LIST 0x01
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] smsutil: sms_decode() allow to pass sms with unknown TPDU size
2017-05-16 17:49 ` [PATCH 1/2] smsutil: sms_decode() allow to pass sms with unknown TPDU size Denis Kenzior
@ 2017-05-16 23:01 ` Alexander Couzens
0 siblings, 0 replies; 5+ messages in thread
From: Alexander Couzens @ 2017-05-16 23:01 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1826 bytes --]
Hi Denis,
> Hi Alexander,
>
> On 05/16/2017 10:49 AM, Alexander Couzens wrote:
> > Certain modems (QMI based) only return a PDU length, but without
> > the length of the TPDU.
>
> It looks like the current QMI_WMS_RESULT_NEW_MSG_NOTIFY assumes that
> the message doesn't have an SMSC preceeding it. Or is it that we are
> not parsing some other parameters?
The nice thing here is:
When receiving a SMS PDU over the notify callback, it doesn't contain a
SC address, but when reading it from NV it does.
> > Without passing this information over ofono_sms_deliver_notify()
> > down to sms_decode(), sms_decode() will fail, as it try to
> > interpret the SC header as PDU header.
>
> How do we know when a PDU contains the SC address and when it doesn't?
I don't know. Sorry, I didn't read (yet) the whole spec.
Originally I though it would be possible and a good idea to detect SC,
but sms_decode_address_field() can exit early with success and
sms_decode() fail later when decoding. A second pass is then required.
I dropped that idea and use fixed expectation, hopefully QMI behave the
same way on different devices.
> [..]
> > b) export sms_decode_address_field() which is part of smsutil.h
> > would mean using this function only to get the length of TPDU and
> > throw away the result (address), which is later called a second
> > time.
>
> In-tree drivers can freely utilize core utilities such as smsutil and
> stkutil. So this isn't really a problem. Since the QMI API is quite
> dumb, then parsing the SC address field twice is probably the least
> evil way to do this.
Ok. Will do it that way.
--
Alexander Couzens
mail: lynxis(a)fe80.eu
jabber: lynxis(a)fe80.eu
mobile: +4915123277221
gpg: 390D CF78 8BF9 AA50 4F8F F1E2 C29E 9DA6 A0DF 8604
[-- Attachment #2: attachment.sig --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-05-16 23:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-16 15:49 [PATCH 1/2] smsutil: sms_decode() allow to pass sms with unknown TPDU size Alexander Couzens
2017-05-16 15:49 ` [PATCH 2/2] qmimodem: receive & delete sms from NV storage on 'Powered' Alexander Couzens
2017-05-16 18:06 ` Denis Kenzior
2017-05-16 17:49 ` [PATCH 1/2] smsutil: sms_decode() allow to pass sms with unknown TPDU size Denis Kenzior
2017-05-16 23:01 ` Alexander Couzens
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.