Yes, that's a good proposal. I will send an updated patch for review tomorrow.
Henrik
Den mån 14 mars 2022 kl 21:39 skrev Sergey Ryazanov <ryazanov....@gmail.com>:
>
> On Mon, Mar 14, 2022 at 9:03 PM Henrik Ginstmark <hen...@ginstmark.se> wrote:
> > Yes, I agree this will break backward compatibility. --get-message and
> > --delete-message should be quite easy to fix with still have SIM as
> > default storage, but for --list-messages I don´t have a clear view how
> > to add an optional argument, jet.
> > A quick fix could be just to add a new command: --list-messages-me.
>
> How about implementing the storage argument as an option for each of
> these commands? Like the SMSC option for the send message command.
> Something like this:
> --list-messages: List SMS messages
> --messages-storage <mem>: Messages storage (sim (default), me)
> --delete-message <id>: Delete SMS message at index <id>
> --messages-storage <mem>: Messages storage (sim (default), me)
> --get-message <id>: Get SMS message at index <id>
> --messages-storage <mem>: Messages storage (sim (default), me)
>
> > Den mån 14 mars 2022 kl 01:26 skrev Sergey Ryazanov
> > <ryazanov....@gmail.com>:
> >> Hello Henrik,
> >>
> >> On Sun, Mar 13, 2022 at 10:25 PM Henrik Ginstmark <hen...@ginstmark.se>
> >> wrote:
> >>> Today it's hard coded to read text messages from SIM card.
> >>> Not all devices store received text messages in SIM, they store
> >>> in me, QMI_WMS_STORAGE_TYPE_NV.
> >>> I have added a switch to choose sim or me for --list-messages,
> >>> --get-message and --delete-message.
> >>>
> >>> Signed-off-by: Henrik Ginstmarh <hen...@ginstmark.se>
> >>> ---
> >>> uqmi/src/commands-wms.c | 69 +++++++++++++++++++++++++++++++++--------
> >>> uqmi/src/commands-wms.h | 10 +++---
> >>> 2 files changed, 61 insertions(+), 18 deletions(-)
> >>>
> >>> diff --git a/uqmi/src/commands-wms.c b/uqmi/src/commands-wms.c
> >>> index a58fd6a..2bcc398 100644
> >>> --- a/uqmi/src/commands-wms.c
> >>> +++ b/uqmi/src/commands-wms.c
> >>> @@ -46,6 +46,14 @@ cmd_wms_list_messages_prepare(struct qmi_dev *qmi,
> >>> struct qmi_request *req, stru
> >>> QMI_INIT(message_tag,
> >>> QMI_WMS_MESSAGE_TAG_TYPE_MT_NOT_READ),
> >>> };
> >>>
> >>> + if (strcmp(arg, "sim") == 0) {
> >>> + } else if (strcmp(arg, "me") == 0) {
> >>> + qmi_set_ptr(&mreq, storage_type, QMI_WMS_STORAGE_TYPE_NV);
> >>> + } else {
> >>> + uqmi_add_error("Invalid value (sim or me)");
> >>> + return QMI_CMD_EXIT;
> >>> + }
> >>> +
> >>> qmi_set_wms_list_messages_request(msg, &mreq);
> >>>
> >>> return QMI_CMD_REQUEST;
> >>> @@ -283,20 +291,37 @@ static void blobmsg_add_hex(struct blob_buf *buf,
> >>> const char *name, unsigned con
> >>> static enum qmi_cmd_result
> >>> cmd_wms_delete_message_prepare(struct qmi_dev *qmi, struct qmi_request
> >>> *req, struct qmi_msg *msg, char *arg)
> >>> {
> >>> - char *err;
> >>> - int id;
> >>> -
> >>> - id = strtoul(arg, &err, 10);
> >>> - if (err && *err) {
> >>> - uqmi_add_error("Invalid message ID");
> >>> - return QMI_CMD_EXIT;
> >>> - }
> >>> -
> >>> static struct qmi_wms_delete_request mreq = {
> >>> QMI_INIT(memory_storage, QMI_WMS_STORAGE_TYPE_UIM),
> >>> QMI_INIT(message_mode, QMI_WMS_MESSAGE_MODE_GSM_WCDMA),
> >>> };
> >>>
> >>> + char *s;
> >>> + int id;
> >>> +
> >>> + s = strchr(arg, ',');
> >>> + if (!s) {
> >>> + uqmi_add_error("Invalid argument");
> >>> + return QMI_CMD_EXIT;
> >>> + }
> >>> + s = strtok(s, ",");
> >>> + if (!s) {
> >>> + uqmi_add_error("No message id");
> >>> + return QMI_CMD_EXIT;
> >>> + }
> >>> + id = strtoul(s, &s, 0);
> >>> + if (s && *s) {
> >>> + uqmi_add_error("Invalid message id");
> >>> + return QMI_CMD_EXIT;
> >>> + }
> >>> + if (strcmp(strtok(arg, ","), "sim") == 0) {
> >>> + } else if (strcmp(strtok(arg, ","), "me") == 0) {
> >>> + qmi_set_ptr(&mreq, memory_storage,
> >>> QMI_WMS_STORAGE_TYPE_NV);
> >>> + } else {
> >>> + uqmi_add_error("Invalid value (sim or me)");
> >>> + return QMI_CMD_EXIT;
> >>> + }
> >>> +
> >>> mreq.set.memory_index = 1;
> >>> mreq.data.memory_index = id;
> >>>
> >>> @@ -449,12 +474,30 @@ cmd_wms_get_message_prepare(struct qmi_dev *qmi,
> >>> struct qmi_request *req, struct
> >>> ),
> >>> QMI_INIT(message_mode, QMI_WMS_MESSAGE_MODE_GSM_WCDMA),
> >>> };
> >>> - char *err;
> >>> +
> >>> + char *s;
> >>> int id;
> >>>
> >>> - id = strtoul(arg, &err, 10);
> >>> - if (err && *err) {
> >>> - uqmi_add_error("Invalid message ID");
> >>> + s = strchr(arg, ',');
> >>> + if (!s) {
> >>> + uqmi_add_error("Invalid argument");
> >>> + return QMI_CMD_EXIT;
> >>> + }
> >>> + s = strtok(s, ",");
> >>> + if (!s) {
> >>> + uqmi_add_error("No message id");
> >>> + return QMI_CMD_EXIT;
> >>> + }
> >>> + id = strtoul(s, &s, 0);
> >>> + if (s && *s) {
> >>> + uqmi_add_error("Invalid message id");
> >>> + return QMI_CMD_EXIT;
> >>> + }
> >>> + if (strcmp(strtok(arg, ","), "sim") == 0) {
> >>> + } else if (strcmp(strtok(arg, ","), "me") == 0) {
> >>> + qmi_set_ptr(&mreq,
> >>> message_memory_storage_id.storage_type, QMI_WMS_STORAGE_TYPE_NV);
> >>> + } else {
> >>> + uqmi_add_error("Invalid value (sim or me)");
> >>> return QMI_CMD_EXIT;
> >>> }
> >>>
> >>> diff --git a/uqmi/src/commands-wms.h b/uqmi/src/commands-wms.h
> >>> index e28b97b..d79acd6 100644
> >>> --- a/uqmi/src/commands-wms.h
> >>> +++ b/uqmi/src/commands-wms.h
> >>> @@ -20,7 +20,7 @@
> >>> */
> >>>
> >>> #define __uqmi_wms_commands \
> >>> - __uqmi_command(wms_list_messages, list-messages, no,
> >>> QMI_SERVICE_WMS), \
> >>> + __uqmi_command(wms_list_messages, list-messages, required,
> >>> QMI_SERVICE_WMS), \
> >>> __uqmi_command(wms_delete_message, delete-message, required,
> >>> QMI_SERVICE_WMS), \
> >>> __uqmi_command(wms_get_message, get-message, required,
> >>> QMI_SERVICE_WMS), \
> >>> __uqmi_command(wms_get_raw_message, get-raw-message, required,
> >>> QMI_SERVICE_WMS), \
> >>> @@ -30,10 +30,10 @@
> >>> __uqmi_command(wms_send_message, send-message, required,
> >>> QMI_SERVICE_WMS)
> >>>
> >>> #define wms_helptext \
> >>> - " --list-messages: List SMS messages\n"
> >>> \
> >>> - " --delete-message <id>: Delete SMS message
> >>> at index <id>\n" \
> >>> - " --get-message <id>: Get SMS message at
> >>> index <id>\n" \
> >>> - " --get-raw-message <id>: Get SMS raw message
> >>> contents at index <id>\n" \
> >>> + " --list-messages <storage>: List SMS messages
> >>> <storage> (sim or me)\n" \
> >>
> >> Most modems still use SIM as SMS storage. By making the storage
> >> argument mandatory, we will require a user to spell a storage name,
> >> even if their modem does not support any other storages. Also this
> >> breaks backward compatibility. Should the storage argument be
> >> optional? Maybe even introduce a dedicated option to specify it.
> >>
> >>> + " --delete-message <storage>,<id>: Delete SMS message
> >>> at index\n" \
> >>> + " --get-message <storage>,<id>: Get SMS message at
> >>> index\n" \
> >>> + " --get-raw-message <storage>,<id>: Get SMS raw message
> >>> contents at index\n" \
> >>> " --send-message <data>: Send SMS message
> >>> (use options below)\n" \
> >>> " --send-message-smsc <nr>: SMSC number\n" \
> >>> " --send-message-target <nr>: Destination number
> >>> (required)\n" \
>
> --
> Sergey
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
