Re: [mm][PATCH 1/5] novatel: Add support for connecting to specific APNs and specifying username/password.

2012-05-02 Thread Aleksander Morgado
Hey Nathan,

On 05/01/2012 06:07 PM, Nathan Williams wrote:
 Subject: [PATCH 1/5] novatel: Add support for connecting to specific APNs and
  specifying username/password.
 
 While here, pass through the other relevant bearer properties.

Did you have time to check the 'bearer-properties' branch I wrote about
some time ago? That branch makes it unnecessary to have bearers
implement user/password properties themselves.

 +apn = mm_at_serial_port_quote_string
(mm_broadband_bearer_get_3gpp_apn (bearer));
 +user = mm_at_serial_port_quote_string (self-priv-user);
 +password = mm_at_serial_port_quote_string (self-priv-password);
 +command = g_strdup_printf ($NWQMICONNECT=,,%s,,,%s,%s,
 +   apn, user, password);

mm_at_serial_port_quote_string() is not in git master yet. I remember a
thread talking about quoting strings, but don't recall getting a patch
about it. Could you also post that one?

Cheers,

-- 
Aleksander
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH 2/5] novatel: Use the ALLOWED_SINGLE_AT property. Saves 5 seconds on probing.

2012-05-02 Thread Aleksander Morgado
On 05/01/2012 06:07 PM, Nathan Williams wrote:
 Date: Thu, 22 Mar 2012 16:18:24 -0400
 Subject: [PATCH 2/5] novatel: Use the ALLOWED_SINGLE_AT property. Saves 5
  seconds on probing.

Is it that the modem exposes only one AT port? Or that it exposes
multiple ports but only one is needed? The property was meant for the
former case only, not for the latter.


 
 Change-Id: I0b0c2cb07781eb32ba6eb4294868ed123f57fd9f
 ---
  plugins/novatel/mm-plugin-novatel.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
 
 diff --git a/plugins/novatel/mm-plugin-novatel.c 
 b/plugins/novatel/mm-plugin-novatel.c
 index 10fa6a6..ccec89c 100644
 --- a/plugins/novatel/mm-plugin-novatel.c
 +++ b/plugins/novatel/mm-plugin-novatel.c
 @@ -94,6 +94,7 @@ mm_plugin_create (void)
MM_PLUGIN_BASE_ALLOWED_SUBSYSTEMS, subsystems,
MM_PLUGIN_BASE_ALLOWED_PRODUCT_IDS, products,
MM_PLUGIN_BASE_ALLOWED_AT, TRUE,
 +  MM_PLUGIN_BASE_ALLOWED_SINGLE_AT, TRUE,

When SINGLE_AT is set, _AT is not needed; it's one or the other.

NULL));
  }
  


-- 
Aleksander
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH 4/5] novatel: Poll for whether the connection still exists.

2012-05-02 Thread Aleksander Morgado
On 05/01/2012 06:09 PM, Nathan Williams wrote:
 +static gboolean
 +poll_connection (MMBroadbandBearerNovatel *bearer)
 +{
 +mm_base_modem_at_command (
 +bearer-priv-connection_modem,
 +$NWQMISTATUS,
 +3,
 +FALSE,
 +(GAsyncReadyCallback)poll_connection_ready,
 +bearer);
 +return TRUE;
 +}

Why the extra connection_modem object in the private struct? There's
already a proper reference to the modem object in the MMBearer parent,
which can be retrieved getting the MM_BEARER_MODEM property.

-- 
Aleksander
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH 5/5] novatel: Remove band-setting implementation to improve connections

2012-05-02 Thread Aleksander Morgado
On 05/01/2012 06:09 PM, Nathan Williams wrote:
 Subject: [PATCH 5/5] novatel: Remove band-setting implementation to improve
  connections
 
 The $NWBAND command seems to disturb the internal state of the modem
 such that it is unlikely to connect to the network, and produces
 widely varying error codes. We don't actively use this functionality,
 it's just that setting use all bands is part of the normal
 ModemManager simple-connect sequence. Remove it so it doesn't get
 triggered.

I would leave this code around, and instead make sure that if no
bands-specific configuration is given in the simple connect properties,
we don't try to do anything with the bands setup. What do you think?

-- 
Aleksander
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH 3/5] novatel: Implement load_access_technologies.

