Re: [RFC 3/3] build: Add Gemalto plugin in Makefile

[Denis Kenzior]

Hi Vincent,

On 01/12/2017 11:06 AM, Vincent Cesson wrote:

From: vcesson 

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


Please squash this patch into patch 1/3.  Since that patch adds the a 
new .c file to plugins/ it should also add it to the build system.


Regards,
-Denis
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [RFC 2/3] udevng: Add Gemalto P-family detection

[Denis Kenzior]

On 01/12/2017 11:06 AM, Vincent Cesson wrote:

From: vcesson 


Please update your Author info.



Add a new function setup, based on telit, to handle Gemalto P-family
discovery. The setup look for USB interfaces:
application=/dev/ttyUSB2
gps=/dev/ttyUSB1
modem=/dev/ttyUSB3
---
 plugins/udevng.c | 38 ++
 1 file changed, 38 insertions(+)



Looks fine to me.

Regards,
-Denis

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [RFC 1/3] plugins: Add Gemalto plugin for Cinterion P-family

[Denis Kenzior]

Hi Vincent,

On 01/12/2017 11:06 AM, Vincent Cesson wrote:

From: vcesson 


Please update your Author info.



Actual cinterion plugin is not compliant with newer Gemalto modems.
Gemalto plugin is based on cinterion with a custom struct to handle the
interfaces Application and Modem.


The old cinterion plugin is used for Serial based devices.  So sure, 
this makes sense.



---
 plugins/gemalto.c | 265 ++
 1 file changed, 265 insertions(+)
 create mode 100644 plugins/gemalto.c

diff --git a/plugins/gemalto.c b/plugins/gemalto.c
new file mode 100644
index 000..4758f62
--- /dev/null
+++ b/plugins/gemalto.c
@@ -0,0 +1,265 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.


You might want to update the copyright years and add your own to the 
above.  Really up to you though :)



+ *
+ *  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 
+#endif
+
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define OFONO_API_SUBJECT_TO_CHANGE
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include 
+#include 
+
+#include 
+
+struct gemalto_data {
+   GAtChat *app;
+   GAtChat *mdm;
+   struct ofono_private_config *config;


What's this?  There's no such structure defined, and you should not be 
hijacking the ofono_ prefix for something that is local to a particular 
driver.



+};
+


The rest looks just fine to me.

Regards,
-Denis

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH 1/1] qmimodem: query_passwd_state can be retried

[Denis Kenzior]

Hi Christophe,

All of the patch looks fine, except:


@@ -436,16 +458,36 @@ static void query_passwd_state_cb(struct qmi_result 
*result,
 {
struct cb_data *cbd = user_data;
ofono_sim_passwd_cb_t cb = cbd->cb;
+   struct ofono_sim *sim = cbd->user;
+   struct sim_data *data = ofono_sim_get_data(sim);
struct sim_status sim_stat;
-
-   DBG("");
-
-   if (!handle_get_card_status_result(result, _stat)) {
+   enum get_card_status_result res;
+
+   res = handle_get_card_status_result(result, _stat);
+   switch (res) {
+   case GET_CARD_STATUS_RESULT_OK:
+   DBG("passwd state %d", sim_stat.passwd_state);
+   data->retry_count = 0;
+   CALLBACK_WITH_SUCCESS(cb, sim_stat.passwd_state, cbd->data);
+   break;
+   case GET_CARD_STATUS_RESULT_TEMP_ERROR:
+   data->retry_count++;
+   if (data->retry_count > MAX_RETRY_COUNT) {
+   DBG("Failed after %d attempts", data->retry_count);
+   data->retry_count = 0;
+   CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+   } else {
+   usleep(2);


You cannot use usleep here.  usleep blocks, so the rest of the daemon is 
unable to process anything else.  So for example, if you have multiple 
modems, usleep would block oFono from receiving / sending any events to 
these, D-Bus API would be blocked, etc.


For an example of how to get around this using GLib timers, see 
drivers/atmodem/atutil.c.


The at_util_sim_state_query (at_util_sim_state_query_new()) is doing 
something extremely similar to what you're trying to do here.


Regards,
-Denis
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH 1/1] atmodem: Fix CGDCONT result parsing.

[Denis Kenzior]

Hi Vincent,

On 01/13/2017 07:27 AM, Vincent Cesson wrote:

CGDCONT result parsing fails if first list contains several ranges. For
example with modem Cinterion PHS8:

 AT+CGDCONT=?
 +CGDCONT: (1-17,101-116),"IP",,,(0),(0-4)

Solution: read first range and jump to second list instead of trying to
close the brackets.
---
 drivers/atmodem/gprs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)



Applied, thanks.

Regards,
-Denis

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 1/1] atmodem: Fix CGDCONT result parsing.

[Vincent Cesson]

CGDCONT result parsing fails if first list contains several ranges. For
example with modem Cinterion PHS8:

 AT+CGDCONT=?
 +CGDCONT: (1-17,101-116),"IP",,,(0),(0-4)

Solution: read first range and jump to second list instead of trying to
close the brackets.
---
 drivers/atmodem/gprs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/atmodem/gprs.c b/drivers/atmodem/gprs.c
index 5ee757a..5724f86 100644
--- a/drivers/atmodem/gprs.c
+++ b/drivers/atmodem/gprs.c
@@ -522,7 +522,7 @@ static void at_cgdcont_test_cb(gboolean ok, GAtResult 
*result,
if (g_at_result_iter_next_range(, , ) == FALSE)
continue;
 
-   if (!g_at_result_iter_close_list())
+   if (!g_at_result_iter_skip_next())
continue;
 
if (g_at_result_iter_open_list())
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: Regarding multiple APN support in ofono/connman

[Daniel Wagner]

Hi Denis,

On 01/12/2017 12:26 AM, Denis Kenzior wrote:
> On 01/11/2017 01:39 PM, Naveen Kumar Danturi wrote:
>> Hi Ofono team,
>> Is there a support in ofono for multiple PDN connections in parallel ?
>> If so what interfaces/API's it exposes to connman to manage multiple
>> APN's ? If so, I have these 2 questions :
> 
> Yes, oFono can activate multiple active contexts, assuming the hardware
> & driver supports this.  See doc/connman-api.txt.  This is the same
> interface ConnMan is using currently.  However, ConnMan only manages a
> single context of type="internet".  I think there were some patches for
> ConnMan to support multiple active internet contexts, but I'm not sure
> if they ever were accepted upstream.

Just for the record, ConnMan supports multiple context of type
"internet" now. See 71cbf35ce180cfed27bea6f4b7d1e5792d58f52c in the
ConnMan repo.

cheers,
daniel
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono