[uqmi] dms: add --get-operating-mode

[Sergey Ryazanov]

Hello Henrik,

On Sat, Dec 4, 2021 at 6:22 PM Henrik Ginstmark <henrik at ginstmark.se> wrote:
> This command make it possible to query if the modem is in flight mode or not.
>
> If you need to change your APN setting it should be done during flight mode on.

Just curious, is this true for any Qualcomm based mode or only for a
specific model?

> To make it possible to automate change of APN setting --get-operating-mode is
> needed.
>
> Signed-off-by: Henrik Ginstmark <henrik at ginstmark.se>

Your patch seems broken. Consider using git-format-patch and
git-send-email to prepare and submit the patch, please.

BTW, it is a good idea to keep the 'PATCH' word in the subject line,
so patchwork (https://patchwork.ozlabs.org/project/openwrt/list/) will
be able to catch your patch. Something like this:

[PATCH uqmi] dms: add --get-operating-mode

See also a couple of nit picks below.

> commands-dms.c
>
> @@ -375,6 +375,38 @@ cmd_dms_reset_prepare(struct qmi_dev *qmi, struct
> qmi_request *req, struct qmi_m
>     return QMI_CMD_REQUEST;
> }
>
> +static void
> +cmd_dms_get_operating_mode_cb(struct qmi_dev *qmi, struct qmi_request
> *req, struct qmi_msg *msg)
> +{
> +    struct qmi_dms_get_operating_mode_response res;
> +

This empty line is odd.

> +    const char *modes[] = {

You could reuse 'modes' from the cmd_dms_set_operating_mode_prepare()
to avoid duplication. Just move the array out from the function and
call it something like 'oper_modes'.

> +        [QMI_DMS_OPERATING_MODE_ONLINE] = "online",
> +        [QMI_DMS_OPERATING_MODE_LOW_POWER] = "low_power",
> +        [QMI_DMS_OPERATING_MODE_FACTORY_TEST] = "factory_test",
> +        [QMI_DMS_OPERATING_MODE_OFFLINE] = "offline",
> +        [QMI_DMS_OPERATING_MODE_RESET] = "reset",
> +        [QMI_DMS_OPERATING_MODE_SHUTTING_DOWN] = "shutting_down",
> +        [QMI_DMS_OPERATING_MODE_PERSISTENT_LOW_POWER] = "persistent_low_power",
> +        [QMI_DMS_OPERATING_MODE_MODE_ONLY_LOW_POWER] = "mode_only_low_power",
> +    };
> +    int s = 0;

This is a magic number usage. Also if a modem returns unknown value
then the code below will report that the modem is 'online'. What can
be misleading. See the 'get_pin_status()' function for an example of
an unknown value handling.

> +
> +    qmi_parse_dms_get_operating_mode_response(msg, &res);
> +    if (res.set.mode &&

This part of the check looks odd. If res.set.mode is zero you skip the
's' variable updation, but s is initialized with zero during the
definition.

> +        res.data.mode < ARRAY_SIZE(modes))
> +        s = res.data.mode;
> +
> +    blobmsg_add_string(&status, NULL, modes[s]);
> +}
> +
> +static enum qmi_cmd_result
> +cmd_dms_get_operating_mode_prepare(struct qmi_dev *qmi, struct
> qmi_request *req, struct qmi_msg *msg, char *arg)
> +{
> +    qmi_set_dms_get_operating_mode_request(msg);
> +    return QMI_CMD_REQUEST;
> +}
> +
> #define cmd_dms_set_operating_mode_cb no_cb
> static enum qmi_cmd_result
> cmd_dms_set_operating_mode_prepare(struct qmi_dev *qmi, struct
> qmi_request *req, struct qmi_msg *msg, char *arg)
>
>
> ---
>
> commands-dms.h
>
> @@ -37,6 +37,7 @@
>     __uqmi_command(dms_get_imsi, get-imsi, no, QMI_SERVICE_DMS), \
>     __uqmi_command(dms_get_imei, get-imei, no, QMI_SERVICE_DMS), \
>     __uqmi_command(dms_get_msisdn, get-msisdn, no, QMI_SERVICE_DMS), \
> +   __uqmi_command(dms_get_operating_mode, get-device-operating-mode,
> no, QMI_SERVICE_DMS), \
>     __uqmi_command(dms_set_operating_mode, set-device-operating-mode,
> required, QMI_SERVICE_DMS), \
>     __uqmi_command(dms_reset, reset-dms, no, QMI_SERVICE_DMS), \
>     __uqmi_command(dms_set_fcc_authentication, fcc-auth, no, QMI_SERVICE_DMS) \
> @@ -67,6 +68,7 @@
>         "  --get-imei:                       Get International Mobile
> Equipment ID\n" \
>         "  --get-msisdn:                     Get the MSISDN (telephone
> number)\n" \
>         "  --reset-dms:                      Reset the DMS service\n" \
> +       "  --get-device-operating-mode       Get the device operating mode\n" \
>         "  --set-device-operating-mode <m>   Set the device operating mode\n" \
>         "                                    (modes: online,
> low_power, factory_test, offline\n" \
>         "                                     reset, shutting_down,
> persistent_low_power,\n" \

-- 
Sergey