Re: [PATCH 5/5] gemalto: gprs: support authentication settings

2020-12-22 Thread Denis Kenzior

Hi Sergey,

On 12/21/20 2:01 PM, Sergey Matyukevich wrote:

From: Sergey Matyukevich 

Add support for gprs contexts with username, password, as well as
specific authentication type.
---
  drivers/gemaltomodem/gprs-context.c | 54 -
  1 file changed, 53 insertions(+), 1 deletion(-)



Looks good to me.

Regards,
-Denis
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org


Re: [PATCH 3/5] gemalto: gprs: support automatic context activation

2020-12-22 Thread Denis Kenzior

Hi Sergey,

On 12/21/20 2:01 PM, Sergey Matyukevich wrote:

From: Sergey Matyukevich 

Implement read_settings function to get configuration for automatic
contexts. Fix the issue uncovered by added support for automatic
context activation: do not use AT+CGACT for the contexts handled
by AT^SWWAN. As per modem specs, CGACT context can not be reused
for SWWAN. Though that worked for some reason when automatic
context was reactivated without proper deactivation.
---
  drivers/gemaltomodem/gprs-context.c | 110 +++-
  1 file changed, 59 insertions(+), 51 deletions(-)

diff --git a/drivers/gemaltomodem/gprs-context.c 
b/drivers/gemaltomodem/gprs-context.c
index 322a5f98..680f01ab 100644
--- a/drivers/gemaltomodem/gprs-context.c
+++ b/drivers/gemaltomodem/gprs-context.c
@@ -50,70 +50,61 @@ struct gprs_context_data {
void *cb_data;
  };
  
-static void cgact_enable_cb(gboolean ok, GAtResult *result,

-   gpointer user_data)
+static void set_gprs_context_interface(struct ofono_gprs_context *gc)
  {
-   struct ofono_gprs_context *gc = user_data;
-   struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
struct ofono_modem *modem;
const char *interface;
-   char buf[64];
+
+   modem = ofono_gprs_context_get_modem(gc);
+   interface = ofono_modem_get_string(modem, "NetworkInterface");
+   ofono_gprs_context_set_interface(gc, interface);
+}
+
+static void swwan_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+   struct ofono_gprs_context *gc = user_data;
+   struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
+   struct ofono_error error;
  
  	DBG("ok %d", ok);
  
  	if (!ok) {

-   struct ofono_error error;
-
+   ofono_error("Unable to activate context");
+   ofono_gprs_context_deactivated(gc, gcd->active_context);
gcd->active_context = 0;


This seems a bit strange.  Are you signaling success to the core too early


-
decode_at_error(, g_at_result_final_response(result));
gcd->cb(, gcd->cb_data);
-
return;
}
-
-   snprintf(buf, sizeof(buf), "AT^SWWAN=1,%u", gcd->active_context);
-   g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL, NULL);
-
-   modem = ofono_gprs_context_get_modem(gc);
-   interface = ofono_modem_get_string(modem, "NetworkInterface");
-   ofono_gprs_context_set_interface(gc, interface);
-
-   /* Use DHCP */
-   ofono_gprs_context_set_ipv4_address(gc, NULL, 0);
-
-   CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);
  }
  
  static void cgdcont_enable_cb(gboolean ok, GAtResult *result,

-   gpointer user_data)
+   gpointer user_data)


nit: This is superfluous and also see doc/coding-style.txt item M4


  {
struct ofono_gprs_context *gc = user_data;
struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
+   struct ofono_error error;
char buf[64];
  
  	DBG("ok %d", ok);
  
  	if (!ok) {

-   struct ofono_error error;
-
gcd->active_context = 0;
-
decode_at_error(, g_at_result_final_response(result));
gcd->cb(, gcd->cb_data);
-
return;
}
  
-	snprintf(buf, sizeof(buf), "AT+CGACT=1,%u", gcd->active_context);

-
-   if (g_at_chat_send(gcd->chat, buf, none_prefix,
-   cgact_enable_cb, gc, NULL) == 0)
-   goto error;
+   snprintf(buf, sizeof(buf), "AT^SWWAN=1,%u", gcd->active_context);
+   if (g_at_chat_send(gcd->chat, buf, none_prefix, swwan_cb, gc, NULL)) {


Note, this returns > 0 when the command has been queued, not that it has been 
sent or replied to yet...



+   set_gprs_context_interface(gc);
+   /* Use DHCP */
+   ofono_gprs_context_set_ipv4_address(gc, NULL, 0);


If these modems can only do DHCP, then might be cleaner to move the 'Use DHCP' 
bits into set_gprs_context_interface.  And maybe rename it to make things 
clearer, if needed.


  
-	return;

+   CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);


Are you sure you don't want to wait until swwan_cb to return success to the 
core?  AT^SWWAN can still fail...?



+   return;
+   }
  
-error:

CALLBACK_WITH_FAILURE(gcd->cb, gcd->cb_data);
  }
  





@@ -185,17 +170,39 @@ static void gemalto_gprs_deactivate_primary(struct 
ofono_gprs_context *gc,
gcd->cb = cb;
gcd->cb_data = data;
  
-	snprintf(buf, sizeof(buf), "AT+CGACT=0,%u", cid);

+   snprintf(buf, sizeof(buf), "AT^SWWAN=0,%u", gcd->active_context);
  
  	if (g_at_chat_send(gcd->chat, buf, none_prefix,

-   cgact_disable_cb, gc, NULL) == 0)
-   goto error;
-
-   return;
+   deactivate_cb, gc, NULL))
+   return;
  

Re: [PATCH 1/5] plugin: gemalto: fix source of gprs notifications

2020-12-22 Thread Denis Kenzior

Hi Sergey,

On 12/21/20 2:01 PM, Sergey Matyukevich wrote:

Modem USB interface does not receive certain gprs context notifications.
Fix gprs chat: use Application USB interface to receive all the modem
notifications.
---
  plugins/gemalto.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)



Patch 1, 2 and 4 applied,  thanks.

Regards,
-Denis
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org


Re: [PATCH] huawei, util: Fix a couple of implicit enum conversions

2020-12-22 Thread Denis Kenzior

Hi Richard,

On 12/20/20 5:18 AM, richard.rojf...@gmail.com wrote:

From: Richard Röjfors 

GCC 10 warns about a couple of implicit conversions;

huawei: Member from the incorrect enum was returned,
both had the value 0, so the code would still work.

drivers/huaweimodem/radio-settings.c: In function ‘band_gsm_from_huawei’:
drivers/huaweimodem/radio-settings.c:107:10: error: implicit conversion from 
‘enum ofono_radio_band_umts’ to ‘enum ofono_radio_band_gsm’ 
[-Werror=enum-conversion]
   107 |   return OFONO_RADIO_BAND_UMTS_ANY;

util: smsutil and util has an enum each for representing
the same thing; The SMS alphabet. They share the same
values, so an explicit type cast makes GCC happy.

src/smsutil.c: In function ‘sms_text_prepare_with_alphabet’:
src/smsutil.c:3594:8: error: implicit conversion from ‘enum sms_alphabet’ to 
‘enum gsm_dialect’ [-Werror=enum-conversion]
  3594 |alphabet, _locking,
---
  drivers/huaweimodem/radio-settings.c | 2 +-
  src/smsutil.c| 5 -
  2 files changed, 5 insertions(+), 2 deletions(-)



I split this into two commits and applied.  Thanks!

Regards,
-Denis
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org