Re: [PATCH] GSM SMS reception code

2011-04-15 Thread Dan Williams
On Thu, 2011-04-14 at 15:39 -0700, Marcel Holtmann wrote:
 Hi Dan,
 
  2) Testcases for PDU decoding; best way to do this is to split out the
  bits of the SMS stuff that doesn't depend on anything MM-related like
  MMCallbackInfo and whatnot into a separate file, like mm-sms-utils.c or
  something, and build that file into libmodem_helpers_la_SOURCES in
  Makefile.am.  That makes it easier to link that code to the testcases
  without including a bunch of random stuff that isn't related to the unit
  tests.  Then we grab a bunch of real-world encoded SMS PDUs and shove
  them through the decoder and make sure it works on those, then we can
  start fuzzing it too and make sure it's more robust.  We'll really want
  testcases as there's quite a bit of variation between devices and
  frankly nobody interprets the standards the same way :(
 
 actually the SMS PDUs are pretty much standard since they are
 transmitted this way over the air.
 
 You are not gonna like this, bu you guys could save yourselves some
 trouble and just use src/smsutil.c from oFono since we have been through
 this whole exercise. Including the unit tests actually.

Probably, yeah.  There's a slight question of licensing since oFono is
GPLv2 while MM is currently v2+.  Still, it already is implemented in
oFono and the license thing isn't too big of an issue.

  4) The whole index thing, figuring out if we can use a generic index
  that's MM specific and maps internally to SIM indexes, or whether we can
  in fact just use the SIM index...
 
 You are going to enter a world of pain if you are trying to deal with
 SIM indexes or any kind of indexes for that matter. This whole indexes
 concept is pretty much ancient from around 1995. Even modern SIM cards
 are too limited for doing this properly.

Right, which is why I suggested instead using MM-specific indexes for
access via the D-Bus API and doing whatever is required internally to
present a clean external UI.  I believe the indexes in the MM D-Bus API
should not reference actual SIM messages, but should be synthetic.  I
don't think it's useful to present each SMS as a D-Bus object, and thus
indexes are the way to go.  But the indexes should be MM specific,
transient, and have no relationship to whatever happens inside MM to
operate on the SMSes.

Dan


___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [PATCH] GSM SMS reception code

2011-04-15 Thread Marcel Holtmann
Hi Dan,

   2) Testcases for PDU decoding; best way to do this is to split out the
   bits of the SMS stuff that doesn't depend on anything MM-related like
   MMCallbackInfo and whatnot into a separate file, like mm-sms-utils.c or
   something, and build that file into libmodem_helpers_la_SOURCES in
   Makefile.am.  That makes it easier to link that code to the testcases
   without including a bunch of random stuff that isn't related to the unit
   tests.  Then we grab a bunch of real-world encoded SMS PDUs and shove
   them through the decoder and make sure it works on those, then we can
   start fuzzing it too and make sure it's more robust.  We'll really want
   testcases as there's quite a bit of variation between devices and
   frankly nobody interprets the standards the same way :(
  
  actually the SMS PDUs are pretty much standard since they are
  transmitted this way over the air.
  
  You are not gonna like this, bu you guys could save yourselves some
  trouble and just use src/smsutil.c from oFono since we have been through
  this whole exercise. Including the unit tests actually.
 
 Probably, yeah.  There's a slight question of licensing since oFono is
 GPLv2 while MM is currently v2+.  Still, it already is implemented in
 oFono and the license thing isn't too big of an issue.

that is what you have to decide since the oFono license is not going to
change any time soon. We did GPLv2 on purpose.

We even saw the FSO project take the oFono SMS implementation and as a
result they had to move from LGPL to GPL.

The oFono SMS implementation is comprehensive and complete since we had
to actually build a phone worthy SMS support with all its gory details.

   4) The whole index thing, figuring out if we can use a generic index
   that's MM specific and maps internally to SIM indexes, or whether we can
   in fact just use the SIM index...
  
  You are going to enter a world of pain if you are trying to deal with
  SIM indexes or any kind of indexes for that matter. This whole indexes
  concept is pretty much ancient from around 1995. Even modern SIM cards
  are too limited for doing this properly.
 
 Right, which is why I suggested instead using MM-specific indexes for
 access via the D-Bus API and doing whatever is required internally to
 present a clean external UI.  I believe the indexes in the MM D-Bus API
 should not reference actual SIM messages, but should be synthetic.  I
 don't think it's useful to present each SMS as a D-Bus object, and thus
 indexes are the way to go.  But the indexes should be MM specific,
 transient, and have no relationship to whatever happens inside MM to
 operate on the SMSes.

One thing that you have to decide it where you wanna store the SMS. Is
this storage inside the daemon or better done in the user session.
Either of it is valid choice, only the SIM card is none of them ;)