2012-05-02 Thread Aleksander Morgado
On 05/01/2012 06:08 PM, Nathan Williams wrote:
 Subject: [PATCH 3/5] novatel: Implement load_access_technologies.
 
 Change-Id: Ib503d900850d3754d79525dbc9a40b7b9c221dd7

Pushed this one, thanks.

-- 
Aleksander
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [mm][PATCH 1/5] novatel: Add support for connecting to specific APNs and specifying username/password.

2012-05-02 Thread Nathan Williams
+networkmanager-list

On Wed, May 2, 2012 at 10:43 AM, Nathan Williams n...@google.com wrote:

 On 05/01/2012 06:07 PM, Nathan Williams wrote:
  Subject: [PATCH 1/5] novatel: Add support for connecting to specific
 APNs and
   specifying username/password.
 
  While here, pass through the other relevant bearer properties.

 Did you have time to check the 'bearer-properties' branch I wrote about
 some time ago? That branch makes it unnecessary to have bearers
 implement user/password properties themselves.


 It certainly seemed like a good idea, and would simplify this, but I
 didn't think it had been merged. I hope you hadn't been waiting on me



  +apn = mm_at_serial_port_quote_string
 (mm_broadband_bearer_get_3gpp_apn (bearer));
  +user = mm_at_serial_port_quote_string (self-priv-user);
  +password = mm_at_serial_port_quote_string (self-priv-password);
  +command = g_strdup_printf ($NWQMICONNECT=,,%s,,,%s,%s,
  +   apn, user, password);

 mm_at_serial_port_quote_string() is not in git master yet. I remember a
 thread talking about quoting strings, but don't recall getting a patch
 about it. Could you also post that one?


 Will do. I had forgotten that I was using it in both of my plugins, and
 meant to include it in the stack with my Samsung changes.

 - Nathan

___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH 2/5] novatel: Use the ALLOWED_SINGLE_AT property. Saves 5 seconds on probing.

2012-05-02 Thread Nathan Williams
On Wed, May 2, 2012 at 6:11 AM, Aleksander Morgado aleksan...@lanedo.comwrote:

 On 05/01/2012 06:07 PM, Nathan Williams wrote:
  Date: Thu, 22 Mar 2012 16:18:24 -0400
  Subject: [PATCH 2/5] novatel: Use the ALLOWED_SINGLE_AT property. Saves 5
   seconds on probing.

 Is it that the modem exposes only one AT port? Or that it exposes
 multiple ports but only one is needed? The property was meant for the
 former case only, not for the latter.


With this modem only one of the four exposed serial ports is an AT port. I
believe the remaining ones are two Qualcomm things and GPS.



 When SINGLE_AT is set, _AT is not needed; it's one or the other.


OK, will revise.

   - Nathan
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH 5/5] novatel: Remove band-setting implementation to improve connections

2012-05-02 Thread Nathan Williams
I'm leery of including code that can run the $NWBAND command to set the
modem's state, since as far as I can tell it simply breaks the modem, at
least until it is power-cycled. Not setting the bands in the simple connect
process unless explicitly requested seems like a fine change on its own,
but I think this is too dangerous to include.

- Nathan

On Wed, May 2, 2012 at 6:29 AM, Aleksander Morgado aleksan...@lanedo.comwrote:

 On 05/01/2012 06:09 PM, Nathan Williams wrote:
  Subject: [PATCH 5/5] novatel: Remove band-setting implementation to
 improve
   connections
 
  The $NWBAND command seems to disturb the internal state of the modem
  such that it is unlikely to connect to the network, and produces
  widely varying error codes. We don't actively use this functionality,
  it's just that setting use all bands is part of the normal
  ModemManager simple-connect sequence. Remove it so it doesn't get
  triggered.

 I would leave this code around, and instead make sure that if no
 bands-specific configuration is given in the simple connect properties,
 we don't try to do anything with the bands setup. What do you think?

 --
 Aleksander

___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [mm][PATCH 1/5] novatel: Add support for connecting to specific APNs and specifying username/password.

2012-05-02 Thread Aleksander Morgado
Hey,

