SIM power patch
Aleksander Morgado
aleksander at aleksander.es
Wed Dec 7 18:34:34 UTC 2016
Hey Kasper,
>
> I created a patch for libqmi/qmicli, for powering up/down the SIM card:
>
> --uim-sim-power=[(ON|OFF),(slot number)]
> Power on/off SIM card
>
> Slot number is 1-based index of the SIM slot in the modem - usually 1 for
> modems with 1 SIM interface.
>
> We are using this function to switch SIM card without resetting the modem.
>
> I have not commited any patches for libqmi before, so I'm not sure how to do
> it..? Also, as this is my first patch for this project, please let me know
> if anything needs to be changed.
>
> Tested with Sierra MC7304.
>
> The patch is attached to this email.
>
Superb, thanks!
See some comments inline below. Patch looks mostly good, but there are
several coding style issues to fix, plus some memleaks (we try to keep
the memleak report clean even in qmicli, so that valgrind reports of
qmicli help us find real leaks in libqmi-glib).
> From 28d9ed85961aae519cf529e259b2c17851b24f7c Mon Sep 17 00:00:00 2001
> From: Kasper Holtze <kasper at holtze.dk>
> Date: Wed, 7 Dec 2016 14:45:38 +0100
> Subject: [PATCH] Added --uim-sim-power option to power up or down the SIM
> card(s)
> ---
> data/qmi-service-uim.json | 34 +++++++++++-
> src/qmicli/qmicli-uim.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 171 insertions(+), 1 deletion(-)
> diff --git a/data/qmi-service-uim.json b/data/qmi-service-uim.json
> index e3f88d4..c95a330 100644
> --- a/data/qmi-service-uim.json
> +++ b/data/qmi-service-uim.json
> @@ -600,6 +600,38 @@
> { "name" : "PIN2 Retries",
> "format" : "guint8" },
> { "name" : "PUK2 Retries",
> - "format" : "guint8" } ] } } ] } } ] } ] }
> + "format" : "guint8" } ] } } ] } } ] } ] },
> +
> + // *********************************************************************************
> + { "name" : "Power OFF SIM",
Could we name it "Power Off SIM" ?
> + "type" : "Message",
> + "service" : "UIM",
> + "id" : "0x0030",
> + "version" : "1.0",
> + "input" : [ { "name" : "Slot",
> + "id" : "0x01",
> + "mandatory" : "yes",
> + "type" : "TLV",
> + "format" : "sequence",
> + "contents" : [ { "name" : "Slot",
> + "format" : "guint8" } ] }
> + ],
> + "output" : [ { "common-ref" : "Operation Result" } ] },
> +
> + // *********************************************************************************
> + { "name" : "Power ON SIM",
Could we name it "Power On SIM" ?
> + "type" : "Message",
> + "service" : "UIM",
> + "id" : "0x0031",
> + "version" : "1.0",
> + "input" : [ { "name" : "Slot",
> + "id" : "0x01",
> + "mandatory" : "yes",
> + "type" : "TLV",
> + "format" : "sequence",
> + "contents" : [ { "name" : "Slot",
> + "format" : "guint8" } ] }
> + ],
> + "output" : [ { "common-ref" : "Operation Result" } ] }
>
> ]
> diff --git a/src/qmicli/qmicli-uim.c b/src/qmicli/qmicli-uim.c
> index 453b46d..7df1556 100644
> --- a/src/qmicli/qmicli-uim.c
> +++ b/src/qmicli/qmicli-uim.c
> @@ -48,6 +48,7 @@ static gchar *verify_pin_str;
> static gchar *unblock_pin_str;
> static gchar *change_pin_str;
> static gchar *get_file_attributes_str;
> +static gchar *sim_power_str;
> static gboolean get_card_status_flag;
> static gboolean get_supported_messages_flag;
> static gboolean reset_flag;
> @@ -86,6 +87,10 @@ static GOptionEntry entries[] = {
> "Get supported messages",
> NULL
> },
> + { "uim-sim-power", 0, 0, G_OPTION_ARG_STRING, &sim_power_str,
> + "Power on/off SIM card",
> + "[(ON|OFF),(slot number)]"
(on|off)
> + },
Also, please don't use tabs to indent.
> { "uim-reset", 0, 0, G_OPTION_ARG_NONE, &reset_flag,
> "Reset the service state",
> NULL
> @@ -127,6 +132,7 @@ qmicli_uim_options_enabled (void)
> !!change_pin_str +
> !!read_transparent_str +
> !!get_file_attributes_str +
> + !!sim_power_str +
This doesn't seem to be properly aligned? Please don't use tabs
> get_card_status_flag +
> get_supported_messages_flag +
> reset_flag +
> @@ -557,6 +563,98 @@ get_supported_messages_ready (QmiClientUim *client,
> operation_shutdown (TRUE);
> }
>
> +static QmiMessageUimPowerOnSimInput *
> +power_on_sim_input_create (guint slot)
"guint8 slot" directly?
> +{
> + QmiMessageUimPowerOnSimInput *input;
> + GError *error = NULL;
> +
> + input = qmi_message_uim_power_on_sim_input_new ();
> +
> + if (!qmi_message_uim_power_on_sim_input_set_slot (input, slot, &error)) {
> + g_printerr ("error: could not create SIM power on input: %s\n", error->message);
> + g_error_free (error);
> + qmi_message_uim_power_on_sim_input_unref(input);
Whitespace before parenthesis.
> + input = NULL;
> + }
> +
> + return input;
> +}
> +
> +static void power_on_sim_ready (QmiClientUim *client,
> + GAsyncResult *res)
> +{
> + QmiMessageUimPowerOnSimOutput *output;
> + GError *error = NULL;
> +
> + output = qmi_client_uim_power_on_sim_finish (client, res, &error);
> + if(!output) {
Whitespace after the if and before parenthesis.
> + g_printerr ("error: operation failed: %s\n", error->message);
> + g_error_free (error);
> + operation_shutdown (FALSE);
> + return;
> + }
> +
> + if(!qmi_message_uim_power_on_sim_output_get_result (output, &error)) {
Whitespace after the if and before parenthesis.
> + g_printerr ("error: could not power on SIM: %s\n", error->message);
> + g_error_free (error);
Not a big deal, but:
qmi_message_uim_power_on_sim_output_unref (output);
> + operation_shutdown (FALSE);
> + return;
> + }
> +
> + g_print ("[%s] Successfully performed SIM power on\n",
> + qmi_device_get_path_display (ctx->device));
> +
> + qmi_message_uim_power_on_sim_output_unref (output);
> + operation_shutdown (TRUE);
> +}
> +
> +static QmiMessageUimPowerOffSimInput *
> +power_off_sim_input_create (guint slot)
"guint8 slot" maybe?
> +{
> + QmiMessageUimPowerOffSimInput *input;
> + GError *error = NULL;
> +
> + input = qmi_message_uim_power_off_sim_input_new ();
> +
> + if(!qmi_message_uim_power_off_sim_input_set_slot (input, slot, &error)) {
Whitespace after the if and before parenthesis.
> + g_printerr ("error: could not create SIM power off input: %s\n", error->message);
> + g_error_free (error);
> + qmi_message_uim_power_off_sim_input_unref(input);
Whitespace before parenthesis.
> + input = NULL;
> + }
> +
> + return input;
> +}
> +
> +static void power_off_sim_ready (QmiClientUim *client,
> + GAsyncResult *res)
> +{
> + QmiMessageUimPowerOffSimOutput *output;
> + GError *error = NULL;
> +
> + output = qmi_client_uim_power_off_sim_finish (client, res, &error);
> + if (!output) {
> + g_printerr ("error: operation failed: %s\n", error->message);
> + g_error_free (error);
> + operation_shutdown (FALSE);
> + return;
> + }
> +
> + if (!qmi_message_uim_power_off_sim_output_get_result (output, &error)) {
> + g_printerr ("error: could not power off SIM: %s\n", error->message);
> + g_error_free (error);
> + operation_shutdown (FALSE);
Not a big deal, but:
qmi_message_uim_power_off_sim_output_unref (output);
> + return;
> + }
> +
> + g_print ("[%s] Successfully performed SIM power off\n",
> + qmi_device_get_path_display(ctx->device));
> +
> + qmi_message_uim_power_off_sim_output_unref(output);
Whitespace before parenthesis.
> + operation_shutdown(TRUE);
Whitespace before parenthesis.
> +}
> +
> static void
> reset_ready (QmiClientUim *client,
> GAsyncResult *res)
> @@ -1219,6 +1317,46 @@ qmicli_uim_run (QmiDevice *device,
> return;
> }
>
> + /* Request to power on/off SIM card? */
> + if (sim_power_str) {
> + QmiMessageUimPowerOnSimInput *qmiMessageUimPowerOnSimInput;
> + QmiMessageUimPowerOffSimInput *qmiMessageUimPowerOffSimInput;
Don't add the input bundles here, declare them after checking the
actual action, see below.
> + gchar **split;
> + gchar *power;
> + guint slot;
> +
> + split = g_strsplit (sim_power_str, ",", -1);
> + qmicli_read_non_empty_string (split[0], "power state", &power);
> + qmicli_read_uint_from_string (split[1], &slot);
Don't leak "split":
g_strfreev (split);
Also,
if (slot > G_MAXUINT8) {
/* print error */
}
> +
> + if (g_str_equal (power, "ON")) {
Instead of g_str_equal() use !g_ascii_strcasecmp() and then you'll
allow both ON and on.
Declare "on" input here, and don't use camelCasedNames:
QmiMessageUimPowerOnSimInput *input;
> + g_debug ("Asynchronously power on SIM card");
> +
> + qmiMessageUimPowerOnSimInput = power_on_sim_input_create (slot);
> + qmi_client_uim_power_on_sim (ctx->client,
> + qmiMessageUimPowerOnSimInput,
Alignment of previous element isn't ok, please don't use tabs.
> + 10,
> + ctx->cancellable,
> + (GAsyncReadyCallback)power_on_sim_ready,
> + NULL);
g_free (power);
> + return;
> + } else if (g_str_equal (power, "OFF")) {
Instead of g_str_equal() use !g_ascii_strcasecmp() and then you'll
allow both OFF and off.
Declare "off" input here and also no camelCasedNames:
QmiMessageUimPowerOffSimInput *input;
> + g_debug ("Asynchronously power off SIM card");
> + qmiMessageUimPowerOffSimInput = power_off_sim_input_create (slot);
> + qmi_client_uim_power_off_sim (ctx->client,
> + qmiMessageUimPowerOffSimInput,
Alignment of previous element isn't ok, please don't use tabs.
> + 10,
> + ctx->cancellable,
> + (GAsyncReadyCallback)power_off_sim_ready,
> + NULL);
g_free (power);
> + return;
> + } else {
> + g_printerr ("error: expected 'ON' or 'OFF', got: %s\n", power);
'on' or 'off'
> + operation_shutdown (FALSE);
> + return;
> + }
> + }
> +
> /* Request to reset UIM service? */
> if (reset_flag) {
> g_debug ("Asynchronously resetting UIM service...");
> --
> 2.7.4
--
Aleksander
https://aleksander.es
More information about the libqmi-devel
mailing list