[MM] Question regarding minimum probing time in MMPluginManager
The following code in MMPluginManager seems to always wait until the minimum probing time is consumed. /* If we didn't use the minimum probing time, wait for it to finish */ else if (ctx-timeout_id 0) { mm_dbg ((Plugin Manager) '%s' port probe finished, last one in device, but minimum probing time not consumed yet ('%lf' seconds elapsed), g_udev_device_get_name (port_probe_ctx-port), g_timer_elapsed (ctx-timer, NULL)); For a modem with known port layout (assuming we have a way to specify in the plugin), once all port probes finish, should MMPluginManager proceed without waiting for the minimum probing time to expire? Is there a potential downside? The obvious upside is cutting the modem startup time. Thanks, Ben ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] Question regarding minimum probing time in MMPluginManager
Aleksander, As always, we're very motivated to shave a second or a few hundred milliseconds off the modem startup time :) I guess a simple solution is to let a plugin specify the max number of ports N, such that the plugin manager will skip the minimum probing time after seeing the completion of N port probes. A more generic solution would be adding a hook in the plugin to decide whether or not to end the probing process after each port probe finishes. How do you think? Thanks, Ben On Fri, Jun 28, 2013 at 12:13 AM, Aleksander Morgado aleksan...@lanedo.com wrote: The following code in MMPluginManager seems to always wait until the minimum probing time is consumed. /* If we didn't use the minimum probing time, wait for it to finish */ else if (ctx-timeout_id 0) { mm_dbg ((Plugin Manager) '%s' port probe finished, last one in device, but minimum probing time not consumed yet ('%lf' seconds elapsed), g_udev_device_get_name (port_probe_ctx-port), g_timer_elapsed (ctx-timer, NULL)); For a modem with known port layout (assuming we have a way to specify in the plugin), once all port probes finish, should MMPluginManager proceed without waiting for the minimum probing time to expire? Is there a potential downside? The obvious upside is cutting the modem startup time. This is just to let the kernel some time to expose all the ports, and should only be a couple of seconds max. If the first probe is exposed and right away probed, and there is no other port around when the probing finishes, the whole device probing is assumed finished. Newer ports appearing afterwards should still get grabbed by the already created modem, but these ports will not take part in the probing decisions. But of course, if we have a way to specify the expected port layout in the plugin itself (in very device-specific plugins like the novatel-lte or altair-lte), then there is no point in waiting the minimum probing time if we already got the expected layout, of course. -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] novatel-lte: propagate error when load_current_bands fails
This patch fixes the following crash when MMIfaceModem::load_current_bands_ready() dereferences a NULL GError pointer, which happens when the novatel-lte plugin fails to load the current bands but does not propagate the error. Thread 0 *CRASHED* ( SIGSEGV @ 0x ) 0x7f04d6c89c36 [ModemManager]- mm-iface-modem.c:3886 load_current_bands_ready 0x7f04d6942236 [libgio-2.0.so.0.3200.4] - gsimpleasyncresult.c:767 g_simple_async_result_complete 0x7f04d6942338 [libgio-2.0.so.0.3200.4] - gsimpleasyncresult.c:779 complete_in_idle_cb 0x7f04d67fad74 [libglib-2.0.so.0.3200.4] - gmain.c:2539 g_main_context_dispatch 0x7f04d67fb0f7 [libglib-2.0.so.0.3200.4] - gmain.c:3146 g_main_context_iterate 0x7f04d67fb551 [libglib-2.0.so.0.3200.4] - gmain.c:3340 g_main_loop_run 0x7f04d6c68795 [ModemManager]- main.c:142]main 0x7f04d6213464 [libc-2.15.so]- libc-start.c:234] __libc_start_main 0x7f04d6c68318 [ModemManager]+ 0x0001d318] --- plugins/novatel/mm-broadband-modem-novatel-lte.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/novatel/mm-broadband-modem-novatel-lte.c b/plugins/novatel/mm-broadband-modem-novatel-lte.c index 0db1686..8cb127a 100644 --- a/plugins/novatel/mm-broadband-modem-novatel-lte.c +++ b/plugins/novatel/mm-broadband-modem-novatel-lte.c @@ -376,7 +376,9 @@ load_current_bands_finish (MMIfaceModem *self, GAsyncResult *res, GError **error) { -/* Never fails */ +if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error)) +return NULL; + return (GArray *) g_array_ref (g_simple_async_result_get_op_res_gpointer ( G_SIMPLE_ASYNC_RESULT (res))); } -- 1.8.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] altair-lte: propagate error when load_{supported, current}_bands fails
This patch fixes a potential crash when MMIfaceModem::load_current_bands_ready() dereferences a NULL GError pointer, which happens when the altair-lte plugin fails to load the current bands but does not propagate the error. It also fixes a similar issue with the plugin fails to load the supported bands, even though MMIfaceModem::load_supported_bands_ready() checks for a NULL GError pointer. --- plugins/altair/mm-broadband-modem-altair-lte.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/plugins/altair/mm-broadband-modem-altair-lte.c b/plugins/altair/mm-broadband-modem-altair-lte.c index 4eeae1c..a2ae2ba 100644 --- a/plugins/altair/mm-broadband-modem-altair-lte.c +++ b/plugins/altair/mm-broadband-modem-altair-lte.c @@ -286,7 +286,9 @@ load_supported_bands_finish (MMIfaceModem *self, GAsyncResult *res, GError **error) { -/* Never fails */ +if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error)) +return NULL; + return (GArray *) g_array_ref (g_simple_async_result_get_op_res_gpointer ( G_SIMPLE_ASYNC_RESULT (res))); } @@ -365,7 +367,9 @@ load_current_bands_finish (MMIfaceModem *self, GAsyncResult *res, GError **error) { -/* Never fails */ +if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error)) +return NULL; + return (GArray *) g_array_ref (g_simple_async_result_get_op_res_gpointer ( G_SIMPLE_ASYNC_RESULT (res))); } -- 1.8.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] altair-lte: avoid sending ATZ when enabling the modem
This patch prevents an ATZ command, which causes the modem to reboot, from being sent to the modem when the modem is being enabled. --- plugins/altair/mm-broadband-modem-altair-lte.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/plugins/altair/mm-broadband-modem-altair-lte.c b/plugins/altair/mm-broadband-modem-altair-lte.c index e645776..4eeae1c 100644 --- a/plugins/altair/mm-broadband-modem-altair-lte.c +++ b/plugins/altair/mm-broadband-modem-altair-lte.c @@ -1044,4 +1044,11 @@ mm_broadband_modem_altair_lte_class_init (MMBroadbandModemAltairLteClass *klass) g_type_class_add_private (object_class, sizeof (MMBroadbandModemAltairLtePrivate)); broadband_modem_class-setup_ports = setup_ports; + +/* The Altair LTE modem reboots itself upon receiving an ATZ command. We + * need to skip the default implementation in MMBroadbandModem to prevent + * an ATZ command from being issued as part of the modem initialization + * sequence when enabling the modem. */ +broadband_modem_class-enabling_modem_init = NULL; +broadband_modem_class-enabling_modem_init_finish = NULL; } -- 1.8.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[mobile-broadband-provider-info PATCH] se: add Teliasonera
--- serviceproviders.xml | 10 ++ 1 file changed, 10 insertions(+) diff --git a/serviceproviders.xml b/serviceproviders.xml index 6809f90..42912fb 100644 --- a/serviceproviders.xml +++ b/serviceproviders.xml @@ -9214,6 +9214,16 @@ conceived. !-- Sweden -- country code=se + provider + nameTeliasonera/name + gsm + network-id mcc=240 mnc=01/ + apn value=online.telia.se + plan type=postpaid/ + usage type=internet/ + /apn + /gsm + /provider provider primary=true name3/name gsm -- 1.8.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[mobile-broadband-provider-info PATCH] in: add mtnl.net APN for MTNL
--- serviceproviders.xml | 10 ++ 1 file changed, 10 insertions(+) diff --git a/serviceproviders.xml b/serviceproviders.xml index 6809f90..d68b55b 100644 --- a/serviceproviders.xml +++ b/serviceproviders.xml @@ -5305,6 +5305,16 @@ conceived. gsm network-id mcc=404 mnc=68/ network-id mcc=404 mnc=69/ + apn value=mtnl.net + usage type=internet/ + nameDelhi (3G Prepaid / Postpaid)/name + usernamemtnl/username + passwordmtnl123/password + /apn + apn value=mtnl.net + usage type=internet/ + nameMumbai (3G Prepaid / Postpaid)/name + /apn apn value=gprsmtnldel plan type=postpaid/ usage type=internet/ -- 1.8.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH v3] novatel-lte: increase number of retries for connection status checks
This patch increases the number of retries, from 4 to 60, for connection status check during a connection / disconnection request, which handles some scenario when the connection / disconnection request takes more than 5 seconds to complete. --- plugins/novatel/mm-broadband-bearer-novatel-lte.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c b/plugins/novatel/mm-broadband-bearer-novatel-lte.c index 31c5650..e0227da 100644 --- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c +++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c @@ -281,7 +281,7 @@ connect_3gpp (MMBroadbandBearer *self, callback, user_data, connect_3gpp); -ctx-retries = 4; +ctx-retries = 60; /* Get a 'net' data port */ ctx-data = mm_base_modem_get_best_data_port (MM_BASE_MODEM (modem), @@ -333,7 +333,7 @@ detailed_disconnect_context_new (MMBroadbandBearer *self, callback, user_data, detailed_disconnect_context_new); -ctx-retries = 4; +ctx-retries = 60; return ctx; } -- 1.8.2.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH] novatel-lte: increase number of retries for connection status checks
Sounds good to me. Changed to 60s. Ben On Tue, May 28, 2013 at 12:47 AM, Aleksander Morgado aleksan...@lanedo.comwrote: On 28/05/13 06:04, Ben Chan wrote: 60s seems a bit excessive as users most likely give up earlier. Revised the patch to have a 30s timeout. Remember that MM lets the connection attempt to be cancelled at any time by Disconnect()-ing the Bearer during the connection attempt (best effort is done to perform the disconnection), so connection managers can actually let users give up whenever they want. Actually, IIRC there's a 120s timeout in NetworkManager by default (to consider not only the dialling, but also additional steps in SimpleConnect() like authentication and such), with an effective timeout at the end of 60s in most cases as that's the timeout MM uses by default for the dialling step. Shouldn't we try to have the same timeout in MM side in all plugins? (i.e. 60s). What do you think? Thanks, Ben On Mon, May 27, 2013 at 12:52 AM, Aleksander Morgado aleksan...@lanedo.com mailto:aleksan...@lanedo.com wrote: On 26/05/13 08:45, Ben Chan wrote: This patch increases the number of retries, from 4 to 10, for connection status check during a connection / disconnection request, which handles some scenario when the connection / disconnection request takes more than 5 seconds to complete. We recently updated most plugins and the generic implementation to wait up to 60s to get the connection up. Do you really think that 10s is enough? --- plugins/novatel/mm-broadband-bearer-novatel-lte.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c b/plugins/novatel/mm-broadband-bearer-novatel-lte.c index 31c5650..677bc3c 100644 --- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c +++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c @@ -281,7 +281,7 @@ connect_3gpp (MMBroadbandBearer *self, callback, user_data, connect_3gpp); -ctx-retries = 4; +ctx-retries = 10; /* Get a 'net' data port */ ctx-data = mm_base_modem_get_best_data_port (MM_BASE_MODEM (modem), @@ -333,7 +333,7 @@ detailed_disconnect_context_new (MMBroadbandBearer *self, callback, user_data, detailed_disconnect_context_new); -ctx-retries = 4; +ctx-retries = 10; return ctx; } -- Aleksander -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH v2] novatel-lte: normalize QMI status when included in DBus error message
This patches normalize a response for the AT$NWQMISTATUS command, by replacing white-space characters with a space, before the response is included in a DBus error message. --- plugins/novatel/mm-broadband-bearer-novatel-lte.c | 33 --- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c b/plugins/novatel/mm-broadband-bearer-novatel-lte.c index 31c5650..a28db66 100644 --- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c +++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c @@ -42,6 +42,22 @@ struct _MMBroadbandBearerNovatelLtePrivate { guint connection_poller; }; +static gchar * +normalize_qmistatus (const gchar *status) +{ +gchar *normalized_status, *iter; + +if (!status) +return NULL; + +normalized_status = g_strdup (status); +for (iter = normalized_status; *iter; iter++) +if (g_ascii_isspace (*iter)) +*iter = ' '; + +return normalized_status; +} + /*/ /* 3GPP Connection sequence */ @@ -146,6 +162,7 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem, DetailedConnectContext *ctx) { const gchar *result; +gchar *normalized_result; GError *error = NULL; result = mm_base_modem_at_command_finish (MM_BASE_MODEM (ctx-modem), @@ -188,10 +205,13 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem, } /* Already exhausted all retries */ +normalized_result = normalize_qmistatus (result); g_simple_async_result_set_error (ctx-result, MM_CORE_ERROR, MM_CORE_ERROR_FAILED, - %s, result); + QMI connect failed: %s, + normalized_result); +g_free (normalized_result); detailed_connect_context_complete_and_free (ctx); } @@ -385,6 +405,7 @@ disconnect_3gpp_status_ready (MMBaseModem *modem, } else { mm_dbg (QMI connection status failed: %s, error-message); g_error_free (error); +result = Unknown error; } if (ctx-retries 0) { @@ -398,13 +419,17 @@ disconnect_3gpp_status_ready (MMBaseModem *modem, /* If $NWQMISTATUS reports a CONNECTED QMI state, returns an error such that * the modem state remains 'connected'. Otherwise, assumes the modem is * disconnected from the network successfully. */ -if (is_connected) +if (is_connected) { +gchar *normalized_result; + +normalized_result = normalize_qmistatus (result); g_simple_async_result_set_error (ctx-result, MM_CORE_ERROR, MM_CORE_ERROR_FAILED, QMI disconnect failed: %s, - result); -else + normalized_result); +g_free (normalized_result); +} else g_simple_async_result_set_op_res_gboolean (ctx-result, TRUE); detailed_disconnect_context_complete_and_free (ctx); -- 1.8.2.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH v2] novatel-lte: increase number of retries for connection status checks
This patch increases the number of retries, from 4 to 30, for connection status check during a connection / disconnection request, which handles some scenario when the connection / disconnection request takes more than 5 seconds to complete. --- plugins/novatel/mm-broadband-bearer-novatel-lte.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c b/plugins/novatel/mm-broadband-bearer-novatel-lte.c index 31c5650..e0227da 100644 --- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c +++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c @@ -281,7 +281,7 @@ connect_3gpp (MMBroadbandBearer *self, callback, user_data, connect_3gpp); -ctx-retries = 4; +ctx-retries = 30; /* Get a 'net' data port */ ctx-data = mm_base_modem_get_best_data_port (MM_BASE_MODEM (modem), @@ -333,7 +333,7 @@ detailed_disconnect_context_new (MMBroadbandBearer *self, callback, user_data, detailed_disconnect_context_new); -ctx-retries = 4; +ctx-retries = 30; return ctx; } -- 1.8.2.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH] novatel-lte: increase number of retries for connection status checks
60s seems a bit excessive as users most likely give up earlier. Revised the patch to have a 30s timeout. Thanks, Ben On Mon, May 27, 2013 at 12:52 AM, Aleksander Morgado aleksan...@lanedo.comwrote: On 26/05/13 08:45, Ben Chan wrote: This patch increases the number of retries, from 4 to 10, for connection status check during a connection / disconnection request, which handles some scenario when the connection / disconnection request takes more than 5 seconds to complete. We recently updated most plugins and the generic implementation to wait up to 60s to get the connection up. Do you really think that 10s is enough? --- plugins/novatel/mm-broadband-bearer-novatel-lte.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c b/plugins/novatel/mm-broadband-bearer-novatel-lte.c index 31c5650..677bc3c 100644 --- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c +++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c @@ -281,7 +281,7 @@ connect_3gpp (MMBroadbandBearer *self, callback, user_data, connect_3gpp); -ctx-retries = 4; +ctx-retries = 10; /* Get a 'net' data port */ ctx-data = mm_base_modem_get_best_data_port (MM_BASE_MODEM (modem), @@ -333,7 +333,7 @@ detailed_disconnect_context_new (MMBroadbandBearer *self, callback, user_data, detailed_disconnect_context_new); -ctx-retries = 4; +ctx-retries = 10; return ctx; } -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] novatel-lte: normalize QMI status when included in DBus error message
This patches normalize a response for the AT$NWQMISTATUS command, by replacing white-space characters with a space, before the response is included in a DBus error message. --- plugins/novatel/mm-broadband-bearer-novatel-lte.c | 41 ++- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c b/plugins/novatel/mm-broadband-bearer-novatel-lte.c index 31c5650..6329414 100644 --- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c +++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c @@ -42,6 +42,22 @@ struct _MMBroadbandBearerNovatelLtePrivate { guint connection_poller; }; +static gchar * +normalize_qmistatus (const gchar *status) +{ +gchar *normalized_status, *iter; + +if (!status) +return NULL; + +normalized_status = g_strdup (status); +for (iter = normalized_status; *iter; iter++) +if (g_ascii_isspace (*iter)) +*iter = ' '; + +return normalized_status; +} + /*/ /* 3GPP Connection sequence */ @@ -145,12 +161,12 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem, GAsyncResult *res, DetailedConnectContext *ctx) { -const gchar *result; +gchar *result; GError *error = NULL; -result = mm_base_modem_at_command_finish (MM_BASE_MODEM (ctx-modem), - res, - error); +result = normalize_qmistatus (mm_base_modem_at_command_finish (MM_BASE_MODEM (ctx-modem), + res, + error)); if (!result) { mm_warn (QMI connection status failed: %s, error-message); if (!g_error_matches (error, MM_MOBILE_EQUIPMENT_ERROR, MM_MOBILE_EQUIPMENT_ERROR_UNKNOWN)) { @@ -159,7 +175,7 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem, return; } g_error_free (error); -result = Unknown error; +result = g_strdup (Unknown error); } else if (is_qmistatus_connected (result)) { MMBearerIpConfig *config; @@ -175,6 +191,7 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem, (GDestroyNotify)mm_bearer_connect_result_unref); g_object_unref (config); detailed_connect_context_complete_and_free (ctx); +g_free (result); return; } @@ -184,6 +201,7 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem, mm_dbg (Retrying status check in a second. %d retries left., ctx-retries); g_timeout_add_seconds (1, (GSourceFunc)connect_3gpp_qmistatus, ctx); +g_free (result); return; } @@ -193,6 +211,7 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem, MM_CORE_ERROR_FAILED, %s, result); detailed_connect_context_complete_and_free (ctx); +g_free (result); } static gboolean @@ -366,18 +385,19 @@ disconnect_3gpp_status_ready (MMBaseModem *modem, GAsyncResult *res, DetailedDisconnectContext *ctx) { -const gchar *result; +gchar *result; GError *error = NULL; gboolean is_connected = FALSE; -result = mm_base_modem_at_command_finish (MM_BASE_MODEM (modem), - res, - error); +result = normalize_qmistatus (mm_base_modem_at_command_finish (MM_BASE_MODEM (modem), + res, + error)); if (result) { mm_dbg (QMI connection status: %s, result); if (is_qmistatus_disconnected (result)) { g_simple_async_result_set_op_res_gboolean (ctx-result, TRUE); detailed_disconnect_context_complete_and_free (ctx); +g_free (result); return; } else if (is_qmistatus_connected (result)) { is_connected = TRUE; @@ -385,6 +405,7 @@ disconnect_3gpp_status_ready (MMBaseModem *modem, } else { mm_dbg (QMI connection status failed: %s, error-message); g_error_free (error); +result = g_strdup (Unknown error); } if (ctx-retries 0) { @@ -392,6 +413,7 @@ disconnect_3gpp_status_ready (MMBaseModem *modem, mm_dbg (Retrying status check in a second. %d retries left., ctx-retries); g_timeout_add_seconds (1, (GSourceFunc)disconnect_3gpp_qmistatus, ctx); +g_free (result); return; } @@ -408,6 +430,7 @@ disconnect_3gpp_status_ready (MMBaseModem *modem, g_simple_async_result_set_op_res_gboolean (ctx-result, TRUE);
[MM] [PATCH] novatel-lte: increase number of retries for connection status checks
This patch increases the number of retries, from 4 to 10, for connection status check during a connection / disconnection request, which handles some scenario when the connection / disconnection request takes more than 5 seconds to complete. --- plugins/novatel/mm-broadband-bearer-novatel-lte.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c b/plugins/novatel/mm-broadband-bearer-novatel-lte.c index 31c5650..677bc3c 100644 --- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c +++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c @@ -281,7 +281,7 @@ connect_3gpp (MMBroadbandBearer *self, callback, user_data, connect_3gpp); -ctx-retries = 4; +ctx-retries = 10; /* Get a 'net' data port */ ctx-data = mm_base_modem_get_best_data_port (MM_BASE_MODEM (modem), @@ -333,7 +333,7 @@ detailed_disconnect_context_new (MMBroadbandBearer *self, callback, user_data, detailed_disconnect_context_new); -ctx-retries = 4; +ctx-retries = 10; return ctx; } -- 1.8.2.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] novatel: fix invalid comparison of unsigned expression
This patch fixes the following invalid comparison of unsigned expression: novatel/mm-plugin-novatel.c:148:29: error: comparison of unsigned expression = 0 is always true [-Werror,-Wtautological-compare] if (ctx-nwdmat_retries = 0) { ~~~ ^ ~ Bug reported on https://code.google.com/p/chromium/issues/detail?id=242150 --- plugins/novatel/mm-plugin-novatel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/novatel/mm-plugin-novatel.c b/plugins/novatel/mm-plugin-novatel.c index 4d09f23..2b4497b 100644 --- a/plugins/novatel/mm-plugin-novatel.c +++ b/plugins/novatel/mm-plugin-novatel.c @@ -145,7 +145,7 @@ custom_init_step (CustomInitContext *ctx) return; } -if (ctx-nwdmat_retries = 0) { +if (ctx-nwdmat_retries 0) { ctx-nwdmat_retries--; mm_at_serial_port_queue_command (ctx-port, $NWDMAT=1, -- 1.8.2.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] device: check for NULL driver in add_port_driver
This patch fixes a crash in MMDevice::add_port_driver() due to g_str_equal() dereferencing a NULL driver returned by mm_device_utils_get_port_driver(). Bug reported on https://code.google.com/p/chromium/issues/detail?id=241823 --- src/mm-device.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mm-device.c b/src/mm-device.c index bfc4b36..54f6456 100644 --- a/src/mm-device.c +++ b/src/mm-device.c @@ -255,6 +255,8 @@ add_port_driver (MMDevice *self, guint i; driver = mm_device_utils_get_port_driver (udev_port); +if (!driver) +return; n_items = (self-priv-drivers ? g_strv_length (self-priv-drivers) : 0); if (n_items 0) { -- 1.8.2.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH v2] sierra: remove comparison of unsigned expression = 0
This patch removes an unnecessary check of unsigned expression = 0, which also fixes the following clang warnings: sierra/mm-broadband-modem-sierra.c:570:18: error: comparison of unsigned expression = 0 is always true [-Werror,-Wtautological-compare] mode = 0 ^ ~ Bug reported on https://code.google.com/p/chromium/issues/detail?id=235989 Patched by Yunlian Jiang yunl...@chromium.org --- plugins/sierra/mm-broadband-modem-sierra.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/plugins/sierra/mm-broadband-modem-sierra.c b/plugins/sierra/mm-broadband-modem-sierra.c index 7cb8e6c..f8cfb74 100644 --- a/plugins/sierra/mm-broadband-modem-sierra.c +++ b/plugins/sierra/mm-broadband-modem-sierra.c @@ -566,9 +566,7 @@ selrat_query_ready (MMBaseModem *self, if (g_regex_match_full (r, response, strlen (response), 0, 0, match_info, error)) { guint mode; -if (mm_get_uint_from_match_info (match_info, 1, mode) -mode = 0 -mode = 7) { +if (mm_get_uint_from_match_info (match_info, 1, mode) mode = 7) { switch (mode) { case 0: result.allowed = MM_MODEM_MODE_ANY; -- 1.8.2.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM][PATCH] remove comparison of unsigned expression = 0
Hi Aleksander / Dan, I've rebased Yunlian's original patch to MM HEAD: https://mail.gnome.org/archives/networkmanager-list/2013-May/msg00026.html Could you please take another look? Thanks, Ben On Tue, Apr 30, 2013 at 1:34 PM, Ben Chan benc...@chromium.org wrote: I fixed the cid check in a different way: https://mail.gnome.org/archives/networkmanager-list/2013-April/msg00223.html Also revise the quality normalization: https://mail.gnome.org/archives/networkmanager-list/2013-April/msg00227.html On Tue, Apr 30, 2013 at 12:09 AM, Aleksander Morgado aleksan...@lanedo.com wrote: On 29/04/13 18:23, Ben Chan wrote: How about the cid case? cid == 0 seems to be an invalid value? Ah, true, cid == 0 wouldn't make sense there. -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH] broadband-bearer: fix cid check in disconnect_3gpp
Aleksander / Dan, is this the right fix? Thanks, Ben On Tue, Apr 30, 2013 at 12:54 PM, Ben Chan benc...@chromium.org wrote: A value 0 is used to denote an invalid/uninitialized CID. This patch fixes a CID check in disconnect_3gpp() of MMBroadbandBearer such that it disables all PDP contexts via AT+CGACT=0 when no specific CID is used (i.e. cid == 0). --- src/mm-broadband-bearer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mm-broadband-bearer.c b/src/mm-broadband-bearer.c index 292420c..193f44a 100644 --- a/src/mm-broadband-bearer.c +++ b/src/mm-broadband-bearer.c @@ -1508,7 +1508,7 @@ disconnect_3gpp (MMBroadbandBearer *self, user_data); /* If no specific CID was used, disable all PDP contexts */ -ctx-cgact_command = (cid = 0 ? +ctx-cgact_command = (cid 0 ? g_strdup_printf (+CGACT=0,%d, cid) : g_strdup_printf (+CGACT=0)); -- 1.8.2.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] broadband-bearer: fix cid check in disconnect_3gpp
A value 0 is used to denote an invalid/uninitialized CID. This patch fixes a CID check in disconnect_3gpp() of MMBroadbandBearer such that it disables all PDP contexts via AT+CGACT=0 when no specific CID is used (i.e. cid == 0). --- src/mm-broadband-bearer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mm-broadband-bearer.c b/src/mm-broadband-bearer.c index 292420c..193f44a 100644 --- a/src/mm-broadband-bearer.c +++ b/src/mm-broadband-bearer.c @@ -1508,7 +1508,7 @@ disconnect_3gpp (MMBroadbandBearer *self, user_data); /* If no specific CID was used, disable all PDP contexts */ -ctx-cgact_command = (cid = 0 ? +ctx-cgact_command = (cid 0 ? g_strdup_printf (+CGACT=0,%d, cid) : g_strdup_printf (+CGACT=0)); -- 1.8.2.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH] iface-modem-3gpp: handle non-deferrable registration state updates
Hi Aleksander / Dan, Would a transition from 'registered' to 'idle'/'searching' considered a 'service' loss from the connection manager's perspective (e.g. the service disappears and then reappears in connection manager)? In practice, a +CEREG change may not necessarily mean that the service disappears. But I guess such a glitch can be smoothed out in the connection manager layer instead of the modem manager layer. I'm happy to update the logic as suggested if that's the expected behavior. Thanks, Ben On Tue, Apr 30, 2013 at 5:23 AM, Aleksander Morgado aleksan...@lanedo.comwrote: Hey Ben, Resurrecting old patch... On 05/03/13 03:37, Ben Chan wrote: This patch changes MMIfaceModem3gpp to differentiate between deferrable and non-deferrable 3GPP registration state updates. Periodic or unsolicited registration state updates are deferrable, while internal updates, e.g. due to modem being disabled, are non-deferrable. I think that we should *not* defer the registration state update unless the modem was connected. This is, if the modem goes from registered to idle or searching, that update must be published in the interface right away. What do you think? Are you able to update the logic like that? -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH] iface-modem-3gpp: handle non-deferrable registration state updates
+Dan On Tue, Apr 30, 2013 at 1:03 PM, Ben Chan benc...@chromium.org wrote: Hi Aleksander / Dan, Would a transition from 'registered' to 'idle'/'searching' considered a 'service' loss from the connection manager's perspective (e.g. the service disappears and then reappears in connection manager)? In practice, a +CEREG change may not necessarily mean that the service disappears. But I guess such a glitch can be smoothed out in the connection manager layer instead of the modem manager layer. I'm happy to update the logic as suggested if that's the expected behavior. Thanks, Ben On Tue, Apr 30, 2013 at 5:23 AM, Aleksander Morgado aleksan...@lanedo.com wrote: Hey Ben, Resurrecting old patch... On 05/03/13 03:37, Ben Chan wrote: This patch changes MMIfaceModem3gpp to differentiate between deferrable and non-deferrable 3GPP registration state updates. Periodic or unsolicited registration state updates are deferrable, while internal updates, e.g. due to modem being disabled, are non-deferrable. I think that we should *not* defer the registration state update unless the modem was connected. This is, if the modem goes from registered to idle or searching, that update must be published in the interface right away. What do you think? Are you able to update the logic like that? -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] broadband-modem: update signal quality normalization
This patch updates normalize_ciev_cind_signal_quality() in MMBroadbandModem to remove an unnecessary check on 'quality = 0' and also makes sure the normalized signal quality is capped at 100 when no maximum is specified. This is revised from a patch originally authored by Yunlian Jiang yunl...@chromium.org. Bug reported on https://code.google.com/p/chromium/issues/detail?id=235989 --- src/mm-broadband-modem.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c index 764b4ed..45d9027 100644 --- a/src/mm-broadband-modem.c +++ b/src/mm-broadband-modem.c @@ -1651,11 +1651,10 @@ normalize_ciev_cind_signal_quality (guint quality, guint min, guint max) { -if (!max -quality = 0) { +if (!max) { /* If we didn't get a max, assume it was 5. Note that we do allow * 0, meaning no signal at all. */ -return (quality * 20); +return (quality = 5) ? (quality * 20) : 100; } if (quality = min -- 1.8.2.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM][PATCH] remove comparison of unsigned expression = 0
I fixed the cid check in a different way: https://mail.gnome.org/archives/networkmanager-list/2013-April/msg00223.html Also revise the quality normalization: https://mail.gnome.org/archives/networkmanager-list/2013-April/msg00227.html On Tue, Apr 30, 2013 at 12:09 AM, Aleksander Morgado aleksan...@lanedo.comwrote: On 29/04/13 18:23, Ben Chan wrote: How about the cid case? cid == 0 seems to be an invalid value? Ah, true, cid == 0 wouldn't make sense there. -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM][PATCH] remove comparison of unsigned expression = 0
How about the cid case? cid == 0 seems to be an invalid value? On Apr 28, 2013 10:55 PM, Aleksander Morgado aleksan...@lanedo.com wrote: On 27/04/13 08:00, Ben Chan wrote: [Ben] Aleksander, do you think the original code was meant to check quality 0? +if (!max) { /* If we didn't get a max, assume it was 5. Note that we do allow * 0, meaning no signal at all. */ According to the comment just after that, zero is allowed. -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM][PATCH] remove comparison of unsigned expression = 0
On Fri, Apr 26, 2013 at 10:29 PM, Yunlian Jiang yunl...@google.com wrote: This patch removes a few unnecessary checks of unsigned expression = 0, which also fixes the following clang warnings: mm-broadband-bearer.c:1511:31: error: comparison of unsigned expression = 0 is always true [-Werror,-Wtautological-compare] ctx-cgact_command = (cid = 0 ? ~~~ ^ ~ 1 error generated. mm-broadband-modem.c:1647:17: error: comparison of unsigned expression = 0 is always true [-Werror,-Wtautological-compare] quality = 0) { ~~~ ^ ~ sierra/mm-broadband-modem-sierra.c:570:18: error: comparison of unsigned expression = 0 is always true [-Werror,-Wtautological-compare] mode = 0 ^ ~ Bug reported on https://code.google.com/p/chromium/issues/detail?id=235989 --- diff --git a/plugins/sierra/mm-broadband-modem-sierra.c b/plugins/sierra/mm-broadband-modem-sierra.c index 301f41e..affc01f 100644 --- a/plugins/sierra/mm-broadband-modem-sierra.c +++ b/plugins/sierra/mm-broadband-modem-sierra.c @@ -567,7 +567,6 @@ selrat_query_ready (MMBaseModem *self, guint mode; if (mm_get_uint_from_match_info (match_info, 1, mode) -mode = 0 mode = 7) { switch (mode) { case 0: diff --git a/src/mm-broadband-bearer.c b/src/mm-broadband-bearer.c index 292420c..fcaad56 100644 --- a/src/mm-broadband-bearer.c +++ b/src/mm-broadband-bearer.c @@ -1508,9 +1508,7 @@ disconnect_3gpp (MMBroadbandBearer *self, user_data); /* If no specific CID was used, disable all PDP contexts */ -ctx-cgact_command = (cid = 0 ? [Ben] Aleksander, do you think the original code was meant to check cid 0? - g_strdup_printf (+CGACT=0,%d, cid) : - g_strdup_printf (+CGACT=0)); +ctx-cgact_command = g_strdup_printf (+CGACT=0,%d, cid); /* If the primary port is NOT connected (doesn't have to be the data port), * we'll send CGACT there */ diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c index 92b2050..bfca363 100644 --- a/src/mm-broadband-modem.c +++ b/src/mm-broadband-modem.c @@ -1643,8 +1643,7 @@ normalize_ciev_cind_signal_quality (guint quality, guint min, guint max) { -if (!max -quality = 0) { [Ben] Aleksander, do you think the original code was meant to check quality 0? +if (!max) { /* If we didn't get a max, assume it was 5. Note that we do allow * 0, meaning no signal at all. */ return (quality * 20); -- 1.8.2.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] qcdm: remove unnecessary NULL check on free()
This patch removes a few unnecessary NULL checks on free(), which also fixes the following clang warnings: result.c:59:27: error: if statement has empty body [-Werror,-Wempty-body] if (v-u.u8_array); ^ result.c:59:27: note: put the semicolon on a separate line to silence this warning result.c:62:28: error: if statement has empty body [-Werror,-Wempty-body] if (v-u.u16_array); ^ result.c:62:28: note: put the semicolon on a separate line to silence this warning Bug reported on https://code.google.com/p/chromium/issues/detail?id=219280 Patched by Yunlian Jiang yunl...@chromium.org --- libqcdm/src/result.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/libqcdm/src/result.c b/libqcdm/src/result.c index d51a050..d508fb7 100644 --- a/libqcdm/src/result.c +++ b/libqcdm/src/result.c @@ -53,14 +53,11 @@ static void val_free (Val *v) { if (v-type == VAL_TYPE_STRING) { -if (v-u.s) -free (v-u.s); +free (v-u.s); } else if (v-type == VAL_TYPE_U8_ARRAY) { -if (v-u.u8_array); -free (v-u.u8_array); +free (v-u.u8_array); } else if (v-type == VAL_TYPE_U16_ARRAY) { -if (v-u.u16_array); -free (v-u.u16_array); +free (v-u.u16_array); } free (v-key); memset (v, 0, sizeof (*v)); -- 1.8.2.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH v2] bearer: allow specifying default IP family for bearers
This patch adds a 'bearer-default-ip-family' property to MMBearer, which specifies the default IP family to use for a bearer when no explicit value is given via the simple connect properties. The default IP family is set to IPv4 in MMBearer but can be overridden by a MMBearer subclass, which allows a modem plugin to specify an appropriate default value. --- libmm-glib/mm-bearer-properties.c | 8 +--- src/mm-bearer-mbim.c | 14 +++--- src/mm-bearer-qmi.c | 18 +- src/mm-bearer.c | 25 + src/mm-bearer.h | 2 ++ src/mm-broadband-bearer.c | 16 6 files changed, 64 insertions(+), 19 deletions(-) diff --git a/libmm-glib/mm-bearer-properties.c b/libmm-glib/mm-bearer-properties.c index 6526ed0..2f59fee 100644 --- a/libmm-glib/mm-bearer-properties.c +++ b/libmm-glib/mm-bearer-properties.c @@ -674,13 +674,7 @@ mm_bearer_properties_init (MMBearerProperties *self) self-priv-allow_roaming = TRUE; self-priv-rm_protocol = MM_MODEM_CDMA_RM_PROTOCOL_UNKNOWN; self-priv-allowed_auth = MM_BEARER_ALLOWED_AUTH_UNKNOWN; - -/* At some point in the future, this default should probably be changed - * to IPV4V6. However, presently support for this PDP type is rare. An - * even better approach would likely be to query which PDP types the - * modem supports (using AT+CGDCONT=?), and set the default accordingly - */ -self-priv-ip_type = MM_BEARER_IP_FAMILY_IPV4; +self-priv-ip_type = MM_BEARER_IP_FAMILY_UNKNOWN; } static void diff --git a/src/mm-bearer-mbim.c b/src/mm-bearer-mbim.c index d96a1b1..bddc9da 100644 --- a/src/mm-bearer-mbim.c +++ b/src/mm-bearer-mbim.c @@ -618,6 +618,7 @@ connect_context_step (ConnectContext *ctx) const gchar *password; MbimAuthProtocol auth; MbimContextIpType ip_type; +MMBearerIpFamily ip_family; GError *error = NULL; /* Setup parameters to use */ @@ -658,7 +659,14 @@ connect_context_step (ConnectContext *ctx) } } -switch (mm_bearer_properties_get_ip_type (ctx-properties)) { +ip_family = mm_bearer_properties_get_ip_type (ctx-properties); +if (ip_family == MM_BEARER_IP_FAMILY_UNKNOWN) { +ip_family = mm_bearer_get_default_ip_family (MM_BEARER (ctx-self)); +mm_dbg (No specific IP family requested, defaulting to %s, +mm_bearer_ip_family_get_string (ip_family)); +} + +switch (ip_family) { case MM_BEARER_IP_FAMILY_IPV4: ip_type = MBIM_CONTEXT_IP_TYPE_IPV4; break; @@ -670,8 +678,8 @@ connect_context_step (ConnectContext *ctx) break; case MM_BEARER_IP_FAMILY_UNKNOWN: default: -mm_dbg (No specific IP family requested, defaulting to IPv4); -ip_type = MBIM_CONTEXT_IP_TYPE_IPV4; +/* A valid default IP family should have been specified */ +g_assert_not_reached (); break; } diff --git a/src/mm-bearer-qmi.c b/src/mm-bearer-qmi.c index 8f9568d..9e8d2bc 100644 --- a/src/mm-bearer-qmi.c +++ b/src/mm-bearer-qmi.c @@ -846,11 +846,21 @@ _connect (MMBearer *self, if (properties) { MMBearerAllowedAuth auth; +MMBearerIpFamily ip_family; ctx-apn = g_strdup (mm_bearer_properties_get_apn (properties)); ctx-user = g_strdup (mm_bearer_properties_get_user (properties)); ctx-password = g_strdup (mm_bearer_properties_get_password (properties)); -switch (mm_bearer_properties_get_ip_type (properties)) { + +ip_family = mm_bearer_properties_get_ip_type (properties); +if (ip_family == MM_BEARER_IP_FAMILY_UNKNOWN) { +ip_family = mm_bearer_get_default_ip_family (self); +mm_dbg (No specific IP family requested, defaulting to %s, +mm_bearer_ip_family_get_string (ip_family)); +ctx-no_ip_family_preference = TRUE; +} + +switch (ip_family) { case MM_BEARER_IP_FAMILY_IPV4: ctx-ipv4 = TRUE; ctx-ipv6 = FALSE; @@ -865,10 +875,8 @@ _connect (MMBearer *self, break; case MM_BEARER_IP_FAMILY_UNKNOWN: default: -mm_dbg (No specific IP family requested, defaulting to IPv4); -ctx-no_ip_family_preference = TRUE; -ctx-ipv4 = TRUE; -ctx-ipv6 = FALSE; +/* A valid default IP family should have been specified */ +g_assert_not_reached (); break; } diff --git a/src/mm-bearer.c b/src/mm-bearer.c index 9e96bde..d047e11 100644 --- a/src/mm-bearer.c +++ b/src/mm-bearer.c @@ -57,6 +57,7 @@ enum { PROP_MODEM, PROP_STATUS, PROP_CONFIG, +PROP_DEFAULT_IP_FAMILY, PROP_LAST }; @@ -73,6 +74,8 @@ struct _MMBearerPrivate { MMBearerStatus status; /*
Re: [MM] [PATCH] bearer: allow specifying default IP family for bearers
Thanks. Submitted a revised patch. - Ben On Mon, Apr 22, 2013 at 12:42 AM, Aleksander Morgado aleksan...@lanedo.comwrote: On 04/22/2013 03:50 AM, Ben Chan wrote: This patch adds a 'bearer-default-ip-family' property to MMBearer, which specifies the default IP family to use for a bearer when no explicit value is given via the simple connect properties. The default IP family is set to IPv4 in MMBearer but can be overridden by a MMBearer subclass, which allows a modem plugin to specify an appropriate default value. The logic to use the new property was added only in the MMBroadbandBearer subclass; all the other available MMBearer subclasses (QMI, MBIM) should also get modified to use this new default value. Some more comments below. --- libmm-glib/mm-bearer-properties.c | 8 +--- src/mm-bearer.c | 25 + src/mm-bearer.h | 2 ++ src/mm-broadband-bearer.c | 25 + 4 files changed, 49 insertions(+), 11 deletions(-) diff --git a/libmm-glib/mm-bearer-properties.c b/libmm-glib/mm-bearer-properties.c index 6526ed0..2f59fee 100644 --- a/libmm-glib/mm-bearer-properties.c +++ b/libmm-glib/mm-bearer-properties.c @@ -674,13 +674,7 @@ mm_bearer_properties_init (MMBearerProperties *self) self-priv-allow_roaming = TRUE; self-priv-rm_protocol = MM_MODEM_CDMA_RM_PROTOCOL_UNKNOWN; self-priv-allowed_auth = MM_BEARER_ALLOWED_AUTH_UNKNOWN; - -/* At some point in the future, this default should probably be changed - * to IPV4V6. However, presently support for this PDP type is rare. An - * even better approach would likely be to query which PDP types the - * modem supports (using AT+CGDCONT=?), and set the default accordingly - */ -self-priv-ip_type = MM_BEARER_IP_FAMILY_IPV4; +self-priv-ip_type = MM_BEARER_IP_FAMILY_UNKNOWN; } static void diff --git a/src/mm-bearer.c b/src/mm-bearer.c index 9e96bde..d047e11 100644 --- a/src/mm-bearer.c +++ b/src/mm-bearer.c @@ -57,6 +57,7 @@ enum { PROP_MODEM, PROP_STATUS, PROP_CONFIG, +PROP_DEFAULT_IP_FAMILY, PROP_LAST }; @@ -73,6 +74,8 @@ struct _MMBearerPrivate { MMBearerStatus status; /* Configuration of the bearer */ MMBearerProperties *config; +/* Default IP family of this bearer */ +MMBearerIpFamily default_ip_family; /* Cancellable for connect() */ GCancellable *connect_cancellable; @@ -844,6 +847,12 @@ mm_bearer_get_config (MMBearer *self) NULL); } +MMBearerIpFamily +mm_bearer_get_default_ip_family (MMBearer *self) +{ +return self-priv-default_ip_family; +} + /*/ static void @@ -969,6 +978,9 @@ set_property (GObject *object, g_variant_unref (dictionary); break; } +case PROP_DEFAULT_IP_FAMILY: +self-priv-default_ip_family = g_value_get_enum (value); +break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; @@ -999,6 +1011,9 @@ get_property (GObject *object, case PROP_CONFIG: g_value_set_object (value, self-priv-config); break; +case PROP_DEFAULT_IP_FAMILY: +g_value_set_enum (value, self-priv-default_ip_family); +break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; @@ -1015,6 +1030,7 @@ mm_bearer_init (MMBearer *self) self-priv-status = MM_BEARER_STATUS_DISCONNECTED; self-priv-reason_3gpp = CONNECTION_FORBIDDEN_REASON_NONE; self-priv-reason_cdma = CONNECTION_FORBIDDEN_REASON_NONE; +self-priv-default_ip_family = MM_BEARER_IP_FAMILY_IPV4; /* Set defaults */ mm_gdbus_bearer_set_interface (MM_GDBUS_BEARER (self), NULL); @@ -,6 +1127,15 @@ mm_bearer_class_init (MMBearerClass *klass) MM_TYPE_BEARER_PROPERTIES, G_PARAM_READWRITE); g_object_class_install_property (object_class, PROP_CONFIG, properties[PROP_CONFIG]); + +properties[PROP_DEFAULT_IP_FAMILY] = +g_param_spec_enum (MM_BEARER_DEFAULT_IP_FAMILY, + Bearer default IP family, + IP family to use for this bearer when no IP family is specified, + MM_TYPE_BEARER_IP_FAMILY, + MM_BEARER_IP_FAMILY_IPV4, + G_PARAM_READWRITE); +g_object_class_install_property (object_class, PROP_DEFAULT_IP_FAMILY, properties[PROP_DEFAULT_IP_FAMILY]); } /*/ diff --git a/src/mm-bearer.h b/src/mm-bearer.h index c617297..cc71bfd
[MM] [PATCH] bearer: allow specifying default IP family for bearers
This patch adds a 'bearer-default-ip-family' property to MMBearer, which specifies the default IP family to use for a bearer when no explicit value is given via the simple connect properties. The default IP family is set to IPv4 in MMBearer but can be overridden by a MMBearer subclass, which allows a modem plugin to specify an appropriate default value. --- libmm-glib/mm-bearer-properties.c | 8 +--- src/mm-bearer.c | 25 + src/mm-bearer.h | 2 ++ src/mm-broadband-bearer.c | 25 + 4 files changed, 49 insertions(+), 11 deletions(-) diff --git a/libmm-glib/mm-bearer-properties.c b/libmm-glib/mm-bearer-properties.c index 6526ed0..2f59fee 100644 --- a/libmm-glib/mm-bearer-properties.c +++ b/libmm-glib/mm-bearer-properties.c @@ -674,13 +674,7 @@ mm_bearer_properties_init (MMBearerProperties *self) self-priv-allow_roaming = TRUE; self-priv-rm_protocol = MM_MODEM_CDMA_RM_PROTOCOL_UNKNOWN; self-priv-allowed_auth = MM_BEARER_ALLOWED_AUTH_UNKNOWN; - -/* At some point in the future, this default should probably be changed - * to IPV4V6. However, presently support for this PDP type is rare. An - * even better approach would likely be to query which PDP types the - * modem supports (using AT+CGDCONT=?), and set the default accordingly - */ -self-priv-ip_type = MM_BEARER_IP_FAMILY_IPV4; +self-priv-ip_type = MM_BEARER_IP_FAMILY_UNKNOWN; } static void diff --git a/src/mm-bearer.c b/src/mm-bearer.c index 9e96bde..d047e11 100644 --- a/src/mm-bearer.c +++ b/src/mm-bearer.c @@ -57,6 +57,7 @@ enum { PROP_MODEM, PROP_STATUS, PROP_CONFIG, +PROP_DEFAULT_IP_FAMILY, PROP_LAST }; @@ -73,6 +74,8 @@ struct _MMBearerPrivate { MMBearerStatus status; /* Configuration of the bearer */ MMBearerProperties *config; +/* Default IP family of this bearer */ +MMBearerIpFamily default_ip_family; /* Cancellable for connect() */ GCancellable *connect_cancellable; @@ -844,6 +847,12 @@ mm_bearer_get_config (MMBearer *self) NULL); } +MMBearerIpFamily +mm_bearer_get_default_ip_family (MMBearer *self) +{ +return self-priv-default_ip_family; +} + /*/ static void @@ -969,6 +978,9 @@ set_property (GObject *object, g_variant_unref (dictionary); break; } +case PROP_DEFAULT_IP_FAMILY: +self-priv-default_ip_family = g_value_get_enum (value); +break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; @@ -999,6 +1011,9 @@ get_property (GObject *object, case PROP_CONFIG: g_value_set_object (value, self-priv-config); break; +case PROP_DEFAULT_IP_FAMILY: +g_value_set_enum (value, self-priv-default_ip_family); +break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; @@ -1015,6 +1030,7 @@ mm_bearer_init (MMBearer *self) self-priv-status = MM_BEARER_STATUS_DISCONNECTED; self-priv-reason_3gpp = CONNECTION_FORBIDDEN_REASON_NONE; self-priv-reason_cdma = CONNECTION_FORBIDDEN_REASON_NONE; +self-priv-default_ip_family = MM_BEARER_IP_FAMILY_IPV4; /* Set defaults */ mm_gdbus_bearer_set_interface (MM_GDBUS_BEARER (self), NULL); @@ -,6 +1127,15 @@ mm_bearer_class_init (MMBearerClass *klass) MM_TYPE_BEARER_PROPERTIES, G_PARAM_READWRITE); g_object_class_install_property (object_class, PROP_CONFIG, properties[PROP_CONFIG]); + +properties[PROP_DEFAULT_IP_FAMILY] = +g_param_spec_enum (MM_BEARER_DEFAULT_IP_FAMILY, + Bearer default IP family, + IP family to use for this bearer when no IP family is specified, + MM_TYPE_BEARER_IP_FAMILY, + MM_BEARER_IP_FAMILY_IPV4, + G_PARAM_READWRITE); +g_object_class_install_property (object_class, PROP_DEFAULT_IP_FAMILY, properties[PROP_DEFAULT_IP_FAMILY]); } /*/ diff --git a/src/mm-bearer.h b/src/mm-bearer.h index c617297..cc71bfd 100644 --- a/src/mm-bearer.h +++ b/src/mm-bearer.h @@ -58,6 +58,7 @@ typedef struct _MMBearerPrivate MMBearerPrivate; #define MM_BEARER_MODEM bearer-modem #define MM_BEARER_STATUS bearer-status #define MM_BEARER_CONFIG bearer-config +#define MM_BEARER_DEFAULT_IP_FAMILY bearer-deafult-ip-family typedef enum { /* underscore_name=mm_bearer_status */ MM_BEARER_STATUS_DISCONNECTED, @@ -103,6 +104,7 @@ const gchar*mm_bearer_get_path(MMBearer *bearer); MMBearerStatus mm_bearer_get_status (MMBearer *bearer); MMBearerProperties *mm_bearer_peek_config (MMBearer *self);
[MM] [PATCH] device: handle NULL returned by g_udev_device_get_driver() gracefully
This patch fixes a crash in mm_device_grab_port() when doing a string comparison on a NULL returned by g_udev_device_get_driver(). Thread 0 *CRASHED* ( SIGSEGV @ 0x ) 0x76b760b4 [libc-2.15.so] - strcmp.c:38 strcmp 0x76c66a7d [libglib-2.0.so.0.3200.4]- ghash.c:1704 g_str_equal 0x76ee0e5d [ModemManager] - mm-device.c:147 mm_device_grab_port 0x76edf9d9 [ModemManager] - mm-manager.c:313 device_added 0x76e95b2d [libgudev-1.0.so.0.1.0] - extras/gudev/gudevmarshal.c:84 g_udev_marshal_VOID__STRING_OBJECT 0x76d1fb2b [libgobject-2.0.so.0.3200.4] - gclosure.c:777 g_closure_invoke 0x76d2b88b [libgobject-2.0.so.0.3200.4] - gsignal.c:3551 signal_emit_unlocked_R 0x76d313c5 [libgobject-2.0.so.0.3200.4] - gsignal.c:3300 g_signal_emit_valist 0x76d31569 [libgobject-2.0.so.0.3200.4] - gsignal.c:3356 g_signal_emit 0x76e93bdd [libgudev-1.0.so.0.1.0] - extras/gudev/gudevclient.c:105 monitor_event 0x76c9beb7 [libglib-2.0.so.0.3200.4]- giounix.c:166 g_io_unix_dispatch 0x76c714c1 [libglib-2.0.so.0.3200.4]- gmain.c:2539 g_main_context_dispatch 0x76c71745 [libglib-2.0.so.0.3200.4]- gmain.c:3146 g_main_context_iterate 0x76c71a59 [libglib-2.0.so.0.3200.4]- gmain.c:3340 g_main_loop_run 0x76ede8ed [ModemManager] - main.c:142 main 0x76b35f79 [libc-2.15.so] - libc-start.c:226 __libc_start_main 0x76edea49 [ModemManager] + 0x00014a49 0x76eb4eab [ld-2.15.so] + 0xaeab --- src/mm-device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mm-device.c b/src/mm-device.c index 7800439..a2755a7 100644 --- a/src/mm-device.c +++ b/src/mm-device.c @@ -145,7 +145,7 @@ get_device_ids (GUdevDevice *device, success = TRUE; goto out; } else if (g_str_has_prefix (parent_subsys, usb) - g_str_equal (g_udev_device_get_driver (parent), qmi_wwan)) { + !g_strcmp0 (g_udev_device_get_driver (parent), qmi_wwan)) { /* Need to look for vendor/product in the parent of the QMI device */ GUdevDevice *qmi_parent; -- 1.8.1.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] serial,blacklist: fix file mode (0755 - 0644)
--- src/77-mm-usb-device-blacklist.rules | 0 src/mm-at-serial-port.c | 0 2 files changed, 0 insertions(+), 0 deletions(-) mode change 100755 = 100644 src/77-mm-usb-device-blacklist.rules mode change 100755 = 100644 src/mm-at-serial-port.c diff --git a/src/77-mm-usb-device-blacklist.rules b/src/77-mm-usb-device-blacklist.rules old mode 100755 new mode 100644 diff --git a/src/mm-at-serial-port.c b/src/mm-at-serial-port.c old mode 100755 new mode 100644 -- 1.8.1.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] broadband-bearer: handle NULL and character escaping of APN value
--- src/mm-broadband-bearer.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/mm-broadband-bearer.c b/src/mm-broadband-bearer.c index c34867d..fe894c9 100644 --- a/src/mm-broadband-bearer.c +++ b/src/mm-broadband-bearer.c @@ -753,7 +753,7 @@ find_cid_ready (MMBaseModem *modem, DetailedConnectContext *ctx) { GVariant *result; -gchar *command; +gchar *apn, *command; GError *error = NULL; const gchar *pdp_type; @@ -783,10 +783,12 @@ find_cid_ready (MMBaseModem *modem, } ctx-cid = g_variant_get_uint32 (result); -command = g_strdup_printf (+CGDCONT=%u,\%s\,\%s\, +apn = mm_at_serial_port_quote_string (mm_bearer_properties_get_apn (mm_bearer_peek_config (MM_BEARER (ctx-self; +command = g_strdup_printf (+CGDCONT=%u,\%s\,%s, ctx-cid, pdp_type, - mm_bearer_properties_get_apn (mm_bearer_peek_config (MM_BEARER (ctx-self; + apn); +g_free (apn); mm_base_modem_at_command_full (ctx-modem, ctx-primary, command, -- 1.8.1.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM][PATCH v4] altair-lte: initial altair lte plugin
Hi Ori I guess the copyright note in mm-broadband-bearer-altair-lte.c may simply be Copyright (C) 2013 Altair Semiconductor as it's a new file, right? - Ben On Wed, Apr 3, 2013 at 4:56 AM, Aleksander Morgado aleksan...@lanedo.comwrote: On 04/02/2013 01:57 AM, Ori Inbar wrote: Hello all This is a log for the plugin run : 2013-04-01T16:19:30.614176-07:00 localhost ModemManager[1366]: debug (Plugin Manager) [/sys/devices/s5p-ehci/usb1/1-1] Checking device support... 2013-04-01T16:19:30.614408-07:00 localhost ModemManager[1366]: debug (Novatel LTE) [ttyACM0] filtered by vendor/product IDs 2013-04-01T16:19:30.614428-07:00 localhost ModemManager[1366]: debug (Samsung) [ttyACM0] filtered by vendor/product IDs 2013-04-01T16:19:30.614445-07:00 localhost ModemManager[1366]: debug (Huawei) [ttyACM0] filtered by vendor/product IDs 2013-04-01T16:19:30.614460-07:00 localhost ModemManager[1366]: debug (Plugin Manager) [ttyACM0] Found '1' plugins to try... 2013-04-01T16:19:30.614473-07:00 localhost ModemManager[1366]: debug (Plugin Manager) [ttyACM0] Will try with plugin 'Altair LTE' Thanks! Looks pretty good :) -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] broadband-modem: fix enable flag in UnsolicitedRegistrationEventsContext
--- src/mm-broadband-modem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c index 9d127c9..b3ec35a 100644 --- a/src/mm-broadband-modem.c +++ b/src/mm-broadband-modem.c @@ -3712,7 +3712,7 @@ unsolicited_registration_events_context_new (MMBroadbandModem *self, callback, user_data, unsolicited_registration_events_context_new); -ctx-enable = FALSE; +ctx-enable = enable; ctx-run_cs = cs_supported; ctx-run_ps = ps_supported; ctx-run_eps = eps_supported; -- 1.8.1.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] broadband-modem: fix disabling of unsolicited registration events
--- src/mm-broadband-modem.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c index 9f45904..9d127c9 100644 --- a/src/mm-broadband-modem.c +++ b/src/mm-broadband-modem.c @@ -3924,7 +3924,7 @@ unsolicited_registration_events_context_step (UnsolicitedRegistrationEventsConte mm_base_modem_at_sequence_full ( MM_BASE_MODEM (ctx-self), mm_base_modem_peek_port_primary (MM_BASE_MODEM (ctx-self)), -cs_registration_sequence, +ctx-enable ? cs_registration_sequence : cs_unregistration_sequence, NULL, /* response processor context */ NULL, /* response processor context free */ NULL, /* cancellable */ @@ -3939,7 +3939,7 @@ unsolicited_registration_events_context_step (UnsolicitedRegistrationEventsConte mm_base_modem_at_sequence_full ( MM_BASE_MODEM (ctx-self), mm_base_modem_peek_port_primary (MM_BASE_MODEM (ctx-self)), -ps_registration_sequence, +ctx-enable ? ps_registration_sequence : ps_unregistration_sequence, NULL, /* response processor context */ NULL, /* response processor context free */ NULL, /* cancellable */ @@ -3954,7 +3954,7 @@ unsolicited_registration_events_context_step (UnsolicitedRegistrationEventsConte mm_base_modem_at_sequence_full ( MM_BASE_MODEM (ctx-self), mm_base_modem_peek_port_primary (MM_BASE_MODEM (ctx-self)), -eps_registration_sequence, +ctx-enable ? eps_registration_sequence : eps_unregistration_sequence, NULL, /* response processor context */ NULL, /* response processor context free */ NULL, /* cancellable */ -- 1.8.1.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] iface-modem-3gpp: handle non-deferrable registration state updates
This patch changes MMIfaceModem3gpp to differentiate between deferrable and non-deferrable 3GPP registration state updates. Periodic or unsolicited registration state updates are deferrable, while internal updates, e.g. due to modem being disabled, are non-deferrable. --- src/mm-iface-modem-3gpp.c | 40 ++-- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c index 6c8bb14..7049ec8 100644 --- a/src/mm-iface-modem-3gpp.c +++ b/src/mm-iface-modem-3gpp.c @@ -1093,7 +1093,8 @@ run_deferred_registration_state_update (MMIfaceModem3gpp *self) static void update_registration_state (MMIfaceModem3gpp *self, - MMModem3gppRegistrationState new_state) + MMModem3gppRegistrationState new_state, + gboolean deferrable) { MMModem3gppRegistrationState old_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; RegistrationStateContext *ctx; @@ -1106,18 +1107,20 @@ update_registration_state (MMIfaceModem3gpp *self, g_assert (ctx); if (ctx-deferred_update_id != 0) { -/* If there is already a deferred 'registration loss' state update and the new update - * is not a registered state, update the deferred state update without extending the - * timeout. */ -if (new_state != MM_MODEM_3GPP_REGISTRATION_STATE_HOME -new_state != MM_MODEM_3GPP_REGISTRATION_STATE_ROAMING) { -mm_info (Modem %s: 3GPP Registration state changed (%s - %s), update deferred, - g_dbus_object_get_object_path (G_DBUS_OBJECT (self)), - mm_modem_3gpp_registration_state_get_string (old_state), - mm_modem_3gpp_registration_state_get_string (new_state)); - -ctx-deferred_new_state = new_state; -return; +if (deferrable) { +/* If there is already a deferred 'registration loss' state update and the new update + * is not a registered state, update the deferred state update without extending the + * timeout. */ +if (new_state != MM_MODEM_3GPP_REGISTRATION_STATE_HOME +new_state != MM_MODEM_3GPP_REGISTRATION_STATE_ROAMING) { +mm_info (Modem %s: 3GPP Registration state changed (%s - %s), update deferred, + g_dbus_object_get_object_path (G_DBUS_OBJECT (self)), + mm_modem_3gpp_registration_state_get_string (old_state), + mm_modem_3gpp_registration_state_get_string (new_state)); + +ctx-deferred_new_state = new_state; +return; +} } /* Otherwise, cancel any deferred registration state update */ @@ -1153,7 +1156,8 @@ update_registration_state (MMIfaceModem3gpp *self, if ((old_state == MM_MODEM_3GPP_REGISTRATION_STATE_HOME || old_state == MM_MODEM_3GPP_REGISTRATION_STATE_ROAMING) (new_state == MM_MODEM_3GPP_REGISTRATION_STATE_SEARCHING || - new_state == MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN)) { + new_state == MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN) +deferrable) { mm_info (Modem %s: 3GPP Registration state changed (%s - %s), update deferred, g_dbus_object_get_object_path (G_DBUS_OBJECT (self)), mm_modem_3gpp_registration_state_get_string (old_state), @@ -1191,7 +1195,7 @@ mm_iface_modem_3gpp_update_cs_registration_state (MMIfaceModem3gpp *self, ctx = get_registration_state_context (self); ctx-cs = state; -update_registration_state (self, get_consolidated_reg_state (ctx)); +update_registration_state (self, get_consolidated_reg_state (ctx), TRUE); } void @@ -1210,7 +1214,7 @@ mm_iface_modem_3gpp_update_ps_registration_state (MMIfaceModem3gpp *self, ctx = get_registration_state_context (self); ctx-ps = state; -update_registration_state (self, get_consolidated_reg_state (ctx)); +update_registration_state (self, get_consolidated_reg_state (ctx), TRUE); } void @@ -1229,7 +1233,7 @@ mm_iface_modem_3gpp_update_eps_registration_state (MMIfaceModem3gpp *self, ctx = get_registration_state_context (self); ctx-eps = state; -update_registration_state (self, get_consolidated_reg_state (ctx)); +update_registration_state (self, get_consolidated_reg_state (ctx), TRUE); } /*/ @@ -1492,7 +1496,7 @@ interface_disabling_step (DisablingContext *ctx) ctx-step++; case DISABLING_STEP_REGISTRATION_STATE: -update_registration_state (ctx-self, MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN); +update_registration_state (ctx-self, MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN, FALSE); mm_iface_modem_3gpp_update_access_technologies (ctx-self,
Re: [MM] [PATCH v2] iface-modem-3gpp: clear deferred registration state update when disabling
On Thu, Feb 28, 2013 at 11:46 PM, Aleksander Morgado aleksan...@lanedo.comwrote: On 02/28/2013 08:11 PM, Ben Chan wrote: --- src/mm-iface-modem-3gpp.c | 15 +++ 1 file changed, 15 insertions(+) I pushed it after modifying it to include the clear_deferred_registration_state_update() call in the first DISABLING_STEP_PERIODIC_REGISTRATION_CHECKS step. As said previously, we don't really mind if we get an unsolicited registration event during the disabling process, as we're disabling anyway. I'm a bit confused. If an unsolicited registration event is received after clearing deferred_update_id, deferred_update_id is set to a new value, which will be scheduled after the modem is disabled. That will cause the modem state to become enabled due to the following code in mm-iface-modem-3gpp.c: mm_iface_modem_update_subsystem_state ( MM_IFACE_MODEM (self), SUBSYSTEM_3GPP, (new_state == MM_MODEM_3GPP_REGISTRATION_STATE_SEARCHING ? MM_MODEM_STATE_SEARCHING : MM_MODEM_STATE_ENABLED), MM_MODEM_STATE_CHANGE_REASON_UNKNOWN); ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH v2] iface-modem-3gpp: clear deferred registration state update when disabling
Yep, that should work. On Fri, Mar 1, 2013 at 12:18 AM, Aleksander Morgado aleksan...@lanedo.comwrote: On 03/01/2013 09:06 AM, Ben Chan wrote: On Thu, Feb 28, 2013 at 11:46 PM, Aleksander Morgado aleksan...@lanedo.com mailto:aleksan...@lanedo.com wrote: On 02/28/2013 08:11 PM, Ben Chan wrote: --- src/mm-iface-modem-3gpp.c | 15 +++ 1 file changed, 15 insertions(+) I pushed it after modifying it to include the clear_deferred_registration_state_update() call in the first DISABLING_STEP_PERIODIC_REGISTRATION_CHECKS step. As said previously, we don't really mind if we get an unsolicited registration event during the disabling process, as we're disabling anyway. I'm a bit confused. If an unsolicited registration event is received after clearing deferred_update_id, deferred_update_id is set to a new value, which will be scheduled after the modem is disabled. That will cause the modem state to become enabled due to the following code in mm-iface-modem-3gpp.c: mm_iface_modem_update_subsystem_state ( MM_IFACE_MODEM (self), SUBSYSTEM_3GPP, (new_state == MM_MODEM_3GPP_REGISTRATION_STATE_SEARCHING ? MM_MODEM_STATE_SEARCHING : MM_MODEM_STATE_ENABLED), MM_MODEM_STATE_CHANGE_REASON_UNKNOWN); Ah, good point; didn't consider that the deferred update could get re-scheduled... In that case we should actually clear it *after* DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS, once we know we will not process those unsolicited events. How does this look? diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c index 1d3fdce..6c8bb14 100644 --- a/src/mm-iface-modem-3gpp.c +++ b/src/mm-iface-modem-3gpp.c @@ -1347,6 +1347,7 @@ typedef enum { DISABLING_STEP_PERIODIC_REGISTRATION_CHECKS, DISABLING_STEP_DISABLE_UNSOLICITED_REGISTRATION_EVENTS, DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS, +DISABLING_STEP_CLEANUP_DEFERRED_REGISTRATION_UPDATE, DISABLING_STEP_CLEANUP_UNSOLICITED_EVENTS, DISABLING_STEP_DISABLE_UNSOLICITED_EVENTS, DISABLING_STEP_REGISTRATION_STATE, @@ -1419,8 +1420,6 @@ interface_disabling_step (DisablingContext *ctx) case DISABLING_STEP_PERIODIC_REGISTRATION_CHECKS: /* Disable periodic registration checks, if they were set */ periodic_registration_check_disable (ctx-self); -/* Prevent any deferred registration state update from happening after the modem is disabled */ -clear_deferred_registration_state_update (ctx-self); /* Fall down to next step */ ctx-step++; @@ -1462,6 +1461,12 @@ interface_disabling_step (DisablingContext *ctx) /* Fall down to next step */ ctx-step++; +case DISABLING_STEP_CLEANUP_DEFERRED_REGISTRATION_UPDATE: +/* Prevent any deferred registration state update from happening after the modem is disabled */ +clear_deferred_registration_state_update (ctx-self); +/* Fall down to next step */ +ctx-step++; + case DISABLING_STEP_CLEANUP_UNSOLICITED_EVENTS: if (MM_IFACE_MODEM_3GPP_GET_INTERFACE (ctx-self)-cleanup_unsolicited_events MM_IFACE_MODEM_3GPP_GET_INTERFACE (ctx-self)-cleanup_unsolicited_events_finish) { -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH] iface-modem-3gpp: clear deferred registration state update when disabling
One question inline On Feb 28, 2013 12:23 AM, Aleksander Morgado aleksan...@lanedo.com wrote: On 02/27/2013 11:04 PM, Ben Chan wrote: --- src/mm-iface-modem-3gpp.c | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) When I reviewed your original patch I actually checked for when we were removing the timeout when disabling, and assumed it was done when we set the GObject's data to NULL, but I mixed the two data objects we handle here, as one of them (the one where this new timeout id is stored) isn't removed during disabling. So, yes, we need this method to remove the timeout. Now, I would add the clearing method just when we start the disabling sequence, next to the periodic_registration_check_disable() call. Or was it added in CLEANUP_UNSOLICITED_REGISTRATION_EVENTS for some reason? Could an unsolicited registration state update happen between invocations of interface_disabling_step()? I was wondering if the deferred_update_id should be cleared after unsolicited registration event handling is removed. Also some minor things below. diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c index fa8da1a..3f3a803 100644 --- a/src/mm-iface-modem-3gpp.c +++ b/src/mm-iface-modem-3gpp.c @@ -1325,6 +1325,18 @@ periodic_registration_check_enable (MMIfaceModem3gpp *self) (GDestroyNotify)registration_check_context_free); } +static void +clear_deferred_registration_state_update (MMIfaceModem3gpp *self) +{ +RegistrationStateContext *ctx = get_registration_state_context (self); + +ctx-deferred_new_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; +if (ctx-deferred_update_id) { +g_source_remove (ctx-deferred_update_id); +ctx-deferred_update_id = 0; +} +} + /*/ typedef struct _DisablingContext DisablingContext; @@ -1435,7 +1447,10 @@ interface_disabling_step (DisablingContext *ctx) ctx-step++; } -case DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS: +case DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS: { No real need to add braces here, unless we're adding new variables. +/* Prevent any deferred registration state update from happending after the modem is disabled */ happending doesn't seem an English word to me :) Oops... typo :) +clear_deferred_registration_state_update (ctx-self); + if (MM_IFACE_MODEM_3GPP_GET_INTERFACE (ctx-self)-cleanup_unsolicited_registration_events MM_IFACE_MODEM_3GPP_GET_INTERFACE (ctx-self)-cleanup_unsolicited_registration_events_finish) { MM_IFACE_MODEM_3GPP_GET_INTERFACE (ctx-self)-cleanup_unsolicited_registration_events ( @@ -1446,6 +1461,7 @@ interface_disabling_step (DisablingContext *ctx) } /* Fall down to next step */ ctx-step++; +} case DISABLING_STEP_CLEANUP_UNSOLICITED_EVENTS: if (MM_IFACE_MODEM_3GPP_GET_INTERFACE (ctx-self)-cleanup_unsolicited_events -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH v2] iface-modem-3gpp: clear deferred registration state update when disabling
--- src/mm-iface-modem-3gpp.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c index fa8da1a..2430056 100644 --- a/src/mm-iface-modem-3gpp.c +++ b/src/mm-iface-modem-3gpp.c @@ -1325,6 +1325,18 @@ periodic_registration_check_enable (MMIfaceModem3gpp *self) (GDestroyNotify)registration_check_context_free); } +static void +clear_deferred_registration_state_update (MMIfaceModem3gpp *self) +{ +RegistrationStateContext *ctx = get_registration_state_context (self); + +ctx-deferred_new_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; +if (ctx-deferred_update_id) { +g_source_remove (ctx-deferred_update_id); +ctx-deferred_update_id = 0; +} +} + /*/ typedef struct _DisablingContext DisablingContext; @@ -1436,6 +1448,9 @@ interface_disabling_step (DisablingContext *ctx) } case DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS: +/* Prevent any deferred registration state update from happening after the modem is disabled */ +clear_deferred_registration_state_update (ctx-self); + if (MM_IFACE_MODEM_3GPP_GET_INTERFACE (ctx-self)-cleanup_unsolicited_registration_events MM_IFACE_MODEM_3GPP_GET_INTERFACE (ctx-self)-cleanup_unsolicited_registration_events_finish) { MM_IFACE_MODEM_3GPP_GET_INTERFACE (ctx-self)-cleanup_unsolicited_registration_events ( -- 1.8.1.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] iface-modem-3gpp: clear deferred registration state update when disabling
--- src/mm-iface-modem-3gpp.c | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c index fa8da1a..3f3a803 100644 --- a/src/mm-iface-modem-3gpp.c +++ b/src/mm-iface-modem-3gpp.c @@ -1325,6 +1325,18 @@ periodic_registration_check_enable (MMIfaceModem3gpp *self) (GDestroyNotify)registration_check_context_free); } +static void +clear_deferred_registration_state_update (MMIfaceModem3gpp *self) +{ +RegistrationStateContext *ctx = get_registration_state_context (self); + +ctx-deferred_new_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; +if (ctx-deferred_update_id) { +g_source_remove (ctx-deferred_update_id); +ctx-deferred_update_id = 0; +} +} + /*/ typedef struct _DisablingContext DisablingContext; @@ -1435,7 +1447,10 @@ interface_disabling_step (DisablingContext *ctx) ctx-step++; } -case DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS: +case DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS: { +/* Prevent any deferred registration state update from happending after the modem is disabled */ +clear_deferred_registration_state_update (ctx-self); + if (MM_IFACE_MODEM_3GPP_GET_INTERFACE (ctx-self)-cleanup_unsolicited_registration_events MM_IFACE_MODEM_3GPP_GET_INTERFACE (ctx-self)-cleanup_unsolicited_registration_events_finish) { MM_IFACE_MODEM_3GPP_GET_INTERFACE (ctx-self)-cleanup_unsolicited_registration_events ( @@ -1446,6 +1461,7 @@ interface_disabling_step (DisablingContext *ctx) } /* Fall down to next step */ ctx-step++; +} case DISABLING_STEP_CLEANUP_UNSOLICITED_EVENTS: if (MM_IFACE_MODEM_3GPP_GET_INTERFACE (ctx-self)-cleanup_unsolicited_events -- 1.8.1.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] iface-modem: fix modem state consolidation upon bearer disconnection
Patch iface-modem: fix invalid modem state consolidation (commit 69aff6183a9e6532b4074c89831d6dcfa81ddcce) incorrectly consolidates the modem state upon the disconnection of a bearer. The modem state remains 'connected' after the last bearer is disconnected. This patch fixes that. --- src/mm-iface-modem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c index 63ef6fc..f02b0bd 100644 --- a/src/mm-iface-modem.c +++ b/src/mm-iface-modem.c @@ -270,7 +270,7 @@ bearer_status_changed (MMBearer *bearer, new_state = MM_MODEM_STATE_DISCONNECTING; break; case MM_BEARER_STATUS_DISCONNECTED: -new_state = get_current_consolidated_state (self, state); +new_state = get_current_consolidated_state (self, MM_MODEM_STATE_UNKNOWN); break; } -- 1.8.1.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH] iface-modem: fix invalid modem state consolidation
I've found a bug in this patch and submitted a fix. Upon the disconnection of bearers, the consolidation of modem state shouldn't take the curent state into account, but simply goes through all subsystems. Thanks, Ben On Thu, Jan 24, 2013 at 12:12 AM, Aleksander Morgado aleksan...@lanedo.comwrote: On 24/01/13 02:16, Ben Chan wrote: This patch fixes get_current_consolidated_state() in MMIfaceModem to avoid invalid state transitions, e.g. from 'enabling' to 'disabled'. https://code.google.com/p/chromium-os/issues/detail?id=38173 I modified the commit log to include the information from the bugreport and then pushed it. Thanks! -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH] modem: use +CEREG to determine EPS network registration status
Thanks Aleksander. I committed a a patch to the Chromium OS tree to enable EPS registration check for the novatel-lte plugin. I'd like to see if you and Dan would like to enable it for any LTE-capable modem by default. If so, I will submit a follow-up patch to the modem code instead of the novatel-lte plugin. Ben On Thu, Feb 14, 2013 at 11:56 PM, Aleksander Morgado aleksan...@lanedo.comwrote: Will you be able to test my patch v2 on some of the modems you mentioned? Sure, can do. The V2 patch doesn't seem to cause problems with the ADU960, but then again, I don't have access to the LTE networks it can use (ATT and Lightsquared). After patching the code to unconditionally enable CEREG, the device responds correctly to CEREG requests: AT+CEREG=2 OK AT+CEREG? +CEREG: 2,1 OK when I'm registered on T-Mobile's GSM network. So while I'd really like to have us try CEREG unconditionally if we can, I'm fine with the V2 patch as it is. I just pushed the v2 patch now. Whenever we can test it properly I guess we can enable this by default, so not a big deal. Ben, will you send a patch to enable it in the novatel-lte plugin? -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH v4] iface-modem-3gpp: defer registration state update when registration is lost
This patch defers the update of 3GPP registration state by 15 seconds when the registration state changes from 'registered' (home / roaming) to 'searching'. This allows a temporary loss of 3GPP registration to recover itself when relying on ModemManager to explicitly disconnect and reconnect to the network. --- I rebase patch v3 on top of HEAD, so that this v4 patch can be applied cleanly on HEAD. src/mm-iface-modem-3gpp.c | 122 ++ 1 file changed, 102 insertions(+), 20 deletions(-) diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c index 7117e9c..fa8da1a 100644 --- a/src/mm-iface-modem-3gpp.c +++ b/src/mm-iface-modem-3gpp.c @@ -29,6 +29,7 @@ #include mm-log.h #define REGISTRATION_CHECK_TIMEOUT_SEC 30 +#define DEFERRED_REGISTRATION_STATE_UPDATE_TIMEOUT_SEC 15 #define SUBSYSTEM_3GPP 3gpp @@ -73,6 +74,8 @@ typedef struct { MMModem3gppRegistrationState cs; MMModem3gppRegistrationState ps; MMModem3gppRegistrationState eps; +MMModem3gppRegistrationState deferred_new_state; +guint deferred_update_id; gboolean manual_registration; GCancellable *pending_registration_cancellable; gboolean reloading_operator; @@ -85,6 +88,9 @@ registration_state_context_free (RegistrationStateContext *ctx) g_cancellable_cancel (ctx-pending_registration_cancellable); g_object_unref (ctx-pending_registration_cancellable); } +if (ctx-deferred_update_id) { +g_source_remove (ctx-deferred_update_id); +} g_slice_free (RegistrationStateContext, ctx); } @@ -104,6 +110,7 @@ get_registration_state_context (MMIfaceModem3gpp *self) ctx-cs = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; ctx-ps = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; ctx-eps = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; +ctx-deferred_new_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; g_object_set_qdata_full ( G_OBJECT (self), @@ -1033,25 +1040,98 @@ update_registration_reload_current_operator_ready (MMIfaceModem3gpp *self, } static void +update_non_registered_state (MMIfaceModem3gpp *self, + MMModem3gppRegistrationState old_state, + MMModem3gppRegistrationState new_state) +{ +/* Not registered neither in home nor roaming network */ +mm_iface_modem_3gpp_clear_current_operator (self); + +/* The property in the interface is bound to the property + * in the skeleton, so just updating here is enough */ +g_object_set (self, + MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, new_state, + NULL); + +mm_iface_modem_update_subsystem_state ( +MM_IFACE_MODEM (self), +SUBSYSTEM_3GPP, +(new_state == MM_MODEM_3GPP_REGISTRATION_STATE_SEARCHING ? + MM_MODEM_STATE_SEARCHING : + MM_MODEM_STATE_ENABLED), +MM_MODEM_STATE_CHANGE_REASON_UNKNOWN); +} + +static gboolean +run_deferred_registration_state_update (MMIfaceModem3gpp *self) +{ +MMModem3gppRegistrationState old_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; +MMModem3gppRegistrationState new_state; +RegistrationStateContext *ctx; + +ctx = get_registration_state_context (self); +ctx-deferred_update_id = 0; + +g_object_get (self, + MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, old_state, + NULL); +new_state = ctx-deferred_new_state; + +/* Only set new state if different */ +if (new_state == old_state) +return FALSE; + +mm_info (Modem %s: (deferred) 3GPP Registration state changed (%s - %s), + g_dbus_object_get_object_path (G_DBUS_OBJECT (self)), + mm_modem_3gpp_registration_state_get_string (old_state), + mm_modem_3gpp_registration_state_get_string (new_state)); + +update_non_registered_state (self, old_state, new_state); + +return FALSE; +} + +static void update_registration_state (MMIfaceModem3gpp *self, MMModem3gppRegistrationState new_state) { MMModem3gppRegistrationState old_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; +RegistrationStateContext *ctx; g_object_get (self, MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, old_state, NULL); +ctx = get_registration_state_context (self); +g_assert (ctx); + +if (ctx-deferred_update_id != 0) { +/* If there is already a deferred 'registration loss' state update and the new update + * is not a registered state, update the deferred state update without extending the + * timeout. */ +if (new_state != MM_MODEM_3GPP_REGISTRATION_STATE_HOME +new_state != MM_MODEM_3GPP_REGISTRATION_STATE_ROAMING) { +mm_info (Modem %s: 3GPP Registration state changed (%s - %s), update deferred, + g_dbus_object_get_object_path (G_DBUS_OBJECT (self)), +
[MM] [PATCH v3] iface-modem-3gpp: defer registration state update when registration is lost
This patch defers the update of 3GPP registration state by 15 seconds when the registration state changes from 'registered' (home / roaming) to 'searching'. This allows a temporary loss of 3GPP registration to recover itself when relying on ModemManager to explicitly disconnect and reconnect to the network. --- This patch addresses the comments from Aleksander Morgado aleksan...@lanedo.com. https://mail.gnome.org/archives/networkmanager-list/2013-February/msg00074.html src/mm-iface-modem-3gpp.c | 122 ++ 1 file changed, 102 insertions(+), 20 deletions(-) diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c index 7a88b2f..d1278d4 100644 --- a/src/mm-iface-modem-3gpp.c +++ b/src/mm-iface-modem-3gpp.c @@ -29,6 +29,7 @@ #include mm-log.h #define REGISTRATION_CHECK_TIMEOUT_SEC 30 +#define DEFERRED_REGISTRATION_STATE_UPDATE_TIMEOUT_SEC 15 #define SUBSYSTEM_3GPP 3gpp @@ -72,6 +73,8 @@ mm_iface_modem_3gpp_bind_simple_status (MMIfaceModem3gpp *self, typedef struct { MMModem3gppRegistrationState cs; MMModem3gppRegistrationState ps; +MMModem3gppRegistrationState deferred_new_state; +guint deferred_update_id; gboolean manual_registration; GCancellable *pending_registration_cancellable; gboolean reloading_operator; @@ -84,6 +87,9 @@ registration_state_context_free (RegistrationStateContext *ctx) g_cancellable_cancel (ctx-pending_registration_cancellable); g_object_unref (ctx-pending_registration_cancellable); } +if (ctx-deferred_update_id) { +g_source_remove (ctx-deferred_update_id); +} g_slice_free (RegistrationStateContext, ctx); } @@ -102,6 +108,7 @@ get_registration_state_context (MMIfaceModem3gpp *self) ctx = g_slice_new0 (RegistrationStateContext); ctx-cs = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; ctx-ps = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; +ctx-deferred_new_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; g_object_set_qdata_full ( G_OBJECT (self), @@ -1019,25 +1026,98 @@ update_registration_reload_current_operator_ready (MMIfaceModem3gpp *self, } static void +update_non_registered_state (MMIfaceModem3gpp *self, + MMModem3gppRegistrationState old_state, + MMModem3gppRegistrationState new_state) +{ +/* Not registered neither in home nor roaming network */ +mm_iface_modem_3gpp_clear_current_operator (self); + +/* The property in the interface is bound to the property + * in the skeleton, so just updating here is enough */ +g_object_set (self, + MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, new_state, + NULL); + +mm_iface_modem_update_subsystem_state ( +MM_IFACE_MODEM (self), +SUBSYSTEM_3GPP, +(new_state == MM_MODEM_3GPP_REGISTRATION_STATE_SEARCHING ? + MM_MODEM_STATE_SEARCHING : + MM_MODEM_STATE_ENABLED), +MM_MODEM_STATE_CHANGE_REASON_UNKNOWN); +} + +static gboolean +run_deferred_registration_state_update (MMIfaceModem3gpp *self) +{ +MMModem3gppRegistrationState old_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; +MMModem3gppRegistrationState new_state; +RegistrationStateContext *ctx; + +ctx = get_registration_state_context (self); +ctx-deferred_update_id = 0; + +g_object_get (self, + MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, old_state, + NULL); +new_state = ctx-deferred_new_state; + +/* Only set new state if different */ +if (new_state == old_state) +return FALSE; + +mm_info (Modem %s: (deferred) 3GPP Registration state changed (%s - %s), + g_dbus_object_get_object_path (G_DBUS_OBJECT (self)), + mm_modem_3gpp_registration_state_get_string (old_state), + mm_modem_3gpp_registration_state_get_string (new_state)); + +update_non_registered_state (self, old_state, new_state); + +return FALSE; +} + +static void update_registration_state (MMIfaceModem3gpp *self, MMModem3gppRegistrationState new_state) { MMModem3gppRegistrationState old_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; +RegistrationStateContext *ctx; g_object_get (self, MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, old_state, NULL); +ctx = get_registration_state_context (self); +g_assert (ctx); + +if (ctx-deferred_update_id != 0) { +/* If there is already a deferred 'registration loss' state update and the new update + * is not a registered state, update the deferred state update without extending the + * timeout. */ +if (new_state != MM_MODEM_3GPP_REGISTRATION_STATE_HOME +new_state != MM_MODEM_3GPP_REGISTRATION_STATE_ROAMING) { +mm_info (Modem %s: 3GPP Registration state changed (%s - %s), update
Re: [MM] [PATCH] iface-modem-3gpp: defer registration state update when registration is lost
Thanks Aleksander. Submitted patch v3. On Thu, Feb 14, 2013 at 2:01 AM, Aleksander Morgado aleksan...@lanedo.comwrote: On 02/12/2013 02:18 AM, Ben Chan wrote: This patch defers the update of 3GPP registration state by 15 seconds when the registration state changes from 'registered' (home / roaming) to 'searching'. This allows a temporary loss of 3GPP registration to recover itself when relying on ModemManager to explicitly disconnect and reconnect to the network. I tried to test this with a QMI modem and an external antenna, but disconnecting the antenna while connected just gets me to signal quality 0, it doesn't get neither unregistered nor disconnected. Are you able to grab me some debug logs of a test case where this happens? See some additional comments inline below. --- src/mm-iface-modem-3gpp.c | 108 +- 1 file changed, 88 insertions(+), 20 deletions(-) diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c index 7a88b2f..f5d703d 100644 --- a/src/mm-iface-modem-3gpp.c +++ b/src/mm-iface-modem-3gpp.c @@ -29,6 +29,7 @@ #include mm-log.h #define REGISTRATION_CHECK_TIMEOUT_SEC 30 +#define DEFERRED_REGISTRATION_STATE_UPDATE_TIMEOUT_SEC 15 #define SUBSYSTEM_3GPP 3gpp @@ -72,6 +73,8 @@ mm_iface_modem_3gpp_bind_simple_status (MMIfaceModem3gpp *self, typedef struct { MMModem3gppRegistrationState cs; MMModem3gppRegistrationState ps; +MMModem3gppRegistrationState deferred_new_state; +guint deferred_update_id; gboolean manual_registration; GCancellable *pending_registration_cancellable; gboolean reloading_operator; @@ -84,6 +87,9 @@ registration_state_context_free (RegistrationStateContext *ctx) g_cancellable_cancel (ctx-pending_registration_cancellable); g_object_unref (ctx-pending_registration_cancellable); } +if (ctx-deferred_update_id) { +g_source_remove (ctx-deferred_update_id); +} g_slice_free (RegistrationStateContext, ctx); } @@ -102,6 +108,7 @@ get_registration_state_context (MMIfaceModem3gpp *self) ctx = g_slice_new0 (RegistrationStateContext); ctx-cs = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; ctx-ps = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; +ctx-deferred_new_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; g_object_set_qdata_full ( G_OBJECT (self), @@ -1019,25 +1026,84 @@ update_registration_reload_current_operator_ready (MMIfaceModem3gpp *self, } static void +update_non_registered_state (MMIfaceModem3gpp *self, + MMModem3gppRegistrationState old_state, + MMModem3gppRegistrationState new_state) +{ +/* Not registered neither in home nor roaming network */ +mm_iface_modem_3gpp_clear_current_operator (self); + +/* The property in the interface is bound to the property + * in the skeleton, so just updating here is enough */ +g_object_set (self, + MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, new_state, + NULL); + +mm_iface_modem_update_subsystem_state ( +MM_IFACE_MODEM (self), +SUBSYSTEM_3GPP, +(new_state == MM_MODEM_3GPP_REGISTRATION_STATE_SEARCHING ? + MM_MODEM_STATE_SEARCHING : + MM_MODEM_STATE_ENABLED), +MM_MODEM_STATE_CHANGE_REASON_UNKNOWN); +} + +static gboolean +run_deferred_registration_state_update (MMIfaceModem3gpp *self) +{ +MMModem3gppRegistrationState old_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; +MMModem3gppRegistrationState new_state; +RegistrationStateContext *ctx; + +ctx = get_registration_state_context (self); +ctx-deferred_update_id = 0; + +g_object_get (self, + MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, old_state, + NULL); +new_state = ctx-deferred_new_state; + +/* Only set new state if it is known and different from the old one */ +if (new_state == MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN || +new_state == old_state) +return FALSE; Why do we bail out if UNKNOWN here? If we end up 15s without getting re-registered and the original new state that we got was UNKNOWN, we would not be doing anything, so the state would still say 'registered' in the interface, while we are not. + +mm_info (Modem %s: (deferred) 3GPP Registration state changed (%s - %s), + g_dbus_object_get_object_path (G_DBUS_OBJECT (self)), + mm_modem_3gpp_registration_state_get_string (old_state), + mm_modem_3gpp_registration_state_get_string (new_state)); + +update_non_registered_state (self, old_state, new_state); + +return FALSE; +} + +static void update_registration_state (MMIfaceModem3gpp
Re: [MM] [PATCH] modem: use +CEREG to determine EPS network registration status
Thanks Aleksander. My patch has a typo bug in unsolicited_registration_events_context_step where the eps_error variable in UnsolicitedRegistrationEventsContext is not properly set to NULL after the ownership of GError object pointed by eps_error is transfered. I've submitted a revised patch. - Ben On Mon, Feb 11, 2013 at 11:47 PM, Aleksander Morgado aleksan...@lanedo.comwrote: On 02/11/2013 07:44 PM, Ben Chan wrote: Aleksander and Dan, How do you feel about this patch? Doesn't look bad, but I really need to read it carefully as it changed lots of things. The idea is that plugins can specify whether they require CEREG by setting MM_IFACE_MODEM_3GPP_EPS_NETWORK_SUPPORTED to TRUE themselves, instead of automatically trying it when LTE support is found, right? I now wonder how other AT-controlled LTE modems out there, not just the Novatel LTE, handle this. Is anyone willing to give it a try with other LTE modems? ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] iface-modem-3gpp: defer registration state update when registration is lost
This patch defers the update of 3GPP registration state by 15 seconds when the registration state changes from 'registered' (home / roaming) to 'searching'. This allows a temporary loss of 3GPP registration to recover itself when relying on ModemManager to explicitly disconnect and reconnect to the network. --- src/mm-iface-modem-3gpp.c | 108 +- 1 file changed, 88 insertions(+), 20 deletions(-) diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c index 7a88b2f..f5d703d 100644 --- a/src/mm-iface-modem-3gpp.c +++ b/src/mm-iface-modem-3gpp.c @@ -29,6 +29,7 @@ #include mm-log.h #define REGISTRATION_CHECK_TIMEOUT_SEC 30 +#define DEFERRED_REGISTRATION_STATE_UPDATE_TIMEOUT_SEC 15 #define SUBSYSTEM_3GPP 3gpp @@ -72,6 +73,8 @@ mm_iface_modem_3gpp_bind_simple_status (MMIfaceModem3gpp *self, typedef struct { MMModem3gppRegistrationState cs; MMModem3gppRegistrationState ps; +MMModem3gppRegistrationState deferred_new_state; +guint deferred_update_id; gboolean manual_registration; GCancellable *pending_registration_cancellable; gboolean reloading_operator; @@ -84,6 +87,9 @@ registration_state_context_free (RegistrationStateContext *ctx) g_cancellable_cancel (ctx-pending_registration_cancellable); g_object_unref (ctx-pending_registration_cancellable); } +if (ctx-deferred_update_id) { +g_source_remove (ctx-deferred_update_id); +} g_slice_free (RegistrationStateContext, ctx); } @@ -102,6 +108,7 @@ get_registration_state_context (MMIfaceModem3gpp *self) ctx = g_slice_new0 (RegistrationStateContext); ctx-cs = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; ctx-ps = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; +ctx-deferred_new_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; g_object_set_qdata_full ( G_OBJECT (self), @@ -1019,25 +1026,84 @@ update_registration_reload_current_operator_ready (MMIfaceModem3gpp *self, } static void +update_non_registered_state (MMIfaceModem3gpp *self, + MMModem3gppRegistrationState old_state, + MMModem3gppRegistrationState new_state) +{ +/* Not registered neither in home nor roaming network */ +mm_iface_modem_3gpp_clear_current_operator (self); + +/* The property in the interface is bound to the property + * in the skeleton, so just updating here is enough */ +g_object_set (self, + MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, new_state, + NULL); + +mm_iface_modem_update_subsystem_state ( +MM_IFACE_MODEM (self), +SUBSYSTEM_3GPP, +(new_state == MM_MODEM_3GPP_REGISTRATION_STATE_SEARCHING ? + MM_MODEM_STATE_SEARCHING : + MM_MODEM_STATE_ENABLED), +MM_MODEM_STATE_CHANGE_REASON_UNKNOWN); +} + +static gboolean +run_deferred_registration_state_update (MMIfaceModem3gpp *self) +{ +MMModem3gppRegistrationState old_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; +MMModem3gppRegistrationState new_state; +RegistrationStateContext *ctx; + +ctx = get_registration_state_context (self); +ctx-deferred_update_id = 0; + +g_object_get (self, + MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, old_state, + NULL); +new_state = ctx-deferred_new_state; + +/* Only set new state if it is known and different from the old one */ +if (new_state == MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN || +new_state == old_state) +return FALSE; + +mm_info (Modem %s: (deferred) 3GPP Registration state changed (%s - %s), + g_dbus_object_get_object_path (G_DBUS_OBJECT (self)), + mm_modem_3gpp_registration_state_get_string (old_state), + mm_modem_3gpp_registration_state_get_string (new_state)); + +update_non_registered_state (self, old_state, new_state); + +return FALSE; +} + +static void update_registration_state (MMIfaceModem3gpp *self, MMModem3gppRegistrationState new_state) { MMModem3gppRegistrationState old_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN; +RegistrationStateContext *ctx; g_object_get (self, MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, old_state, NULL); +ctx = get_registration_state_context (self); +g_assert (ctx); + +/* Cancel any deferred registration state update */ +if (ctx-deferred_update_id != 0) { +g_source_remove (ctx-deferred_update_id); +ctx-deferred_update_id = 0; +} + /* Only set new state if different */ if (new_state == old_state) return; if (new_state == MM_MODEM_3GPP_REGISTRATION_STATE_HOME || new_state == MM_MODEM_3GPP_REGISTRATION_STATE_ROAMING) { -RegistrationStateContext *ctx; - -ctx = get_registration_state_context (self); - /* If already
[MM] [PATCH] modem: use +CEREG to determine EPS network registration status
This patch adds the support for solicited/unsolicited EPS network registration status via AT+CEREG, which is configurable via the 'iface-modem-3gpp-eps-network-supported' property of the MMIfaceModem3gpp interface and is disabled by default. --- src/mm-broadband-modem-qmi.c | 3 + src/mm-broadband-modem.c | 165 +++-- src/mm-iface-modem-3gpp.c | 51 - src/mm-iface-modem-3gpp.h | 10 ++- src/mm-modem-helpers.c | 82 +++- src/mm-modem-helpers.h | 1 + src/tests/test-modem-helpers.c | 121 +++--- 7 files changed, 365 insertions(+), 68 deletions(-) diff --git a/src/mm-broadband-modem-qmi.c b/src/mm-broadband-modem-qmi.c index aa82f0e..05be718 100644 --- a/src/mm-broadband-modem-qmi.c +++ b/src/mm-broadband-modem-qmi.c @@ -3984,6 +3984,7 @@ static void modem_3gpp_run_registration_checks (MMIfaceModem3gpp *self, gboolean cs_supported, gboolean ps_supported, +gboolean eps_supported, GAsyncReadyCallback callback, gpointer user_data) { @@ -4138,6 +4139,7 @@ static void modem_3gpp_disable_unsolicited_registration_events (MMIfaceModem3gpp *self, gboolean cs_supported, gboolean ps_supported, +gboolean eps_supported, GAsyncReadyCallback callback, gpointer user_data) { @@ -4183,6 +4185,7 @@ static void modem_3gpp_enable_unsolicited_registration_events (MMIfaceModem3gpp *self, gboolean cs_supported, gboolean ps_supported, + gboolean eps_supported, GAsyncReadyCallback callback, gpointer user_data) { diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c index 3778b56..505c42a 100644 --- a/src/mm-broadband-modem.c +++ b/src/mm-broadband-modem.c @@ -86,6 +86,7 @@ enum { PROP_MODEM_3GPP_REGISTRATION_STATE, PROP_MODEM_3GPP_CS_NETWORK_SUPPORTED, PROP_MODEM_3GPP_PS_NETWORK_SUPPORTED, +PROP_MODEM_3GPP_EPS_NETWORK_SUPPORTED, PROP_MODEM_CDMA_CDMA1X_REGISTRATION_STATE, PROP_MODEM_CDMA_EVDO_REGISTRATION_STATE, PROP_MODEM_CDMA_CDMA1X_NETWORK_SUPPORTED, @@ -129,6 +130,7 @@ struct _MMBroadbandModemPrivate { MMModem3gppRegistrationState modem_3gpp_registration_state; gboolean modem_3gpp_cs_network_supported; gboolean modem_3gpp_ps_network_supported; +gboolean modem_3gpp_eps_network_supported; /* Implementation helpers */ GPtrArray *modem_3gpp_registration_regex; @@ -3280,6 +3282,7 @@ registration_state_changed (MMAtSerialPort *port, gulong lac = 0, cell_id = 0; MMModemAccessTechnology act = MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN; gboolean cgreg = FALSE; +gboolean cereg = FALSE; GError *error = NULL; if (!mm_3gpp_parse_creg_response (match_info, @@ -3288,6 +3291,7 @@ registration_state_changed (MMAtSerialPort *port, cell_id, act, cgreg, + cereg, error)) { mm_warn (error parsing unsolicited registration: %s, error error-message ? error-message : (unknown)); @@ -3298,6 +3302,8 @@ registration_state_changed (MMAtSerialPort *port, /* Report new registration state */ if (cgreg) mm_iface_modem_3gpp_update_ps_registration_state (MM_IFACE_MODEM_3GPP (self), state); +else if (cereg) +mm_iface_modem_3gpp_update_eps_registration_state (MM_IFACE_MODEM_3GPP (self), state); else mm_iface_modem_3gpp_update_cs_registration_state (MM_IFACE_MODEM_3GPP (self), state); @@ -3484,12 +3490,16 @@ typedef struct { GSimpleAsyncResult *result; gboolean cs_supported; gboolean ps_supported; +gboolean eps_supported; gboolean run_cs; gboolean run_ps; +gboolean run_eps; gboolean running_cs; gboolean running_ps; +gboolean running_eps; GError *cs_error; GError *ps_error; +GError *eps_error; } RunRegistrationChecksContext; static void @@ -3500,6 +3510,8 @@ run_registration_checks_context_complete_and_free (RunRegistrationChecksContext g_error_free (ctx-cs_error); if (ctx-ps_error) g_error_free (ctx-ps_error); +if (ctx-eps_error) +g_error_free (ctx-eps_error);
[MM] [PATCH] novatel-lte: retry $NWQMISTATUS check upon error during disconnect
$NWQMISTATUS sometimes returns 'ERROR'. This patch modifies the Novatel LTE plugin to retry $NWQMISTATUS (up to 5 times) to determine if the disconnect operation succeeds. It also changes the plugin to assume that the disconnect operation succeeds if $NWQMISTATUS fails to report the current connection status. --- plugins/novatel/mm-broadband-bearer-novatel-lte.c | 68 +-- 1 file changed, 51 insertions(+), 17 deletions(-) diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c b/plugins/novatel/mm-broadband-bearer-novatel-lte.c index 80c7464..4b67cd6 100644 --- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c +++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c @@ -325,6 +325,7 @@ typedef struct { MMAtSerialPort *secondary; MMPort *data; GSimpleAsyncResult *result; +gint retries; } DetailedDisconnectContext; static DetailedDisconnectContext * @@ -348,6 +349,7 @@ detailed_disconnect_context_new (MMBroadbandBearer *self, callback, user_data, detailed_disconnect_context_new); +ctx-retries = 4; return ctx; } @@ -373,34 +375,72 @@ disconnect_3gpp_finish (MMBroadbandBearer *self, return !g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error); } +static gboolean disconnect_3gpp_qmistatus (DetailedDisconnectContext *ctx); + static void -disconnect_3gpp_status_complete (MMBaseModem *modem, - GAsyncResult *res, - DetailedDisconnectContext *ctx) +disconnect_3gpp_status_ready (MMBaseModem *modem, + GAsyncResult *res, + DetailedDisconnectContext *ctx) { const gchar *result; GError *error = NULL; +gboolean is_connected = FALSE; result = mm_base_modem_at_command_finish (MM_BASE_MODEM (modem), res, error); -if (error) { -mm_dbg(QMI connection status failed: %s, error-message); +if (result) { +mm_dbg (QMI connection status: %s, result); +if (is_qmistatus_disconnected (result)) { +g_simple_async_result_set_op_res_gboolean (ctx-result, TRUE); +detailed_disconnect_context_complete_and_free (ctx); +return; +} else if (is_qmistatus_connected (result)) { +is_connected = TRUE; +} +} else { +mm_dbg (QMI connection status failed: %s, error-message); g_error_free (error); } -if (result is_qmistatus_disconnected (result)) -g_simple_async_result_set_op_res_gboolean (ctx-result, TRUE); -else +if (ctx-retries 0) { +ctx-retries--; +mm_dbg (Retrying status check in a second. %d retries left., +ctx-retries); +g_timeout_add_seconds (1, (GSourceFunc)disconnect_3gpp_qmistatus, ctx); +return; +} + +/* If $NWQMISTATUS reports a CONNECTED QMI state, returns an error such that + * the modem state remains 'connected'. Otherwise, assumes the modem is + * disconnected from the network successfully. */ +if (is_connected) g_simple_async_result_set_error (ctx-result, MM_CORE_ERROR, MM_CORE_ERROR_FAILED, - Error checking if disconnected (%s), - result ? result : no result); + QMI disconnect failed: %s, + result); +else +g_simple_async_result_set_op_res_gboolean (ctx-result, TRUE); detailed_disconnect_context_complete_and_free (ctx); } +static gboolean +disconnect_3gpp_qmistatus (DetailedDisconnectContext *ctx) +{ +mm_base_modem_at_command ( +ctx-modem, +$NWQMISTATUS, +3, /* timeout */ +FALSE, /* allow_cached */ +(GAsyncReadyCallback)disconnect_3gpp_status_ready, /* callback */ +ctx); /* user_data */ + +return FALSE; +} + + static void disconnect_3gpp_check_status (MMBaseModem *modem, GAsyncResult *res, @@ -416,13 +456,7 @@ disconnect_3gpp_check_status (MMBaseModem *modem, g_error_free (error); } -mm_base_modem_at_command ( -ctx-modem, -$NWQMISTATUS, -3, /* timeout */ -FALSE, /* allow_cached */ -(GAsyncReadyCallback)disconnect_3gpp_status_complete, -ctx); /* user_data */ +disconnect_3gpp_qmistatus (ctx); } static void -- 1.8.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] novatel-lte: use +CFUN=4 for power down
--- plugins/novatel/mm-broadband-modem-novatel-lte.c | 26 1 file changed, 26 insertions(+) diff --git a/plugins/novatel/mm-broadband-modem-novatel-lte.c b/plugins/novatel/mm-broadband-modem-novatel-lte.c index d32c397..9a21c4b 100644 --- a/plugins/novatel/mm-broadband-modem-novatel-lte.c +++ b/plugins/novatel/mm-broadband-modem-novatel-lte.c @@ -149,6 +149,30 @@ modem_init (MMIfaceModem *self, } /*/ +/* Modem power down (Modem interface) */ + +static gboolean +modem_power_down_finish (MMIfaceModem *self, + GAsyncResult *res, + GError **error) +{ +return !!mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, error); +} + +static void +modem_power_down (MMIfaceModem *self, + GAsyncReadyCallback callback, + gpointer user_data) +{ +mm_base_modem_at_command (MM_BASE_MODEM (self), + +CFUN=4, + 6, + FALSE, + callback, + user_data); +} + +/*/ /* Create Bearer (Modem interface) */ static MMBearer * @@ -745,6 +769,8 @@ iface_modem_init (MMIfaceModem *iface) { iface-modem_init = modem_init; iface-modem_init_finish = modem_init_finish; +iface-modem_power_down = modem_power_down; +iface-modem_power_down_finish = modem_power_down_finish; iface-create_bearer = modem_create_bearer; iface-create_bearer_finish = modem_create_bearer_finish; iface-create_sim = modem_create_sim; -- 1.8.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] iface-modem: add a missing step increment in interface_initialization_step
--- src/mm-iface-modem.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c index 10144aa..722664c 100644 --- a/src/mm-iface-modem.c +++ b/src/mm-iface-modem.c @@ -3979,6 +3979,8 @@ interface_initialization_step (InitializationContext *ctx) g_object_unref (sim); return; } +/* Fall down to next step */ +ctx-step++; case INITIALIZATION_STEP_OWN_NUMBERS: /* Own numbers is meant to be loaded only once during the whole -- 1.8.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] iface-modem: rearrange initialization steps
This patch rearranges the initialization steps in MMIfaceModem such that the following SIM related operations happen at the end of the initialization: - INITIALIZATION_STEP_UNLOCK_REQUIRED - INITIALIZATION_STEP_SIM - INITIALIZATION_STEP_OWN_NUMBERS The rationale of this change is that the SIM interface of some modems may require some time to initialize before it responds to SIM related AT commands. By rearranging the initialization steps to execute non-SIM related AT commands first, some of the latency for the SIM initialization can be absorbed. --- src/mm-iface-modem.c | 118 +-- 1 file changed, 59 insertions(+), 59 deletions(-) diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c index 6b812e2..3827a99 100644 --- a/src/mm-iface-modem.c +++ b/src/mm-iface-modem.c @@ -3401,12 +3401,12 @@ typedef enum { INITIALIZATION_STEP_REVISION, INITIALIZATION_STEP_EQUIPMENT_ID, INITIALIZATION_STEP_DEVICE_ID, -INITIALIZATION_STEP_UNLOCK_REQUIRED, -INITIALIZATION_STEP_SIM, -INITIALIZATION_STEP_OWN_NUMBERS, INITIALIZATION_STEP_SUPPORTED_MODES, INITIALIZATION_STEP_SUPPORTED_BANDS, INITIALIZATION_STEP_POWER_STATE, +INITIALIZATION_STEP_UNLOCK_REQUIRED, +INITIALIZATION_STEP_SIM, +INITIALIZATION_STEP_OWN_NUMBERS, INITIALIZATION_STEP_LAST } InitializationStep; @@ -3872,62 +3872,6 @@ interface_initialization_step (InitializationContext *ctx) /* Fall down to next step */ ctx-step++; -case INITIALIZATION_STEP_UNLOCK_REQUIRED: -/* Only check unlock required if we were previously not unlocked */ -if (mm_gdbus_modem_get_unlock_required (ctx-skeleton) != MM_MODEM_LOCK_NONE) { -mm_iface_modem_update_lock_info (ctx-self, - MM_MODEM_LOCK_UNKNOWN, /* ask */ - (GAsyncReadyCallback)modem_update_lock_info_ready, - ctx); -return; -} -/* Fall down to next step */ -ctx-step++; - -case INITIALIZATION_STEP_SIM: -/* If the modem doesn't need any SIM, skip */ -if (MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-create_sim -MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-create_sim_finish) { -MMSim *sim = NULL; - -g_object_get (ctx-self, - MM_IFACE_MODEM_SIM, sim, - NULL); -if (!sim) { -MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-create_sim ( -MM_IFACE_MODEM (ctx-self), -(GAsyncReadyCallback)sim_new_ready, -ctx); -return; -} - -/* If already available the sim object, relaunch initialization. - * This will try to load any missing property value that couldn't be - * retrieved before due to having the SIM locked. */ -mm_sim_initialize (sim, - NULL, /* TODO: cancellable */ - (GAsyncReadyCallback)sim_reinit_ready, - ctx); -g_object_unref (sim); -return; -} - -case INITIALIZATION_STEP_OWN_NUMBERS: -/* Own numbers is meant to be loaded only once during the whole - * lifetime of the modem. Therefore, if we already have them loaded, - * don't try to load them again. */ -if (mm_gdbus_modem_get_own_numbers (ctx-skeleton) == NULL -MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_own_numbers -MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_own_numbers_finish) { -MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_own_numbers ( -ctx-self, -(GAsyncReadyCallback)load_own_numbers_ready, -ctx); -return; -} -/* Fall down to next step */ -ctx-step++; - case INITIALIZATION_STEP_SUPPORTED_MODES: g_assert (MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_supported_modes != NULL); g_assert (MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_supported_modes_finish != NULL); @@ -3995,6 +3939,62 @@ interface_initialization_step (InitializationContext *ctx) /* Fall down to next step */ ctx-step++; +case INITIALIZATION_STEP_UNLOCK_REQUIRED: +/* Only check unlock required if we were previously not unlocked */ +if (mm_gdbus_modem_get_unlock_required (ctx-skeleton) != MM_MODEM_LOCK_NONE) { +mm_iface_modem_update_lock_info (ctx-self, + MM_MODEM_LOCK_UNKNOWN, /* ask */ + (GAsyncReadyCallback)modem_update_lock_info_ready, + ctx); +return; +} +/* Fall down to next step */ +ctx-step++; + +case
[MM] [PATCH] iface-modem: fix invalid modem state consolidation
This patch fixes get_current_consolidated_state() in MMIfaceModem to avoid invalid state transitions, e.g. from 'enabling' to 'disabled'. https://code.google.com/p/chromium-os/issues/detail?id=38173 --- src/mm-iface-modem.c | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c index 6b812e2..f6af878 100644 --- a/src/mm-iface-modem.c +++ b/src/mm-iface-modem.c @@ -205,7 +205,7 @@ mm_iface_modem_wait_for_final_state (MMIfaceModem *self, /*/ -static MMModemState get_current_consolidated_state (MMIfaceModem *self); +static MMModemState get_current_consolidated_state (MMIfaceModem *self, MMModemState modem_state); typedef struct { MMBearer *self; @@ -270,7 +270,7 @@ bearer_status_changed (MMBearer *bearer, new_state = MM_MODEM_STATE_DISCONNECTING; break; case MM_BEARER_STATUS_DISCONNECTED: -new_state = get_current_consolidated_state (self); +new_state = get_current_consolidated_state (self, state); break; } @@ -1184,7 +1184,7 @@ mm_iface_modem_update_state (MMIfaceModem *self, /* Enabled may really be searching or registered */ if (new_state == MM_MODEM_STATE_ENABLED) -new_state = get_current_consolidated_state (self); +new_state = get_current_consolidated_state (self, new_state); /* Update state only if different */ if (new_state != old_state) { @@ -1260,9 +1260,9 @@ subsystem_state_array_free (GArray *array) } static MMModemState -get_current_consolidated_state (MMIfaceModem *self) +get_current_consolidated_state (MMIfaceModem *self, MMModemState modem_state) { -MMModemState consolidated = MM_MODEM_STATE_DISABLED; +MMModemState consolidated = modem_state; GArray *subsystem_states; if (G_UNLIKELY (!state_update_context_quark)) @@ -1293,6 +1293,7 @@ get_current_consolidated_state (MMIfaceModem *self) static MMModemState get_updated_consolidated_state (MMIfaceModem *self, +MMModemState modem_state, const gchar *subsystem, MMModemState subsystem_state) { @@ -1345,7 +1346,7 @@ get_updated_consolidated_state (MMIfaceModem *self, g_array_append_val (subsystem_states, s); } -return get_current_consolidated_state (self); +return get_current_consolidated_state (self, modem_state); } void @@ -1364,7 +1365,7 @@ mm_iface_modem_update_subsystem_state (MMIfaceModem *self, /* We may have different subsystems being handled (e.g. 3GPP and CDMA), and * the registration status value is unique, so if we get subsystem-specific * state updates, we'll need to merge all to get a consolidated one. */ -consolidated = get_updated_consolidated_state (self, subsystem, new_state); +consolidated = get_updated_consolidated_state (self, state, subsystem, new_state); /* Don't update registration-related states while disabling/enabling */ if (state == MM_MODEM_STATE_ENABLING || -- 1.8.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] iface-modem, iface-modem-simple: log time taken for succeeded operations
This patch logs the time taken for succeeded initialization, enabling, disabling, connection, and disconnection operations, which helps evaluate how to optimize modem plugins. --- libmm-glib/mm-common-helpers.c |7 +++ libmm-glib/mm-common-helpers.h |2 ++ src/mm-iface-modem-simple.c| 18 +- src/mm-iface-modem.c | 24 4 files changed, 50 insertions(+), 1 deletions(-) diff --git a/libmm-glib/mm-common-helpers.c b/libmm-glib/mm-common-helpers.c index 3f100b6..406a99a 100644 --- a/libmm-glib/mm-common-helpers.c +++ b/libmm-glib/mm-common-helpers.c @@ -960,3 +960,10 @@ mm_utils_check_for_single_value (guint32 value) return TRUE; } + +gdouble +mm_utils_subtract_time (const GTimeVal *end_time, const GTimeVal *start_time) +{ +return (end_time-tv_sec - start_time-tv_sec) + + (end_time-tv_usec - start_time-tv_usec) / 1.0e6; +} diff --git a/libmm-glib/mm-common-helpers.h b/libmm-glib/mm-common-helpers.h index 6155b58..42bbe20 100644 --- a/libmm-glib/mm-common-helpers.h +++ b/libmm-glib/mm-common-helpers.h @@ -102,4 +102,6 @@ gchar*mm_utils_bin2hexstr (const guint8 *bin, gsize len); gboolean mm_utils_check_for_single_value (guint32 value); +gdouble mm_utils_subtract_time (const GTimeVal *end_time, const GTimeVal *start_time); + #endif /* MM_COMMON_HELPERS_H */ diff --git a/src/mm-iface-modem-simple.c b/src/mm-iface-modem-simple.c index 12aa152..b0bcf46 100644 --- a/src/mm-iface-modem-simple.c +++ b/src/mm-iface-modem-simple.c @@ -196,6 +196,8 @@ typedef struct { /* Results to set */ MMBearer *bearer; + +GTimeVal start_time; } ConnectionContext; static void @@ -203,7 +205,6 @@ connection_context_free (ConnectionContext *ctx) { g_assert (ctx-state_changed_id == 0); g_assert (ctx-state_changed_wait_id == 0); - g_variant_unref (ctx-dictionary); if (ctx-properties) g_object_unref (ctx-properties); @@ -516,6 +517,8 @@ bearer_list_find_disconnected (MMBearer *bearer, static void connection_step (ConnectionContext *ctx) { +GTimeVal end_time; + switch (ctx-step) { case CONNECTION_STEP_FIRST: /* Fall down to next step */ @@ -738,6 +741,9 @@ connection_step (ConnectionContext *ctx) case CONNECTION_STEP_LAST: mm_info (Simple connect state (%d/%d): All done, ctx-step, CONNECTION_STEP_LAST); +g_get_current_time (end_time); +mm_info (Connection steps took %.3f second(s)., + mm_utils_subtract_time (end_time, ctx-start_time)); /* All done, yey! */ mm_gdbus_modem_simple_complete_connect ( ctx-skeleton, @@ -828,6 +834,7 @@ handle_connect (MmGdbusModemSimple *skeleton, ctx-invocation = g_object_ref (invocation); ctx-self = g_object_ref (self); ctx-dictionary = g_variant_ref (dictionary); +g_get_current_time (ctx-start_time); mm_base_modem_authorize (MM_BASE_MODEM (self), invocation, @@ -846,6 +853,7 @@ typedef struct { gchar *bearer_path; GList *bearers; MMBearer *current; +GTimeVal start_time; } DisconnectionContext; static void @@ -887,6 +895,12 @@ disconnect_next_bearer (DisconnectionContext *ctx) /* No more bearers? all done! */ if (!ctx-bearers) { +GTimeVal end_time; + +g_get_current_time (end_time); +mm_info (Disconnection steps took %.3f second(s)., + mm_utils_subtract_time (end_time, ctx-start_time)); + mm_gdbus_modem_simple_complete_disconnect (ctx-skeleton, ctx-invocation); disconnection_context_free (ctx); @@ -977,6 +991,8 @@ handle_disconnect (MmGdbusModemSimple *skeleton, ctx-bearer_path = g_strdup (ctx-bearer_path); } +g_get_current_time (ctx-start_time); + mm_base_modem_authorize (MM_BASE_MODEM (self), invocation, MM_AUTHORIZATION_DEVICE_CONTROL, diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c index 6b812e2..2edbcf4 100644 --- a/src/mm-iface-modem.c +++ b/src/mm-iface-modem.c @@ -2911,6 +2911,7 @@ struct _DisablingContext { MMModemState previous_state; GSimpleAsyncResult *result; MmGdbusModem *skeleton; +GTimeVal start_time; }; static void @@ -2935,6 +2936,8 @@ mm_iface_modem_disable_finish (MMIfaceModem *self, static void interface_disabling_step (DisablingContext *ctx) { +GTimeVal end_time; + switch (ctx-step) { case DISABLING_STEP_FIRST: /* Fall down to next step */ @@ -2954,6 +2957,10 @@ interface_disabling_step (DisablingContext *ctx) ctx-step++; case DISABLING_STEP_LAST: +g_get_current_time (end_time); +mm_info (Disabling steps took %.3f second(s)., + mm_utils_subtract_time (end_time, ctx-start_time)); + /* We are done without errors! */
Re: [MM] [PATCH] iface-modem,iface-modem-simple: log time taken for succeeded operations
There may be better ways to collect this kind of performance data than outputting them to ModemManager log. Is there a commonly used method for ModemManager to report performance metrics in a way that other tools can collect and analyze them? - Ben On Mon, Jan 21, 2013 at 8:30 PM, Ben Chan benc...@chromium.org wrote: This patch logs the time taken for succeeded initialization, enabling, disabling, connection, and disconnection operations, which helps evaluate how to optimize modem plugins. --- libmm-glib/mm-common-helpers.c |7 +++ libmm-glib/mm-common-helpers.h |2 ++ src/mm-iface-modem-simple.c| 18 +- src/mm-iface-modem.c | 24 4 files changed, 50 insertions(+), 1 deletions(-) diff --git a/libmm-glib/mm-common-helpers.c b/libmm-glib/mm-common-helpers.c index 3f100b6..406a99a 100644 --- a/libmm-glib/mm-common-helpers.c +++ b/libmm-glib/mm-common-helpers.c @@ -960,3 +960,10 @@ mm_utils_check_for_single_value (guint32 value) return TRUE; } + +gdouble +mm_utils_subtract_time (const GTimeVal *end_time, const GTimeVal *start_time) +{ +return (end_time-tv_sec - start_time-tv_sec) + + (end_time-tv_usec - start_time-tv_usec) / 1.0e6; +} diff --git a/libmm-glib/mm-common-helpers.h b/libmm-glib/mm-common-helpers.h index 6155b58..42bbe20 100644 --- a/libmm-glib/mm-common-helpers.h +++ b/libmm-glib/mm-common-helpers.h @@ -102,4 +102,6 @@ gchar*mm_utils_bin2hexstr (const guint8 *bin, gsize len); gboolean mm_utils_check_for_single_value (guint32 value); +gdouble mm_utils_subtract_time (const GTimeVal *end_time, const GTimeVal *start_time); + #endif /* MM_COMMON_HELPERS_H */ diff --git a/src/mm-iface-modem-simple.c b/src/mm-iface-modem-simple.c index 12aa152..b0bcf46 100644 --- a/src/mm-iface-modem-simple.c +++ b/src/mm-iface-modem-simple.c @@ -196,6 +196,8 @@ typedef struct { /* Results to set */ MMBearer *bearer; + +GTimeVal start_time; } ConnectionContext; static void @@ -203,7 +205,6 @@ connection_context_free (ConnectionContext *ctx) { g_assert (ctx-state_changed_id == 0); g_assert (ctx-state_changed_wait_id == 0); - g_variant_unref (ctx-dictionary); if (ctx-properties) g_object_unref (ctx-properties); @@ -516,6 +517,8 @@ bearer_list_find_disconnected (MMBearer *bearer, static void connection_step (ConnectionContext *ctx) { +GTimeVal end_time; + switch (ctx-step) { case CONNECTION_STEP_FIRST: /* Fall down to next step */ @@ -738,6 +741,9 @@ connection_step (ConnectionContext *ctx) case CONNECTION_STEP_LAST: mm_info (Simple connect state (%d/%d): All done, ctx-step, CONNECTION_STEP_LAST); +g_get_current_time (end_time); +mm_info (Connection steps took %.3f second(s)., + mm_utils_subtract_time (end_time, ctx-start_time)); /* All done, yey! */ mm_gdbus_modem_simple_complete_connect ( ctx-skeleton, @@ -828,6 +834,7 @@ handle_connect (MmGdbusModemSimple *skeleton, ctx-invocation = g_object_ref (invocation); ctx-self = g_object_ref (self); ctx-dictionary = g_variant_ref (dictionary); +g_get_current_time (ctx-start_time); mm_base_modem_authorize (MM_BASE_MODEM (self), invocation, @@ -846,6 +853,7 @@ typedef struct { gchar *bearer_path; GList *bearers; MMBearer *current; +GTimeVal start_time; } DisconnectionContext; static void @@ -887,6 +895,12 @@ disconnect_next_bearer (DisconnectionContext *ctx) /* No more bearers? all done! */ if (!ctx-bearers) { +GTimeVal end_time; + +g_get_current_time (end_time); +mm_info (Disconnection steps took %.3f second(s)., + mm_utils_subtract_time (end_time, ctx-start_time)); + mm_gdbus_modem_simple_complete_disconnect (ctx-skeleton, ctx-invocation); disconnection_context_free (ctx); @@ -977,6 +991,8 @@ handle_disconnect (MmGdbusModemSimple *skeleton, ctx-bearer_path = g_strdup (ctx-bearer_path); } +g_get_current_time (ctx-start_time); + mm_base_modem_authorize (MM_BASE_MODEM (self), invocation, MM_AUTHORIZATION_DEVICE_CONTROL, diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c index 6b812e2..2edbcf4 100644 --- a/src/mm-iface-modem.c +++ b/src/mm-iface-modem.c @@ -2911,6 +2911,7 @@ struct _DisablingContext { MMModemState previous_state; GSimpleAsyncResult *result; MmGdbusModem *skeleton; +GTimeVal start_time; }; static void @@ -2935,6 +2936,8 @@ mm_iface_modem_disable_finish (MMIfaceModem *self, static void interface_disabling_step (DisablingContext *ctx) { +GTimeVal end_time
[MM] [PATCH v2] modem: log time taken for succeeded operations
This patch logs the time taken for succeeded initialization, enabling, disabling, connection, and disconnection operations, which helps evaluate how to optimize modem plugins. --- libmm-glib/mm-common-helpers.c |7 +++ libmm-glib/mm-common-helpers.h |2 ++ src/mm-broadband-modem.c | 22 ++ src/mm-iface-modem-simple.c| 17 + src/mm-iface-modem.c | 24 5 files changed, 72 insertions(+), 0 deletions(-) diff --git a/libmm-glib/mm-common-helpers.c b/libmm-glib/mm-common-helpers.c index 3f100b6..406a99a 100644 --- a/libmm-glib/mm-common-helpers.c +++ b/libmm-glib/mm-common-helpers.c @@ -960,3 +960,10 @@ mm_utils_check_for_single_value (guint32 value) return TRUE; } + +gdouble +mm_utils_subtract_time (const GTimeVal *end_time, const GTimeVal *start_time) +{ +return (end_time-tv_sec - start_time-tv_sec) + + (end_time-tv_usec - start_time-tv_usec) / 1.0e6; +} diff --git a/libmm-glib/mm-common-helpers.h b/libmm-glib/mm-common-helpers.h index 6155b58..42bbe20 100644 --- a/libmm-glib/mm-common-helpers.h +++ b/libmm-glib/mm-common-helpers.h @@ -102,4 +102,6 @@ gchar*mm_utils_bin2hexstr (const guint8 *bin, gsize len); gboolean mm_utils_check_for_single_value (guint32 value); +gdouble mm_utils_subtract_time (const GTimeVal *end_time, const GTimeVal *start_time); + #endif /* MM_COMMON_HELPERS_H */ diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c index 0a7af2c..135e725 100644 --- a/src/mm-broadband-modem.c +++ b/src/mm-broadband-modem.c @@ -7460,6 +7460,7 @@ typedef struct { DisablingStep step; MMModemState previous_state; gboolean disabled; +GTimeVal start_time; } DisablingContext; static void disabling_step (DisablingContext *ctx); @@ -7618,6 +7619,8 @@ disabling_wait_for_final_state_ready (MMIfaceModem *self, static void disabling_step (DisablingContext *ctx) { +GTimeVal end_time; + /* Don't run new steps if we're cancelled */ if (disabling_context_complete_and_free_if_cancelled (ctx)) return; @@ -7746,6 +7749,9 @@ disabling_step (DisablingContext *ctx) case DISABLING_STEP_LAST: mm_info (Modem fully disabled...); ctx-disabled = TRUE; +g_get_current_time (end_time); +mm_info (Modem disabling steps took %.3f second(s)., + mm_utils_subtract_time (end_time, ctx-start_time)); /* All disabled without errors! */ g_simple_async_result_set_op_res_gboolean (G_SIMPLE_ASYNC_RESULT (ctx-result), TRUE); disabling_context_complete_and_free (ctx); @@ -7768,6 +7774,7 @@ disable (MMBaseModem *self, ctx-result = g_simple_async_result_new (G_OBJECT (self), callback, user_data, disable); ctx-cancellable = (cancellable ? g_object_ref (cancellable) : NULL); ctx-step = DISABLING_STEP_FIRST; +g_get_current_time (ctx-start_time); disabling_step (ctx); } @@ -7798,6 +7805,7 @@ typedef struct { EnablingStep step; MMModemState previous_state; gboolean enabled; +GTimeVal start_time; } EnablingContext; static void enabling_step (EnablingContext *ctx); @@ -7940,6 +7948,8 @@ enabling_wait_for_final_state_ready (MMIfaceModem *self, static void enabling_step (EnablingContext *ctx) { +GTimeVal end_time; + /* Don't run new steps if we're cancelled */ if (enabling_context_complete_and_free_if_cancelled (ctx)) return; @@ -8068,6 +8078,9 @@ enabling_step (EnablingContext *ctx) case ENABLING_STEP_LAST: mm_info (Modem fully enabled...); ctx-enabled = TRUE; +g_get_current_time (end_time); +mm_info (Modem enabling steps took %.3f second(s)., + mm_utils_subtract_time (end_time, ctx-start_time)); /* All enabled without errors! */ g_simple_async_result_set_op_res_gboolean (G_SIMPLE_ASYNC_RESULT (ctx-result), TRUE); enabling_context_complete_and_free (ctx); @@ -8118,6 +8131,7 @@ enable (MMBaseModem *self, ctx-result = result; ctx-cancellable = g_object_ref (cancellable); ctx-step = ENABLING_STEP_FIRST; +g_get_current_time (ctx-start_time); enabling_step (ctx); return; } @@ -8179,6 +8193,7 @@ typedef struct { GSimpleAsyncResult *result; InitializeStep step; gpointer ports_ctx; +GTimeVal start_time; } InitializeContext; static void initialize_step (InitializeContext *ctx); @@ -8356,6 +8371,8 @@ INTERFACE_INIT_READY_FN (iface_modem_firmware, MM_IFACE_MODEM_FIRMWARE, FALSE) static void initialize_step (InitializeContext *ctx) { +GTimeVal end_time; + /* Don't run new steps if we're cancelled */ if (initialize_context_complete_and_free_if_cancelled (ctx)) return; @@ -8523,6 +8540,10 @@ initialize_step (InitializeContext *ctx) mm_info (Modem fully initialized); +g_get_current_time (end_time); +mm_info
Re: [MM] [PATCH] iface-modem,iface-modem-simple: log time taken for succeeded operations
Submitted a revised patch. On Mon, Jan 21, 2013 at 8:39 PM, Ben Chan benc...@chromium.org wrote: There may be better ways to collect this kind of performance data than outputting them to ModemManager log. Is there a commonly used method for ModemManager to report performance metrics in a way that other tools can collect and analyze them? - Ben On Mon, Jan 21, 2013 at 8:30 PM, Ben Chan benc...@chromium.org wrote: This patch logs the time taken for succeeded initialization, enabling, disabling, connection, and disconnection operations, which helps evaluate how to optimize modem plugins. --- libmm-glib/mm-common-helpers.c |7 +++ libmm-glib/mm-common-helpers.h |2 ++ src/mm-iface-modem-simple.c| 18 +- src/mm-iface-modem.c | 24 4 files changed, 50 insertions(+), 1 deletions(-) diff --git a/libmm-glib/mm-common-helpers.c b/libmm-glib/mm-common-helpers.c index 3f100b6..406a99a 100644 --- a/libmm-glib/mm-common-helpers.c +++ b/libmm-glib/mm-common-helpers.c @@ -960,3 +960,10 @@ mm_utils_check_for_single_value (guint32 value) return TRUE; } + +gdouble +mm_utils_subtract_time (const GTimeVal *end_time, const GTimeVal *start_time) +{ +return (end_time-tv_sec - start_time-tv_sec) + + (end_time-tv_usec - start_time-tv_usec) / 1.0e6; +} diff --git a/libmm-glib/mm-common-helpers.h b/libmm-glib/mm-common-helpers.h index 6155b58..42bbe20 100644 --- a/libmm-glib/mm-common-helpers.h +++ b/libmm-glib/mm-common-helpers.h @@ -102,4 +102,6 @@ gchar*mm_utils_bin2hexstr (const guint8 *bin, gsize len); gboolean mm_utils_check_for_single_value (guint32 value); +gdouble mm_utils_subtract_time (const GTimeVal *end_time, const GTimeVal *start_time); + #endif /* MM_COMMON_HELPERS_H */ diff --git a/src/mm-iface-modem-simple.c b/src/mm-iface-modem-simple.c index 12aa152..b0bcf46 100644 --- a/src/mm-iface-modem-simple.c +++ b/src/mm-iface-modem-simple.c @@ -196,6 +196,8 @@ typedef struct { /* Results to set */ MMBearer *bearer; + +GTimeVal start_time; } ConnectionContext; static void @@ -203,7 +205,6 @@ connection_context_free (ConnectionContext *ctx) { g_assert (ctx-state_changed_id == 0); g_assert (ctx-state_changed_wait_id == 0); - g_variant_unref (ctx-dictionary); if (ctx-properties) g_object_unref (ctx-properties); @@ -516,6 +517,8 @@ bearer_list_find_disconnected (MMBearer *bearer, static void connection_step (ConnectionContext *ctx) { +GTimeVal end_time; + switch (ctx-step) { case CONNECTION_STEP_FIRST: /* Fall down to next step */ @@ -738,6 +741,9 @@ connection_step (ConnectionContext *ctx) case CONNECTION_STEP_LAST: mm_info (Simple connect state (%d/%d): All done, ctx-step, CONNECTION_STEP_LAST); +g_get_current_time (end_time); +mm_info (Connection steps took %.3f second(s)., + mm_utils_subtract_time (end_time, ctx-start_time)); /* All done, yey! */ mm_gdbus_modem_simple_complete_connect ( ctx-skeleton, @@ -828,6 +834,7 @@ handle_connect (MmGdbusModemSimple *skeleton, ctx-invocation = g_object_ref (invocation); ctx-self = g_object_ref (self); ctx-dictionary = g_variant_ref (dictionary); +g_get_current_time (ctx-start_time); mm_base_modem_authorize (MM_BASE_MODEM (self), invocation, @@ -846,6 +853,7 @@ typedef struct { gchar *bearer_path; GList *bearers; MMBearer *current; +GTimeVal start_time; } DisconnectionContext; static void @@ -887,6 +895,12 @@ disconnect_next_bearer (DisconnectionContext *ctx) /* No more bearers? all done! */ if (!ctx-bearers) { +GTimeVal end_time; + +g_get_current_time (end_time); +mm_info (Disconnection steps took %.3f second(s)., + mm_utils_subtract_time (end_time, ctx-start_time)); + mm_gdbus_modem_simple_complete_disconnect (ctx-skeleton, ctx-invocation); disconnection_context_free (ctx); @@ -977,6 +991,8 @@ handle_disconnect (MmGdbusModemSimple *skeleton, ctx-bearer_path = g_strdup (ctx-bearer_path); } +g_get_current_time (ctx-start_time); + mm_base_modem_authorize (MM_BASE_MODEM (self), invocation, MM_AUTHORIZATION_DEVICE_CONTROL, diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c index 6b812e2..2edbcf4 100644 --- a/src/mm-iface-modem.c +++ b/src/mm-iface-modem.c @@ -2911,6 +2911,7 @@ struct _DisablingContext { MMModemState previous_state; GSimpleAsyncResult *result; MmGdbusModem *skeleton; +GTimeVal start_time; }; static void @@ -2935,6 +2936,8 @@ mm_iface_modem_disable_finish (MMIfaceModem
[MM] [PATCH] iface-modem: schedule signal quality check more often initially
This patch modifies MMIfaceModem to schedule the periodic signal quality check with a period of 3s instead of 30s (up to 5 periods) initially until a non-zero signal quality value is obtained and then switch back to the 30s period. --- src/mm-iface-modem.c | 36 +--- 1 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c index be4b70d..edb9b7d 100644 --- a/src/mm-iface-modem.c +++ b/src/mm-iface-modem.c @@ -27,9 +27,10 @@ #include mm-log.h #include mm-context.h -#define SIGNAL_QUALITY_RECENT_TIMEOUT_SEC 60 -#define SIGNAL_QUALITY_CHECK_TIMEOUT_SEC 30 -#define ACCESS_TECHNOLOGIES_CHECK_TIMEOUT_SEC 30 +#define SIGNAL_QUALITY_RECENT_TIMEOUT_SEC60 +#define SIGNAL_QUALITY_INITIAL_CHECK_TIMEOUT_SEC 3 +#define SIGNAL_QUALITY_CHECK_TIMEOUT_SEC 30 +#define ACCESS_TECHNOLOGIES_CHECK_TIMEOUT_SEC30 #define STATE_UPDATE_CONTEXT_TAG state-update-context-tag #define SIGNAL_QUALITY_UPDATE_CONTEXT_TAG signal-quality-update-context-tag @@ -996,6 +997,8 @@ mm_iface_modem_update_signal_quality (MMIfaceModem *self, /*/ typedef struct { +guint interval; +guint initial_retries; guint timeout_source; gboolean running; } SignalQualityCheckContext; @@ -1008,6 +1011,8 @@ signal_quality_check_context_free (SignalQualityCheckContext *ctx) g_free (ctx); } +static gboolean periodic_signal_quality_check (MMIfaceModem *self); + static void signal_quality_check_ready (MMIfaceModem *self, GAsyncResult *res) @@ -1029,8 +1034,20 @@ signal_quality_check_ready (MMIfaceModem *self, * mm_iface_modem_shutdown when this function is invoked as a callback of * load_signal_quality. */ ctx = g_object_get_qdata (G_OBJECT (self), signal_quality_check_context_quark); -if (ctx) +if (ctx) { +if (ctx-interval == SIGNAL_QUALITY_INITIAL_CHECK_TIMEOUT_SEC +(signal_quality != 0 || --ctx-initial_retries == 0)) { +ctx-interval = SIGNAL_QUALITY_CHECK_TIMEOUT_SEC; +if (ctx-timeout_source) { +mm_dbg (Periodic signal quality checks rescheduled (interval = %ds), ctx-interval); +g_source_remove(ctx-timeout_source); +ctx-timeout_source = g_timeout_add_seconds (ctx-interval, + (GSourceFunc)periodic_signal_quality_check, + self); +} +} ctx-running = FALSE; +} } static gboolean @@ -1043,7 +1060,7 @@ periodic_signal_quality_check (MMIfaceModem *self) /* Only launch a new one if not one running already OR if the last one run * was more than 15s ago. */ if (!ctx-running || -(time (NULL) - get_last_signal_quality_update_time (self) 15)) { +(time (NULL) - get_last_signal_quality_update_time (self) (ctx-interval / 2))) { ctx-running = TRUE; MM_IFACE_MODEM_GET_INTERFACE (self)-load_signal_quality ( self, @@ -1097,9 +1114,14 @@ periodic_signal_quality_check_enable (MMIfaceModem *self) } /* Create context and keep it as object data */ -mm_dbg (Periodic signal quality checks enabled); ctx = g_new0 (SignalQualityCheckContext, 1); -ctx-timeout_source = g_timeout_add_seconds (SIGNAL_QUALITY_CHECK_TIMEOUT_SEC, +/* Schedule the signal quality check using a shorter period, up to 5 + * periods, initially until a non-zero signal quality value is obtained + * and then switch back to the normal period. */ +ctx-interval = SIGNAL_QUALITY_INITIAL_CHECK_TIMEOUT_SEC; +ctx-initial_retries = 5; +mm_dbg (Periodic signal quality checks enabled (interval = %ds), ctx-interval); +ctx-timeout_source = g_timeout_add_seconds (ctx-interval, (GSourceFunc)periodic_signal_quality_check, self); g_object_set_qdata_full (G_OBJECT (self), -- 1.7.7.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH 1/2] core: add 'hotplugged' flag to indicate if modem is newly plugged in
This patch adds a 'hotplugged' flag to MMBaseModem to indicate if a modem is newly plugged in. A plugin can use this information to determine if, for example, the modem needs to be soft reset using the ATZ command. Dan Williams d...@redhat.com contributed the idea of implementation. --- src/mm-base-modem.c | 18 ++ src/mm-base-modem.h |4 src/mm-device.c | 21 + src/mm-device.h |4 +++- src/mm-manager.c| 15 --- src/mm-plugin.c |2 ++ 6 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/mm-base-modem.c b/src/mm-base-modem.c index c66ec53..86ff812 100644 --- a/src/mm-base-modem.c +++ b/src/mm-base-modem.c @@ -67,6 +67,7 @@ struct _MMBaseModemPrivate { guint vendor_id; guint product_id; +gboolean hotplugged; gboolean valid; guint max_timeouts; @@ -411,6 +412,23 @@ mm_base_modem_initialize (MMBaseModem *self, } void +mm_base_modem_set_hotplugged (MMBaseModem *self, + gboolean hotplugged) +{ +g_return_if_fail (MM_IS_BASE_MODEM (self)); + +self-priv-hotplugged = hotplugged; +} + +gboolean +mm_base_modem_get_hotplugged (MMBaseModem *self) +{ +g_return_val_if_fail (MM_IS_BASE_MODEM (self), FALSE); + +return self-priv-hotplugged; +} + +void mm_base_modem_set_valid (MMBaseModem *self, gboolean new_valid) { diff --git a/src/mm-base-modem.h b/src/mm-base-modem.h index bdacf86..9d4e3a1 100644 --- a/src/mm-base-modem.h +++ b/src/mm-base-modem.h @@ -143,6 +143,10 @@ MMAtSerialPort *mm_base_modem_get_best_at_port (MMBaseModem *self, GError MMPort *mm_base_modem_get_best_data_port(MMBaseModem *self); GList*mm_base_modem_get_data_ports(MMBaseModem *self); +void mm_base_modem_set_hotplugged (MMBaseModem *self, + gboolean hotplugged); +gboolean mm_base_modem_get_hotplugged (MMBaseModem *self); + void mm_base_modem_set_valid(MMBaseModem *self, gboolean valid); gboolean mm_base_modem_get_valid(MMBaseModem *self); diff --git a/src/mm-device.c b/src/mm-device.c index e8330d6..a37b26d 100644 --- a/src/mm-device.c +++ b/src/mm-device.c @@ -68,6 +68,9 @@ struct _MMDevicePrivate { /* When exported, a reference to the object manager */ GDBusObjectManagerServer *object_manager; + +/* Whether the device was hot-plugged. */ +gboolean hotplugged; }; /*/ @@ -593,14 +596,24 @@ mm_device_get_port_probe_list (MMDevice *self) return copy; } +gboolean +mm_device_get_hotplugged (MMDevice *self) +{ +return self-priv-hotplugged; +} + /*/ MMDevice * -mm_device_new (GUdevDevice *udev_device) +mm_device_new (GUdevDevice *udev_device, gboolean hotplugged) { -return MM_DEVICE (g_object_new (MM_TYPE_DEVICE, -MM_DEVICE_UDEV_DEVICE, udev_device, -NULL)); +MMDevice *device; + +device = MM_DEVICE (g_object_new (MM_TYPE_DEVICE, + MM_DEVICE_UDEV_DEVICE, udev_device, + NULL)); +device-priv-hotplugged = hotplugged; +return device; } static void diff --git a/src/mm-device.h b/src/mm-device.h index 4849905..e7065ed 100644 --- a/src/mm-device.h +++ b/src/mm-device.h @@ -58,7 +58,7 @@ struct _MMDeviceClass { GType mm_device_get_type (void); -MMDevice *mm_device_new (GUdevDevice *udev_device); +MMDevice *mm_device_new (GUdevDevice *udev_device, gboolean hotplugged); void mm_device_grab_port(MMDevice*self, GUdevDevice *udev_port); @@ -96,4 +96,6 @@ GList *mm_device_get_port_probe_list (MMDevice *self); const gchar *mm_device_utils_get_port_driver (GUdevDevice *udev_port); +gbooleanmm_device_get_hotplugged (MMDevice *self); + #endif /* MM_DEVICE_H */ diff --git a/src/mm-manager.c b/src/mm-manager.c index c5185b3..688348f 100644 --- a/src/mm-manager.c +++ b/src/mm-manager.c @@ -216,7 +216,8 @@ find_physical_device (GUdevDevice *child) static void device_added (MMManager *manager, - GUdevDevice *port) + GUdevDevice *port, + gboolean hotplugged) { MMDevice *device; const char *subsys, *name, *physdev_path, *physdev_subsys; @@ -292,7 +293,7 @@ device_added (MMManager *manager, FindDeviceSupportContext *ctx; /* Keep the device listed in the Manager */ -device = mm_device_new (physdev); +device = mm_device_new (physdev, hotplugged); g_hash_table_insert (manager-priv-devices, g_strdup (physdev_path), device); @@ -394,7 +395,7 @@ handle_uevent
[MM] [PATCH 2/2] novatel-lte: skip soft reset if modem is newly plugged in
Soft resetting a Novatel LTE modem can a significant amount of time (3-4 seconds). If the modem is newly plugged in, skip the unnecessary soft reset when enabling the modem. --- plugins/novatel/mm-broadband-modem-novatel-lte.c | 108 ++ 1 files changed, 108 insertions(+), 0 deletions(-) diff --git a/plugins/novatel/mm-broadband-modem-novatel-lte.c b/plugins/novatel/mm-broadband-modem-novatel-lte.c index 08954b5..d32c397 100644 --- a/plugins/novatel/mm-broadband-modem-novatel-lte.c +++ b/plugins/novatel/mm-broadband-modem-novatel-lte.c @@ -43,6 +43,112 @@ G_DEFINE_TYPE_EXTENDED (MMBroadbandModemNovatelLte, mm_broadband_modem_novatel_l G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM_3GPP, iface_modem_3gpp_init)); /*/ +/* Initializing the modem (Modem interface) */ + +static gboolean +modem_init_finish (MMIfaceModem *self, + GAsyncResult *res, + GError **error) +{ +return !g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error); +} + +static void +modem_init_sequence_ready (MMBaseModem *self, + GAsyncResult *res, + GSimpleAsyncResult *simple) +{ +GError *error = NULL; + +mm_base_modem_at_sequence_full_finish (MM_BASE_MODEM (self), res, NULL, error); +if (error) +g_simple_async_result_take_error (simple, error); +else { +MMAtSerialPort *secondary; + +/* Disable echo in secondary port as well, if any */ +secondary = mm_base_modem_peek_port_secondary (MM_BASE_MODEM (self)); +if (secondary) +/* No need to wait for the reply */ +mm_base_modem_at_command_full (MM_BASE_MODEM (self), + secondary, + E0, + 3, + FALSE, + FALSE, /* raw */ + NULL, /* cancellable */ + NULL, + NULL); + +g_simple_async_result_set_op_res_gboolean (simple, TRUE); +} + +g_simple_async_result_complete (simple); +g_object_unref (simple); +} + +static const MMBaseModemAtCommand modem_init_sequence[] = { +/* Init command. ITU rec v.250 (6.1.1) says: + * The DTE should not include additional commands on the same command line + * after the Z command because such commands may be ignored. + * So run ATZ alone. + */ +{ Z, 6, FALSE, mm_base_modem_response_processor_no_result_continue }, + +/* Ensure echo is off after the init command */ +{ E0 V1, 3, FALSE, NULL }, + +/* Some phones (like Blackberries) don't support +CMEE=1, so make it + * optional. It completely violates 3GPP TS 27.007 (9.1) but what can we do... + */ +{ +CMEE=1, 3, FALSE, NULL }, + +/* Additional OPTIONAL initialization */ +{ X4 C1, 3, FALSE, NULL }, + +{ NULL } +}; + +static void +modem_init (MMIfaceModem *self, +GAsyncReadyCallback callback, +gpointer user_data) +{ +MMAtSerialPort *primary; +GSimpleAsyncResult *result; +guint init_sequence_index; + +result = g_simple_async_result_new (G_OBJECT (self), +callback, +user_data, +modem_init); + +primary = mm_base_modem_peek_port_primary (MM_BASE_MODEM (self)); +if (!primary) { +g_simple_async_result_set_error ( +result, +MM_CORE_ERROR, +MM_CORE_ERROR_FAILED, +Need primary AT port to run modem init sequence); +g_simple_async_result_complete_in_idle (result); +g_object_unref (result); +return; +} + +/* Skip ATZ if the device was hotplugged. */ +init_sequence_index = mm_base_modem_get_hotplugged (MM_BASE_MODEM (self)) ? 1 : 0; + +mm_base_modem_at_sequence_full (MM_BASE_MODEM (self), +primary, +modem_init_sequence[init_sequence_index], +NULL, /* response_processor_context */ +NULL, /* response_processor_context_free */ +NULL, /* cancellable */ + (GAsyncReadyCallback)modem_init_sequence_ready, +result); +} + +/*/ /* Create Bearer (Modem interface) */ static MMBearer * @@ -637,6 +743,8 @@ mm_broadband_modem_novatel_lte_init (MMBroadbandModemNovatelLte *self) static void iface_modem_init (MMIfaceModem *iface) { +
Re: [MM/QMI] Unable to select firmware when there is no SIM
Do you have any preference/suggestion on how we should approach this problem? I guess we may need to add additional checks in the code where it currently assumes a modem object in a fully initialized state. - Ben On Sun, Jan 6, 2013 at 3:10 AM, Aleksander Morgado aleksan...@lanedo.comwrote: Hey hey Ben, I encounter an issue with firmware selection on my Novatel Gobi 3000 (E396) modem. When the UMTS firmware is initially selected and there is no SIM card inserted, ModemManager can't fully initialize the modem and firmware interface, which prevents me from selecting the CDMA firmware. Is it possible to continue the initialization of the modem and firmware interface when the SIM check fails? I guess we can allow that, yes. The modem itself would be in 'failed' state when there is no SIM though; the only thing we have to do is to let the Firmware interface get initialized even if we're in 'failed' state. -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM/QMI] Unable to select firmware when there is no SIM
Thanks for the patch and it works. - Ben On Mon, Jan 7, 2013 at 1:26 PM, Aleksander Morgado aleksan...@lanedo.comwrote: On 07/01/13 19:53, Ben Chan wrote: Do you have any preference/suggestion on how we should approach this problem? I guess we may need to add additional checks in the code where it currently assumes a modem object in a fully initialized state. I don't think there should be many Firmware-related code (if any) where we require a specific modem state. The attached patch (untested!!) should allow launching the Firmware interface when the modem is in Failed state. It should at least give some hints of where to start. -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM/QMI] Unable to select firmware when there is no SIM
The patch addresses the firmware selection issue I mentioned. I think it can be pushed as it is. Thanks! On Mon, Jan 7, 2013 at 10:55 PM, Aleksander Morgado aleksan...@lanedo.comwrote: On 07/01/13 23:34, Ben Chan wrote: Thanks for the patch and it works. Oh; cool. Nothing else needed then? -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM/QMI] Unable to select firmware when there is no SIM
Hi Aleksander, I encounter an issue with firmware selection on my Novatel Gobi 3000 (E396) modem. When the UMTS firmware is initially selected and there is no SIM card inserted, ModemManager can't fully initialize the modem and firmware interface, which prevents me from selecting the CDMA firmware. Is it possible to continue the initialization of the modem and firmware interface when the SIM check fails? Thanks, Ben ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] iface-modem-3gpp, iface-modem-cdma: check for deallocated RegistrationCheckContext
This patch fixes a crash in periodic_registration_checks_ready() due to access of an already deallocated RegistrationCheckContext. Thread 0 *CRASHED* ( SIGSEGV @ 0x ) 0x7fc344d355cd [ModemManager] - mm-iface-modem-cdma.c:1112 periodic_registration_checks_ready 0x7fc3449ea266 [libgio-2.0.so.0.3200.4] - gsimpleasyncresult.c:767 g_simple_async_result_complete 0x7fc3449ea368 [libgio-2.0.so.0.3200.4] - gsimpleasyncresult.c:779 complete_in_idle_cb 0x7fc344851dc4 [libglib-2.0.so.0.3200.4] - gmain.c:2539 g_main_context_dispatch 0x7fc344852147 [libglib-2.0.so.0.3200.4] - gmain.c:3146 g_main_context_iterate 0x7fc3448525a1 [libglib-2.0.so.0.3200.4] - gmain.c:3340 g_main_loop_run 0x7fc344d0f154 [ModemManager] - main.c:158 main 0x7fc34426a474 [libc-2.15.so] - libc-start.c:234 __libc_start_main 0x7fc344d0eb68 [ModemManager] + 0x0001bb68 --- src/mm-iface-modem-3gpp.c |3 ++- src/mm-iface-modem-cdma.c |3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c index 1e0cd30..7a88b2f 100644 --- a/src/mm-iface-modem-3gpp.c +++ b/src/mm-iface-modem-3gpp.c @@ -1147,7 +1147,8 @@ periodic_registration_checks_ready (MMIfaceModem3gpp *self, /* Remove the running tag */ ctx = g_object_get_qdata (G_OBJECT (self), registration_check_context_quark); -ctx-running = FALSE; +if (ctx) +ctx-running = FALSE; } static gboolean diff --git a/src/mm-iface-modem-cdma.c b/src/mm-iface-modem-cdma.c index 8478df5..d582fb3 100644 --- a/src/mm-iface-modem-cdma.c +++ b/src/mm-iface-modem-cdma.c @@ -1109,7 +1109,8 @@ periodic_registration_checks_ready (MMIfaceModemCdma *self, /* Remove the running tag */ ctx = g_object_get_qdata (G_OBJECT (self), registration_check_context_quark); -ctx-running = FALSE; +if (ctx) +ctx-running = FALSE; } static gboolean -- 1.7.7.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH] iface-modem, novatel-lte: disable network scan in LTE mode
In LTE mode, AT+COPS=? simply replies OK (confirmed with the modem vendor) - Ben On Thu, Jan 3, 2013 at 7:46 AM, Aleksander Morgado aleksan...@lanedo.comwrote: AT+COPS=? ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH] iface-modem, novatel-lte: disable network scan in LTE mode
Also, based on my experiments, the modem sometimes seems to hang when running AT+COPS=?. This patch avoids AT+COPS=? from being issued when the modem is in LTE mdoe. On Thu, Jan 3, 2013 at 11:10 AM, Ben Chan benc...@chromium.org wrote: In LTE mode, AT+COPS=? simply replies OK (confirmed with the modem vendor) - Ben On Thu, Jan 3, 2013 at 7:46 AM, Aleksander Morgado aleksan...@lanedo.comwrote: AT+COPS=? ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH] iface-modem, novatel-lte: disable network scan in LTE mode
On Thu, Jan 3, 2013 at 1:17 PM, Aleksander Morgado aleksan...@lanedo.comwrote: On 03/01/13 20:13, Ben Chan wrote: Also, based on my experiments, the modem sometimes seems to hang when running AT+COPS=?. This patch avoids AT+COPS=? from being issued when the modem is in LTE mdoe. From what I can see, it also avoids the Scan when the access tech is unknown: +if ((access_tech == MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN) || +(access_tech MM_MODEM_ACCESS_TECHNOLOGY_LTE)) { So you're really limiting the scanning for networks only to when the modem is already connected to a given network which is not LTE. I don't think we shouldn't ban from scanning when access tech is UNKNOWN. I include UNKNOWN just to be safe as there may be a period of time when the access technology is not yet determined. Also, the reply to AT+COPS=? may really take a loong time; you sure it gets stuck? From what I've seen before, the scan operation timed out and the modem didn't respond to AT commands anymore. However, that's not frequently reproducible. I wonder how the scan works when the Novatel LTE modem is handled by the QMI driver. IIRC Dan said we wouldn't need this specific plugin any more once the modem is handled by the QMI implementation. I haven't tried the QMI driver for this modem, but wonder if it behaves in a similar way as the limitation is most likely a firmware issue. -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH] iface-modem, novatel-lte: disable network scan in LTE mode
On Thu, Jan 3, 2013 at 1:37 PM, Aleksander Morgado aleksan...@lanedo.comwrote: Also, based on my experiments, the modem sometimes seems to hang when running AT+COPS=?. This patch avoids AT+COPS=? from being issued when the modem is in LTE mdoe. From what I can see, it also avoids the Scan when the access tech is unknown: +if ((access_tech == MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN) || +(access_tech MM_MODEM_ACCESS_TECHNOLOGY_LTE)) { So you're really limiting the scanning for networks only to when the modem is already connected to a given network which is not LTE. I don't think we shouldn't ban from scanning when access tech is UNKNOWN. I include UNKNOWN just to be safe as there may be a period of time when the access technology is not yet determined. That's what I mean; it should be perfectly valid to be able to scan for networks when the modem is not yet registered anywhere, i.e. when the access tech is unknown. Let me remove the check for unknown access technology. Also, the reply to AT+COPS=? may really take a loong time; you sure it gets stuck? From what I've seen before, the scan operation timed out and the modem didn't respond to AT commands anymore. However, that's not frequently reproducible. So this patch is just avoiding the need to parse an empty list as the AT+COPS=? command replies just OK by default when in LTE mode. Is it really worth the complication of subclassing the scan method just to avoid that? :-/ The only purpose of this patch is to skip AT+COPS=? in LTE mode as that causes the modem to sometimes behave weirdly :-/ I wonder how the scan works when the Novatel LTE modem is handled by the QMI driver. IIRC Dan said we wouldn't need this specific plugin any more once the modem is handled by the QMI implementation. I haven't tried the QMI driver for this modem, but wonder if it behaves in a similar way as the limitation is most likely a firmware issue. It would be great if you could check the overall behaviour of the Novatel LTE modem with the QMI driver, to see if there is any reason not to switch to it... Will do -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH v2] iface-modem, novatel-lte: disable network scan in LTE mode
--- plugins/novatel/mm-broadband-modem-novatel-lte.c | 72 +- src/mm-iface-modem.c | 20 ++ src/mm-iface-modem.h |3 + 3 files changed, 94 insertions(+), 1 deletions(-) diff --git a/plugins/novatel/mm-broadband-modem-novatel-lte.c b/plugins/novatel/mm-broadband-modem-novatel-lte.c index 249a4a1..d667182 100644 --- a/plugins/novatel/mm-broadband-modem-novatel-lte.c +++ b/plugins/novatel/mm-broadband-modem-novatel-lte.c @@ -29,15 +29,18 @@ #include mm-sim-novatel-lte.h #include mm-errors-types.h #include mm-iface-modem.h +#include mm-iface-modem-3gpp.h #include mm-iface-modem-messaging.h #include mm-log.h #include mm-modem-helpers.h #include mm-serial-parsers.h static void iface_modem_init (MMIfaceModem *iface); +static void iface_modem_3gpp_init (MMIfaceModem3gpp *iface); G_DEFINE_TYPE_EXTENDED (MMBroadbandModemNovatelLte, mm_broadband_modem_novatel_lte, MM_TYPE_BROADBAND_MODEM, 0, -G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM, iface_modem_init)); +G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM, iface_modem_init) +G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM_3GPP, iface_modem_3gpp_init)); /*/ /* Create Bearer (Modem interface) */ @@ -519,6 +522,66 @@ reset (MMIfaceModem *self, } /*/ +/* Scan networks (3GPP interface) */ + +static GList * +scan_networks_finish (MMIfaceModem3gpp *self, + GAsyncResult *res, + GError **error) +{ +const gchar *result; + +result = mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, error); +if (!result) +return NULL; + +return mm_3gpp_parse_cops_test_response (result, error); +} + +static void +scan_networks (MMIfaceModem3gpp *self, + GAsyncReadyCallback callback, + gpointer user_data) +{ +MMModemAccessTechnology access_tech; + +mm_dbg (scanning for networks (Novatel LTE)...); + +access_tech = mm_iface_modem_get_access_technologies (MM_IFACE_MODEM (self)); +/* The Novatel LTE modem does not properly support AT+COPS=? in LTE mode. + * Thus, do not try to scan networks when the current access technologies + * include LTE. + */ +if (access_tech MM_MODEM_ACCESS_TECHNOLOGY_LTE) { +GSimpleAsyncResult *simple; +gchar *access_tech_string; + +access_tech_string = mm_modem_access_technology_build_string_from_mask (access_tech); +mm_warn (Couldn't scan for networks with access technologies: %s, access_tech_string); +simple = g_simple_async_result_new (G_OBJECT (self), +callback, +user_data, +scan_networks); +g_simple_async_result_set_error (simple, + MM_CORE_ERROR, + MM_CORE_ERROR_UNSUPPORTED, + Couldn't scan for networks with access technologies: %s, + access_tech_string); +g_simple_async_result_complete_in_idle (simple); +g_object_unref (simple); +g_free (access_tech_string); +return; +} + +mm_base_modem_at_command (MM_BASE_MODEM (self), + +COPS=?, + 120, + FALSE, + callback, + user_data); +} + +/*/ MMBroadbandModemNovatelLte * mm_broadband_modem_novatel_lte_new (const gchar *device, @@ -564,6 +627,13 @@ iface_modem_init (MMIfaceModem *iface) } static void +iface_modem_3gpp_init (MMIfaceModem3gpp *iface) +{ +iface-scan_networks = scan_networks; +iface-scan_networks_finish = scan_networks_finish; +} + +static void mm_broadband_modem_novatel_lte_class_init (MMBroadbandModemNovatelLteClass *klass) { } diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c index d25b03d..e94bc42 100644 --- a/src/mm-iface-modem.c +++ b/src/mm-iface-modem.c @@ -3930,6 +3930,26 @@ mm_iface_modem_shutdown (MMIfaceModem *self) /*/ +MMModemAccessTechnology +mm_iface_modem_get_access_technologies (MMIfaceModem *self) +{ +MMModemAccessTechnology access_tech = MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN; +MmGdbusModem *skeleton; + +g_object_get (self, + MM_IFACE_MODEM_DBUS_SKELETON, skeleton, + NULL); + +if (skeleton) { +access_tech = mm_gdbus_modem_get_access_technologies (skeleton); +g_object_unref
[MM] [PATCH v3] iface-modem, novatel-lte: disable network scan in LTE mode
--- plugins/novatel/mm-broadband-modem-novatel-lte.c | 102 +- src/mm-iface-modem.c | 20 src/mm-iface-modem.h |3 + 3 files changed, 124 insertions(+), 1 deletions(-) diff --git a/plugins/novatel/mm-broadband-modem-novatel-lte.c b/plugins/novatel/mm-broadband-modem-novatel-lte.c index 249a4a1..08954b5 100644 --- a/plugins/novatel/mm-broadband-modem-novatel-lte.c +++ b/plugins/novatel/mm-broadband-modem-novatel-lte.c @@ -29,15 +29,18 @@ #include mm-sim-novatel-lte.h #include mm-errors-types.h #include mm-iface-modem.h +#include mm-iface-modem-3gpp.h #include mm-iface-modem-messaging.h #include mm-log.h #include mm-modem-helpers.h #include mm-serial-parsers.h static void iface_modem_init (MMIfaceModem *iface); +static void iface_modem_3gpp_init (MMIfaceModem3gpp *iface); G_DEFINE_TYPE_EXTENDED (MMBroadbandModemNovatelLte, mm_broadband_modem_novatel_lte, MM_TYPE_BROADBAND_MODEM, 0, -G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM, iface_modem_init)); +G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM, iface_modem_init) +G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM_3GPP, iface_modem_3gpp_init)); /*/ /* Create Bearer (Modem interface) */ @@ -519,6 +522,96 @@ reset (MMIfaceModem *self, } /*/ +/* Scan networks (3GPP interface) */ + +static GList * +scan_networks_finish (MMIfaceModem3gpp *self, + GAsyncResult *res, + GError **error) +{ +if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error)) +return NULL; + +return g_simple_async_result_get_op_res_gpointer (G_SIMPLE_ASYNC_RESULT (res)); +} + +static void +cops_query_ready (MMBroadbandModemNovatelLte *self, + GAsyncResult *res, + GSimpleAsyncResult *operation_result) +{ +const gchar *response; +GError *error = NULL; +GList *scan_result; + +response = mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, error); +if (error) { +g_simple_async_result_take_error (operation_result, error); +g_simple_async_result_complete (operation_result); +g_object_unref (operation_result); +return; +} + +scan_result = mm_3gpp_parse_cops_test_response (response, error); +if (error) { +g_simple_async_result_take_error (operation_result, error); +g_simple_async_result_complete (operation_result); +g_object_unref (operation_result); +return; +} + +g_simple_async_result_set_op_res_gpointer (operation_result, + scan_result, + NULL); +g_simple_async_result_complete (operation_result); +g_object_unref (operation_result); +} + +static void +scan_networks (MMIfaceModem3gpp *self, + GAsyncReadyCallback callback, + gpointer user_data) +{ +GSimpleAsyncResult *result; +MMModemAccessTechnology access_tech; + +mm_dbg (scanning for networks (Novatel LTE)...); + +result = g_simple_async_result_new (G_OBJECT (self), +callback, +user_data, +scan_networks); + +access_tech = mm_iface_modem_get_access_technologies (MM_IFACE_MODEM (self)); +/* The Novatel LTE modem does not properly support AT+COPS=? in LTE mode. + * Thus, do not try to scan networks when the current access technologies + * include LTE. + */ +if (access_tech MM_MODEM_ACCESS_TECHNOLOGY_LTE) { +gchar *access_tech_string; + +access_tech_string = mm_modem_access_technology_build_string_from_mask (access_tech); +mm_warn (Couldn't scan for networks with access technologies: %s, access_tech_string); +g_simple_async_result_set_error (result, + MM_CORE_ERROR, + MM_CORE_ERROR_UNSUPPORTED, + Couldn't scan for networks with access technologies: %s, + access_tech_string); +g_simple_async_result_complete_in_idle (result); +g_object_unref (result); +g_free (access_tech_string); +return; +} + +mm_base_modem_at_command (MM_BASE_MODEM (self), + +COPS=?, + 120, + FALSE, + (GAsyncReadyCallback)cops_query_ready, + result); +} + +/*/ MMBroadbandModemNovatelLte *
Re: [MM] [PATCH v2] iface-modem, novatel-lte: disable network scan in LTE mode
Thanks for the notes. Submitted patch v3. - Ben On Thu, Jan 3, 2013 at 3:58 PM, Aleksander Morgado aleksan...@lanedo.comwrote: +static GList * +scan_networks_finish (MMIfaceModem3gpp *self, + GAsyncResult *res, + GError **error) +{ +const gchar *result; + +result = mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, error); +if (!result) +return NULL; + +return mm_3gpp_parse_cops_test_response (result, error); +} + +static void +scan_networks (MMIfaceModem3gpp *self, + GAsyncReadyCallback callback, + gpointer user_data) +{ +MMModemAccessTechnology access_tech; + +mm_dbg (scanning for networks (Novatel LTE)...); + +access_tech = mm_iface_modem_get_access_technologies (MM_IFACE_MODEM (self)); +/* The Novatel LTE modem does not properly support AT+COPS=? in LTE mode. + * Thus, do not try to scan networks when the current access technologies + * include LTE. + */ +if (access_tech MM_MODEM_ACCESS_TECHNOLOGY_LTE) { +GSimpleAsyncResult *simple; +gchar *access_tech_string; + +access_tech_string = mm_modem_access_technology_build_string_from_mask (access_tech); +mm_warn (Couldn't scan for networks with access technologies: %s, access_tech_string); +simple = g_simple_async_result_new (G_OBJECT (self), +callback, +user_data, +scan_networks); +g_simple_async_result_set_error (simple, + MM_CORE_ERROR, + MM_CORE_ERROR_UNSUPPORTED, + Couldn't scan for networks with access technologies: %s, + access_tech_string); +g_simple_async_result_complete_in_idle (simple); +g_object_unref (simple); +g_free (access_tech_string); +return; +} + +mm_base_modem_at_command (MM_BASE_MODEM (self), + +COPS=?, + 120, + FALSE, + callback, + user_data); +} You cannot mix in the same async method a code execution path using mm_base_modem_at_command() with another one using GSimpleAsyncResult and completion in idle. When you use mm_base_modem_at_command(), in finish() you're expected to use mm_base_modem_at_command_finish(), like you did. But when you use GSimpleAsyncResult and completion in idle you should use g_simple_async_result_propagate_error() in finish(). The fact that it may work as expected is due to how at_command_finish() is implemented; but you shouldn't rely on that. So you'll need to use GSimpleAsyncResult for both cases, i.e. provide a _ready() GAsyncReadyCallback in mm_base_modem_at_command() to which you pass 'simple' as user_data; and then complete the 'simple' from within the _ready() method. In this way, you can safely call g_simple_async_result_propagate_error() in finish(). See for example: http://cgit.freedesktop.org/ModemManager/ModemManager/tree/plugins/simtech/mm-broadband-modem-simtech.c#n416 And btw, a hint for when you just always need to report an error in idle in the async method (avoids the need to create a GSimpleAsyncResult yourself): http://developer.gnome.org/gio/unstable/GSimpleAsyncResult.html#g-simple-async-report-error-in-idle -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] iface-modem, novatel-lte: disable network scan in LTE mode
--- plugins/novatel/mm-broadband-modem-novatel-lte.c | 73 +- src/mm-iface-modem.c | 20 ++ src/mm-iface-modem.h |3 + 3 files changed, 95 insertions(+), 1 deletions(-) diff --git a/plugins/novatel/mm-broadband-modem-novatel-lte.c b/plugins/novatel/mm-broadband-modem-novatel-lte.c index 249a4a1..5b1a63b 100644 --- a/plugins/novatel/mm-broadband-modem-novatel-lte.c +++ b/plugins/novatel/mm-broadband-modem-novatel-lte.c @@ -29,15 +29,18 @@ #include mm-sim-novatel-lte.h #include mm-errors-types.h #include mm-iface-modem.h +#include mm-iface-modem-3gpp.h #include mm-iface-modem-messaging.h #include mm-log.h #include mm-modem-helpers.h #include mm-serial-parsers.h static void iface_modem_init (MMIfaceModem *iface); +static void iface_modem_3gpp_init (MMIfaceModem3gpp *iface); G_DEFINE_TYPE_EXTENDED (MMBroadbandModemNovatelLte, mm_broadband_modem_novatel_lte, MM_TYPE_BROADBAND_MODEM, 0, -G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM, iface_modem_init)); +G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM, iface_modem_init) +G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM_3GPP, iface_modem_3gpp_init)); /*/ /* Create Bearer (Modem interface) */ @@ -519,6 +522,67 @@ reset (MMIfaceModem *self, } /*/ +/* Scan networks (3GPP interface) */ + +static GList * +scan_networks_finish (MMIfaceModem3gpp *self, + GAsyncResult *res, + GError **error) +{ +const gchar *result; + +result = mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, error); +if (!result) +return NULL; + +return mm_3gpp_parse_cops_test_response (result, error); +} + +static void +scan_networks (MMIfaceModem3gpp *self, + GAsyncReadyCallback callback, + gpointer user_data) +{ +MMModemAccessTechnology access_tech; + +mm_dbg (scanning for networks (Novatel LTE)...); + +access_tech = mm_iface_modem_get_access_technologies (MM_IFACE_MODEM (self)); +/* The Novatel LTE modem does not properly support AT+COPS=? in LTE mode. + * Thus, do not try to scan networks when the current access technologies + * include LTE or cannot be determined. + */ +if ((access_tech == MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN) || +(access_tech MM_MODEM_ACCESS_TECHNOLOGY_LTE)) { +GSimpleAsyncResult *simple; +gchar *access_tech_string; + +access_tech_string = mm_modem_access_technology_build_string_from_mask (access_tech); +mm_warn (Couldn't scan for networks with access technologies: %s, access_tech_string); +simple = g_simple_async_result_new (G_OBJECT (self), +callback, +user_data, +scan_networks); +g_simple_async_result_set_error (simple, + MM_CORE_ERROR, + MM_CORE_ERROR_UNSUPPORTED, + Couldn't scan for networks with access technologies: %s, + access_tech_string); +g_simple_async_result_complete_in_idle (simple); +g_object_unref (simple); +g_free (access_tech_string); +return; +} + +mm_base_modem_at_command (MM_BASE_MODEM (self), + +COPS=?, + 120, + FALSE, + callback, + user_data); +} + +/*/ MMBroadbandModemNovatelLte * mm_broadband_modem_novatel_lte_new (const gchar *device, @@ -564,6 +628,13 @@ iface_modem_init (MMIfaceModem *iface) } static void +iface_modem_3gpp_init (MMIfaceModem3gpp *iface) +{ +iface-scan_networks = scan_networks; +iface-scan_networks_finish = scan_networks_finish; +} + +static void mm_broadband_modem_novatel_lte_class_init (MMBroadbandModemNovatelLteClass *klass) { } diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c index d25b03d..e94bc42 100644 --- a/src/mm-iface-modem.c +++ b/src/mm-iface-modem.c @@ -3930,6 +3930,26 @@ mm_iface_modem_shutdown (MMIfaceModem *self) /*/ +MMModemAccessTechnology +mm_iface_modem_get_access_technologies (MMIfaceModem *self) +{ +MMModemAccessTechnology access_tech = MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN; +MmGdbusModem *skeleton; + +g_object_get (self, + MM_IFACE_MODEM_DBUS_SKELETON, skeleton, + NULL); + +if (skeleton) { +
Re: [MM] [PATCH v2] serial-port: avoid opening a serial port that has been disposed
Thanks for the patches! Ben On Wed, Dec 12, 2012 at 3:59 AM, Aleksander Morgado aleksan...@lanedo.comwrote: Aleksander, thanks for the patch. I've been running suspend/resume test with the patch. Seems like those glib warnings (and the crash) are gone except for the following: So, I pushed now the patch for both the Icera and HSO plugins; see commit dc9bbef. (ModemManager:2858): GLib-GObject-WARNING **: gsignal.c:2576: instance `0x78624028' has no handler with id `148' Also fixed this one, which was popping up when trying to reproduce the issue in an Option/HSO modem; see commit f20922b. Cheers, -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH v2] serial-port: avoid opening a serial port that has been disposed
Aleksander, thanks for the patch. I've been running suspend/resume test with the patch. Seems like those glib warnings (and the crash) are gone except for the following: (ModemManager:2858): GLib-GObject-WARNING **: gsignal.c:2576: instance `0x78624028' has no handler with id `148' Thanks, Ben On Fri, Nov 30, 2012 at 5:05 AM, Aleksander Morgado aleksan...@lanedo.comwrote: On 30/11/12 13:32, Aleksander Morgado wrote: * Whenever we wait for an unsolicited message to keep on processing an operation, we also need to consider the case where the port is forced to be closed due to being removed. At least *Icera* and *HSO* are affected by this issue. Ben, are you able to retest with the attached patch applied? Cheers, -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH v2] serial-port: avoid opening a serial port that has been disposed
The crash happens to be a reuse of an already disposed MMSerialPort object. I'm now tracing the new/dispose paths. Ben /* Don't read any input if the current command isn't done being sent yet */ info = g_queue_peek_nth (priv-queue, 0); 49f60: 6940ldr r0, [r0, #20] - 0x151F60 49f62: f7c9 ebf6 blx 13750 _init+0x1254 ==20047== Invalid read of size 4 ==20047==at 0x151F60: ??? (in /usr/sbin/ModemManager) ==20047== Address 0x14 is not stack'd, malloc'd or (recently) free'd ==20047== ==20047== ==20047== Process terminating with default action of signal 11 (SIGSEGV) ==20047== Access not within mapped region at address 0x14 ==20047==at 0x151F60: ??? (in /usr/sbin/ModemManager) ==20047== If you believe this happened as a result of a stack ==20047== overflow in your program's main thread (unlikely but ==20047== possible), you can try to increase the size of the ==20047== main thread stack using the --main-stacksize= flag. ==20047== The main thread stack size used in this run was 8388608. (ModemManager:2858): GLib-GObject-CRITICAL **: g_object_ref: assertion `G_IS_OBJECT (object)' failed (ModemManager:2858): GLib-GObject-CRITICAL **: g_object_ref: assertion `G_IS_OBJECT (object)' failed (ModemManager:2858): GLib-GObject-CRITICAL **: g_object_unref: assertion `G_IS_OBJECT (object)' failed (ModemManager:2858): GLib-GObject-CRITICAL **: g_object_unref: assertion `G_IS_OBJECT (object)' failed (ModemManager:2858): GLib-GObject-WARNING **: gsignal.c:2576: instance `0x78624028' has no handler with id `148' (ModemManager:2858): GLib-GObject-WARNING **: invalid unclassed pointer in cast to `MMSerialPort' (ModemManager:2858): GLib-GObject-CRITICAL **: g_type_instance_get_private: assertion `instance != NULL instance-g_class != NULL' failed On Thu, Nov 29, 2012 at 11:26 AM, Dan Williams d...@redhat.com wrote: On Tue, 2012-11-27 at 13:19 -0800, Ben Chan wrote: Yes, it's related to the data_available (mm-serial-port.c:767) crash (crosbug.com/35391). I'm running suspend/resume stress test with ModemManager under valgrind. Any way to get MM running with --debug in those logs? Also, in your sources, does line 767 match up with: info = g_queue_peek_nth (priv-queue, 0); or some other line? One other thing to do, put an mm_info ((%s): disposing, mm_port_get_device (MM_PORT (self))); into dispose() and lets see when it gets disposed and if anything tries to open it during/after dispose. Drop another of these into mm_serial_port_new(). Dan Thanks, Ben On Tue, Nov 27, 2012 at 1:13 PM, Aleksander Morgado aleksan...@lanedo.com wrote: On 11/27/2012 09:39 PM, Ben Chan wrote: --- src/mm-serial-port.c | 10 ++ 1 files changed, 10 insertions(+), 0 deletions(-) Don't think it's the correct approach. I don't think we ever run g_object_run_dispose() ourselves for port objects, and that means that whenever we got the port object disposed it was because it was the last valid reference. And that means that you won't be able to read priv-disposed afterwards as that would mean reading already freed memory. There clearly is a missing reference around, or a timeout or other source which has the port as user_data and is not being properly cleaned up (e.g. removing the timeout or event source when the port is disposed). Running it under valgrind should confirm this. Is this related to the data_available() crashes during suspend/resume? diff --git a/src/mm-serial-port.c b/src/mm-serial-port.c index 0a8820d..a33c745 100644 --- a/src/mm-serial-port.c +++ b/src/mm-serial-port.c @@ -69,6 +69,7 @@ static guint signals[LAST_SIGNAL] = { 0 }; typedef struct { guint32 open_count; +gboolean disposed; gboolean forced_close; int fd; GHashTable *reply_cache; @@ -849,6 +850,12 @@ mm_serial_port_open (MMSerialPort *self, GError **error) device = mm_port_get_device (MM_PORT (self)); +/* If the MMSerialPort object has been disposed, just return an error. */ +if (priv-disposed) { +mm_info ((%s) skipped opening serial port that has been disposed, device); +return FALSE; +} + if (priv-open_count) { /* Already open */ goto success; @@ -1537,6 +1544,9 @@ dispose (GObject *object) { MMSerialPortPrivate *priv = MM_SERIAL_PORT_GET_PRIVATE (object); +/* Mark
[MM] [PATCH] serial-port: avoid opening a serial port that has been disposed
--- src/mm-serial-port.c |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/src/mm-serial-port.c b/src/mm-serial-port.c index 0a8820d..dee2fec 100644 --- a/src/mm-serial-port.c +++ b/src/mm-serial-port.c @@ -849,6 +849,13 @@ mm_serial_port_open (MMSerialPort *self, GError **error) device = mm_port_get_device (MM_PORT (self)); +/* If we forced closing the port, the MMSerialPort object has been disposed. + * Just return an error. */ +if (priv-forced_close) { +mm_info ((%s) skipped opening serial port that has been disposed, device); +return FALSE; +} + if (priv-open_count) { /* Already open */ goto success; -- 1.7.7.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH] serial-port: avoid opening a serial port that has been disposed
I guess the major concern would be using priv-forced_close as an indicator of whether the serial port is allowed to reopen or not. How about using another variable to tag the MMSerialPort object as being disposed? There are a few call sites calling mm_serial_port_open(), so it seems better to check inside mm_serial_port_open(). How do you think? I can revise the patch if that makes sense. Thanks, Ben On Tue, Nov 27, 2012 at 12:03 PM, Dan Williams d...@redhat.com wrote: On Tue, 2012-11-27 at 10:57 -0800, Ben Chan wrote: --- src/mm-serial-port.c |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) I guess I'd rather fix the code that's attempting to re-open the port if it's not supposed to do that. This has the effect of making ports single-use only, which wasn't originally the intention. Forcing a port closed regardless of the open count doesn't necessarily mean the port object itself has been disposed. Dan diff --git a/src/mm-serial-port.c b/src/mm-serial-port.c index 0a8820d..dee2fec 100644 --- a/src/mm-serial-port.c +++ b/src/mm-serial-port.c @@ -849,6 +849,13 @@ mm_serial_port_open (MMSerialPort *self, GError **error) device = mm_port_get_device (MM_PORT (self)); +/* If we forced closing the port, the MMSerialPort object has been disposed. + * Just return an error. */ +if (priv-forced_close) { +mm_info ((%s) skipped opening serial port that has been disposed, device); +return FALSE; +} + if (priv-open_count) { /* Already open */ goto success; ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH v2] serial-port: avoid opening a serial port that has been disposed
--- src/mm-serial-port.c | 10 ++ 1 files changed, 10 insertions(+), 0 deletions(-) diff --git a/src/mm-serial-port.c b/src/mm-serial-port.c index 0a8820d..a33c745 100644 --- a/src/mm-serial-port.c +++ b/src/mm-serial-port.c @@ -69,6 +69,7 @@ static guint signals[LAST_SIGNAL] = { 0 }; typedef struct { guint32 open_count; +gboolean disposed; gboolean forced_close; int fd; GHashTable *reply_cache; @@ -849,6 +850,12 @@ mm_serial_port_open (MMSerialPort *self, GError **error) device = mm_port_get_device (MM_PORT (self)); +/* If the MMSerialPort object has been disposed, just return an error. */ +if (priv-disposed) { +mm_info ((%s) skipped opening serial port that has been disposed, device); +return FALSE; +} + if (priv-open_count) { /* Already open */ goto success; @@ -1537,6 +1544,9 @@ dispose (GObject *object) { MMSerialPortPrivate *priv = MM_SERIAL_PORT_GET_PRIVATE (object); +/* Mark the MMSerialPort object as disposed to prevent it from being re-opened. */ +priv-disposed = TRUE; + if (priv-timeout_id) { g_source_remove (priv-timeout_id); priv-timeout_id = 0; -- 1.7.7.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH v2] serial-port: avoid opening a serial port that has been disposed
Yes, it's related to the data_available (mm-serial-port.c:767) crash ( crosbug.com/35391). I'm running suspend/resume stress test with ModemManager under valgrind. Thanks, Ben On Tue, Nov 27, 2012 at 1:13 PM, Aleksander Morgado aleksan...@lanedo.comwrote: On 11/27/2012 09:39 PM, Ben Chan wrote: --- src/mm-serial-port.c | 10 ++ 1 files changed, 10 insertions(+), 0 deletions(-) Don't think it's the correct approach. I don't think we ever run g_object_run_dispose() ourselves for port objects, and that means that whenever we got the port object disposed it was because it was the last valid reference. And that means that you won't be able to read priv-disposed afterwards as that would mean reading already freed memory. There clearly is a missing reference around, or a timeout or other source which has the port as user_data and is not being properly cleaned up (e.g. removing the timeout or event source when the port is disposed). Running it under valgrind should confirm this. Is this related to the data_available() crashes during suspend/resume? diff --git a/src/mm-serial-port.c b/src/mm-serial-port.c index 0a8820d..a33c745 100644 --- a/src/mm-serial-port.c +++ b/src/mm-serial-port.c @@ -69,6 +69,7 @@ static guint signals[LAST_SIGNAL] = { 0 }; typedef struct { guint32 open_count; +gboolean disposed; gboolean forced_close; int fd; GHashTable *reply_cache; @@ -849,6 +850,12 @@ mm_serial_port_open (MMSerialPort *self, GError **error) device = mm_port_get_device (MM_PORT (self)); +/* If the MMSerialPort object has been disposed, just return an error. */ +if (priv-disposed) { +mm_info ((%s) skipped opening serial port that has been disposed, device); +return FALSE; +} + if (priv-open_count) { /* Already open */ goto success; @@ -1537,6 +1544,9 @@ dispose (GObject *object) { MMSerialPortPrivate *priv = MM_SERIAL_PORT_GET_PRIVATE (object); +/* Mark the MMSerialPort object as disposed to prevent it from being re-opened. */ +priv-disposed = TRUE; + if (priv-timeout_id) { g_source_remove (priv-timeout_id); priv-timeout_id = 0; -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] broadband-modem: check for NULL response in parse_caps_cpin and parse_caps_cgmm
--- src/mm-broadband-modem.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c index 9161c82..070526f 100644 --- a/src/mm-broadband-modem.c +++ b/src/mm-broadband-modem.c @@ -465,6 +465,9 @@ parse_caps_cpin (MMBaseModem *self, GVariant **result, GError **result_error) { +if (!response) +return FALSE; + if (strcasestr (response, SIM PIN) || strcasestr (response, SIM PUK) || strcasestr (response, PH-SIM PIN) || @@ -498,6 +501,9 @@ parse_caps_cgmm (MMBaseModem *self, GVariant **result, GError **result_error) { +if (!response) +return FALSE; + /* This check detects some really old Motorola GPRS dongles and phones */ if (strstr (response, GSM900) || strstr (response, GSM1800) || -- 1.7.7.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH v2] broadband-modem: check for NULL response in parse_caps_{cpin, cgmm, gcap}
--- src/mm-broadband-modem.c | 11 ++- 1 files changed, 10 insertions(+), 1 deletions(-) diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c index 9161c82..174de9d 100644 --- a/src/mm-broadband-modem.c +++ b/src/mm-broadband-modem.c @@ -435,10 +435,13 @@ parse_caps_gcap (MMBaseModem *self, const ModemCaps *cap = modem_caps; guint32 ret = 0; +if (!response) +return FALSE; + /* Some modems (Huawei E160g) won't respond to +GCAP with no SIM, but * will respond to ATI. Ignore the error and continue. */ -if (response strstr (response, +CME ERROR:)) +if (strstr (response, +CME ERROR:)) return FALSE; while (cap-name) { @@ -465,6 +468,9 @@ parse_caps_cpin (MMBaseModem *self, GVariant **result, GError **result_error) { +if (!response) +return FALSE; + if (strcasestr (response, SIM PIN) || strcasestr (response, SIM PUK) || strcasestr (response, PH-SIM PIN) || @@ -498,6 +504,9 @@ parse_caps_cgmm (MMBaseModem *self, GVariant **result, GError **result_error) { +if (!response) +return FALSE; + /* This check detects some really old Motorola GPRS dongles and phones */ if (strstr (response, GSM900) || strstr (response, GSM1800) || -- 1.7.7.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH] novatel-lte: use $NWMDN to read own number when +CNUM fails
The number(s) returned by +CNUM include the number return by $NWMDN. Ideally, we should be able to just use +CNUM. But probably due to some firmware issue, +CNUM sometimes returns ERROR unexpectedly shortly after the SIM interface becomes ready. It also returns ERROR for an unactivated SIM.$NWMDN seems to always behave properly, so that this patch uses it as a fallback. And also because $NWMDN only returns one number, I still try +CNUM first when possible. Thanks, Ben On Mon, Nov 5, 2012 at 4:48 AM, Aleksander Morgado aleksan...@lanedo.comwrote: On 04/11/12 02:00, Ben Chan wrote: +CNUM may return ERROR when the modem fails to read the own numbers from the SIM card or when the SIM card hasn't been activated. Use $NWMDN to read the MDN as a fallback, which distinguishes these two cases. Just wondering, shouldn't the plugin try to read always both? OwnNumbers is an array of strings, you can both the one retrieved from +CNUM (if any) and the MDN in there. -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] novatel-lte: use $NWMDN to read own number when +CNUM fails
+CNUM may return ERROR when the modem fails to read the own numbers from the SIM card or when the SIM card hasn't been activated. Use $NWMDN to read the MDN as a fallback, which distinguishes these two cases. --- plugins/novatel/mm-broadband-modem-novatel-lte.c | 103 ++ 1 files changed, 103 insertions(+), 0 deletions(-) diff --git a/plugins/novatel/mm-broadband-modem-novatel-lte.c b/plugins/novatel/mm-broadband-modem-novatel-lte.c index 6f2e7e2..249a4a1 100644 --- a/plugins/novatel/mm-broadband-modem-novatel-lte.c +++ b/plugins/novatel/mm-broadband-modem-novatel-lte.c @@ -158,6 +158,107 @@ modem_after_sim_unlock (MMIfaceModem *self, } /*/ +/* Load own numbers (Modem interface) */ + +static GStrv +load_own_numbers_finish (MMIfaceModem *self, + GAsyncResult *res, + GError **error) +{ +GVariant *result; +GStrv own_numbers; + +result = mm_base_modem_at_sequence_finish (MM_BASE_MODEM (self), res, NULL, error); +if (!result) +return NULL; + +own_numbers = (GStrv) g_variant_dup_strv (result, NULL); +return own_numbers; +} + +static gboolean +response_processor_cnum_ignore_at_errors (MMBaseModem *self, + gpointer none, + const gchar *command, + const gchar *response, + gboolean last_command, + const GError *error, + GVariant **result, + GError **result_error) +{ +GStrv own_numbers; + +if (error) { +/* Ignore AT errors (ie, ERROR or CMx ERROR) */ +if (error-domain != MM_MOBILE_EQUIPMENT_ERROR || last_command) +*result_error = g_error_copy (error); + +return FALSE; +} + +own_numbers = mm_3gpp_parse_cnum_exec_response (response, result_error); +if (!own_numbers) +return FALSE; + +*result = g_variant_new_strv ((const gchar *const *) own_numbers, -1); +g_strfreev (own_numbers); +return TRUE; +} + +static gboolean +response_processor_nwmdn_ignore_at_errors (MMBaseModem *self, + gpointer none, + const gchar *command, + const gchar *response, + gboolean last_command, + const GError *error, + GVariant **result, + GError **result_error) +{ +GArray *array; +GStrv own_numbers; +gchar *mdn; + +if (error) { +/* Ignore AT errors (ie, ERROR or CMx ERROR) */ +if (error-domain != MM_MOBILE_EQUIPMENT_ERROR || last_command) +*result_error = g_error_copy (error); + +return FALSE; +} + +mdn = g_strdup (mm_strip_tag (response, $NWMDN:)); +array = g_array_new (TRUE, TRUE, sizeof (gchar *)); +g_array_append_val (array, mdn); +own_numbers = (GStrv) g_array_free (array, FALSE); + +*result = g_variant_new_strv ((const gchar *const *) own_numbers, -1); +g_strfreev (own_numbers); +return TRUE; +} + +static const MMBaseModemAtCommand own_numbers_commands[] = { +{ +CNUM, 3, TRUE, response_processor_cnum_ignore_at_errors }, +{ $NWMDN, 3, TRUE, response_processor_nwmdn_ignore_at_errors }, +{ NULL } +}; + +static void +load_own_numbers (MMIfaceModem *self, + GAsyncReadyCallback callback, + gpointer user_data) +{ +mm_dbg (loading (Novatel LTE) own numbers...); +mm_base_modem_at_sequence ( +MM_BASE_MODEM (self), +own_numbers_commands, +NULL, /* response_processor_context */ +NULL, /* response_processor_context_free */ +callback, +user_data); +} + +/*/ /* Load supported bands (Modem interface) */ /* @@ -449,6 +550,8 @@ iface_modem_init (MMIfaceModem *iface) iface-create_sim_finish = modem_create_sim_finish; iface-modem_after_sim_unlock = modem_after_sim_unlock; iface-modem_after_sim_unlock_finish = modem_after_sim_unlock_finish; +iface-load_own_numbers = load_own_numbers; +iface-load_own_numbers_finish = load_own_numbers_finish; iface-load_supported_bands = load_supported_bands; iface-load_supported_bands_finish = load_supported_bands_finish; iface-load_current_bands = load_current_bands; -- 1.7.7.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM PATCH] iface-modem: load own numbers after SIM initialization
In 3GPP, own numbers are loaded from the SIM card, the loading of own numbers should be scheduled after the SIM card is ready. --- src/mm-iface-modem.c | 34 +- 1 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c index 082846e..d25b03d 100644 --- a/src/mm-iface-modem.c +++ b/src/mm-iface-modem.c @@ -3128,10 +3128,10 @@ typedef enum { INITIALIZATION_STEP_REVISION, INITIALIZATION_STEP_EQUIPMENT_ID, INITIALIZATION_STEP_DEVICE_ID, -INITIALIZATION_STEP_OWN_NUMBERS, INITIALIZATION_STEP_UNLOCK_REQUIRED, INITIALIZATION_STEP_UNLOCK_RETRIES, INITIALIZATION_STEP_SIM, +INITIALIZATION_STEP_OWN_NUMBERS, INITIALIZATION_STEP_SUPPORTED_MODES, INITIALIZATION_STEP_SUPPORTED_BANDS, INITIALIZATION_STEP_LAST @@ -3649,22 +3649,6 @@ interface_initialization_step (InitializationContext *ctx) /* Fall down to next step */ ctx-step++; -case INITIALIZATION_STEP_OWN_NUMBERS: -/* Own numbers is meant to be loaded only once during the whole - * lifetime of the modem. Therefore, if we already have them loaded, - * don't try to load them again. */ -if (mm_gdbus_modem_get_own_numbers (ctx-skeleton) == NULL -MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_own_numbers -MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_own_numbers_finish) { -MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_own_numbers ( -ctx-self, -(GAsyncReadyCallback)load_own_numbers_ready, -ctx); -return; -} -/* Fall down to next step */ -ctx-step++; - case INITIALIZATION_STEP_UNLOCK_REQUIRED: /* Only check unlock required if we were previously not unlocked */ if (mm_gdbus_modem_get_unlock_required (ctx-skeleton) != MM_MODEM_LOCK_NONE) { @@ -3710,6 +3694,22 @@ interface_initialization_step (InitializationContext *ctx) return; } +case INITIALIZATION_STEP_OWN_NUMBERS: +/* Own numbers is meant to be loaded only once during the whole + * lifetime of the modem. Therefore, if we already have them loaded, + * don't try to load them again. */ +if (mm_gdbus_modem_get_own_numbers (ctx-skeleton) == NULL +MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_own_numbers +MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_own_numbers_finish) { +MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_own_numbers ( +ctx-self, +(GAsyncReadyCallback)load_own_numbers_ready, +ctx); +return; +} +/* Fall down to next step */ +ctx-step++; + case INITIALIZATION_STEP_SUPPORTED_MODES: g_assert (MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_supported_modes != NULL); g_assert (MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_supported_modes_finish != NULL); -- 1.7.7.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM PATCH] novatel-lte: increase the wait after SIM unlock to 3 seconds
After repeated stress tests on a few Novatel E362 modems and SIM cards, it is revealed that a 3-second wait after SIM unlock is necessary for reliably reading ICCID and IMSI through the SIM interface. --- plugins/novatel/mm-broadband-modem-novatel-lte.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/novatel/mm-broadband-modem-novatel-lte.c b/plugins/novatel/mm-broadband-modem-novatel-lte.c index da855fc..6f2e7e2 100644 --- a/plugins/novatel/mm-broadband-modem-novatel-lte.c +++ b/plugins/novatel/mm-broadband-modem-novatel-lte.c @@ -152,9 +152,9 @@ modem_after_sim_unlock (MMIfaceModem *self, user_data, modem_after_sim_unlock); -/* A 2-second wait is necessary for SIM to become ready. +/* A 3-second wait is necessary for SIM to become ready. * Otherwise, a subsequent AT+CRSM command will likely fail. */ -g_timeout_add_seconds (2, (GSourceFunc)after_sim_unlock_wait_cb, result); +g_timeout_add_seconds (3, (GSourceFunc)after_sim_unlock_wait_cb, result); } /*/ -- 1.7.7.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM PATCH 2/2] novatel-lte: retry $NWQMISTATUS upon an unknown error during connecting
The $NWQMISTATUS command sometimes replies an ERROR shortly after calling the $NWQMICONNECT command, but then replies the proper QMI status if we retry it. This behavior is observed on an E362 modem with 4.08 firmware. (ttyUSB0): -- 'AT$NWQMICONNECT=,,CR' (ttyUSB0): -- 'CRLFOKCRLF' (ttyUSB0): -- 'AT$NWQMISTATUSCR' (ttyUSB0): -- 'CRLFERRORCRLF' Got failure code 100: Unknown error QMI connection status failed: Unknown error --- plugins/novatel/mm-broadband-bearer-novatel-lte.c | 14 -- 1 files changed, 8 insertions(+), 6 deletions(-) diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c b/plugins/novatel/mm-broadband-bearer-novatel-lte.c index e570024..e1944bb 100644 --- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c +++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c @@ -191,12 +191,14 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem, error); if (!result) { mm_warn (QMI connection status failed: %s, error-message); -g_simple_async_result_take_error (ctx-result, error); -detailed_connect_context_complete_and_free (ctx); -return; -} - -if (is_qmistatus_connected (result)) { +if (!g_error_matches (error, MM_MOBILE_EQUIPMENT_ERROR, MM_MOBILE_EQUIPMENT_ERROR_UNKNOWN)) { +g_simple_async_result_take_error (ctx-result, error); +detailed_connect_context_complete_and_free (ctx); +return; +} +g_error_free (error); +result = Unknown error; +} else if (is_qmistatus_connected (result)) { MMBearerIpConfig *config; mm_dbg(Connected); -- 1.7.7.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM PATCH 1/2] novatel-lte: handle $NWQMISTATUS responses for firmware 4.08
In firmware 4.08, the $NWQMISTATUS command returns different values for QMI state to indicate the current connection state. This patch modifies the code to handle $NWQMISTATUS responses in firmware 1.41 and 4.08. --- plugins/novatel/mm-broadband-bearer-novatel-lte.c | 24 +++- 1 files changed, 18 insertions(+), 6 deletions(-) diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c b/plugins/novatel/mm-broadband-bearer-novatel-lte.c index e7e76f6..e570024 100644 --- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c +++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c @@ -33,6 +33,7 @@ #include mm-modem-helpers.h #define CONNECTION_CHECK_TIMEOUT_SEC 5 +#define QMISTATUS_TAG $NWQMISTATUS: G_DEFINE_TYPE (MMBroadbandBearerNovatelLte, mm_broadband_bearer_novatel_lte, MM_TYPE_BROADBAND_BEARER); @@ -121,6 +122,20 @@ connect_3gpp_finish (MMBroadbandBearer *self, static gboolean connect_3gpp_qmistatus (DetailedConnectContext *ctx); +static gboolean +is_qmistatus_connected(const gchar *str) +{ +str = mm_strip_tag (str, QMISTATUS_TAG); +return g_strrstr (str, QMI State: CONNECTED) || g_strrstr (str, QMI State: QMI_WDS_PKT_DATA_CONNECTED); +} + +static gboolean +is_qmistatus_disconnected(const gchar *str) +{ +str = mm_strip_tag (str, QMISTATUS_TAG); +return g_strrstr (str, QMI State: DISCONNECTED) || g_strrstr (str, QMI State: QMI_WDS_PKT_DATA_DISCONNECTED); +} + static void poll_connection_ready (MMBaseModem *modem, GAsyncResult *res, @@ -136,8 +151,7 @@ poll_connection_ready (MMBaseModem *modem, return; } -result = mm_strip_tag (result, $NWQMISTATUS:); -if (g_strrstr (result, QMI State: DISCONNECTED)) { +if (is_qmistatus_disconnected (result)) { mm_bearer_report_disconnection (MM_BEARER (bearer)); g_source_remove (bearer-priv-connection_poller); bearer-priv-connection_poller = 0; @@ -182,8 +196,7 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem, return; } -result = mm_strip_tag (result, $NWQMISTATUS:); -if (g_strrstr (result, QMI State: CONNECTED)) { +if (is_qmistatus_connected (result)) { MMBearerIpConfig *config; mm_dbg(Connected); @@ -372,8 +385,7 @@ disconnect_3gpp_status_complete (MMBaseModem *modem, g_error_free (error); } -result = mm_strip_tag (result, $NWQMISTATUS:); -if (result g_strrstr (result, QMI State: DISCONNECTED)) +if (result is_qmistatus_disconnected (result)) g_simple_async_result_set_op_res_gboolean (ctx-result, TRUE); else g_simple_async_result_set_error (ctx-result, -- 1.7.7.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[PATCH] icera: improve parsing of access technologies in %NWSTATE response
--- plugins/icera/mm-broadband-modem-icera.c | 40 +++--- 1 files changed, 20 insertions(+), 20 deletions(-) diff --git a/plugins/icera/mm-broadband-modem-icera.c b/plugins/icera/mm-broadband-modem-icera.c index db70252..8267ba6 100644 --- a/plugins/icera/mm-broadband-modem-icera.c +++ b/plugins/icera/mm-broadband-modem-icera.c @@ -322,27 +322,27 @@ ipdpact_received (MMAtSerialPort *port, static MMModemAccessTechnology nwstate_to_act (const gchar *str) { +MMModemAccessTechnology technologies = 0; + /* small 'g' means CS, big 'G' means PS */ -if (!strcmp (str, 2g)) -return MM_MODEM_ACCESS_TECHNOLOGY_GSM; -else if (!strcmp (str, 2G-GPRS)) -return MM_MODEM_ACCESS_TECHNOLOGY_GPRS; -else if (!strcmp (str, 2G-EDGE)) -return MM_MODEM_ACCESS_TECHNOLOGY_EDGE; -else if (!strcmp (str, 3G)) -return MM_MODEM_ACCESS_TECHNOLOGY_UMTS; -else if (!strcmp (str, 3g)) -return MM_MODEM_ACCESS_TECHNOLOGY_UMTS; -else if (!strcmp (str, R99)) -return MM_MODEM_ACCESS_TECHNOLOGY_UMTS; -else if (!strcmp (str, 3G-HSDPA) || !strcmp (str, HSDPA)) -return MM_MODEM_ACCESS_TECHNOLOGY_HSDPA; -else if (!strcmp (str, 3G-HSUPA) || !strcmp (str, HSUPA)) -return MM_MODEM_ACCESS_TECHNOLOGY_HSUPA; -else if (!strcmp (str, 3G-HSDPA-HSUPA) || !strcmp (str, HSDPA-HSUPA)) -return MM_MODEM_ACCESS_TECHNOLOGY_HSPA; - -return MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN; +if (strstr (str, 2g) || strstr (str, 2G)) +technologies |= MM_MODEM_ACCESS_TECHNOLOGY_GSM; +if (strstr (str, GPRS)) +technologies |= MM_MODEM_ACCESS_TECHNOLOGY_GPRS; +if (strstr (str, EDGE)) +technologies |= MM_MODEM_ACCESS_TECHNOLOGY_EDGE; +if (strstr (str, 3g) || strstr (str, 3G) || strstr (str, R99)) +technologies |= MM_MODEM_ACCESS_TECHNOLOGY_UMTS; +if (strstr (str, HSDPA-HSUPA) || strstr (str, HSUPA-HSDPA)) +technologies |= MM_MODEM_ACCESS_TECHNOLOGY_HSPA; +if (strstr (str, HSDPA)) +technologies |= MM_MODEM_ACCESS_TECHNOLOGY_HSDPA; +if (strstr (str, HSUPA)) +technologies |= MM_MODEM_ACCESS_TECHNOLOGY_HSUPA; +if (strstr (str, HSPA+)) +technologies |= MM_MODEM_ACCESS_TECHNOLOGY_HSPA_PLUS; + +return technologies; } static void -- 1.7.7.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [PATCH] icera: improve parsing of access technologies in %NWSTATE response
On Tue, Oct 16, 2012 at 12:49 AM, Aleksander Morgado aleksan...@lanedo.com wrote: I pushed this patch as is, but then I realized that the report of current access technologies is supposed to give which is the *current* access technology being active. We allow reporting more than one for the cases where several access technologies are given simultaneously (e.g. cdma1x + evdo + lte). So not sure if there is much benefit in giving e.g. 4 different technologies like umts, hsdpa, hsupa, hspa when the modem reports 3G-HSDPA-HSUPA. In this example, umts shouldn't really be given I guess, and also we know that HSPA is just HSDPA+HSUPA, so giving HSPA only was enough... so... should I revert the patch? :-/ I guess that makes sense :) The previous patch tries to address your comment in crosbug.com/33483. Let's revert it. The real issue in crosbug.com/33483 is that nwstate_to_act doesn't handle HSPA+, so I think we can fix it as follows: Thanks, Ben From f90c8bea9dae89aceb6d7d0cf037cbc4bc73d2df Mon Sep 17 00:00:00 2001 From: Ben Chan benc...@chromium.org Date: Tue, 16 Oct 2012 00:55:40 -0700 Subject: [PATCH] icera: parse HSPA+ access technology in %NWSTATE response --- plugins/icera/mm-broadband-modem-icera.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/plugins/icera/mm-broadband-modem-icera.c b/plugins/icera/mm-broadband-modem-icera.c index db70252..697e8cc 100644 --- a/plugins/icera/mm-broadband-modem-icera.c +++ b/plugins/icera/mm-broadband-modem-icera.c @@ -341,6 +341,8 @@ nwstate_to_act (const gchar *str) return MM_MODEM_ACCESS_TECHNOLOGY_HSUPA; else if (!strcmp (str, 3G-HSDPA-HSUPA) || !strcmp (str, HSDPA-HSUPA)) return MM_MODEM_ACCESS_TECHNOLOGY_HSPA; +else if (!strcmp (str, 3G-HSDPA-HSUPA-HSPA+) || !strcmp (str, HSDPA-HSUPA-HSPA+)) +return MM_MODEM_ACCESS_TECHNOLOGY_HSPA_PLUS; return MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN; } -- 1.7.7.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] glib assertions
FYI, while running suspend/resume stress tests on various modems, MM gives a few glib assertion warnings. I'm going to run gdb to get a backtrace, but wonder if you know where the issue could potentially be. (ModemManager:648): GLib-GIO-CRITICAL **: g_dbus_connection_emit_signal: assertion `G_IS_DBUS_CONNECTION (connection)' failed (ModemManager:648): GLib-GObject-CRITICAL **: g_object_ref: assertion `G_IS_OBJECT (object)' failed (ModemManager:648): GLib-GObject-CRITICAL **: g_object_unref: assertion `G_IS_OBJECT (object)' failed Also, I had to take out the following assertion in MMBroadbandModem disabling_step() to prevent a crash. It seems like a race when mm_base_modem_disable() is being called on MMBaseModem and MMBroardbandModem is being disposed. Should we turn the assertion into a condition check instead? case DISABLING_STEP_IFACE_MODEM: g_assert (ctx-self-priv-modem_dbus_skeleton != NULL); Thread 0 (crashed) 0 libc-2.15.so!raise [raise.c : 64 + 0x10] rbx = 0x7fcfdec08260 r12 = 0x7fcf52c4 r13 = 0x7fcfdec073e0 r14 = 0x7fcfdd8d3960 r15 = 0x7fcfdeabd800 rip = 0x7fcfdd224aa5 rsp = 0x7fff300c3e18 rbp = 0x7fcfdd8d4260 Found by: given as instruction pointer in context 1 libc-2.15.so!abort [abort.c : 91 + 0x9] rbx = 0x7fcfdec08260 r12 = 0x7fcf52c4 r13 = 0x7fcfdec073e0 r14 = 0x7fcfdd8d3960 r15 = 0x7fcfdeabd800 rip = 0x7fcfdd225f07 rsp = 0x7fff300c3e20 rbp = 0x7fcfdd8d4260 Found by: call frame info 2 libglib-2.0.so.0.3200.4!g_assertion_message [gtestutils.c : 1861 + 0x4] rbx = 0x7fcfdec08260 r12 = 0x7fcf52c4 r13 = 0x7fcfdec073e0 r14 = 0x7fcfdd8d3960 r15 = 0x7fcfdeabd800 rip = 0x7fcfdd81b83d rsp = 0x7fff300c3f50 rbp = 0x7fcfdd8d4260 Found by: call frame info 3 libglib-2.0.so.0.3200.4!g_assertion_message_expr [gtestutils.c : 1872 + 0xc] rbx = 0x7fcfdddc52d0 r12 = 0x7fcf52c4 r13 = 0x r14 = 0x7fcfdd8d3960 r15 = 0x7fcfdeabd800 rip = 0x7fcfdd81bd12 rsp = 0x7fff300c3fe0 rbp = 0x1b45 Found by: call frame info 4 ModemManager!disabling_step [mm-broadband-modem.c : 6981 + 0x20] rbx = 0x7fcfdec0ed00 r12 = 0x0041 r13 = 0x7fcfdeac4f00 r14 = 0x7fcfdd8d3960 r15 = 0x7fcfdeabd800 rip = 0x7fcfddda6571 rsp = 0x7fff300c4010 rbp = 0x7fcfdeaf4200 Found by: call frame info 5 ModemManager!handle_enable_auth_ready [mm-iface-modem.c : 1216 + 0x4] rbx = 0x7fcfdec0fd30 r12 = 0x0041 r13 = 0x7fcfdeac4f00 r14 = 0x7fcfdd8d3960 r15 = 0x7fcfdeabd800 rip = 0x7fcfddd84379 rsp = 0x7fff300c4020 rbp = 0x7fcfdeaf4200 Found by: call frame info 6 libgio-2.0.so.0.3200.4!g_simple_async_result_complete [gsimpleasyncresult.c : 767 + 0xd] rbx = 0x7fcfdeb03e30 r12 = 0x0041 r13 = 0x7fcfdeac4f00 r14 = 0x7fcfdd8d3960 r15 = 0x7fcfdeabd800 rip = 0x7fcfddbd0447 rsp = 0x7fff300c4050 rbp = 0x7fcfdec0e650 Found by: call frame info 7 ModemManager!authorize_ready [mm-base-modem.c : 1015 + 0x7] rbx = 0x7fcfdeb03e30 r12 = 0x0041 r13 = 0x7fcfdeac4f00 r14 = 0x7fcfdd8d3960 r15 = 0x7fcfdeabd800 rip = 0x7fcfddd7a161 rsp = 0x7fff300c4070 rbp = 0x7fcfdec0e650 Found by: call frame info 8 libgio-2.0.so.0.3200.4!g_simple_async_result_complete [gsimpleasyncresult.c : 767 + 0xd] rbx = 0x7fcfdeb040f0 r12 = 0x0041 r13 = 0x7fcfdeac4f00 r14 = 0x7fcfdd8d3960 r15 = 0x7fcfdeabd800 rip = 0x7fcfddbd0447 rsp = 0x7fff300c4090 rbp = 0x7fcfdec0e650 Found by: call frame info 9 libgio-2.0.so.0.3200.4!complete_in_idle_cb [gsimpleasyncresult.c : 779 + 0x4] rbx = 0x7fcfdec0e650 r12 = 0x0041 r13 = 0x7fcfdeac4f00 r14 = 0x7fcfdd8d3960 r15 = 0x7fcfdeabd800 rip = 0x7fcfddbd0549 rsp = 0x7fff300c40b0 rbp = 0x0001 Found by: call frame info 10 libglib-2.0.so.0.3200.4!g_main_context_dispatch [gmain.c : 2539 + 0x3] rbx = 0x7fcfdec0e650 r12 = 0x0041 r13 = 0x7fcfdeac4f00 r14 = 0x7fcfdd8d3960 r15 = 0x7fcfdeabd800 rip = 0x7fcfdd7fa5c3 rsp = 0x7fff300c40c0 rbp = 0x0001 Found by: call frame info 11 libglib-2.0.so.0.3200.4!g_main_context_iterate [gmain.c : 3146 + 0x7] rbx = 0x7fcfdeabd800 r12 = 0x7fcfdd807d70 r13 = 0x r14 = 0x0002 r15 = 0x0001 rip = 0x7fcfdd7fa940 rsp = 0x7fff300c4150 rbp = 0x0001 Found by: call frame info 12
[MM] [PATCH] broadband-modem: increase timeout for ATZ
Some modem like Novatel LTE can take up to 5 seconds to complete the ATZ command. Increase the timeout for ATZ from 3 seconds to 6 seconds. --- src/mm-broadband-modem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c index 4a2fdb8..10100c8 100644 --- a/src/mm-broadband-modem.c +++ b/src/mm-broadband-modem.c @@ -2295,7 +2295,7 @@ static const MMBaseModemAtCommand modem_init_sequence[] = { * after the Z command because such commands may be ignored. * So run ATZ alone. */ -{ Z, 3, FALSE, mm_base_modem_response_processor_no_result_continue }, +{ Z, 6, FALSE, mm_base_modem_response_processor_no_result_continue }, /* Ensure echo is off after the init command */ { E0 V1, 3, FALSE, NULL }, -- 1.7.12 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] broadband-modem: disconnect bearers only if the bearer list still exists
This patch fixes a crash when MMBroadbandModem tries to access an already cleared bearer list during the disabling steps. Thread 0 *CRASHED* ( SIGSEGV @ 0x ) 0x7f6eed4c40a3 [ModemManager] - mm-bearer-list.c:259 mm_bearer_list_disconnect_all_bearers 0x7f6eed4cd6f8 [ModemManager] - mm-iface-modem.c:1216 handle_enable_auth_ready 0x7f6eed332676 [libgio-2.0.so.0.3000.2] - gsimpleasyncresult.c:749 g_simple_async_result_complete 0x7f6eed4c5750 [ModemManager] - mm-base-modem.c:1015 authorize_ready 0x7f6eed332676 [libgio-2.0.so.0.3000.2] - gsimpleasyncresult.c:749 g_simple_async_result_complete 0x7f6eed332788 [libgio-2.0.so.0.3000.2] - gsimpleasyncresult.c:761 complete_in_idle_cb 0x7f6eecf36f44 [libglib-2.0.so.0.3000.2] - gmain.c:2441 g_main_context_dispatch 0x7f6eecf37597 [libglib-2.0.so.0.3000.2] - gmain.c:3089 g_main_context_iterate 0x7f6eecf37b51 [libglib-2.0.so.0.3000.2] - gmain.c:3297 g_main_loop_run 0x7f6eed4b5ad1 [ModemManager] - main.c:150 main 0x7f6eec95141c [libc-2.15.so] - libc-start.c:234 __libc_start_main 0x7f6eed4b55e8 [ModemManager] + 0x0001a5e8 --- src/mm-broadband-modem.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c index 4a2fdb8..64df821 100644 --- a/src/mm-broadband-modem.c +++ b/src/mm-broadband-modem.c @@ -6883,11 +6883,15 @@ disabling_step (DisablingContext *ctx) ctx-step++; case DISABLING_STEP_DISCONNECT_BEARERS: -mm_bearer_list_disconnect_all_bearers ( -ctx-self-priv-modem_bearer_list, -(GAsyncReadyCallback)bearer_list_disconnect_all_bearers_ready, -ctx); -return; +if (ctx-self-priv-modem_bearer_list) { +mm_bearer_list_disconnect_all_bearers ( +ctx-self-priv-modem_bearer_list, +(GAsyncReadyCallback)bearer_list_disconnect_all_bearers_ready, +ctx); +return; +} +/* Fall down to next step */ +ctx-step++; case DISABLING_STEP_IFACE_SIMPLE: /* Fall down to next step */ -- 1.7.12 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] glib assertions
= 0x7fcfdeac7970 r12 = 0x0001 r13 = 0x r14 = 0x r15 = 0x rip = 0x7fcfdd7fad7a rsp = 0x7fff300c41a0 rbp = 0x0001 Found by: call frame info 13 ModemManager!main [main.c : 150 + 0x4] rbx = 0x7fcfdeabd010 r12 = 0x0001 r13 = 0x r14 = 0x r15 = 0x rip = 0x7fcfddd6a492 rsp = 0x7fff300c41c0 rbp = 0x0001 Found by: call frame info 14 libc-2.15.so!__libc_start_main [libc-start.c : 234 + 0x16] rbx = 0x r12 = 0x7fcfddd69f80 r13 = 0x7fff300c42e0 r14 = 0x r15 = 0x rip = 0x7fcfdd21141d rsp = 0x7fff300c4210 rbp = 0x Found by: call frame info 15 ModemManager + 0x1afa8 rbx = 0x r12 = 0x7fcfddd69f80 r13 = 0x7fff300c42e0 r14 = 0x r15 = 0x rip = 0x7fcfddd69fa9 rsp = 0x7fff300c42d0 rbp = 0x Found by: call frame info Thanks, Ben ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH v2] modem-helpers: handle the case when operator name is Unknown
Some modems report Unknown as the operator name when failed to obtain the actual value: -- 'AT+COPS=3,0;+COPS?CR' -- 'CRLF+COPS: 0,0,Unknown,0CRLFCRLFOKCRLF' This patch prevents Unknown from being treated as a valid operator name. --- src/mm-modem-helpers.c | 10 +- 1 files changed, 9 insertions(+), 1 deletions(-) diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c index db0bfbd..6692b6f 100644 --- a/src/mm-modem-helpers.c +++ b/src/mm-modem-helpers.c @@ -1468,7 +1468,15 @@ mm_3gpp_parse_operator (const gchar *reply, */ if (!g_utf8_validate (operator, -1, NULL)) { g_free (operator); -operator = NULL; +return NULL; +} + +/* Some modems (Novatel LTE) return the operator name as Unknown when + * it fails to obtain the operator name. Return NULL in such case. + */ +if (g_ascii_strcasecmp (operator, unknown) == 0) { +g_free (operator); +return NULL; } } -- 1.7.7.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] Power down during init
How long? I think the timeout is 3 seconds now so maybe it should be increased. could be up to 30s, which is likely a firmware issue. - Ben ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] modem-helpers: handle the case when operator name is Unknown
Some modems report Unknown as the operator name when failed to obtain the actual value: -- 'AT+COPS=3,0;+COPS?CR' -- 'CRLF+COPS: 0,0,Unknown,0CRLFCRLFOKCRLF' This patch prevents Unknown from being treated as a valid operator name. --- src/mm-modem-helpers.c | 12 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c index db0bfbd..9aa360d 100644 --- a/src/mm-modem-helpers.c +++ b/src/mm-modem-helpers.c @@ -1456,6 +1456,8 @@ mm_3gpp_parse_operator (const gchar *reply, } if (operator) { +gchar *casefolded_operator; + /* Some modems (Option HSO) return the operator name as a hexadecimal * string of the bytes of the operator name as encoded by the current * character set. @@ -1468,8 +1470,18 @@ mm_3gpp_parse_operator (const gchar *reply, */ if (!g_utf8_validate (operator, -1, NULL)) { g_free (operator); +return NULL; +} + +/* Some modems (Novatel LTE) return the operator name as Unknown when + * it fails to obtain the operator name. Return NULL in such case. + */ +casefolded_operator = g_utf8_casefold (operator, -1); +if (g_utf8_collate (casefolded_operator, unknown) == 0) { +g_free (operator); operator = NULL; } +g_free (casefolded_operator); } return operator; -- 1.7.7.3 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list