On 05/02/2012 04:43 PM, Nathan Williams wrote:
 +networkmanager-list
 
 On Wed, May 2, 2012 at 10:43 AM, Nathan Williams n...@google.com
 mailto:n...@google.com wrote:
 
 On 05/01/2012 06:07 PM, Nathan Williams wrote:
  Subject: [PATCH 1/5] novatel: Add support for connecting to
 specific APNs and
   specifying username/password.
 
  While here, pass through the other relevant bearer properties.
 
 Did you have time to check the 'bearer-properties' branch I
 wrote about
 some time ago? That branch makes it unnecessary to have bearers
 implement user/password properties themselves.
 
 
 It certainly seemed like a good idea, and would simplify this, but I
 didn't think it had been merged. I hope you hadn't been waiting on
 me
  

Actually, I had almost forgot about that branch :-) No big deal, I'll
merge that patch as it is (when the quote_string() comes in), and then
update the branch before merging it.

 
 
  +apn = mm_at_serial_port_quote_string
 (mm_broadband_bearer_get_3gpp_apn (bearer));
  +user = mm_at_serial_port_quote_string (self-priv-user);
  +password = mm_at_serial_port_quote_string
 (self-priv-password);
  +command = g_strdup_printf ($NWQMICONNECT=,,%s,,,%s,%s,
  +   apn, user, password);
 
 mm_at_serial_port_quote_string() is not in git master yet. I
 remember a
 thread talking about quoting strings, but don't recall getting a
 patch
 about it. Could you also post that one?
 
 
 Will do. I had forgotten that I was using it in both of my plugins,
 and meant to include it in the stack with my Samsung changes.
 

Ok.

-- 
Aleksander
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH 2/5] novatel: Use the ALLOWED_SINGLE_AT property. Saves 5 seconds on probing.

2012-05-02 Thread Aleksander Morgado
On 05/02/2012 04:53 PM, Nathan Williams wrote:
 On Wed, May 2, 2012 at 6:11 AM, Aleksander Morgado
 aleksan...@lanedo.com mailto:aleksan...@lanedo.com wrote:
 
 On 05/01/2012 06:07 PM, Nathan Williams wrote:
  Date: Thu, 22 Mar 2012 16:18:24 -0400
  Subject: [PATCH 2/5] novatel: Use the ALLOWED_SINGLE_AT property.
 Saves 5
   seconds on probing.
 
 Is it that the modem exposes only one AT port? Or that it exposes
 multiple ports but only one is needed? The property was meant for the
 former case only, not for the latter.
 
 
 With this modem only one of the four exposed serial ports is an AT port.
 I believe the remaining ones are two Qualcomm things and GPS.
 

Ah, good then.

  
 
 When SINGLE_AT is set, _AT is not needed; it's one or the other.
 
 
 OK, will revise.
 

Great, thanks.

-- 
Aleksander
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH 5/5] novatel: Remove band-setting implementation to improve connections

2012-05-02 Thread Aleksander Morgado

 I'm leery of including code that can run the $NWBAND command to set the
 modem's state, since as far as I can tell it simply breaks the modem, at
 least until it is power-cycled. Not setting the bands in the simple
 connect process unless explicitly requested seems like a fine change on
 its own, but I think this is too dangerous to include.
 

Ok, then. Pushed already.

-- 
Aleksander
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] Add a utility routine to do ITU V.250 quoting of strings for AT commands.

2012-05-02 Thread Nathan Williams
Needed by the Novatel plugin.

- Nathan


0001-Add-a-utility-routine-to-do-ITU-V.250-quoting-of-str.patch
Description: Binary data
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH 2/5] novatel: Use the ALLOWED_SINGLE_AT property. Saves 5 seconds on probing.

2012-05-02 Thread Nathan Williams
Updated patch attached.

- Nathan

On Wed, May 2, 2012 at 11:25 AM, Aleksander Morgado
aleksan...@lanedo.comwrote:

 On 05/02/2012 04:53 PM, Nathan Williams wrote:
  On Wed, May 2, 2012 at 6:11 AM, Aleksander Morgado
  aleksan...@lanedo.com mailto:aleksan...@lanedo.com wrote:
 
  On 05/01/2012 06:07 PM, Nathan Williams wrote:
   Date: Thu, 22 Mar 2012 16:18:24 -0400
   Subject: [PATCH 2/5] novatel: Use the ALLOWED_SINGLE_AT property.
  Saves 5
seconds on probing.
 
  Is it that the modem exposes only one AT port? Or that it exposes
  multiple ports but only one is needed? The property was meant for the
  former case only, not for the latter.
 
 
  With this modem only one of the four exposed serial ports is an AT port.
  I believe the remaining ones are two Qualcomm things and GPS.
 

 Ah, good then.

 
 
  When SINGLE_AT is set, _AT is not needed; it's one or the other.
 
 
  OK, will revise.
 

 Great, thanks.

 --
 Aleksander