The only thing that you might wanna consider is that the you actually
wanna have conversations and not random inbox/outbox list with a random
index.

Regards

Marcel


___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [PATCH] GSM SMS reception code

2011-04-14 Thread Dan Williams
On Tue, 2011-04-12 at 17:05 -0400, Nathan Williams wrote:
 Here's the implementation I've done of SMS reception code for GSM
 modems, exclusive of the bug fixes discussed here earlier. It
 implements the SmsReceived signal and the Get, Delete, and List
 commands; it does not group multi-part messages together into one, but
 does skip over the user-data header to avoid presenting garbage to the
 user. 
  
 Credit to Torgny Johansson torgny.johans...@ericsson.com for the
 initial basis of this code.

Looks great, pushed with a few cleanups as
487972c1ac40b057c35dac51958f04c13f6137d8.

Some suggestions for further patches:

1) give sms_parse_pdu() a GError **error argument, and set that
internally when things fail so that callers can get usable error
information instead of calling mm_err().  If you want to define new
errors, see mm-errors.c and mm-errors.h.  Those files basically map the
ETSI standard error numbers to GErrors, so we should add a bunch for the
CMS errors, and then we can also add MM-specific errors with higher
numbers.  The numbers are only used internally for mapping the returned
numberic CMS/CME to a GError, while dbus-glib translates them into the
D-Bus familiar format of org.freedesktop.ModemManager. instead.

2) Testcases for PDU decoding; best way to do this is to split out the
bits of the SMS stuff that doesn't depend on anything MM-related like
MMCallbackInfo and whatnot into a separate file, like mm-sms-utils.c or
something, and build that file into libmodem_helpers_la_SOURCES in
Makefile.am.  That makes it easier to link that code to the testcases
without including a bunch of random stuff that isn't related to the unit
tests.  Then we grab a bunch of real-world encoded SMS PDUs and shove
them through the decoder and make sure it works on those, then we can
start fuzzing it too and make sure it's more robust.  We'll really want
testcases as there's quite a bit of variation between devices and
frankly nobody interprets the standards the same way :(

3) Do we need to do endian conversions here for BE arches when decoding
or constructing the PDU?

4) The whole index thing, figuring out if we can use a generic index
that's MM specific and maps internally to SIM indexes, or whether we can
in fact just use the SIM index...

Keep the patches coming :)

Thanks!
Dan

___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [PATCH] GSM SMS reception code

2011-04-14 Thread Marcel Holtmann
Hi Dan,

 2) Testcases for PDU decoding; best way to do this is to split out the
 bits of the SMS stuff that doesn't depend on anything MM-related like
 MMCallbackInfo and whatnot into a separate file, like mm-sms-utils.c or
 something, and build that file into libmodem_helpers_la_SOURCES in
 Makefile.am.  That makes it easier to link that code to the testcases
 without including a bunch of random stuff that isn't related to the unit
 tests.  Then we grab a bunch of real-world encoded SMS PDUs and shove
 them through the decoder and make sure it works on those, then we can
 start fuzzing it too and make sure it's more robust.  We'll really want
 testcases as there's quite a bit of variation between devices and
 frankly nobody interprets the standards the same way :(

actually the SMS PDUs are pretty much standard since they are
transmitted this way over the air.

You are not gonna like this, bu you guys could save yourselves some
trouble and just use src/smsutil.c from oFono since we have been through
this whole exercise. Including the unit tests actually.

 4) The whole index thing, figuring out if we can use a generic index
 that's MM specific and maps internally to SIM indexes, or whether we can
 in fact just use the SIM index...

You are going to enter a world of pain if you are trying to deal with
SIM indexes or any kind of indexes for that matter. This whole indexes
concept is pretty much ancient from around 1995. Even modern SIM cards
are too limited for doing this properly.

Regards

Marcel


___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


[PATCH] GSM SMS reception code

2011-04-12 Thread Nathan Williams
Here's the implementation I've done of SMS reception code for GSM modems,
exclusive of the bug fixes discussed here earlier. It implements the
SmsReceived signal and the Get, Delete, and List commands; it does not group
multi-part messages together into one, but does skip over the user-data
header to avoid presenting garbage to the user.

Credit to Torgny Johansson torgny.johans...@ericsson.com for the initial
basis of this code.

- Nathan
diff --git a/introspection/org.freedesktop.ModemManager.Modem.Gsm.SMS.xml b/introspection/org.freedesktop.ModemManager.Modem.Gsm.SMS.xml
index 081ecc5..15953b8 100644
--- a/introspection/org.freedesktop.ModemManager.Modem.Gsm.SMS.xml
+++ b/introspection/org.freedesktop.ModemManager.Modem.Gsm.SMS.xml
@@ -36,6 +36,7 @@
   validity : uint (0..255) - Specifies when the SMS expires in SMSC (optional)
   class: uint (0..3) - Message importance and location (optional)
   completed: boolean - Whether all message parts have been received or not (optional)
