gobi2000: Reading enabled locks
Aleksander Morgado
aleksander at aleksander.es
Wed Nov 12 03:20:24 PST 2014
On 12/11/14 11:09, Torsten Hilbrich wrote:
> Am 11.11.2014 um 17:11 schrieb Aleksander Morgado:
>> This wouldn't be the correct approach I'm afraid.
>>
>> This case is very similar to the SMS messaging implementation in QMI
>> modems, where QMI is used if WMS is supported and otherwise we
>> fallback to AT-based SMS messaging see:
>> http://cgit.freedesktop.org/ModemManager/ModemManager/commit/src/mm-broadband-modem-qmi.c?id=fcede1a80acfdc299c5aa0015012a68846dfc763
>>
>> So, instead of subclassing a whole new object, the generic
>> MMBroadbandModemQmi should: if qmi_client_dms_uim_get_ck_status()
>> works, use that approach. If qmi_client_dms_uim_get_ck_status() fails
>> because the command isn't supported, then call parent interface's
>> load_enabled_facility_locks(), which is AT-based.
>
> I have now implemented an alternate approach. I noticed that the qmi get
> ck status command didn't have any support to retrieve the SIM lock
> facility (which in the AT+CLCK command is named SC). So the current code
> simply cannot fill the MM_MODEM_3GPP_FACILITY_SIM bit.
>
> However, this information is available as part of the pin status. So I
> added a new function to retrieve the PIN enabled/disabled state based on
> that PIN status. This function is called when the other facility locks
> were queried.
>
> Patch and logs of a test run with initially disabled PIN which is then
> set to enabled are attached. Of course, I also tested the opposite
> direction.
>
> These are the new debug messages added:
>
> <debug> [1415786280.215306] [mm-broadband-modem-qmi.c:3396]
> get_sim_lock_status_via_pin_status(): Retrieving PIN status to check for
> enabled PIN
> <debug> [1415786280.222673] [mm-broadband-modem-qmi.c:3371]
> get_sim_lock_status_via_pin_status_ready(): PIN is reported disabled
>
Ah, that makes sense, yes.
See review of the patch below.
> From c0f8cff0c414d04bdbb394da6acb1b82af0d2f12 Mon Sep 17 00:00:00 2001
> From: Torsten Hilbrich <torsten.hilbrich at secunet.com>
> Date: Wed, 12 Nov 2014 10:38:44 +0100
> Subject: [PATCH] qmi: Retrieve MM_MODEM_3GPP_FACILITY_SIM
>
> This facility cannot retrieved via 'DMS UIM Get CK Status' and seems
> currently unimplemented for mm-broadband-modem-qmi. The SIM enabled
> status is however available via 'DMS UIM Get PIN Status'.
>
> Using this message to add the retrieval of PIN status just after the
> other facilities were queried.
>
> Succesfully tested with Gobi2000 (05c6:9205), both retrieval of 3gpp
> SIM lock status and enabling/disabling the PIN.
> ---
> src/mm-broadband-modem-qmi.c | 66 ++++++++++++++++++++++++++++++++++++++----
> src/mm-modem-helpers-qmi.c | 25 ++++++++++++++++
> src/mm-modem-helpers-qmi.h | 1 +
> 3 files changed, 87 insertions(+), 5 deletions(-)
>
> diff --git a/src/mm-broadband-modem-qmi.c b/src/mm-broadband-modem-qmi.c
> index 277d43d..e3457eb 100644
> --- a/src/mm-broadband-modem-qmi.c
> +++ b/src/mm-broadband-modem-qmi.c
> @@ -3345,6 +3345,66 @@ modem_3gpp_load_enabled_facility_locks_finish (MMIfaceModem3gpp *self,
> }
>
> static void
> +get_sim_lock_status_via_pin_status_ready (QmiClientDms *client,
> + GAsyncResult *res,
> + LoadEnabledFacilityLocksContext *ctx)
> +{
> + QmiMessageDmsUimGetPinStatusOutput *output;
> + GError *error = NULL;
> + gboolean enabled;
> +
> + output = qmi_client_dms_uim_get_pin_status_finish (client, res, &error);
'error' is leaking if qmi_client_dms_uim_get_pin_status_finish() fails.
You probably don't want to know what error was, so you can just pass
NULL instead of the &error.
> + if (!output ||
> + !qmi_message_dms_uim_get_pin_status_output_get_result (output, &error)) {
'error' is leaking if
qmi_message_dms_uim_get_pin_status_output_get_result() fails. You
probably don't want to know what error was, so you can just pass NULL
instead of the &error.
And as 'error' is no longer used, just remove the variable as well :)
> + mm_dbg ("Couldn't query PIN status, assuming SIM PIN is disabled");
> + enabled = FALSE;
> + } else {
> + QmiDmsUimPinStatus current_status;
Whiteline here, between variables and first method.
> + if (qmi_message_dms_uim_get_pin_status_output_get_pin1_status (
> + output,
> + ¤t_status,
> + NULL, /* verify_retries_left */
> + NULL, /* unblock_retries_left */
> + NULL))
> + {
Put this closing brace in the same line as the NULL))
> + enabled = pin_enabled_from_qmi_uim_pin_status(current_status);
> + mm_dbg ("PIN is reported %s", (enabled ? "enabled" : "disabled"));
> + } else {
> + mm_dbg ("Couldn't find PIN1 status in the result, assuming SIM PIN is disabled");
> + enabled = FALSE;
> + }
'output' is leaking, you're missing the unref:
qmi_message_dms_uim_get_pin_status_output_unref (output);
> + }
> +
> + if (enabled) {
> + ctx->locks |= (MM_MODEM_3GPP_FACILITY_SIM);
> + } else {
> + ctx->locks &= ~(MM_MODEM_3GPP_FACILITY_SIM);
> + }
> +
> + /* No more facilities to query, all done */
> + g_simple_async_result_set_op_res_gpointer (ctx->result,
> + GUINT_TO_POINTER (ctx->locks),
> + NULL);
> + load_enabled_facility_locks_context_complete_and_free (ctx);
> +}
> +
> +/* the SIM lock cannot be queried with the qmi_get_ck_status function,
> + * therefore using the PIN status */
> +static void
> +get_sim_lock_status_via_pin_status (LoadEnabledFacilityLocksContext *ctx)
> +{
> + mm_dbg ("Retrieving PIN status to check for enabled PIN");
> + /* if the SIM is locked or not can only be queried by locking at
> + * the PIN status */
> + qmi_client_dms_uim_get_pin_status (QMI_CLIENT_DMS (ctx->client),
> + NULL,
> + 5,
> + NULL,
> + (GAsyncReadyCallback)get_sim_lock_status_via_pin_status_ready,
> + ctx);
> +}
> +
> +static void
> dms_uim_get_ck_status_ready (QmiClientDms *client,
> GAsyncResult *res,
> LoadEnabledFacilityLocksContext *ctx)
> @@ -3422,11 +3482,7 @@ get_next_facility_lock_status (LoadEnabledFacilityLocksContext *ctx)
> }
> }
>
> - /* No more facilities to query, all done */
> - g_simple_async_result_set_op_res_gpointer (ctx->result,
> - GUINT_TO_POINTER (ctx->locks),
> - NULL);
> - load_enabled_facility_locks_context_complete_and_free (ctx);
> + get_sim_lock_status_via_pin_status(ctx);
> }
>
> static void
> diff --git a/src/mm-modem-helpers-qmi.c b/src/mm-modem-helpers-qmi.c
> index c673697..df334c3 100644
> --- a/src/mm-modem-helpers-qmi.c
> +++ b/src/mm-modem-helpers-qmi.c
> @@ -98,6 +98,31 @@ mm_modem_lock_from_qmi_uim_pin_status (QmiDmsUimPinStatus status,
>
> /*****************************************************************************/
>
> +gboolean pin_enabled_from_qmi_uim_pin_status(QmiDmsUimPinStatus status)
Add a "mm_" prefix to the method name so that we know it's not static;
also put the return type in its own line when defining the function. And
missing whitespace before the open parenthesis :)
gboolean
mm_pin_enabled_from_qmi_uim_pin_status (QmiDmsUimPinStatus status)
> +{
> + switch (status) {
> + case QMI_DMS_UIM_PIN_STATUS_ENABLED_NOT_VERIFIED:
> + case QMI_DMS_UIM_PIN_STATUS_ENABLED_VERIFIED:
> + case QMI_DMS_UIM_PIN_STATUS_BLOCKED:
> + case QMI_DMS_UIM_PIN_STATUS_PERMANENTLY_BLOCKED:
> + case QMI_DMS_UIM_PIN_STATUS_UNBLOCKED:
> + case QMI_DMS_UIM_PIN_STATUS_CHANGED:
> + /* assume the PIN to be enabled then */
> + return TRUE;
> +
> + case QMI_DMS_UIM_PIN_STATUS_DISABLED:
> + case QMI_DMS_UIM_PIN_STATUS_NOT_INITIALIZED:
> + /* assume the PIN to be disabled then */
> + return FALSE;
> +
> + default:
> + /* by default assume disabled */
> + return FALSE;
> + }
> +}
> +
> +/*****************************************************************************/
> +
> QmiDmsUimFacility
> mm_3gpp_facility_to_qmi_uim_facility (MMModem3gppFacility mm)
> {
> diff --git a/src/mm-modem-helpers-qmi.h b/src/mm-modem-helpers-qmi.h
> index fb3ef8d..70e13e3 100644
> --- a/src/mm-modem-helpers-qmi.h
> +++ b/src/mm-modem-helpers-qmi.h
> @@ -31,6 +31,7 @@ MMModemMode mm_modem_mode_from_qmi_radio_interface (QmiDmsRadioInterface network
> MMModemLock mm_modem_lock_from_qmi_uim_pin_status (QmiDmsUimPinStatus status,
> gboolean pin1);
>
> +gboolean pin_enabled_from_qmi_uim_pin_status(QmiDmsUimPinStatus status);
The "mm_" prefix will also be needed here; plus the whitespace before
the open parenthesis.
> QmiDmsUimFacility mm_3gpp_facility_to_qmi_uim_facility (MMModem3gppFacility mm);
>
> GArray *mm_modem_bands_from_qmi_band_capabilities (QmiDmsBandCapability qmi_bands,
> --
> 1.7.10.4
--
Aleksander
https://aleksander.es
More information about the ModemManager-devel
mailing list