0001-novatel-Use-the-ALLOWED_SINGLE_AT-property.-Saves-5-.patch
Description: Binary data
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH 4/5] novatel: Poll for whether the connection still exists.

2012-05-02 Thread Nathan Williams
Excellent point. Revised.

- Nathan

On Wed, May 2, 2012 at 6:24 AM, Aleksander Morgado aleksan...@lanedo.comwrote:

 On 05/01/2012 06:09 PM, Nathan Williams wrote:
  +static gboolean
  +poll_connection (MMBroadbandBearerNovatel *bearer)
  +{
  +mm_base_modem_at_command (
  +bearer-priv-connection_modem,
  +$NWQMISTATUS,
  +3,
  +FALSE,
  +(GAsyncReadyCallback)poll_connection_ready,
  +bearer);
  +return TRUE;
  +}

 Why the extra connection_modem object in the private struct? There's
 already a proper reference to the modem object in the MMBearer parent,
 which can be retrieved getting the MM_BEARER_MODEM property.

 --
 Aleksander



0001-novatel-Poll-for-whether-the-connection-still-exists.patch
Description: Binary data
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH 2/5] novatel: Use the ALLOWED_SINGLE_AT property. Saves 5 seconds on probing.

2012-05-02 Thread Bjørn Mork
Nathan Williams n...@google.com writes:
 On Wed, May 2, 2012 at 6:11 AM, Aleksander Morgado 
 aleksan...@lanedo.comwrote:

 On 05/01/2012 06:07 PM, Nathan Williams wrote:
  Date: Thu, 22 Mar 2012 16:18:24 -0400
  Subject: [PATCH 2/5] novatel: Use the ALLOWED_SINGLE_AT property. Saves 5
   seconds on probing.

 Is it that the modem exposes only one AT port? Or that it exposes
 multiple ports but only one is needed? The property was meant for the
 former case only, not for the latter.


 With this modem only one of the four exposed serial ports is an AT port. I
 believe the remaining ones are two Qualcomm things and GPS.

That sounds very likely.  A wild guess based on similar devices is that
the four ports are AT, QCDM, NMEA and QMI.  You could try writing
$GPS_START to the suspected NMEA port and see if that enables output
from the GPS.  QCDM could be tested using the tests in libqcdm (included
with ModemManager).

Testing for a QMI port is currenly non-trivial. You will have to modify
the cdc-wdm or qmi_wwan drivers to bind to the device and see if it
replies to QMI.  A nice test util for someone to write (I've been
planning to, but haven't had the time and probably won't either...)
would be something using libusb to send a CDC embedded QMI message (the
subsystem version check looks like a good candidate) to a given
device/interface.  Should be fairly easy to write.



Bjørn
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH] Add a utility routine to do ITU V.250 quoting of strings for AT commands.

2012-05-02 Thread Aleksander Morgado
On 05/02/2012 07:11 PM, Nathan Williams wrote:
 Needed by the Novatel plugin.
 
 - Nathan
 
 
 0001-Add-a-utility-routine-to-do-ITU-V.250-quoting-of-str.patch
 
 
 From 26d1efa84314a77432aa0121aaa978f8abb4f598 Mon Sep 17 00:00:00 2001
 From: Nathan Williams n...@chromium.org
 Date: Fri, 23 Mar 2012 14:05:40 -0400
 Subject: [PATCH] Add a utility routine to do ITU V.250 quoting of strings for
  AT commands.
 
 BUG=chromium-os:27096,chromium-os:27063
 TEST=None
 Change-Id: Ic1d24a9e4b7421db7f8d16c52535bd6d2780423e

Pushed, thanks.

-- 
Aleksander
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [mm][PATCH 1/5] novatel: Add support for connecting to specific APNs and specifying username/password.

