5:35 p.m.
Some ZTE modems don't support sending a CBS topics list of more than 50 characters.
Bertrand Aygon (2):
cbs: split the topics list for ZTE modem.
zte: specify the vendor to send cbs topics in split mode.
drivers/atmodem/cbs.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++--
plugins/zte.c | 2 +-
2 files changed, 84 insertions(+), 4 deletions(-)
--
1.7.4.1
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
5:35 p.m.
New subject: [PATCH 1/2] cbs: split the topics list for ZTE modem.
---
drivers/atmodem/cbs.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 83 insertions(+), 3 deletions(-)
diff --git a/drivers/atmodem/cbs.c b/drivers/atmodem/cbs.c
index e2e1ca4..3c250d2 100644
--- a/drivers/atmodem/cbs.c
+++ b/drivers/atmodem/cbs.c
@@ -47,6 +47,8 @@ struct cbs_data {
GAtChat *chat;
gboolean cscb_mode_1;
unsigned int vendor;
+ unsigned int string_limit;
+ gchar *rt; /* remaining topics */
};
static void at_cbm_notify(GAtResult *result, gpointer user_data)
@@ -105,6 +107,56 @@ static void at_cscb_set_cb(gboolean ok, GAtResult *result, gpointer
user_data)
cb(&error, cbd->data);
}
+static void at_cscb_set_cb_split(gboolean ok, GAtResult *result, gpointer user_data)
+{
+ struct cb_data *cbd = user_data;
+ struct cbs_data *data = ofono_cbs_get_data(cbd->user);
+ ofono_cbs_set_cb_t cb = cbd->cb;
+ char *buf;
+ unsigned int id;
+
+ DBG("%s", data->rt);
+
+ if (strlen(data->rt) > data->string_limit) {
+ gchar *topics_split = g_strndup(data->rt, data->string_limit);
+ gchar *new_rt;
+
+ if (data->rt[data->string_limit] != ',') {
+ char *token = strrchr(topics_split, ',');
+ *token = 0;
+ }
+
+ new_rt = g_strdup((char *) ((void *) data->rt +
+ strlen(topics_split) + 1));
+
+ g_free(data->rt);
+ data->rt = new_rt;
+
+ buf = g_strdup_printf("AT+CSCB=0,\"%s\"", topics_split);
+
+ id = g_at_chat_send(data->chat, buf, none_prefix,
+ at_cscb_set_cb_split, cbd, NULL);
+
+ g_free(topics_split);
+ } else {
+ buf = g_strdup_printf("AT+CSCB=0,\"%s\"", data->rt);
+
+ id = g_at_chat_send(data->chat, buf, none_prefix,
+ at_cscb_set_cb, cbd, NULL);
+
+ g_free(data->rt);
+ }
+
+ g_free(buf);
+
+ if (id > 0)
+ return;
+
+ g_free(cbd);
+
+ CALLBACK_WITH_FAILURE(cb, user_data);
+}
+
static void at_cbs_set_topics(struct ofono_cbs *cbs, const char *topics,
ofono_cbs_set_cb_t cb, void *user_data)
{
@@ -113,7 +165,7 @@ static void at_cbs_set_topics(struct ofono_cbs *cbs, const char
*topics,
char *buf;
unsigned int id;
- DBG("");
+ DBG("%s", topics);
/* For the Qualcomm based devices it is required to clear
* the list of topics first. Otherwise setting the new
@@ -128,14 +180,40 @@ static void at_cbs_set_topics(struct ofono_cbs *cbs, const char
*topics,
g_at_chat_send(data->chat, "AT+CSCB=1", none_prefix,
NULL, NULL, NULL);
break;
+ case OFONO_VENDOR_ZTE:
+ data->string_limit = 50;
+ g_at_chat_send(data->chat, "AT+CSCB=1", none_prefix,
+ NULL, NULL, NULL);
+ break;
default:
break;
}
- buf = g_strdup_printf("AT+CSCB=0,\"%s\"", topics);
+ if (data->string_limit && strlen(topics) > data->string_limit) {
+ gchar *topics_split = g_strndup(topics, data->string_limit);
+
+ if (topics[data->string_limit] != ',') {
+ char *token = strrchr(topics_split, ',');
+ *token = 0;
+ }
+
+ data->rt = g_strdup((char *) ((void *) topics +
+ strlen(topics_split) + 1));
+
+ cbd->user = cbs;
- id = g_at_chat_send(data->chat, buf, none_prefix,
+ buf = g_strdup_printf("AT+CSCB=0,\"%s\"", topics_split);
+
+ id = g_at_chat_send(data->chat, buf, none_prefix,
+ at_cscb_set_cb_split, cbd, NULL);
+
+ g_free(topics_split);
+ } else {
+ buf = g_strdup_printf("AT+CSCB=0,\"%s\"", topics);
+
+ id = g_at_chat_send(data->chat, buf, none_prefix,
at_cscb_set_cb, cbd, g_free);
+ }
g_free(buf);
@@ -240,6 +318,8 @@ static int at_cbs_probe(struct ofono_cbs *cbs, unsigned int vendor,
data = g_new0(struct cbs_data, 1);
data->chat = g_at_chat_clone(chat);
data->vendor = vendor;
+ data->string_limit = 0;
+ data->rt = NULL;
ofono_cbs_set_data(cbs, data);
--
1.7.4.1
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
11:07 p.m.
New subject: [PATCH 1/2] cbs: split the topics list for ZTE modem.
Hi Bertrand,
drivers/atmodem/cbs.c | 86
+++++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 83 insertions(+), 3 deletions(-)
diff --git a/drivers/atmodem/cbs.c b/drivers/atmodem/cbs.c
index e2e1ca4..3c250d2 100644
--- a/drivers/atmodem/cbs.c
+++ b/drivers/atmodem/cbs.c
@@ -47,6 +47,8 @@ struct cbs_data {
GAtChat *chat;
gboolean cscb_mode_1;
unsigned int vendor;
+ unsigned int string_limit;
+ gchar *rt; /* remaining topics */
Using rt is not a good variable name.
};
static void at_cbm_notify(GAtResult *result, gpointer user_data)
@@ -105,6 +107,56 @@ static void at_cscb_set_cb(gboolean ok, GAtResult *result, gpointer
user_data)
cb(&error, cbd->data);
}
+static void at_cscb_set_cb_split(gboolean ok, GAtResult *result, gpointer user_data)
+{
+ struct cb_data *cbd = user_data;
+ struct cbs_data *data = ofono_cbs_get_data(cbd->user);
+ ofono_cbs_set_cb_t cb = cbd->cb;
+ char *buf;
+ unsigned int id;
+
+ DBG("%s", data->rt);
Where is the handling if this command responds with ERROR?
+ if (strlen(data->rt) > data->string_limit) {
+ gchar *topics_split = g_strndup(data->rt, data->string_limit);
+ gchar *new_rt;
+
+ if (data->rt[data->string_limit] != ',') {
+ char *token = strrchr(topics_split, ',');
+ *token = 0;
+ }
+
+ new_rt = g_strdup((char *) ((void *) data->rt +
+ strlen(topics_split) + 1));
+
+ g_free(data->rt);
+ data->rt = new_rt;
This is code duplication. Split it our into a proper helper function.
+ buf = g_strdup_printf("AT+CSCB=0,\"%s\"",
topics_split);
+
+ id = g_at_chat_send(data->chat, buf, none_prefix,
+ at_cscb_set_cb_split, cbd, NULL);
+
+ g_free(topics_split);
+ } else {
+ buf = g_strdup_printf("AT+CSCB=0,\"%s\"", data->rt);
+
+ id = g_at_chat_send(data->chat, buf, none_prefix,
+ at_cscb_set_cb, cbd, NULL);
This is a memory leak btw. Who is freeing cbd?
+ g_free(data->rt);
+ }
+
+ g_free(buf);
+
+ if (id > 0)
+ return;
+
+ g_free(cbd);
+
+ CALLBACK_WITH_FAILURE(cb, user_data);
+}
+
static void at_cbs_set_topics(struct ofono_cbs *cbs, const char *topics,
ofono_cbs_set_cb_t cb, void *user_data)
{
@@ -113,7 +165,7 @@ static void at_cbs_set_topics(struct ofono_cbs *cbs, const char
*topics,
char *buf;
unsigned int id;
- DBG("");
+ DBG("%s", topics);
/* For the Qualcomm based devices it is required to clear
* the list of topics first. Otherwise setting the new
@@ -128,14 +180,40 @@ static void at_cbs_set_topics(struct ofono_cbs *cbs, const char
*topics,
g_at_chat_send(data->chat, "AT+CSCB=1", none_prefix,
NULL, NULL, NULL);
break;
+ case OFONO_VENDOR_ZTE:
+ data->string_limit = 50;
+ g_at_chat_send(data->chat, "AT+CSCB=1", none_prefix,
+ NULL, NULL, NULL);
+ break;
Why are we copying the Qualcomm quirk over for ZTE?
default:
break;
}
- buf = g_strdup_printf("AT+CSCB=0,\"%s\"", topics);
+ if (data->string_limit && strlen(topics) > data->string_limit) {
+ gchar *topics_split = g_strndup(topics, data->string_limit);
+
+ if (topics[data->string_limit] != ',') {
+ char *token = strrchr(topics_split, ',');
+ *token = 0;
+ }
+
+ data->rt = g_strdup((char *) ((void *) topics +
+ strlen(topics_split) + 1));
+
+ cbd->user = cbs;
- id = g_at_chat_send(data->chat, buf, none_prefix,
+ buf = g_strdup_printf("AT+CSCB=0,\"%s\"", topics_split);
+
+ id = g_at_chat_send(data->chat, buf, none_prefix,
+ at_cscb_set_cb_split, cbd, NULL);
+
+ g_free(topics_split);
+ } else {
+ buf = g_strdup_printf("AT+CSCB=0,\"%s\"", topics);
+
+ id = g_at_chat_send(data->chat, buf, none_prefix,
at_cscb_set_cb, cbd, g_free);
+ }
g_free(buf);
@@ -240,6 +318,8 @@ static int at_cbs_probe(struct ofono_cbs *cbs, unsigned int vendor,
data = g_new0(struct cbs_data, 1);
data->chat = g_at_chat_clone(chat);
data->vendor = vendor;
+ data->string_limit = 0;
+ data->rt = NULL;
ofono_cbs_set_data(cbs, data);
Regards
Marcel
11:14 p.m.
New subject: [PATCH 1/2] cbs: split the topics list for ZTE modem.
Hi Marcel,
Thanks for your review.
> drivers/atmodem/cbs.c | 86
+++++++++++++++++++++++++++++++++++++++++++++++--
> 1 files changed, 83 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/atmodem/cbs.c b/drivers/atmodem/cbs.c
> index e2e1ca4..3c250d2 100644
> --- a/drivers/atmodem/cbs.c
> +++ b/drivers/atmodem/cbs.c
> @@ -47,6 +47,8 @@ struct cbs_data {
> GAtChat *chat;
> gboolean cscb_mode_1;
> unsigned int vendor;
> + unsigned int string_limit;
> + gchar *rt; /* remaining topics */
Using rt is not a good variable name.
Ok, I will use r_topics. I would prefer to avoid remaining_topics, to help with 80c
limitation.
> };
>
> static void at_cbm_notify(GAtResult *result, gpointer user_data)
> @@ -105,6 +107,56 @@ static void at_cscb_set_cb(gboolean ok,
GAtResult *result, gpointer user_data)
> cb(&error, cbd->data);
> }
>
> +static void at_cscb_set_cb_split(gboolean ok, GAtResult *result,
gpointer user_data)
> +{
> + struct cb_data *cbd = user_data;
> + struct cbs_data *data = ofono_cbs_get_data(cbd->user);
> + ofono_cbs_set_cb_t cb = cbd->cb;
> + char *buf;
> + unsigned int id;
> +
> + DBG("%s", data->rt);
Where is the handling if this command responds with ERROR?
Oops, I missed it. I will include it.
> + if (strlen(data->rt) > data->string_limit) {
> + gchar *topics_split = g_strndup(data->rt, data-
>string_limit);
> + gchar *new_rt;
> +
> + if (data->rt[data->string_limit] != ',') {
> + char *token = strrchr(topics_split, ',');
> + *token = 0;
> + }
> +
> + new_rt = g_strdup((char *) ((void *) data->rt +
> + strlen(topics_split) + 1));
> +
> + g_free(data->rt);
> + data->rt = new_rt;
This is code duplication. Split it our into a proper helper function.
Ok.
> + buf = g_strdup_printf("AT+CSCB=0,\"%s\"",
topics_split);
> +
> + id = g_at_chat_send(data->chat, buf, none_prefix,
> + at_cscb_set_cb_split, cbd, NULL);
> +
> + g_free(topics_split);
> + } else {
> + buf = g_strdup_printf("AT+CSCB=0,\"%s\"", data->rt);
> +
> + id = g_at_chat_send(data->chat, buf, none_prefix,
> + at_cscb_set_cb, cbd, NULL);
This is a memory leak btw. Who is freeing cbd?
I will add the g_free is the GDestroyNotify.
> + g_free(data->rt);
> + }
> +
> + g_free(buf);
> +
> + if (id > 0)
> + return;
> +
> + g_free(cbd);
> +
> + CALLBACK_WITH_FAILURE(cb, user_data);
> +}
> +
> static void at_cbs_set_topics(struct ofono_cbs *cbs, const char
*topics,
> ofono_cbs_set_cb_t cb, void *user_data)
> {
> @@ -113,7 +165,7 @@ static void at_cbs_set_topics(struct ofono_cbs
*cbs, const char *topics,
> char *buf;
> unsigned int id;
>
> - DBG("");
> + DBG("%s", topics);
>
> /* For the Qualcomm based devices it is required to clear
> * the list of topics first. Otherwise setting the new
> @@ -128,14 +180,40 @@ static void at_cbs_set_topics(struct ofono_cbs
*cbs, const char *topics,
> g_at_chat_send(data->chat, "AT+CSCB=1", none_prefix,
> NULL, NULL, NULL);
> break;
> + case OFONO_VENDOR_ZTE:
> + data->string_limit = 50;
> + g_at_chat_send(data->chat, "AT+CSCB=1", none_prefix,
> + NULL, NULL, NULL);
> + break;
Why are we copying the Qualcomm quirk over for ZTE?
Because previously, ZTE modem is using QUALCOMM vendor for CBS creation.
So I do not want to change this behaviour.
> default:
> break;
> }
>
> - buf = g_strdup_printf("AT+CSCB=0,\"%s\"", topics);
> + if (data->string_limit && strlen(topics) > data->string_limit) {
> + gchar *topics_split = g_strndup(topics, data-
>string_limit);
> +
> + if (topics[data->string_limit] != ',') {
> + char *token = strrchr(topics_split, ',');
> + *token = 0;
> + }
> +
> + data->rt = g_strdup((char *) ((void *) topics +
> + strlen(topics_split) + 1));
> +
> + cbd->user = cbs;
>
> - id = g_at_chat_send(data->chat, buf, none_prefix,
> + buf = g_strdup_printf("AT+CSCB=0,\"%s\"", topics_split);
> +
> + id = g_at_chat_send(data->chat, buf, none_prefix,
> + at_cscb_set_cb_split, cbd, NULL);
> +
> + g_free(topics_split);
> + } else {
> + buf = g_strdup_printf("AT+CSCB=0,\"%s\"", topics);
> +
> + id = g_at_chat_send(data->chat, buf, none_prefix,
> at_cscb_set_cb, cbd, g_free);
> + }
>
> g_free(buf);
>
> @@ -240,6 +318,8 @@ static int at_cbs_probe(struct ofono_cbs *cbs,
unsigned int vendor,
> data = g_new0(struct cbs_data, 1);
> data->chat = g_at_chat_clone(chat);
> data->vendor = vendor;
> + data->string_limit = 0;
> + data->rt = NULL;
>
> ofono_cbs_set_data(cbs, data);
>
Thanks again,
Bertrand
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
11:38 p.m.
New subject: [PATCH 1/2] cbs: split the topics list for ZTE modem.
Hi Betrand,
> > /* For the Qualcomm based devices it is required to
clear
> > * the list of topics first. Otherwise setting the new
> > @@ -128,14 +180,40 @@ static void at_cbs_set_topics(struct ofono_cbs
> *cbs, const char *topics,
> > g_at_chat_send(data->chat, "AT+CSCB=1", none_prefix,
> > NULL, NULL, NULL);
> > break;
> > + case OFONO_VENDOR_ZTE:
> > + data->string_limit = 50;
> > + g_at_chat_send(data->chat, "AT+CSCB=1", none_prefix,
> > + NULL, NULL, NULL);
> > + break;
>
> Why are we copying the Qualcomm quirk over for ZTE?
Because previously, ZTE modem is using QUALCOMM vendor for CBS creation.
So I do not want to change this behaviour.
test it again. I am not even 100% that ZTE needs this behavior if the
limitation was on the command line length. However it might. So please
double check and comment it properly.
Regards
Marcel
12:29 a.m.
New subject: [PATCH 1/2] cbs: split the topics list for ZTE modem.
Hi,
> > > /* For the Qualcomm based devices it is required to
clear
> > > * the list of topics first. Otherwise setting the new
> > > @@ -128,14 +180,40 @@ static void at_cbs_set_topics(struct
ofono_cbs
> > *cbs, const char *topics,
> > > g_at_chat_send(data->chat, "AT+CSCB=1", none_prefix,
> > > NULL, NULL, NULL);
> > > break;
> > > + case OFONO_VENDOR_ZTE:
> > > + data->string_limit = 50;
> > > + g_at_chat_send(data->chat, "AT+CSCB=1", none_prefix,
> > > + NULL, NULL, NULL);
> > > + break;
> >
> > Why are we copying the Qualcomm quirk over for ZTE?
>
> Because previously, ZTE modem is using QUALCOMM vendor for CBS
creation.
> So I do not want to change this behaviour.
test it again. I am not even 100% that ZTE needs this behavior if the
limitation was on the command line length. However it might. So please
double check and comment it properly.
Ok, so ZTE require the CSCB=1 in case the topic is already set:
at+cscb=1
OK
AT+CSCB=0,"2000,3000,4000,5000,6000,7000,8000,9000,10000"
OK
at+cscb?
+CSCB: 0, "10000,9000,8000,7000,6000,5000,4000,3000,2000",""
OK
AT+CSCB=0,"2000,3000,4000,5000,6000,7000,8000,9000,10000"
+CMS ERROR: 302
Regards,
Bertrand
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
5:35 p.m.
New subject: [PATCH 2/2] zte: specify the vendor to send cbs topics in split mode.
---
plugins/zte.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/plugins/zte.c b/plugins/zte.c
index 9fa9940..c572860 100644
--- a/plugins/zte.c
+++ b/plugins/zte.c
@@ -297,7 +297,7 @@ static void zte_post_online(struct ofono_modem *modem)
ofono_netreg_create(modem, OFONO_VENDOR_ZTE, "atmodem", data->aux);
- ofono_cbs_create(modem, OFONO_VENDOR_QUALCOMM_MSM,
+ ofono_cbs_create(modem, OFONO_VENDOR_ZTE,
"atmodem", data->aux);
ofono_ussd_create(modem, OFONO_VENDOR_QUALCOMM_MSM,
"atmodem", data->aux);
--
1.7.4.1
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
4065
days inactive
4065
days old
6 comments
3 participants
participants (3)
-
Aygon, Bertrand -
Bertrand Aygon
-
Marcel Holtmann