+  index: uint - Index of message (for Get and Delete) (optional)
 /tp:docstring
   /arg
 /method
diff --git a/src/mm-charsets.c b/src/mm-charsets.c
index dd1ae08..cbdf388 100644
--- a/src/mm-charsets.c
+++ b/src/mm-charsets.c
@@ -427,16 +427,16 @@ mm_charset_utf8_to_unpacked_gsm (const char *utf8, guint32 *out_len)
 
 guint8 *
 gsm_unpack (const guint8 *gsm,
-guint32 nchars,
+guint32 num_septets,
 guint8 start_offset,  /* in _bits_ */
 guint32 *out_unpacked_len)
 {
 GByteArray *unpacked;
 int i;
 
-unpacked = g_byte_array_sized_new (nchars + 1);
+unpacked = g_byte_array_sized_new (num_septets + 1);
 
-for (i = 0; i  nchars; i++) {
+for (i = 0; i  num_septets; i++) {
 guint8 bits_here, bits_in_next, octet, offset, c;
 guint32 start_bit;
 
diff --git a/src/mm-charsets.h b/src/mm-charsets.h
index efd89f3..50b0cce 100644
--- a/src/mm-charsets.h
+++ b/src/mm-charsets.h
@@ -53,7 +53,7 @@ guint8 *mm_charset_utf8_to_unpacked_gsm (const char *utf8, guint32 *out_len);
 guint8 *mm_charset_gsm_unpacked_to_utf8 (const guint8 *gsm, guint32 len);
 
 guint8 *gsm_unpack (const guint8 *gsm,
-guint32 nchars, /* number of gsm characters, not octets */
+guint32 num_septets,
 guint8 start_offset,  /* in bits */
 guint32 *out_unpacked_len);
 
diff --git a/src/mm-generic-gsm.c b/src/mm-generic-gsm.c
index 98713b0..24c3d24 100644
--- a/src/mm-generic-gsm.c
+++ b/src/mm-generic-gsm.c
@@ -167,6 +167,16 @@ static void ciev_received (MMAtSerialPort *port,
GMatchInfo *info,
gpointer user_data);
 
+static void cmti_received (MMAtSerialPort *port,
+   GMatchInfo *info,
+   gpointer user_data);
+
+#define GS_HASH_TAG get-sms
+static GValue *simple_string_value (const char *str);
+static GValue *simple_uint_value (guint32 i);
+static GValue *simple_boolean_value (gboolean b);
+static void simple_free_gvalue (gpointer data);
+
 MMModem *
 mm_generic_gsm_new (const char *device,
 const char *driver,
@@ -814,6 +824,9 @@ mm_generic_gsm_grab_port (MMGenericGsm *self,
 
 regex = g_regex_new (\\r\\n\\+CIEV: (\\d+),(\\d)\\r\\n, G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL);
 mm_at_serial_port_add_unsolicited_msg_handler (MM_AT_SERIAL_PORT (port), regex, ciev_received, self, NULL);
+
+regex = g_regex_new (\\r\\n\\+CMTI: \(\\S+)\,(\\d+)\\r\\n, G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL);
+mm_at_serial_port_add_unsolicited_msg_handler (MM_AT_SERIAL_PORT (port), regex, cmti_received, self, NULL);
 g_regex_unref (regex);
 
 if (ptype == MM_PORT_TYPE_PRIMARY) {
@@ -1293,6 +1306,32 @@ ciev_received (MMAtSerialPort *port,
 }
 
 static void
+cmti_received (MMAtSerialPort *port,
+   GMatchInfo *info,
+   gpointer user_data)
+{
+MMGenericGsm *self = MM_GENERIC_GSM (user_data);
+
+guint idx=0;
+char *mem;
+char *str;
+
+mem = g_match_info_fetch (info, 1);
+
+str = g_match_info_fetch (info, 2);
+if (str)
+idx = atoi (str);
+g_free (str);
+
+/* todo: parse pdu to know if the sms is complete */
+mm_modem_gsm_sms_received (MM_MODEM_GSM_SMS (self),
+   idx,
+   TRUE);
+
+/* todo: send mm_modem_gsm_sms_completed if complete */
+}
+
+static void
 cmer_cb (MMAtSerialPort *port,
  GString *response,
  GError *error,
@@ -1380,6 +1419,11 @@ mm_generic_gsm_enable_complete (MMGenericGsm *self,
 /* Try to enable XON/XOFF flow control */
 mm_at_serial_port_queue_command (priv-primary, +IFC=1,1, 3, NULL, NULL);
 
+/* Enable SMS notifications */
+mm_at_serial_port_queue_command (priv-primary, +CNMI=2,1,2,1,0, 3, NULL, NULL);
+/* Set SMS storage