Hi Aleksander,

I'll create a merge request for the same. But I see a problem:

Comment in libmm-glib/mm-sms.c :mm_sms_get_timestamp() states that the time 
returned is ISO8601 complaint. However, ISO8601 mandates that year must be in 
YYYY format and not just YY. What we are showing in timestamp field is 
YYMMDDHHMMSS+ZZ. This is more in-line with the 7 octet timestamp received in 
SMS PDU and also shown using AT Command:

AT+CMGR=1
+CMGR: "REC UNREAD","+85291234567",,"07/02/18,00:05:10+32"

OK

Now, we cannot do YYYY as SMS PDU only has YY.

More important, you will notice in above AT command output that the timezone is 
"+32" which is actually "Quarters" and not "Hours" as MM is showing now

So we've below ways to fix this:

1) change sms_decode_timestamp() to populate +ZZ as "quarters" and not "hours". 
This then becomes in-line with AT command reporting.
2) change format to YYMMDDHHMMSS+HHMM

So what is the preference?

Thanks
Amol


The information in this email communication (inclusive of attachments) is 
confidential to 4RF Limited and the intended recipient(s). If you are not the 
intended recipient(s), please note that any use, disclosure, distribution or 
copying of this information or any part thereof is strictly prohibited and that 
the author accepts no liability for the consequences of any action taken on the 
basis of the information provided. If you have received this email in error, 
please notify the sender immediately by return email and then delete all 
instances of this email from your system. 4RF Limited will not accept 
responsibility for any consequences associated with the use of this email 
(including, but not limited to, damages sustained as a result of any viruses 
and/or any action or lack of action taken in reliance on it).-----Original 
Message-----
From: Aleksander Morgado <[email protected]>
Sent: Wednesday, 18 September 2019 6:07 PM
To: Amol Lad <[email protected]>
Cc: ModemManager (development) <[email protected]>
Subject: Re: [PATCH] Include minutes in SMS timezone field

Hey Amol

On Wed, Sep 18, 2019 at 2:00 PM Amol Lad <[email protected]> wrote:
>
> Hi,
>
> Below is a sample received SMS. I'm in timezone "+0530" but the timestamp 
> only shows "+05". I've attempted to fix this in master. Thus, if the timezone 
> field notes not include minutes then it will be shows as "+hh" and if it 
> includes minutes then it will be shown as "+hhmm"
>
> # mmcli -s 1
>   -----------------------
>   General    | dbus path: /org/freedesktop/ModemManager1/SMS/1
>   -----------------------
>   Content    |    number: +919591657038
>              |      text: Hello lte
>   -----------------------
>   Properties |  pdu type: deliver
>              |     state: received
>              |   storage: me
>              |      smsc: +919845686186
>              | timestamp: 190918143442+05
>
>

This email cannot be applied with "git am" I'm afraid, could you send the patch 
as an attachment, or even better, open a new Merge Request in gitlab?
https://gitlab.freedesktop.org/mobile-broadband/ModemManager


> From d00803e9625831cb27a4e2cacb941fed991baa68 Mon Sep 17 00:00:00 2001
>
> The information in this email communication (inclusive of attachments)
> is confidential to 4RF Limited and the intended recipient(s). If you
> are not the intended recipient(s), please note that any use,
> disclosure, distribution or copying of this information or any part
> thereof is strictly prohibited and that the author accepts no
> liability for the consequences of any action taken on the basis of the
> information provided. If you have received this email in error, please
> notify the sender immediately by return email and then delete all
> instances of this email from your system. 4RF Limited will not accept
> responsibility for any consequences associated with the use of this
> email (including, but not limited to, damages sustained as a result of
> any viruses and/or any action or lack of action taken in reliance on
> it).From: "amol.lad" <[email protected]>
> Date: Wed, 18 Sep 2019 17:16:31 +0530
> Subject: [PATCH] Include minutes in sms timezone field
>
> ---
>  src/mm-sms-part-3gpp.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/src/mm-sms-part-3gpp.c b/src/mm-sms-part-3gpp.c index
> 7e3f5374..60a7ae0b 100644
> --- a/src/mm-sms-part-3gpp.c
> +++ b/src/mm-sms-part-3gpp.c
> @@ -159,20 +159,25 @@ sms_decode_address (const guint8 *address, int
> len)  static char *  sms_decode_timestamp (const guint8 *timestamp)  {
> -    /* YYMMDDHHMMSS+ZZ */
> +    /* YYMMDDHHMMSS+ZZ or YYMMDDHHMMSS+ZZZZ */
>      char *timestr;
> -    int quarters, hours;
> +    int quarters, hours, mins;
>
> -    timestr = g_malloc0 (16);
> +    timestr = g_malloc0 (18);
>      sms_semi_octets_to_bcd_string (timestr, timestamp, 6);
>      quarters = ((timestamp[6] & 0x7) * 10) + ((timestamp[6] >> 4) & 0xf);
>      hours = quarters / 4;
> +    mins = (quarters % 4) * 15;
>      if (timestamp[6] & 0x08)
>          timestr[12] = '-';
>      else
>          timestr[12] = '+';
>      timestr[13] = (hours / 10) + '0';
>      timestr[14] = (hours % 10) + '0';
> +    if (mins) {
> +        timestr[15] = (mins / 10) + '0';
> +        timestr[16] = (mins % 10) + '0';
> +    }
>      /* TODO(njw): Change timestamp rep to something that includes 
> quarter-hours */
>      return timestr;

You can also remove that old TODO comment because you're actually implementing 
the missing bits, right?

>  }
> --
> 2.17.1
>
> _______________________________________________
> ModemManager-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel



--
Aleksander
https://aleksander.es
_______________________________________________
ModemManager-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel

Reply via email to