Re: [PATCH 3/4] isimodem2.5: Changes to add isimodem2.5
Hi Marcel, Please sync with Aki if it might not be better to do the vendor quirk system that we are using with atmodem. I am open here for suggestions and you guys have more experience in what can be shared and what is different. we will sync with Aki if there is a better way of implementing this, we have got a couple of really good review comments from him already. (Thanks also to Mika Liljeberg for suggesting something similar) I guess the question is what will be most efficient and easiest to maintain. Best Regards, Jessica Nilsson ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 3/4] isimodem2.5: Changes to add isimodem2.5
Hi Aki, The CBS topics set is simply not implemented in the current cbs driver. I suggest you create a patch for adding that into the current driver. And same for the clear_topics callback. Yes, I might be able to do that. I can not test it though, I haven't got any isimodem HW. I assume you mean submitting a patch to the isimodem for this? This has no business being in the isimodem driver. I suppose it was included by mistake. Yes, sorry. So the IMEI is not stored on the modem at all. This obviously needs a quirk, but I wonder if it would be cleaner to handle this in the modem plugin. See for example how GPIO lines are handled in the n900 driver. Right, we will look into that and probably get back to you on this with questions. Is there actually anything different here, except for the name change? It's using a forked version of GIsiPipe, but I'm pretty sure we can just add the missing bits into the existing GIsiPipe class. Yes, that could be a solution. Just adding the decoding for NET_MODEM_ subblocks here would be enough. In fact, these subblocks look almost identical to the existing NET_ subblocks, so adding just extra case statements with the new codepoints would probably work, too. That would certainly look neater. :-) I think this is a bad idea. I know you're using NET_CS_CONTROL_REQ here to implement the unregister callback, but the this is basically just wrapping existing PN_MTC functionality. In general, ISI modems don't seem to understand what a deregister even means. In fact, even the D-Bus method seems of little value, and I would remove it altogether. In any case, leaving this unimplemented is my preference. Ok, we can see if this works for us, but you are probably right. This is different from the current netreg driver. However, it's easy enough to alternate, see below. I would suggest probing for both PN_NETWORK and PN_MODEM_NETWORK here. Then which ever answers first gets used. After we know with which resource we're working with, overloading a few of the ISI messages is enough to make this driver work with both versions of modems. Seems pretty straighforward. I didn't go through all of the patch, but from what I can see, we can pretty well fold the WG2.5 modem support into the existing drivers. We'll get back to you on that when we have had another look at this and your other suggestions. Thanks for your comments. :-) Best Regards, Jessica ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 3/4] isimodem2.5: Changes to add isimodem2.5
Hi Jessica, Updates in Makefile.am - adding of isimodem2.5 Adding of drivers/isimodem2.5/ files to fully listed functionality. Regards, Jessica Nilsson --- Makefile.am| 36 +- drivers/isimodem2.5/call-barring.c | 503 ++ drivers/isimodem2.5/call-forwarding.c | 613 +++ drivers/isimodem2.5/call-settings.c| 1093 + drivers/isimodem2.5/call.h | 237 + drivers/isimodem2.5/cbs.c | 433 ++ drivers/isimodem2.5/checkpatch.pl | 2789 drivers/isimodem2.5/debug.c| 694 +++ drivers/isimodem2.5/debug.h| 66 + drivers/isimodem2.5/devinfo.c | 246 + drivers/isimodem2.5/gpds-context.c | 788 drivers/isimodem2.5/gpds.c | 395 ++ drivers/isimodem2.5/gpds.h | 295 ++ drivers/isimodem2.5/gss.h | 84 + drivers/isimodem2.5/info.h | 53 + drivers/isimodem2.5/isimodem.c | 534 +++ drivers/isimodem2.5/isimodem.h | 69 + drivers/isimodem2.5/isiutil.h | 64 + drivers/isimodem2.5/mce.h | 65 + drivers/isimodem2.5/network-registration.c | 1158 + drivers/isimodem2.5/network.h | 257 ++ drivers/isimodem2.5/phonebook.c| 1320 ++ drivers/isimodem2.5/radio-settings.c | 313 ++ drivers/isimodem2.5/simu_resps.h | 6800 drivers/isimodem2.5/sms.c | 869 drivers/isimodem2.5/sms.h | 188 + drivers/isimodem2.5/ss.h | 174 + drivers/isimodem2.5/ssn.c | 456 ++ drivers/isimodem2.5/timeout.h | 33 + drivers/isimodem2.5/uicc.c | 3316 ++ drivers/isimodem2.5/uicc.h | 253 ++ drivers/isimodem2.5/uicc_interface.h | 40 + drivers/isimodem2.5/ussd.c | 341 ++ drivers/isimodem2.5/voicecall.c| 1555 +++ 34 files changed, 26128 insertions(+), 2 deletions(-) create mode 100644 drivers/isimodem2.5/call-barring.c create mode 100644 drivers/isimodem2.5/call-forwarding.c create mode 100644 drivers/isimodem2.5/call-settings.c create mode 100644 drivers/isimodem2.5/call.h create mode 100644 drivers/isimodem2.5/cbs.c create mode 100755 drivers/isimodem2.5/checkpatch.pl create mode 100644 drivers/isimodem2.5/debug.c create mode 100644 drivers/isimodem2.5/debug.h create mode 100644 drivers/isimodem2.5/devinfo.c create mode 100644 drivers/isimodem2.5/gpds-context.c create mode 100644 drivers/isimodem2.5/gpds.c create mode 100644 drivers/isimodem2.5/gpds.h create mode 100644 drivers/isimodem2.5/gss.h create mode 100644 drivers/isimodem2.5/info.h create mode 100644 drivers/isimodem2.5/isimodem.c create mode 100644 drivers/isimodem2.5/isimodem.h create mode 100644 drivers/isimodem2.5/isiutil.h create mode 100644 drivers/isimodem2.5/mce.h create mode 100644 drivers/isimodem2.5/network-registration.c create mode 100644 drivers/isimodem2.5/network.h create mode 100644 drivers/isimodem2.5/phonebook.c create mode 100644 drivers/isimodem2.5/radio-settings.c create mode 100644 drivers/isimodem2.5/simu_resps.h create mode 100644 drivers/isimodem2.5/sms.c create mode 100644 drivers/isimodem2.5/sms.h create mode 100644 drivers/isimodem2.5/ss.h create mode 100644 drivers/isimodem2.5/ssn.c create mode 100644 drivers/isimodem2.5/timeout.h create mode 100644 drivers/isimodem2.5/uicc.c create mode 100644 drivers/isimodem2.5/uicc.h create mode 100644 drivers/isimodem2.5/uicc_interface.h create mode 100644 drivers/isimodem2.5/ussd.c create mode 100644 drivers/isimodem2.5/voicecall.c this is not gonna work this way. No one can review such a massive patch. Actually Evolution even refused to format this for me. So please split this into one patch per atom driver. In addition, I don't a point in a filename. So isimodem25 or isimodem2 or something like that please. I am open for proposals. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 3/4] isimodem2.5: Changes to add isimodem2.5
Hi Marcel, thanks for your input. this is not gonna work this way. No one can review such a massive patch. Actually Evolution even refused to format this for me. :-) Right you are, it's huge So please split this into one patch per atom driver. We are starting to look at this split up. Do you have any other suggestions? In addition, I don't a point in a filename. So isimodem25 or isimodem2 or something like that please. I am open for proposals. Let's go for isimodem25 Best Regards, Jessica Nilsson ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 3/4] isimodem2.5: Changes to add isimodem2.5
Hi, In addition, I don't a point in a filename. So isimodem25 or isimodem2 or something like that please. I am open for proposals. Let's go for isimodem25 Quite a bit of the code is practically a copy-paste from the isimodem driver. Have you looked into the possibility of supporting both ISI interface versions in the same driver, or at least sharing some of the atom drivers? MikaL ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 3/4] isimodem2.5: Changes to add isimodem2.5
Hi Jessica, So please split this into one patch per atom driver. We are starting to look at this split up. Do you have any other suggestions? I really want one patch per atom driver. In addition, I don't a point in a filename. So isimodem25 or isimodem2 or something like that please. I am open for proposals. Let's go for isimodem25 Please sync with Aki if it might not be better to do the vendor quirk system that we are using with atmodem. I am open here for suggestions and you guys have more experience in what can be shared and what is different. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 3/4] isimodem2.5: Changes to add isimodem2.5
Hi Jessica,
2010/12/8 Jessica Nilsson jessica.j.nils...@stericsson.com:
+static void reset_buf(char *buf, char *buf_2, int buf_len)
+{
+ memset(buf, '\0', buf_len);
+ memset(buf_2, '\0', buf_len);
+}
+
+static int get_topics_len(const char *topics)
+{
+ int i = 0;
+ int k = 0;
+ int length = 0;
+ char buf[6];
+ char buf_2[6];
+
+ reset_buf(buf, buf_2, 6);
+
+ while (*topics != '\0') {
+ if (*topics == ',') {
+ reset_buf(buf, buf_2, 6);
+ k = 0;
+ length++;
+ } else if (*topics != ',' *topics != '-') {
+ buf[k] = *topics;
+ k++;
+ } else if (*topics == '-') {
+ topics++;
+ i++;
+ k = 0;
+
+ while (*topics != ',' *topics != '\0') {
+ buf_2[k] = *topics;
+ topics++;
+ i++;
+ k++;
+ }
+
+ length = length + atoi(buf_2) - atoi(buf) + 1;
+ k = 0;
+ }
+
+ topics++;
+ i++;
+ }
+
+ topics = topics - i;
+ return length;
+}
+
+static void parse_topics(const char *topics, gint16 *topics_parsed)
+{
+ int j = 0;
+ int k = 0;
+ char buf[6];
+ char buf_2[6];
+
+ reset_buf(buf, buf_2, 6);
+
+ while (*topics != '\0') {
+ if (*topics != ',' *topics != '-') {
+ buf[j] = *topics;
+ j++;
+ } else if (*topics == '-') {
+ topics++;
+ j = 0;
+
+ while (*topics != ',' *topics != '\0') {
+ buf_2[j] = *topics;
+ topics++;
+ j++;
+ }
+
+ for (j = 0; j = (atoi(buf_2) - atoi(buf)); j++) {
+ topics_parsed[k] = atoi(buf) + j;
+ topics_parsed[k] = g_ntohs(topics_parsed[k]);
+ k++;
+ }
+
+ j = 0;
+ } else if (*topics == ',') {
+ topics_parsed[k] = atoi(buf);
+ topics_parsed[k] = g_ntohs(topics_parsed[k]);
+ reset_buf(buf, buf_2, 6);
+ j = 0;
+ k++;
+ }
+
+ topics++;
+ }
+}
+
+static gboolean set_resp_cb(GIsiClient *client, const void *restrict data,
+ size_t len, uint16_t object, void *opaque)
+{
+ const unsigned char *msg = data;
+ struct isi_cb_data *cbd = opaque;
+ ofono_cbs_set_cb_t cb = cbd-cb;
+
+ DBG();
+
+ if (!msg) {
+ DBG(ISI client error: %d, g_isi_client_error(client));
+ goto error;
+ }
+
+ if (len 3 || msg[0] != SMS_CB_ROUTING_RESP)
+ goto error;
+
+ if (msg[2] != SMS_OK)
+ goto error;
+
+ cbs_subscription_nr = msg[1];
+ CALLBACK_WITH_SUCCESS(cb, cbd-data);
+ goto out;
+error:
+ CALLBACK_WITH_FAILURE(cb, cbd-data);
+out:
+ g_free(cbd);
+ return TRUE;
+}
+
+static void isi_set_topics(struct ofono_cbs *cbs, const char *topics,
+ ofono_cbs_set_cb_t cb, void *data)
+{
+ struct cbs_data *cd = ofono_cbs_get_data(cbs);
+ struct isi_cb_data *cbd = isi_cb_data_new(cbs, cb, data);
+ int topics_len = get_topics_len(topics);
+ gint16 topics_out[topics_len];
+
+ uint8_t msg[] = {
+ SMS_CB_ROUTING_REQ,
+ SMS_ROUTING_SET,
+ cbs_subscription_nr,
+ 0x00, /*Subscription type*/
+ 0x00, /*Fillers*/
+ 0x00,
+ 0x01, /*Number of subblocks*/
+ 0x00,
+ 0x26, /*Subblock*/
+ 0x00,
+ topics_len * 2 + 6, /*Subblock length*/
+ 0x00,
+ topics_len, /*Number of topics*/
+ };
+ struct iovec iov[2] = {
+ { msg, sizeof(msg) },
+ { topics_out, sizeof(topics_out) },
+ };
+ parse_topics(topics, topics_out);
+
+ DBG();
+
+ if (g_isi_request_vmake(cd-client, iov, 2, CBS_TIMEOUT,
+ set_resp_cb, cbd))
+ return;
+
+ CALLBACK_WITH_FAILURE(cb, data);
+ g_free(cbd);
+}
The CBS topics