2012-05-02 Thread Aleksander Morgado
On 05/02/2012 05:19 PM, Aleksander Morgado wrote:
 Hey,
 
 On 05/02/2012 04:43 PM, Nathan Williams wrote:
 +networkmanager-list

 On Wed, May 2, 2012 at 10:43 AM, Nathan Williams n...@google.com
 mailto:n...@google.com wrote:

 On 05/01/2012 06:07 PM, Nathan Williams wrote:
  Subject: [PATCH 1/5] novatel: Add support for connecting to
 specific APNs and
   specifying username/password.
 
  While here, pass through the other relevant bearer properties.

 Did you have time to check the 'bearer-properties' branch I
 wrote about
 some time ago? That branch makes it unnecessary to have bearers
 implement user/password properties themselves.


 It certainly seemed like a good idea, and would simplify this, but I
 didn't think it had been merged. I hope you hadn't been waiting on
 me
  
 
 Actually, I had almost forgot about that branch :-) No big deal, I'll
 merge that patch as it is (when the quote_string() comes in), and then
 update the branch before merging it.
 

Pushed now, thanks. Will update the bearer-properties branch this/next
week, and get it merged as well.

-- 
Aleksander
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH 2/5] novatel: Use the ALLOWED_SINGLE_AT property. Saves 5 seconds on probing.

2012-05-02 Thread Aleksander Morgado
On 05/02/2012 07:13 PM, Nathan Williams wrote:
 
 0001-novatel-Use-the-ALLOWED_SINGLE_AT-property.-Saves-5-.patch
 
 
 From ad53cf829f42a5570e8eab46bb1ab0065110f3d8 Mon Sep 17 00:00:00 2001
 From: Nathan Williams n...@chromium.org
 Date: Thu, 22 Mar 2012 16:18:24 -0400
 Subject: [PATCH] novatel: Use the ALLOWED_SINGLE_AT property. Saves 5 seconds
  on probing.
 
 Change-Id: I0b0c2cb07781eb32ba6eb4294868ed123f57fd9f

Pushed now, thanks.

-- 
Aleksander
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH 4/5] novatel: Poll for whether the connection still exists.

2012-05-02 Thread Aleksander Morgado
On 05/02/2012 07:14 PM, Nathan Williams wrote:
 0001-novatel-Poll-for-whether-the-connection-still-exists.patch
 
 
 From 976d9e914a07d8757054088025b26587e7c1853e Mon Sep 17 00:00:00 2001
 From: Nathan Williams n...@chromium.org
 Date: Fri, 6 Apr 2012 13:12:15 -0400
 Subject: [PATCH] novatel: Poll for whether the connection still exists.
 
 Novatel E362 firmware doesn't notify us by unsolicited message if the
 connection goes away, so set up a polling loop to check.
 
 While here, inline a method that's only used in one place so that the
 containing function is single-exit and single-cleanup.
 
 Change-Id: If72f7c6ef06de3fb22530d42f62a8dddc6fecfda

And pushed this last one as well.

-- 
Aleksander
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH 2/5] novatel: Use the ALLOWED_SINGLE_AT property. Saves 5 seconds on probing.

2012-05-02 Thread Aleksander Morgado

 Testing for a QMI port is currenly non-trivial. You will have to modify
 the cdc-wdm or qmi_wwan drivers to bind to the device and see if it
 replies to QMI.  A nice test util for someone to write (I've been
 planning to, but haven't had the time and probably won't either...)
 would be something using libusb to send a CDC embedded QMI message (the
 subsystem version check looks like a good candidate) to a given
 device/interface.  Should be fairly easy to write.
 

That check will possibly be done directly through libqmi/libqmi-glib. In
my libqmi-glib, opening a QmiDevice involves doing the subsystem version
check already.

-- 
Aleksander
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH 2/5] novatel: Use the ALLOWED_SINGLE_AT property. Saves 5 seconds on probing.

2012-05-02 Thread Bjørn Mork
Aleksander Morgado aleksan...@lanedo.com writes:

 Testing for a QMI port is currenly non-trivial. You will have to modify
 the cdc-wdm or qmi_wwan drivers to bind to the device and see if it
 replies to QMI.  A nice test util for someone to write (I've been
 planning to, but haven't had the time and probably won't either...)
 would be something using libusb to send a CDC embedded QMI message (the
 subsystem version check looks like a good candidate) to a given
 device/interface.  Should be fairly easy to write.
 

 That check will possibly be done directly through libqmi/libqmi-glib. In
 my libqmi-glib, opening a QmiDevice involves doing the subsystem version
 check already.

Yes, adding a libusb-based CDC driver backend in addition to the
character device backed would be useful, at least for testing and
verifying new devices.


Bjørn
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list