Exports SMS over D-Bus and mis cleanups
cover Exports SMS over D-Bus and mis cleanups
From: Inaky Perez-Gonzalez <[email protected]>
Hi All
This patchset is the current state of my tree that changes the D-Bus interface for SMS:
- adds object based management of SMS messages
- adds a cancelation operation for in-transit / pending messages
- holds messages waiting for acknoledgement (delivery report) -- this is still not fully integrated with the code that was commited last days (thus once the delivery arrives the message is not automatically cleaned up as "confirmed").
- generates truly unique SMS message IDs using hashing of contents and receiver address
- miscelaeous small cleanups / additions, carryover from a previous submit that got neither not acked or no resolution was agreed upon
Please review and suggest what else needs to be done / changed.
Thx,
The following changes since commit 1fedd096a0ba2ce8625a9e4d1c2ce25bb8f6dfe4:
Marcel Holtmann (1):
Check sanity the MNC length value from the SIM card
are available in the git repository at:
git://gitorious.org/~inakypg/ofono/ofono-inakypg.git master
Patches follow for reviewing convenience.
Inaky Perez-Gonzalez (23):
documentation: add note about referencing standards
util.h: Add BUILD_BUG_ON() and friends for compile-time assert checking smutil.h: add missing header file dependencies
write_file: make transaction-safe
doc: explain debugging options to -d, add a pointer in -h to manpage
SMS: introduce message ID API
introduce DECLARE_SMS_ADDR_STR()
export sms_assembly_encode_address
SMS: implement SHA256-based message IDs [incomplete]
sms: add doc about the extensions D-Bus API (not yet implemented) struct tx_queue_entry: add fields and destructor
SMS: produce a unique, persistent name for in-transit messages
SMS: introduce bare state machine and transitions
SMS: export outgoing messages over D-Bus (skeleton)
SMS: split sms_send_message() into a D-Bus front end and an internal API
SMS: introduce wait-for-ack state and infrastructure
SMS: introduce sms_msg_cancel and its D-Bus wrapper
SMS: rename tx_queue_entry->msg to ->dbus_msg for clarity
SMS: Implement D-Bus SMS-MSG::GetProperties
SMS: send D-Bus SMS-MSG::ProperyChanged signals when message changes status
SMS: make D-Bus SendMessage and Cancel fully synchronous
SMS: set the SRR bit in outgoing PDUs if WFA is requested
sms_text_prepare: document @use_delivery_reports
HACKING | 10 + Makefile.am | 5 +- doc/ofonod.8 | 5 +- doc/sms-api.txt | 94 ++++++++ doc/standards.txt | 8 + src/main.c | 4 +- src/sms.c | 512 +++++++++++++++++++++++++++++++++++----- src/smsutil.c | 202 ++++++++++++++++- src/smsutil.h | 125 ++++++++++ src/storage.c | 42 +++- src/util.h | 28 +++ test/test-sms-msg-state-change | 24 ++ unit/test-sms-msg-id.c | 212 +++++++++++++++++ 13 files changed, 1199 insertions(+), 72 deletions(-) create mode 100644 doc/standards.txt create mode 100755 test/test-sms-msg-state-change create mode 100644 unit/test-sms-msg-id.c
PATCH 1/23 documentation: add note about referencing standards
From: Inaky Perez-Gonzalez <[email protected]>
--- doc/standards.txt | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) create mode 100644 doc/standards.txt
doc/standards.txt new file +8 -0
@@ -0,0 +1,8 @@ +Referencing standards in the source +=================================== + +When referencing standard documents use raw numbers xx.xxx for 3GPP +documents or xxx.xxx for ETSI document (eg: 23.040). If needing to +point to an specific section/subsection, explicitly say "Section foo" + +3GPP specs can be found in http://3gpp.org/ftp/Specs.
Signature · 1 line
1.6.6.1
Re: [SMS D-Bus 01/23] documentation: add note about referencing standards
Hi Inaky,
Quoted 6 lines · Inaky Perez-Gonzalez
From: Inaky Perez-Gonzalez <[email protected]>
--- doc/standards.txt | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) create mode 100644 doc/standards.txt
This one has been applied, thanks.
Regards,
-Denis
PATCH 2/23 util.h: Add BUILD_BUG_ON() and friends for compile-time assert checking
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
These have been stolen from the Linux kernel source; come pretty handy to make build-time consistency checks and thus avoid run-time surprises.
--- src/util.h | 28 ++++++++++++++++++++++++++++ 1 files changed, 28 insertions(+), 0 deletions(-)
src/util.h +28 -0
@@ -80,3 +80,31 @@ char *sim_string_to_utf8(const unsigned char *buffer, int length);unsigned char *utf8_to_sim_string (const char *utf,int max_length,int *out_length); +/* +* Build time consistency checks +* +* Stolen from the Linux kernel 2.6.35-rc; as most taken from the +* Linux kernel, this is licensed GPL v2 or newer. +*/ + +/* Force a compilation error if condition is true */ +#define BUILD_BUG_ON (condition) ((void)BUILD_BUG_ON_ZERO(condition)) + +/* Force a compilation error if condition is constant and true */ +#define MAYBE_BUILD_BUG_ON (cond) ((void)sizeof(char[1 - 2 * !!(cond)])) + +/* Force a compilation error if a constant expression is not a power of 2 */ +#define BUILD_BUG_ON_NOT_POWER_OF_2 (n) \ + BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0)) + +/* Force a compilation error if condition is true, but also produce a +result (of value 0 and type size_t), so the expression can be used +e.g. in a structure initializer (or where-ever else comma expressions +aren't permitted). */ +#define BUILD_BUG_ON_ZERO (e) (sizeof(struct { int:-!!(e); })) +#define BUILD_BUG_ON_NULL (e) ((void *)sizeof(struct { int:-!!(e); })) + +/* +* End of code stolen from the Linux kernel 2.6.35-rc. From now, code +* is GPL v2 only. +*/
Signature · 1 line
1.6.6.1
Re: [SMS D-Bus 02/23] util.h: Add BUILD_BUG_ON() and friends for compile-time assert checking
Hi Inaky,
Quoted 6 lines
These have been stolen from the Linux kernel source; come pretty handy to make build-time consistency checks and thus avoid run-time surprises.
--- src/util.h | 28 ++++++++++++++++++++++++++++ 1 files changed, 28 insertions(+), 0 deletions(-)
I still think that using include/bug.h is way better. And that way we could more easily sync this with ConnMan. Since this might come in handy for ConnMan as well.
Regards
Marcel
Re: [SMS D-Bus 02/23] util.h: Add BUILD_BUG_ON() and friends for compile-time assert checking
Hey Marcel
Quoted 9 lines · Marcel Holtmann
Hi Inaky,
Quoted 6 lines
These have been stolen from the Linux kernel source; come pretty handy to make build-time consistency checks and thus avoid run-time surprises.
--- src/util.h | 28 ++++++++++++++++++++++++++++ 1 files changed, 28 insertions(+), 0 deletions(-)
Quoted 3 lines
I still think that using
include/bug.his way better. And that way we could more easily sync this with ConnMan. Since this might come in handy for ConnMan as well.
I don't have a problem with it, but I am still waiting for Denis' take on it.
Re: [SMS D-Bus 02/23] util.h: Add BUILD_BUG_ON() and friends for compile-time assert checking
In reply to Inaky Perez-GonzalezHi Inaky,
Quoted 9 lines
From: Inaky Perez-Gonzalez <[email protected]>
These have been stolen from the Linux kernel source; come pretty handy to make build-time consistency checks and thus avoid run-time surprises.
--- src/util.h | 28 ++++++++++++++++++++++++++++ 1 files changed, 28 insertions(+), 0 deletions(-)
This stuff really doesn't belong in util.h. That file is ancient and if I were to do it again, I'd rename it into textutil.h or gsmutil.h. I only really want to keep text conversions there.
I suggest putting this stuff into include/bug.h per Marcel's earlier email.
Regards,
-Denis
Re: [SMS D-Bus 02/23] util.h: Add BUILD_BUG_ON() and friends for compile-time assert checking
Quoted 12 lines · Denis Kenzior
Hi Inaky,
Quoted 9 lines
From: Inaky Perez-Gonzalez <[email protected]>
These have been stolen from the Linux kernel source; come pretty handy to make build-time consistency checks and thus avoid run-time surprises.
--- src/util.h | 28 ++++++++++++++++++++++++++++ 1 files changed, 28 insertions(+), 0 deletions(-)
Quoted 5 lines
This stuff really doesn't belong in
util.h. That file is ancient and if I were to do it again, I'd rename it intotextutil.horgsmutil.h. I only really want to keep text conversions there.I suggest putting this stuff into
include/bug.hper Marcel's earlier email.
Okey dokey, will do so.
PATCH 3/23 smutil.h: add missing header file dependencies
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
--- src/smsutil.h | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
src/smsutil.h +9 -0
@@ -18,6 +18,14 @@* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA * */ +#ifndef __smsutil_h__ +#define __smsutil_h__ + +#include <time.h> +#include <glib/gtypes.h> +#include <glib/gslist.h> +#include <glib/gmessages.h> +#include <glib/gchecksum.h> #define CBS_MAX_GSM_CHARS 93
@@ -539,3 +547,4 @@ GSList *cbs_optimize_ranges(GSList *ranges);gboolean cbs_topic_in_range (unsigned int topic,GSList *ranges);char *ussd_decode (int dcs,int len,const unsigned char *data); +#endif /* #ifndef __smsutil_h__ */
Signature · 1 line
1.6.6.1
Re: [SMS D-Bus 03/23] smutil.h: add missing header file dependencies
Hi Inaky,
src/smsutil.h line 22
Quoted 13 lines
src/smsutil.h | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)src/smsutil.h +2
-0@@ -18,6 +18,14 @@* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA * */ +#ifndef __smsutil_h__ +#define __smsutil_h__
for internal headers I don't want the circular inclusion protection. It buys us nothing. And leaving it out will warn us when we do circular inclusion. That is a clear indication that something is a bit too complicated if it is needed.
Regards
Marcel
Re: [SMS D-Bus 03/23] smutil.h: add missing header file dependencies
Quoted 5 lines · Marcel Holtmann
Hi Inaky,
Quoted 3 lines
src/smsutil.h | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
Quoted 15 lines
src/smsutil.h line 22
Quoted 10 lines
src/smsutil.h +2
-0@@ -18,6 +18,14 @@* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA * */ +#ifndef __smsutil_h__ +#define __smsutil_h__for internal headers I don't want the circular inclusion protection. It buys us nothing. And leaving it out will warn us when we do circular inclusion. That is a clear indication that something is a bit too complicated if it is needed.
Yeah, I just forgot to drop it.
PATCH 4/23 write_file: make transaction-safe
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
write_file(), as written wasn't transaction-safe; a crash bewtween a file being open and the buffer being written before a safe close would leave the file with a set of undetermined contents.
Modified to the file is written to a temporary file name; once completed, it is renamed to the final name. This way, a crash in the middle doesn't leave half-baked files.
--- src/storage.c | 42 +++++++++++++++++++++++++++++++----------- 1 files changed, 31 insertions(+), 11 deletions(-)
src/storage.c +31 -11
@@ -98,11 +98,21 @@ ssize_t read_file(unsigned char *buffer, size_t len,return r; } +/* +* Write a buffer to a file in a transactionally safe form +* +* Given a buffer, write it to a file named after +* @path_fmt+args. However, to make sure the file contents are +* consistent (ie: a crash right after opening or during write() +* doesn't leave a file half baked), the contents are written to a +* file with a temporary name and when closed, it is renamed to the +* specified name (@path_fmt+args). +*/ ssize_t write_file (const unsigned char *buffer,size_t len,mode_t mode,const char *path_fmt, ...) {va_list ap; -char *path; +char *tmp_path, *path;ssize_t r;int fd;
@@ -110,26 +120,36 @@ ssize_t write_file(const unsigned char *buffer, size_t len, mode_t mode, path =g_strdup_vprintf (path_fmt, ap);va_end (ap); -if (create_dirs (path, mode |S_IXUSR ) !=0 ) { -g_free (path); -return -1 ; - } + tmp_path =g_strdup_printf ("%s.XXXXXX.tmp" , path); - fd =TFR (open (path,O_WRONLY |O_CREAT |O_TRUNC , mode)); -if (fd ==-1 ) { -g_free (path); -return -1 ; - } + r =-1 ; +if (create_dirs (path, mode |S_IXUSR ) !=0 ) + goto error_create_dirs; + fd =TFR (g_mkstemp_full (tmp_path,O_WRONLY |O_CREAT |O_TRUNC , mode)); +if (fd ==-1 ) + goto error_mkstemp_full; r =TFR (write (fd, buffer, len));TFR (close (fd));if (r != (ssize_t ) len) { -unlink (path); r =-1 ; + goto error_write; } +/* Now that the file contents are written, rename to the real +* file name; this way we are uniquely sure that the whole +* thing is there. */ +unlink (path); +/* conserve @r's value from 'write' */ +if (link (tmp_path, path) ==-1 ) + r =-1 ; +error_write: +unlink (tmp_path); +error_mkstemp_full: +error_create_dirs: +g_free (tmp_path);g_free (path);return r; }
Signature · 1 line
1.6.6.1
Re: [SMS D-Bus 04/23] write_file: make transaction-safe
Hi Inaky,
Quoted 9 lines · Inaky Perez-Gonzalez
From: Inaky Perez-Gonzalez <[email protected]>
write_file(), as written wasn't transaction-safe; a crash bewtween a file being open and the buffer being written before a safe close would leave the file with a set of undetermined contents.Modified to the file is written to a temporary file name; once completed, it is renamed to the final name. This way, a crash in the middle doesn't leave half-baked files.
Patch is fine, but:
Applying: write_file: make transaction-safe /home/denkenz/ofono-master/.git/rebase-apply/patch:13: trailing whitespace. /*/home/denkenz/ofono-master/.git/rebase-apply/patch:22: trailing whitespace.
*//home/denkenz/ofono-master/.git/rebase-apply/patch:75: trailing whitespace.
g_free(tmp_path);
fatal: 3 lines add whitespace errors.
Regards,
-Denis
Re: [SMS D-Bus 04/23] write_file: make transaction-safe
Quoted 13 lines · Denis Kenzior
Hi Inaky,
Quoted 9 lines · Inaky Perez-Gonzalez
From: Inaky Perez-Gonzalez <[email protected]>
write_file(), as written wasn't transaction-safe; a crash bewtween a file being open and the buffer being written before a safe close would leave the file with a set of undetermined contents.Modified to the file is written to a temporary file name; once completed, it is renamed to the final name. This way, a crash in the middle doesn't leave half-baked files.
Quoted 5 lines
Patch is fine, but:
Applying:
write_file: make transaction-safe/home/denkenz/ofono-master/.git/rebase-apply/patch:13: trailing whitespace. /*
Yup, sorry, I missed those. They have been cleaned up already -- next resubmit will have it fixed.
PATCH 5/23 doc: explain debugging options to -d, add a pointer in -h to manpage
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
Modified HACKING and man page to have more formation on what are the debugging options and how to enable them.
--- HACKING | 10 ++++++++++ doc/ofonod.8 | 5 ++++- src/main.c | 4 +++- 3 files changed, 17 insertions(+), 2 deletions(-)
HACKING +10 -0
@@ -81,3 +81,13 @@ automatically includes this option. For production installations or distribution packaging it is important that the "--enable-maintainer-mode" option is NOT used. + +Note multiple arguments to -d can be specified, colon, comma or space +separated. The arguments are relative source code filenames for which +debugging output should be enabled; output shell-style globs are +accepted (e.g.: 'plugins/*:src/main.c'). + +Other debugging settings that can be toggled: + + - Environment variable OFONO_AT_DEBUG (set to 1): enable AT commands + debugging
doc/ofonod.8 +4 -1
@@ -18,7 +18,10 @@ is used to manage \fID-Bus\fP permissions for oFono. .SH OPTIONS .TP .B --debug, -d -Enable debug information output. +Enable debug information output. Note multiple arguments to -d can be +specified, colon, comma or space separated. The arguments are relative +source code filenames for which debugging output should be enabled; +output shell-style globs are accepted (e.g.: "plugins/*:src/main.c"). .TP .B --nodetach, -n Don't run as daemon in background.
src/main.c +3 -1
@@ -110,7 +110,9 @@ static gboolean parse_debug(const char *key, const char *value,static GOptionEntry options[] = { {"debug" ,'d' ,G_OPTION_FLAG_OPTIONAL_ARG ,G_OPTION_ARG_CALLBACK , parse_debug, -"Specify debug options to enable" ,"DEBUG" }, +"Specify debug options to enable (see the " +"man page for ofonod(8) for more information)." , +"DEBUG" }, {"nodetach" ,'n' ,G_OPTION_FLAG_REVERSE ,G_OPTION_ARG_NONE , &option_detach,"Don't run as daemon in background" },
Signature · 1 line
1.6.6.1
Re: [SMS D-Bus 05/23] doc: explain debugging options to -d, add a pointer in -h to manpage
Hi Inaky,
Please remember not to go over 50 characters on your git commit description header and more than 72 characters on your git commit description body.
src/main.c static gboolean parse_debug(const char *key, const char *value, · line 114
Quoted 7 lines
src/main.c +2
-1@@ -110,7 +110,9 @@ static gboolean parse_debug(const char *key, const char *value,static GOptionEntry options[] = { {"debug" ,'d' ,G_OPTION_FLAG_OPTIONAL_ARG ,G_OPTION_ARG_CALLBACK , parse_debug, -"Specify debug options to enable" ,"DEBUG" }, +"Specify debug options to enable (see the " +"man page for ofonod(8) for more information)." ,
I still think mentioning the man page is a bit redundant.
src/main.c static gboolean parse_debug(const char *key, const char *value, · line 118
Quoted 4 lines
src/main.c +1
-0+ "DEBUG" }, {"nodetach" ,'n' ,G_OPTION_FLAG_REVERSE ,G_OPTION_ARG_NONE , &option_detach, "Don't run as daemon in background" },
Regards,
-Denis
Re: [SMS D-Bus 05/23] doc: explain debugging options to -d, add a pointer in -h to manpage
Quoted 16 lines · Denis Kenzior
Hi Inaky,
Please remember not to go over 50 characters on your git commit description header and more than 72 characters on your git commit description body.
src/main.c static gboolean parse_debug(const char *key, const char *value, · line 114
Quoted 7 lines
src/main.c +2
-1@@ -110,7 +110,9 @@ static gboolean parse_debug(const char *key, const char *value,static GOptionEntry options[] = { {"debug" ,'d' ,G_OPTION_FLAG_OPTIONAL_ARG ,G_OPTION_ARG_CALLBACK , parse_debug, -"Specify debug options to enable" ,"DEBUG" }, +"Specify debug options to enable (see the " +"man page for ofonod(8) for more information)." ,I still think mentioning the man page is a bit redundant.
Quoted 7 lines
src/main.c static gboolean parse_debug(const char *key, const char *value, · line 118
Quoted 4 lines
src/main.c +1
-0+ "DEBUG" }, {"nodetach" ,'n' ,G_OPTION_FLAG_REVERSE ,G_OPTION_ARG_NONE , &option_detach, "Don't run as daemon in background" },Regards,
-Denis
Let's just drop this one then.
Re: [SMS D-Bus 05/23] doc: explain debugging options to -d, add a pointer in -h to manpage
In reply to Denis KenziorQuoted 16 lines · Denis Kenzior
Hi Inaky,
Please remember not to go over 50 characters on your git commit description header and more than 72 characters on your git commit description body.
src/main.c static gboolean parse_debug(const char *key, const char *value, · line 114
Quoted 7 lines
src/main.c +2
-1@@ -110,7 +110,9 @@ static gboolean parse_debug(const char *key, const char *value,static GOptionEntry options[] = { {"debug" ,'d' ,G_OPTION_FLAG_OPTIONAL_ARG ,G_OPTION_ARG_CALLBACK , parse_debug, -"Specify debug options to enable" ,"DEBUG" }, +"Specify debug options to enable (see the " +"man page for ofonod(8) for more information)." ,I still think mentioning the man page is a bit redundant.
Quoted 7 lines
src/main.c static gboolean parse_debug(const char *key, const char *value, · line 118
Quoted 4 lines
src/main.c +1
-0+ "DEBUG" }, {"nodetach" ,'n' ,G_OPTION_FLAG_REVERSE ,G_OPTION_ARG_NONE , &option_detach, "Don't run as daemon in background" },Regards,
-Denis
Bah, hit enter too soon, sorry -- meant drop the .c code adding that, but keep the additions to the man page. I'll redo that one.
PATCH 6/23 SMS: introduce message ID API
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
This adds a simple API to use for generating unique IDs for SMS messages.
--- Makefile.am | 5 +- src/smsutil.c | 191 +++++++++++++++++++++++++++++++++++++++++++ src/smsutil.h | 82 +++++++++++++++++++ unit/test-sms-msg-id.c | 212 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 489 insertions(+), 1 deletions(-) create mode 100644 unit/test-sms-msg-id.c
Makefile.am +4 -1
@@ -363,7 +363,7 @@ unit_objects = noinst_PROGRAMS = unit/test-common unit/test-util unit/test-idmap \ unit/test-sms unit/test-simutil \ unit/test-mux unit/test-caif \ - unit/test-stkutil + unit/test-stkutil unit/test-sms-msg-id unit_test_common_SOURCES = unit/test-common.c src/common.c unit_test_common_LDADD = @GLIB_LIBS@
@@ -402,6 +402,9 @@ unit_test_caif_SOURCES = unit/test-caif.c $(gatchat_sources) \ unit_test_caif_LDADD = @GLIB_LIBS@ unit_objects += $(unit_test_caif_OBJECTS) +unit_test_sms_msg_id_SOURCES = unit/test-sms-msg-id.c src/util.c src/smsutil.c src/storage.c +unit_test_sms_msg_id_LDADD = @GLIB_LIBS@ + noinst_PROGRAMS += gatchat/gsmdial gatchat/test-server gatchat/test-qcdm gatchat_gsmdial_SOURCES = gatchat/gsmdial.c $(gatchat_sources)
src/smsutil.c +191 -0
194 lines not on this page · raw message
src/smsutil.h +82 -0
86 lines not on this page · raw message
unit/test-sms-msg-id.c new file +212 -0
212 lines not on this page · raw message
Signature · 1 line
1.6.6.1
PATCH 7/23 introduce DECLARE_SMS_ADDR_STR()
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
Introduce DECLARE_SMS_ADDR_STR(), which declares a string variable of the right size for passing to sms_assembly_decode_address(). This way we detach each client having to have the knowledge of what the right size is, leaving that decission to the infrastructure provider. Updated couple of sites in smsutil.c to use it vs a raw declaration.
--- src/smsutil.c | 4 ++-- src/smsutil.h | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-)
src/smsutil.c +2 -2
@@ -2314,7 +2314,7 @@ static void sms_assembly_load(struct sms_assembly *assembly, const struct dirent *dir) {struct sms_address addr; -char straddr[25 ]; +DECLARE_SMS_ADDR_STR (straddr);guint16 ref;guint8 max;guint8 seq;
@@ -2391,7 +2391,7 @@ static gboolean sms_assembly_store(struct sms_assembly *assembly, {unsigned char buf[177 ];int len; -char straddr[25 ]; +DECLARE_SMS_ADDR_STR (straddr);if (!assembly->imsi)return FALSE;
src/smsutil.h +3 -0
@@ -440,6 +440,9 @@ gboolean sms_decode_unpacked_stk_pdu(const unsigned char *pdu, int len,gboolean sms_encode (const struct sms *in,int *len,int *tpdu_len,unsigned char *pdu); +enum {SMS_ADDR_STR_SIZE =25 }; +#define DECLARE_SMS_ADDR_STR (a) char a[SMS_ADDR_STR_SIZE] +gboolean sms_decode_address_field (const unsigned char *pdu,int len,int *offset,gboolean sc,struct sms_address *out);
Signature · 1 line
1.6.6.1
Re: [SMS D-Bus 07/23] introduce DECLARE_SMS_ADDR_STR()
Hi Inaky,
src/smsutil.h gboolean sms_decode_unpacked_stk_pdu(const unsigned char *pdu, int len, · line 448
Quoted 6 lines
src/smsutil.h +3
-0+enum {SMS_ADDR_STR_SIZE =25 }; +#define DECLARE_SMS_ADDR_STR (a) char a[SMS_ADDR_STR_SIZE] +gboolean sms_decode_address_field (const unsigned char *pdu,int len,int *offset,gboolean sc,struct sms_address *out);
Sounds fine to me, but can we get rid of the enum declaration? simply char a[25] seems to be enough. Might want to add a comment explaining why it is 25. Namely that sms address is encoded into 12 bytes, stored in hex format and space for a null.
Regards,
-Denis
Re: [SMS D-Bus 07/23] introduce DECLARE_SMS_ADDR_STR()
Quoted 13 lines · Denis Kenzior
Hi Inaky,
src/smsutil.h gboolean sms_decode_unpacked_stk_pdu(const unsigned char *pdu, int len, · line 448
Quoted 6 lines
src/smsutil.h +3
-0+enum {SMS_ADDR_STR_SIZE =25 }; +#define DECLARE_SMS_ADDR_STR (a) char a[SMS_ADDR_STR_SIZE] +gboolean sms_decode_address_field (const unsigned char *pdu,int len,int *offset,gboolean sc,struct sms_address *out);Sounds fine to me, but can we get rid of the enum declaration? simply char a[25] seems to be enough. Might want to add a comment explaining why it is 25. Namely that sms address is encoded into 12 bytes, stored in hex format and space for a null.
Ack, will do.
PATCH 8/23 export sms_assembly_encode_address
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
For generating unique message naming (for persistence and D-Bus object naming), we'll be using the address. Instead of repeating the functions, we reuse the same infrastructure used by the SMS assembly code.
--- src/smsutil.c | 4 ++-- src/smsutil.h | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-)
src/smsutil.c +2 -2
@@ -2293,8 +2293,8 @@ static gboolean sms_assembly_extract_address(const char *straddr,return sms_decode_address_field (pdu, len, &offset, FALSE, out); } -static gboolean sms_assembly_encode_address (const struct sms_address *in, -char *straddr) +gboolean sms_assembly_encode_address (const struct sms_address *in, +char *straddr) {unsigned char pdu[12 ];int offset =0 ;
src/smsutil.h +2 -0
@@ -505,6 +505,8 @@ GSList *sms_assembly_add_fragment(struct sms_assembly *assembly,const struct sms_address *addr, guint16 ref, guint8 max, guint8 seq);void sms_assembly_expire (struct sms_assembly *assembly,time_t before); +gboolean sms_assembly_encode_address (const struct sms_address *in, +char *straddr);struct status_report_assembly *status_report_assembly_new (const char *imsi);void status_report_assembly_free (struct status_report_assembly *assembly);
Signature · 1 line
1.6.6.1
Re: [SMS D-Bus 08/23] export sms_assembly_encode_address
Hi Inaky,
src/smsutil.h GSList *sms_assembly_add_fragment(struct sms_assembly *assembly, · line 512
Quoted 6 lines
void
sms_assembly_expire(struct sms_assembly *assembly, time_t before);src/smsutil.h +2
-0+gboolean sms_assembly_encode_address (const struct sms_address *in, +char *straddr);struct status_report_assembly *status_report_assembly_new (const char *imsi);void status_report_assembly_free (struct status_report_assembly *assembly);
Can we rename this into sms_address_to_hex_string? That seems to reflect the intent a bit better.
Regards,
-Denis
Re: [SMS D-Bus 08/23] export sms_assembly_encode_address
Quoted 11 lines · Denis Kenzior
Hi Inaky,
src/smsutil.h GSList *sms_assembly_add_fragment(struct sms_assembly *assembly, · line 512
Quoted 6 lines
void
sms_assembly_expire(struct sms_assembly *assembly, time_t before);src/smsutil.h +2
-0+gboolean sms_assembly_encode_address (const struct sms_address *in, +char *straddr);struct status_report_assembly *status_report_assembly_new (const char *imsi);void status_report_assembly_free (struct status_report_assembly *assembly);Can we rename this into
sms_address_to_hex_string? That seems to reflect the intent a bit better.
Ok -- this will then touch a few more other sites in the code.
Re: [SMS D-Bus 08/23] export sms_assembly_encode_address
Hi Inaky,
Quoted 7 lines · Inaky Perez-Gonzalez
Quoted 6 lines · Denis Kenzior
Hi Inaky,
Quoted 4 lines
void
sms_assembly_expire(struct sms_assembly *assembly, time_t before);src/smsutil.h +2
-0+gboolean sms_assembly_encode_address (const struct sms_address *in, +char *straddr);
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
struct
status_report_assembly*status_report_assembly_new(const char *imsi); voidstatus_report_assembly_free(struct status_report_assembly *assembly);Can we rename this into
sms_address_to_hex_string? That seems to reflect the intent a bit better.Ok -- this will then touch a few more other sites in the code.
That is fine.
Regards,
-Denis
PATCH 9/23 SMS: implement SHA256-based message IDs [incomplete]
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
This patch removes the sequential SMS message identification gig and replaces it with a 16-bit hash cookie based off message contents.
FIXME: [incomplete] need to figure out how to do so that identical messages sent to the same number also have a different ID.
Note that in order to feed the reference 16-bit ID to set_ref_and_to(), the UUID has to be computed as early as possible.
--- src/sms.c | 64 +++++++++++++++++++++++++++++++++++++++--------------------- 1 files changed, 41 insertions(+), 23 deletions(-)
src/sms.c +41 -23
@@ -55,7 +55,6 @@ struct ofono_sms {DBusMessage *pending;struct ofono_phone_number sca;struct sms_assembly *assembly; -unsigned int next_msg_id;guint ref;GQueue *txq;gint tx_source;
@@ -450,9 +449,13 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,const char *to;const char *text;GSList *msg_list; -int ref_offset; +int ref_offset, ref;struct tx_queue_entry *entry;struct ofono_modem *modem; +struct sms_msg_id sms_msg_id; +DECLARE_SMS_MSG_ID_STRBUF (msg_id_str); +struct sms_address receiver; +DECLARE_SMS_ADDR_STR (receiver_str); if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &to, DBUS_TYPE_STRING, &text,
@@ -462,31 +465,40 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,if (valid_phone_number_format (to) == FALSE)return __ofono_error_invalid_format (msg); +/* +* Instead of using the telephone number/address we got from +* D-Bus, we do the reverse formatting, so we get something +* that has been normalized--this is used later and we do it +* here to simplify error handling. +*/ +sms_address_from_string (&receiver, to); +if (sms_assembly_encode_address (&receiver, receiver_str) + == FALSE) +return __ofono_error_failed (msg); + msg_list =sms_text_prepare (text,0 , TRUE, &ref_offset, sms->use_delivery_reports);if (!msg_list)return __ofono_error_invalid_format (msg); -DBG ("ref: %d, offset: %d" , sms->ref, ref_offset); +sms_msg_id_init (&sms_msg_id); +sms_msg_id_hash (&sms_msg_id, text,strlen (text)); +sms_msg_id_hash (&sms_msg_id, receiver_str,strlen (receiver_str)); +sms_msg_id_hash (&sms_msg_id,NULL ,0 ); +sms_msg_id_memcpy (&ref,sizeof (ref), &sms_msg_id); +sms_msg_id_printf (&sms_msg_id, msg_id_str,sizeof (msg_id_str)); -set_ref_and_to (msg_list, sms->ref, ref_offset, to); +set_ref_and_to (msg_list, ref, ref_offset, to); entry =create_tx_queue_entry (msg_list); - -sms_address_from_string (&entry->receiver, to); +ofono_debug ("SMS: %p: ref %08x, offset %d, msg id: %s\n" , + entry, ref, ref_offset, msg_id_str); + entry->msg_id = ref; + entry->receiver = receiver;g_slist_foreach (msg_list, (GFunc )g_free,NULL );g_slist_free (msg_list); - -if (ref_offset !=0 ) { -if (sms->ref ==65536 ) - sms->ref =1 ; -else - sms->ref = sms->ref +1 ; - } - entry->msg =dbus_message_ref (msg); - entry->msg_id = sms->next_msg_id++; entry->status_report = sms->use_delivery_reports;g_queue_push_tail (sms->txq, entry);
@@ -544,6 +556,9 @@ static void dispatch_text_message(struct ofono_sms *sms,struct tm remote;struct tm local;const char *str = buf; +struct sms_msg_id sms_msg_id; +unsigned int id; +DECLARE_SMS_MSG_ID_STRBUF (msg_id_str);if (!message)return ;
@@ -585,11 +600,18 @@ static void dispatch_text_message(struct ofono_sms *sms,g_dbus_send_message (conn, signal); -if (cls !=SMS_CLASS_0 ) { -__ofono_history_sms_received (modem, sms->next_msg_id, str, +sms_msg_id_init (&sms_msg_id); +sms_msg_id_hash (&sms_msg_id, message,strlen (message)); +sms_msg_id_hash (&sms_msg_id, addr->address,strlen (addr->address)); +/* FIXME: add RX time? */ +sms_msg_id_hash (&sms_msg_id,NULL ,0 ); +sms_msg_id_memcpy (&id,sizeof (id), &sms_msg_id); +sms_msg_id_printf (&sms_msg_id, msg_id_str,sizeof (msg_id_str)); +ofono_debug ("SMS RX: msg id is %s\n" , msg_id_str); + +if (cls !=SMS_CLASS_0 ) +__ofono_history_sms_received (modem, id, str, &remote, &local, message); - sms->next_msg_id +=1 ; - } }static void sms_dispatch (struct ofono_sms *sms,GSList *sms_list)
@@ -972,8 +994,6 @@ static void sms_remove(struct ofono_atom *atom)if (sms->settings) {g_key_file_set_integer (sms->settings,SETTINGS_GROUP , -"NextMessageId" , sms->next_msg_id); -g_key_file_set_integer (sms->settings,SETTINGS_GROUP ,"NextReference" , sms->ref); g_key_file_set_boolean(sms->settings, SETTINGS_GROUP, "UseDeliveryReports",
@@ -1065,8 +1085,6 @@ static void sms_load_settings(struct ofono_sms *sms, const char *imsi) sms->imsi =g_strdup (imsi); - sms->next_msg_id =g_key_file_get_integer (sms->settings,SETTINGS_GROUP , -"NextMessageId" ,NULL ); sms->ref =g_key_file_get_integer (sms->settings,SETTINGS_GROUP ,"NextReference" ,NULL ); sms->use_delivery_reports =
Signature · 1 line
1.6.6.1
PATCH 10/23 sms: add doc about the extensions D-Bus API (not yet implemented)
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
--- doc/sms-api.txt | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 94 insertions(+), 0 deletions(-)
doc/sms-api.txt +94 -0
@@ -46,3 +46,97 @@ Signals PropertyChanged(string name, variant value) Properties string ServiceCenterAddress Contains the number of the SMS service center. + + + +SMS / Messaging interface +========================= + +Requirements: + + - provide an interface for management of text messages over cell + networks + + - maintain messaging data while "in transit", meaning, while Ofono + owns it since the time it is submitted for delivery until it is + deemed delivered to the network or had failure (undeliverable / + undelivered). + + - has to be persistent across daemon invocations (needs offline db) + + - provides methods to submit a message for delivery, query status, + cancel delivery, as well as signals on status change + + +High level interface + + MSGHANDLE := objectpath + + Each MSGHANDLE is a message object "in transit" (aka: owned by + oFono). So it becomes a D-Bus object name that supports the + org.ofono.Message interface. + + MSGDATA := struct / dict + + Data for the message. Still TBD what exactly will come in here, + other than the obvious destination and text data. + +org.ofono.SmsManager [interface] + + This interface hangs off the modem object in oFono. + + Marcel suggests in the future we could have an "any" modem that + orutes routes things automatically. Dennis says that in any case, + most of the times we have a single modem object dangling around. + + MSGHANDLE SendMessage(MSGDATA) [method] + + Submit a message for delivery / processing. oFono owns it from + now on until successful delivery, cancellation (by the user) + or cancellation (by oFono). User has to keep a copy around as + oFono only offers persistence to satisfy its needs. + + PendingMessages [property] + + List of MSGHANDLEs which are currently pending/in-transit + + GetProperties() [method] + + Get manager's properties; one of them being the list of + pending messages. + + ProperyChanged() [signal] + + A property has changed. + + IncomingMessage() [signal] + + Message arrivedl; class != CLASS_0 FIXME + + ImmediateMessage() [signal] + + Message arrived; class == CLASS_0 FIXME + + +org.ofono.Message [interface] + + MSGDATA GetProperties() [method] + + Returns all the data associated to a message, as well as its + current status. + + Cancel() [method] + + Cancel oFono's handling of the message, wether is submitting + it, retrying submissions, etc. + + PropertyChanged() [signal] + + Some property in the message has changed. If status change, + provide old and new status -- to ease up state machine + processing. + + Dennis: subscribers can subscribe to signals from all object + that support an interface, so it's easy to subscribe to get + the signals from all the objects without having to subscribe + to each object.
Signature · 1 line
1.6.6.1
Re: [SMS D-Bus 10/23] sms: add doc about the extensions D-Bus API (not yet implemented)
Hi Inaky,
Quoted 25 lines
doc/sms-api.txt +25
-0+ + + +SMS / Messaging interface +========================= + +Requirements: + + - provide an interface for management of text messages over cell + networks + + - maintain messaging data while "in transit", meaning, while Ofono + owns it since the time it is submitted for delivery until it is + deemed delivered to the network or had failure (undeliverable / + undelivered). + + - has to be persistent across daemon invocations (needs offline db) + + - provides methods to submit a message for delivery, query status, + cancel delivery, as well as signals on status change + + +High level interface + + MSGHANDLE := objectpath
doc/sms-api.txt Signals PropertyChanged(string name, variant value) · line 83
Quoted 10 lines
doc/sms-api.txt +10
-0+ + Each MSGHANDLE is a message object "in transit" (aka: owned by + oFono). So it becomes a D-Bus object name that supports the + org.ofono.Message interface. + + MSGDATA := struct / dict + + Data for the message. Still TBD what exactly will come in here, + other than the obvious destination and text data. +
Please put this type of stuff into doc/overview.txt or doc/sms.txt. I'd like to stick to the API conventions in doc/foo-api.txt.
Quoted 45 lines
doc/sms-api.txt +45
-0+org.ofono.SmsManager [interface] + + This interface hangs off the modem object in oFono. + + Marcel suggests in the future we could have an "any" modem that + orutes routes things automatically. Dennis says that in any case, + most of the times we have a single modem object dangling around. + + MSGHANDLE SendMessage(MSGDATA) [method] + + Submit a message for delivery / processing. oFono owns it from + now on until successful delivery, cancellation (by the user) + or cancellation (by oFono). User has to keep a copy around as + oFono only offers persistence to satisfy its needs. + + PendingMessages [property] + + List of MSGHANDLEs which are currently pending/in-transit + + GetProperties() [method] + + Get manager's properties; one of them being the list of + pending messages. + + ProperyChanged() [signal] + + A property has changed. + + IncomingMessage() [signal] + + Message arrivedl; class != CLASS_0 FIXME + + ImmediateMessage() [signal] + + Message arrived; class == CLASS_0 FIXME + + +org.ofono.Message [interface] + + MSGDATA GetProperties() [method] + + Returns all the data associated to a message, as well as its + current status. + + Cancel() [method]
doc/sms-api.txt Signals PropertyChanged(string name, variant value) · line 138
Quoted 10 lines
doc/sms-api.txt +10
-0+ + Cancel oFono's handling of the message, wether is submitting + it, retrying submissions, etc. + + PropertyChanged() [signal] + + Some property in the message has changed. If status change, + provide old and new status -- to ease up state machine + processing. +
Please convert this to proper doc/sms-api.txt style and remove the old API.
doc/sms-api.txt Signals PropertyChanged(string name, variant value) · line 142
Quoted 4 lines
doc/sms-api.txt +4
-0+ Dennis: subscribers can subscribe to signals from all object + that support an interface, so it's easy to subscribe to get + the signals from all the objects without having to subscribe + to each object.
This is a task that needs to be added to the TODO for multiple interfaces. In particular NetworkOperator, VoiceCall, PrimaryContext, etc.
Regards,
-Denis
Re: [SMS D-Bus 10/23] sms: add doc about the extensions D-Bus API (not yet implemented)
Quoted 4 lines · Denis Kenzior
Hi Inaky,
Quoted 2 lines
(quoted diff) +2
-0+ +
....
Quoted 10 lines
doc/sms-api.txt Signals PropertyChanged(string name, variant value) · line 142
Quoted 4 lines
doc/sms-api.txt +4
-0+ Dennis: subscribers can subscribe to signals from all object + that support an interface, so it's easy to subscribe to get + the signals from all the objects without having to subscribe + to each object.This is a task that needs to be added to the TODO for multiple interfaces. In particular NetworkOperator, VoiceCall, PrimaryContext, etc.
Regards,
-Denis
K, will report all these too.
PATCH 11/23 struct tx_queue_entry: add fields and destructor
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
Add a name field that will be used to contain a permanent name for each SMS message in-transit, to be used for persistent storage and for D-Bus object naming.
The persist name will be used to denote if persistence has to be implemented in this in-transit SMS or not.
Introduce also a destructor function to encapsulate all the release steps for this data type, as more are to be added later.
--- src/sms.c | 26 +++++++++++++++++++++++--- 1 files changed, 23 insertions(+), 3 deletions(-)
src/sms.c +23 -3
@@ -76,6 +76,7 @@ struct pending_pdu {int pdu_len; }; +struct tx_queue_entry {struct pending_pdu *pdus;unsigned char num_pdus;
@@ -291,6 +292,25 @@ static DBusMessage *sms_set_property(DBusConnection *conn, DBusMessage *msg,return __ofono_error_invalid_args (msg); } + +/* +* Destroy/release the contents of a 'struct tx_queue_entry' +* +* This *only* releases resources allocated *inside* @entry. The +* caller still has to explicitly free @entry (if needed) itself. +*/ +static void tx_queue_entry_destroy (struct tx_queue_entry *entry) +{ +g_free (entry->pdus); +} + +static void tx_queue_entry_destroy_free (gpointer _entry,gpointer unused) +{ +struct tx_queue_entry *entry = _entry; +tx_queue_entry_destroy (entry); +g_free (entry); +} +static void tx_finished (const struct ofono_error *error,int mr,void *data) {struct ofono_sms *sms = data;
@@ -320,7 +340,7 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data)time (NULL ), OFONO_HISTORY_SMS_STATUS_SUBMIT_FAILED); -g_free (entry->pdus); +tx_queue_entry_destroy (entry);g_free (entry);if (g_queue_peek_head (sms->txq)) {
@@ -353,7 +373,7 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data)time (NULL ), OFONO_HISTORY_SMS_STATUS_SUBMITTED); -g_free (entry->pdus); +tx_queue_entry_destroy (entry);g_free (entry);if (g_queue_peek_head (sms->txq)) {
@@ -987,7 +1007,7 @@ static void sms_remove(struct ofono_atom *atom) }if (sms->txq) { -g_queue_foreach (sms->txq, (GFunc )g_free,NULL ); +g_queue_foreach (sms->txq, tx_queue_entry_destroy_free,NULL );g_queue_free (sms->txq); sms->txq =NULL ; }
Signature · 1 line
1.6.6.1
Re: [SMS D-Bus 11/23] struct tx_queue_entry: add fields and destructor
Hi Inaky,
<snip>
Quoted 24 lines
src/sms.c +19
-0+ +/* +* Destroy/release the contents of a 'struct tx_queue_entry' +* +* This *only* releases resources allocated *inside* @entry. The +* caller still has to explicitly free @entry (if needed) itself. +*/ +static void tx_queue_entry_destroy (struct tx_queue_entry *entry) +{ +g_free (entry->pdus); +} + +static void tx_queue_entry_destroy_free (gpointer _entry,gpointer unused) +{ +struct tx_queue_entry *entry = _entry; +tx_queue_entry_destroy (entry); +g_free (entry); +} +static void tx_finished (const struct ofono_error *error,int mr,void *data) {struct ofono_sms *sms = data;
src/sms.c static void tx_finished(const struct ofono_error *error, int mr, void *data) · line 344
Quoted 6 lines
src/sms.c +1
-1@@ -320,7 +340,7 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data)time (NULL ), OFONO_HISTORY_SMS_STATUS_SUBMIT_FAILED); -g_free (entry->pdus); +tx_queue_entry_destroy (entry);g_free (entry);
It seems we can use tx_queue_entry_destroy_free here...
src/sms.c static void tx_finished(const struct ofono_error *error, int mr, void *data) · line 377
Quoted 9 lines
src/sms.c +1
-1if (g_queue_peek_head (sms->txq)) {@@ -353,7 +373,7 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data)time (NULL ), OFONO_HISTORY_SMS_STATUS_SUBMITTED); -g_free (entry->pdus); +tx_queue_entry_destroy (entry);g_free (entry);
Same comment as above.
src/sms.c static void sms_remove(struct ofono_atom *atom) · line 1013
Quoted 11 lines
src/sms.c +1
-1if (g_queue_peek_head (sms->txq)) {@@ -987,7 +1007,7 @@ static void sms_remove(struct ofono_atom *atom) }if (sms->txq) { -g_queue_foreach (sms->txq, (GFunc )g_free,NULL ); +g_queue_foreach (sms->txq, tx_queue_entry_destroy_free,NULL );g_queue_free (sms->txq); sms->txq =NULL ; }
Can we simply unify the two functions and simply call it tx_queue_entry_free? For symmetry renaming create_tx_queue_entry to tx_queue_entry_new would be nice.
Regards,
-Denis
Re: [SMS D-Bus 11/23] struct tx_queue_entry: add fields and destructor
Quoted 6 lines · Denis Kenzior
Hi Inaky,
<snip>
....
Quoted 10 lines
src/sms.c static void sms_remove(struct ofono_atom *atom) · line 1013
Quoted 6 lines
src/sms.c +1
-1if (sms->txq) { -g_queue_foreach (sms->txq, (GFunc )g_free,NULL ); +g_queue_foreach (sms->txq, tx_queue_entry_destroy_free,NULL );g_queue_free (sms->txq); sms->txq =NULL ; }Can we simply unify the two functions and simply call it
tx_queue_entry_free? For symmetry renamingcreate_tx_queue_entrytotx_queue_entry_newwould be nice.
Makes sense -- thought it cannot be done, but I realized sms_msg_cancel() needs a _destroy_free(), not just a _destroy(), so that settles that last one. Will do the _create() rename too.
Re: [SMS D-Bus 11/23] struct tx_queue_entry: add fields and destructor
Hi Inaky,
Quoted 18 lines · Inaky Perez-Gonzalez
Quoted 6 lines · Denis Kenzior
Hi Inaky,
<snip>
....
Quoted 10 lines
src/sms.c static void sms_remove(struct ofono_atom *atom) · line 1013
Quoted 6 lines
src/sms.c +1
-1if (sms->txq) { -g_queue_foreach (sms->txq, (GFunc )g_free,NULL ); +g_queue_foreach (sms->txq, tx_queue_entry_destroy_free,NULL );g_queue_free (sms->txq); sms->txq =NULL ; }Can we simply unify the two functions and simply call it
tx_queue_entry_free? For symmetry renamingcreate_tx_queue_entrytotx_queue_entry_newwould be nice.
Quoted 5 lines
Makes sense -- thought it cannot be done, but I realized
sms_msg_cancel()needs a_destroy_free(), not just a_destroy(), so that settles that last one. Will do the_create()rename too.
Good, can you resubmit this one separately soonish? I'm touching this area of the code because of Andrew's changes for STK Send SMS command handling.
Regards,
-Denis
Re: [SMS D-Bus 11/23] struct tx_queue_entry: add fields and destructor
Quoted 10 lines · Denis Kenzior
Hi Inaky,
....
Quoted 3 lines
settles that last one. Will do the
_create()rename too.Good, can you resubmit this one separately soonish? I'm touching this area of the code because of Andrew's changes for STK Send SMS command handling.
I'll resubmit the whole series of stuff you haven't merged yet with all the feedback incorporated. Should be done tomorrow the latest.
Re: [SMS D-Bus 11/23] struct tx_queue_entry: add fields and destructor
Hi Inaky,
Quoted 8 lines · Inaky Perez-Gonzalez
Quoted 7 lines · Denis Kenzior
Hi Inaky,
....
Quoted 3 lines
settles that last one. Will do the
_create()rename too.
Quoted 7 lines
Quoted 3 lines
Good, can you resubmit this one separately soonish? I'm touching this area of the code because of Andrew's changes for STK Send SMS command handling.
I'll resubmit the whole series of stuff you haven't merged yet with all the feedback incorporated. Should be done tomorrow the latest.
The reason I'm asking is that I was about to do almost the exact same set of changes. Tomorrow is fine though.
Regards,
-Denis
Re: [SMS D-Bus 11/23] struct tx_queue_entry: add fields and destructor
Quoted 11 lines · Denis Kenzior
...
Quoted 7 lines
Quoted 3 lines
Good, can you resubmit this one separately soonish? I'm touching this area of the code because of Andrew's changes for STK Send SMS command handling.
I'll resubmit the whole series of stuff you haven't merged yet with all the feedback incorporated. Should be done tomorrow the latest.
The reason I'm asking is that I was about to do almost the exact same set of changes. Tomorrow is fine though.
Dennis, I am running behind on this one. The commits have been regenerated with the feedback you gave -- however, I picked up a bug when hitting cancel when the message is in _QUEUED state and I still haven't been able to tackle it. Quite hard to reproduce as the transition out of _QUEUED happens quite fast; currently working on a python script to automate the test.
If you want, I can submit with the buglet, but I'd rather *not* do that.
Re: [SMS D-Bus 11/23] struct tx_queue_entry: add fields and destructor
Hi Inaky,
Quoted 13 lines · Inaky Perez-Gonzalez
Quoted 11 lines · Denis Kenzior
...
Quoted 7 lines
Quoted 3 lines
Good, can you resubmit this one separately soonish? I'm touching this area of the code because of Andrew's changes for STK Send SMS command handling.
I'll resubmit the whole series of stuff you haven't merged yet with all the feedback incorporated. Should be done tomorrow the latest.
The reason I'm asking is that I was about to do almost the exact same set of changes. Tomorrow is fine though.
Quoted 9 lines
Dennis, I am running behind on this one. The commits have been regenerated with the feedback you gave -- however, I picked up a bug when hitting cancel when the message is in
_QUEUEDstate and I still haven't been able to tackle it. Quite hard to reproduce as the transition out of_QUEUEDhappens quite fast; currently working on a python script to automate the test.If you want, I can submit with the buglet, but I'd rather *not* do that.
No worries, take your time. Just as a heads up, I pushed some changes to src/sms.c today.
Regards,
-Denis
Re: [SMS D-Bus 11/23] struct tx_queue_entry: add fields and destructor
Quoted 23 lines · Denis Kenzior
Hi Inaky,
Quoted 20 lines · Inaky Perez-Gonzalez
Quoted 11 lines · Denis Kenzior
...
Quoted 7 lines
Quoted 3 lines
Good, can you resubmit this one separately soonish? I'm touching this area of the code because of Andrew's changes for STK Send SMS command handling.
I'll resubmit the whole series of stuff you haven't merged yet with all the feedback incorporated. Should be done tomorrow the latest.
The reason I'm asking is that I was about to do almost the exact same set of changes. Tomorrow is fine though.
Dennis, I am running behind on this one. The commits have been regenerated with the feedback you gave -- however, I picked up a bug when hitting cancel when the message is in
_QUEUEDstate and I still haven't been able to tackle it. Quite hard to reproduce as the transition out of_QUEUEDhappens quite fast; currently working on a python script to automate the test.
Quoted 5 lines
Quoted 2 lines
If you want, I can submit with the buglet, but I'd rather *not* do that.
No worries, take your time. Just as a heads up, I pushed some changes to
src/sms.ctoday.
Ack, thx. I'll do a rebase before pushing once I am done chasing that bugger.
Re: [SMS D-Bus 11/23] struct tx_queue_entry: add fields and destructor
In reply to Denis KenziorQuoted 23 lines · Denis Kenzior
Hi Inaky,
Quoted 20 lines · Inaky Perez-Gonzalez
Quoted 11 lines · Denis Kenzior
...
Quoted 7 lines
Quoted 3 lines
Good, can you resubmit this one separately soonish? I'm touching this area of the code because of Andrew's changes for STK Send SMS command handling.
I'll resubmit the whole series of stuff you haven't merged yet with all the feedback incorporated. Should be done tomorrow the latest.
The reason I'm asking is that I was about to do almost the exact same set of changes. Tomorrow is fine though.
Dennis, I am running behind on this one. The commits have been regenerated with the feedback you gave -- however, I picked up a bug when hitting cancel when the message is in
_QUEUEDstate and I still haven't been able to tackle it. Quite hard to reproduce as the transition out of_QUEUEDhappens quite fast; currently working on a python script to automate the test.
Quoted 5 lines
Quoted 2 lines
If you want, I can submit with the buglet, but I'd rather *not* do that.
No worries, take your time. Just as a heads up, I pushed some changes to
src/sms.ctoday.
Grrr -- found the bug and deal with it. However, when I tried to pull your tree to rebase on top, I got weird conflicts on stkutil.c -- don't have time to diagnose now, but just wanted to ask: did you do a rebase?
Re: [SMS D-Bus 11/23] struct tx_queue_entry: add fields and destructor
Inaky,
Quoted 27 lines · Inaky Perez-Gonzalez
Quoted 26 lines · Denis Kenzior
Hi Inaky,
Quoted 22 lines · Inaky Perez-Gonzalez
Quoted 11 lines · Denis Kenzior
...
Quoted 7 lines
Quoted 3 lines
Good, can you resubmit this one separately soonish? I'm touching this area of the code because of Andrew's changes for STK Send SMS command handling.
I'll resubmit the whole series of stuff you haven't merged yet with all the feedback incorporated. Should be done tomorrow the latest.
The reason I'm asking is that I was about to do almost the exact same set of changes. Tomorrow is fine though.
Dennis, I am running behind on this one. The commits have been regenerated with the feedback you gave -- however, I picked up a bug when hitting cancel when the message is in
_QUEUEDstate and I still haven't been able to tackle it. Quite hard to reproduce as the transition out of_QUEUEDhappens quite fast; currently working on a python script to automate the test.If you want, I can submit with the buglet, but I'd rather *not* do that.
Quoted 8 lines
Quoted 2 lines
No worries, take your time. Just as a heads up, I pushed some changes to
src/sms.ctoday.Grrr -- found the bug and deal with it. However, when I tried to pull your tree to rebase on top, I got weird conflicts on
stkutil.c-- don't have time to diagnose now, but just wanted to ask: did you do a rebase?
Yep.
Regards,
-Denis
Re: [SMS D-Bus 11/23] struct tx_queue_entry: add fields and destructor
Quoted 4 lines · Denis Kenzior
Inaky,
....
Quoted 1 line
Quoted 1 line
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
No worries, take your time. Just as a heads up, I pushed some changes to
src/sms.ctoday.Grrr -- found the bug and deal with it. However, when I tried to pull your tree to rebase on top, I got weird conflicts on
stkutil.c-- don't have time to diagnose now, but just wanted to ask: did you do a rebase?Yep.
Rats. Don't. Rebase. A. Public. Tree. Please! :)
Re: [SMS D-Bus 11/23] struct tx_queue_entry: add fields and destructor
Quoted 8 lines · Inaky Perez-Gonzalez
Quoted 7 lines · Denis Kenzior
Inaky,
....
Quoted 4 lines
Quoted 3 lines
No worries, take your time. Just as a heads up, I pushed some changes to
src/sms.ctoday.
Quoted 9 lines
Quoted 5 lines
Quoted 3 lines
Grrr -- found the bug and deal with it. However, when I tried to pull your tree to rebase on top, I got weird conflicts on
stkutil.c-- don't have time to diagnose now, but just wanted to ask: did you do a rebase?Yep.
Rats. Don't. Rebase. A. Public. Tree. Please! :)
Maybe we're not talking about the same thing. My workflow is
git commit / git am
git fetch
git rebase
git push
Regards,
-Denis
Re: [SMS D-Bus 11/23] struct tx_queue_entry: add fields and destructor
Quoted 19 lines · Denis Kenzior
Quoted 8 lines · Inaky Perez-Gonzalez
Quoted 7 lines · Denis Kenzior
Inaky,
....
Quoted 4 lines
Quoted 3 lines
No worries, take your time. Just as a heads up, I pushed some changes to
src/sms.ctoday.Quoted 9 lines
Quoted 5 lines
Quoted 3 lines
Grrr -- found the bug and deal with it. However, when I tried to pull your tree to rebase on top, I got weird conflicts on
stkutil.c-- don't have time to diagnose now, but just wanted to ask: did you do a rebase?Yep.
Rats. Don't. Rebase. A. Public. Tree. Please! :)
Quoted 6 lines
Maybe we're not talking about the same thing. My workflow is
git commit / git am
git fetch
git rebase
git push
gee, didn't see this -- well, if the rebase is changing the commit IDs, which it probably does, then you are breaking the commit history. Would depend on the details.
Anyway, I am not so sure that is the problem. Spent the whole day going around it and I still couldn't figure the sucker out. Will keep trying.
Re: [SMS D-Bus 11/23] struct tx_queue_entry: add fields and destructor
Hi Inaky,
Quoted 7 lines
Quoted 6 lines
Maybe we're not talking about the same thing. My workflow is
git commit / git am
git fetch
git rebase
git push
Quoted 6 lines
gee, didn't see this -- well, if the rebase is changing the commit IDs, which it probably does, then you are breaking the commit history. Would depend on the details.
Anyway, I am not so sure that is the problem. Spent the whole day going around it and I still couldn't figure the sucker out. Will keep trying.
the ofono.git master tree never got re-based. All commit ids are fully atomic. So that is not the problem.
Regards
Marcel
Re: [SMS D-Bus 11/23] struct tx_queue_entry: add fields and destructor
In reply to Denis KenziorQuoted 21 lines · Denis Kenzior
Hi Inaky,
Quoted 18 lines · Inaky Perez-Gonzalez
Quoted 16 lines · Denis Kenzior
Hi Inaky,
<snip>
....
src/sms.c static void sms_remove(struct ofono_atom *atom) · line 1013
Quoted 6 lines
src/sms.c +1
-1if (sms->txq) { -g_queue_foreach (sms->txq, (GFunc )g_free,NULL ); +g_queue_foreach (sms->txq, tx_queue_entry_destroy_free,NULL );g_queue_free (sms->txq); sms->txq =NULL ; }Can we simply unify the two functions and simply call it
tx_queue_entry_free? For symmetry renamingcreate_tx_queue_entrytotx_queue_entry_newwould be nice.
Quoted 7 lines
Quoted 3 lines
Makes sense -- thought it cannot be done, but I realized
sms_msg_cancel()needs a_destroy_free(), not just a_destroy(), so that settles that last one. Will do the_create()rename too.Good, can you resubmit this one separately soonish? I'm touching this area of the code because of Andrew's changes for STK Send SMS command handling.
Sigh, your changes broke havoc on mine, as they completely change the code flow. I started trying to integrate Friday after giving up on that rebase (or not) issue and just applying commits as patches, but it is almost like starting from scratch.
Heads up, this means I'll have to retest the heck out of it and yet another delay once I figure out how to reshape everything for the new code flow.
PATCH 12/23 SMS: produce a unique, persistent name for in-transit messages
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
This name will be used for persistence and D-Bus object naming. It is not very humanly readable, but most dependable to avoid collisions.
--- src/sms.c | 12 ++++++++++++ src/smsutil.h | 6 ++++++ 2 files changed, 18 insertions(+), 0 deletions(-)
src/sms.c +12 -0
@@ -77,6 +77,9 @@ struct pending_pdu { }; +/* +* @name: Name for the SMS message object (used by D-Bus) +*/ struct tx_queue_entry {struct pending_pdu *pdus;unsigned char num_pdus;
@@ -86,6 +89,7 @@ struct tx_queue_entry {DBusMessage *msg;gboolean status_report;struct sms_address receiver; +char *name; }; static void set_sca(struct ofono_sms *sms,
@@ -302,6 +306,7 @@ static DBusMessage *sms_set_property(DBusConnection *conn, DBusMessage *msg,static void tx_queue_entry_destroy (struct tx_queue_entry *entry) {g_free (entry->pdus); +g_free (entry->name); }static void tx_queue_entry_destroy_free (gpointer _entry,gpointer unused)
@@ -461,6 +466,9 @@ static struct tx_queue_entry *create_tx_queue_entry(GSList *msg_list)* is created by create_tx_queue_entry() and g_queue_push_tail() * appends that entry to the SMS transmit queue. Then the tx_next() * function is scheduled to run to process the queue. +* +* @sms is the main SMS driver struct, @entry and @msg_list represent +* the current message being processed. */ static DBusMessage *sms_send_message (DBusConnection *conn,DBusMessage *msg,void *data)
@@ -476,6 +484,7 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,DECLARE_SMS_MSG_ID_STRBUF (msg_id_str);struct sms_address receiver;DECLARE_SMS_ADDR_STR (receiver_str); +const char *sms_path =__ofono_atom_get_path (sms->atom); if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &to, DBUS_TYPE_STRING, &text,
@@ -520,6 +529,9 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,g_slist_free (msg_list); entry->msg =dbus_message_ref (msg); entry->status_report = sms->use_delivery_reports; + entry->name =g_strdup_printf (SMS_MSG_NAME_FMT , sms_path, + msg_id_str, entry->num_pdus); +ofono_debug ("sms/entry %p name %s\n" , entry, entry->name);g_queue_push_tail (sms->txq, entry);
src/smsutil.h +6 -0
@@ -634,4 +634,10 @@ void sms_msg_id_memcpy(void *, size_t, const struct sms_msg_id *);/* for unit testing only */ gsize __sms_msg_id_hash_length (void ); +/* +* UNIQUE-ID_#PDUS -- note we use underscores as to generate a name +* suitable to be a D-Bus name. +*/ +#define SMS_MSG_NAME_FMT "%s/%s_%i" +#endif /* #ifndef __smsutil_h__ */
Signature · 1 line
1.6.6.1
PATCH 13/23 SMS: introduce bare state machine and transitions
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
This adds state to outgoing/in-transit SMS messages. This will be used later on for persistence / D-Bus, when the SMS life cycle is expanded.
The state is a variable in the 'struct tx_queue_entry' which gets updated as messages go through the hoops.
--- src/sms.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 107 insertions(+), 0 deletions(-)
src/sms.c +107 -0
@@ -50,6 +50,35 @@ static gboolean tx_next(gpointer user_data);static GSList *g_drivers =NULL ; +/* +* SMS TX message's state +* +* When a message is queued to be delivered, it will transition +* through a set of states. +* +* Allowed transition table (Allowed, Not-allowed) from left to right: +* +* UNINITIALIZED CANCELING FAILED +* | QUEUED DONE | CANCELLED EXPIRED +* UNINITIALIZED - A N N N N N +* QUEUED N - A A N A N +* DONE A N - N N N N +* CANCELING N N N - A A A +* CANCELLED A N N N - N N +* FAILED A N N N N - N +* EXPIRED A N N N N N - +*/ +enum ofono_sms_tx_state { +OFONO_SMS_TX_ST_UNINITIALIZED , +OFONO_SMS_TX_ST_QUEUED , +OFONO_SMS_TX_ST_DONE , +OFONO_SMS_TX_ST_CANCELING , +OFONO_SMS_TX_ST_CANCELLED , +OFONO_SMS_TX_ST_FAILED , +OFONO_SMS_TX_ST_EXPIRED , + __OFONO_SMS_TX_ST_INVALID, +}; +struct ofono_sms {int flags;DBusMessage *pending;
@@ -79,6 +108,7 @@ struct pending_pdu {/* * @name: Name for the SMS message object (used by D-Bus) +* @state: Current state of the (in-transit) SMS */ struct tx_queue_entry {struct pending_pdu *pdus;
@@ -90,6 +120,7 @@ struct tx_queue_entry {gboolean status_report;struct sms_address receiver;char *name; +enum ofono_sms_tx_state state; }; static void set_sca(struct ofono_sms *sms,
@@ -297,6 +328,77 @@ static DBusMessage *sms_set_property(DBusConnection *conn, DBusMessage *msg, } +/* Check if a state transition is legal */ +static void ofono_sms_tx_state_check (const char *file,unsigned line, +const struct tx_queue_entry *entry, +enum ofono_sms_tx_state state_old, +enum ofono_sms_tx_state state_new, +unsigned states_allowed_bm) +{ +if (((1 << state_new) & states_allowed_bm) ==0 ) +ofono_warn ("%s:%d: SW BUG? Forbidden state change " +"%p %u -> %u\n" , + file, line, entry, state_old, state_new); +} + + +/* +* Set a pending SMS's state +* +* This is just syntatic sugar that validates that the transition is +* correct and warns out otherwise. The transition table is defined in +* the doc block for 'enum ofono_sms_tx_state'. +* +* In case of inconsistency, we just warn and press forward. +*/ +#define ofono_sms_tx_state_set (entry, new_state) \ + __ofono_sms_tx_state_set(entry, new_state, __FILE__, __LINE__) + +static void __ofono_sms_tx_state_set (struct tx_queue_entry *entry, +enum ofono_sms_tx_state state_new, +const char *file,unsigned line) +{ +enum ofono_sms_tx_state state_old = entry->state; + +switch (state_old) { +case OFONO_SMS_TX_ST_UNINITIALIZED : +ofono_sms_tx_state_check ( + file, line, entry, state_old, state_new, +1 <<OFONO_SMS_TX_ST_QUEUED ); +break ; +case OFONO_SMS_TX_ST_QUEUED : +ofono_sms_tx_state_check ( + file, line, entry, state_old, state_new, +1 <<OFONO_SMS_TX_ST_DONE + |1 <<OFONO_SMS_TX_ST_CANCELING + |1 <<OFONO_SMS_TX_ST_FAILED ); +break ; +case OFONO_SMS_TX_ST_CANCELING : +ofono_sms_tx_state_check ( + file, line, entry, state_old, state_new, +1 <<OFONO_SMS_TX_ST_CANCELLED + |1 <<OFONO_SMS_TX_ST_FAILED + |1 <<OFONO_SMS_TX_ST_EXPIRED ); +break ; +case OFONO_SMS_TX_ST_DONE : +case OFONO_SMS_TX_ST_CANCELLED : +case OFONO_SMS_TX_ST_FAILED : +case OFONO_SMS_TX_ST_EXPIRED : +ofono_sms_tx_state_check ( + file, line, entry, state_old, state_new, +1 <<OFONO_SMS_TX_ST_UNINITIALIZED ); +break ; +case __OFONO_SMS_TX_ST_INVALID: +default : +ofono_warn ("%s:%d: SW BUG? Bad state change %p %u -> %u\n" , + file, line, entry, state_old, state_new); + } +ofono_debug ("%s:%d: SMS state change: %p %u -> %u\n" , + file, line, entry, state_old, state_new); + entry->state = state_new; +} + +/* * Destroy/release the contents of a 'struct tx_queue_entry' *
@@ -307,6 +409,7 @@ static void tx_queue_entry_destroy(struct tx_queue_entry *entry) {g_free (entry->pdus);g_free (entry->name); + entry->state = __OFONO_SMS_TX_ST_INVALID; }static void tx_queue_entry_destroy_free (gpointer _entry,gpointer unused)
@@ -337,6 +440,7 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data)DBG ("Max retries reached, giving up" ); +ofono_sms_tx_state_set (entry,OFONO_SMS_TX_ST_CANCELING ); entry =g_queue_pop_head (sms->txq);__ofono_dbus_pending_reply (&entry->msg,__ofono_error_failed (entry->msg));
@@ -345,6 +449,7 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data)time (NULL ), OFONO_HISTORY_SMS_STATUS_SUBMIT_FAILED); +ofono_sms_tx_state_set (entry,OFONO_SMS_TX_ST_FAILED );tx_queue_entry_destroy (entry);g_free (entry);
@@ -378,6 +483,7 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data)time (NULL ), OFONO_HISTORY_SMS_STATUS_SUBMITTED); +ofono_sms_tx_state_set (entry,OFONO_SMS_TX_ST_DONE );tx_queue_entry_destroy (entry);g_free (entry);
@@ -534,6 +640,7 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,ofono_debug ("sms/entry %p name %s\n" , entry, entry->name);g_queue_push_tail (sms->txq, entry); +ofono_sms_tx_state_set (entry,OFONO_SMS_TX_ST_QUEUED ); modem =__ofono_atom_get_modem (sms->atom); __ofono_history_sms_send_pending(modem, entry->msg_id, to,
Signature · 1 line
1.6.6.1
PATCH 14/23 SMS: export outgoing messages over D-Bus (skeleton)
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
This creates two frunctions, sms_msg_[un]register() which will add/remove a D-Bus object for each SMS message when in transit.
Future changes make sms_msg_register() need information that is not available at the time create_tx_queue_entry() is called, thus why registration happens later.
--- src/sms.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 57 insertions(+), 1 deletions(-)
src/sms.c +57 -1
@@ -41,6 +41,10 @@#define SMS_MANAGER_FLAG_CACHED 0x1 +/* D-Bus interface name for SMS messages (not for the manager!) */ +static +const char SMS_MSG_INTERFACE [] ="org.ofono.SMSMessage" ; +#define SETTINGS_STORE "sms"#define SETTINGS_GROUP "Settings"
@@ -108,6 +112,7 @@ struct pending_pdu {/* * @name: Name for the SMS message object (used by D-Bus) +* @dbus_path: D-Bus path for this object * @state: Current state of the (in-transit) SMS */ struct tx_queue_entry {
@@ -120,6 +125,7 @@ struct tx_queue_entry {gboolean status_report;struct sms_address receiver;char *name; +char *dbus_path;enum ofono_sms_tx_state state; };
@@ -400,6 +406,55 @@ static void __ofono_sms_tx_state_set(struct tx_queue_entry *entry,/* +* D-Bus SMS Message interface +* +* NOTE: the sms_msg_{methods,signals} tables should be const, but +* then g_dbus_register_interface() type warns. +*/ + +static +GDBusMethodTable sms_msg_methods[] = { + { } +}; + + +static +GDBusSignalTable sms_msg_signals[] = { + {"PropertyChanged" ,"sv" }, + { } +}; + + +static +void sms_msg_register (struct tx_queue_entry *sms_msg) +{ +g_assert (sms_msg->name !=NULL ); + sms_msg->dbus_path =g_strdup (sms_msg->name); +if (!g_dbus_register_interface (ofono_dbus_get_connection (), + sms_msg->dbus_path,SMS_MSG_INTERFACE , + sms_msg_methods, sms_msg_signals, +NULL , sms_msg,NULL )) { +ofono_error ("%s: Could not create %s interface" , + sms_msg->dbus_path,SMS_MSG_INTERFACE ); +g_free (sms_msg->dbus_path); + } +ofono_debug ("%s: %d: sms %p: MSG registered @ %s" , + __FILE__, __LINE__, sms_msg, sms_msg->dbus_path); +} + + +static +void sms_msg_unregister (struct tx_queue_entry *sms_msg) +{ +g_dbus_unregister_interface (ofono_dbus_get_connection (), + sms_msg->dbus_path,SMS_MSG_INTERFACE ); +g_free (sms_msg->dbus_path); +ofono_debug ("%s: %d: sms %p: MSG unregistered" , + __FILE__, __LINE__, sms_msg); +} + + +/* * Destroy/release the contents of a 'struct tx_queue_entry' * * This *only* releases resources allocated *inside* @entry. The
@@ -407,6 +462,7 @@ static void __ofono_sms_tx_state_set(struct tx_queue_entry *entry,*/ static void tx_queue_entry_destroy (struct tx_queue_entry *entry) { +sms_msg_unregister (entry);g_free (entry->pdus);g_free (entry->name); entry->state = __OFONO_SMS_TX_ST_INVALID;
@@ -556,7 +612,6 @@ static struct tx_queue_entry *create_tx_queue_entry(GSList *msg_list)DBG ("pdu_len: %d, tpdu_len: %d" , pdu->pdu_len, pdu->tpdu_len); } -return entry; }
@@ -638,6 +693,7 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg, entry->name =g_strdup_printf (SMS_MSG_NAME_FMT , sms_path, msg_id_str, entry->num_pdus);ofono_debug ("sms/entry %p name %s\n" , entry, entry->name); +sms_msg_register (entry);g_queue_push_tail (sms->txq, entry);ofono_sms_tx_state_set (entry,OFONO_SMS_TX_ST_QUEUED );
Signature · 1 line
1.6.6.1
Re: [SMS D-Bus 14/23] SMS: export outgoing messages over D-Bus (skeleton)
Quoted 8 lines · Inaky Perez-Gonzalez
From: Inaky Perez-Gonzalez <[email protected]>
This creates two frunctions, sms_msg_[un]register() which will add/remove a D-Bus object for each SMS message when in transit.
Future changes make
sms_msg_register()need information that is not available at the timecreate_tx_queue_entry()is called, thus why registration happens later.
Found a few rough corners here when integrating the final parts of status reports into the SMS state-machine. A re-submit will have this incremental patch merged:
src/sms.c +22 -6
@@ -532,8 +532,15 @@ GDBusSignalTable sms_msg_signals[] = { }; +/** +* +* @returns 0 if ok, 0 on error +* +* On success, sms_msg->dbus_path is non-NULL; this is needed as the +* cleanup path on error are not easy to break up. +*/ static -void sms_msg_register (struct tx_queue_entry *sms_msg) +int sms_msg_register (struct tx_queue_entry *sms_msg) {g_assert (sms_msg->name !=NULL ); sms_msg->dbus_path =g_strdup (sms_msg->name);
@@ -544,20 +551,26 @@ void sms_msg_register(struct tx_queue_entry *sms_msg)ofono_error ("%s: Could not create %s interface" , sms_msg->dbus_path,SMS_MSG_INTERFACE );g_free (sms_msg->dbus_path); + sms_msg->dbus_path =NULL ; +return 1 ; + }else { +ofono_debug ("%s: %d: sms %p @ %s: MSG registered" , + __FILE__, __LINE__, sms_msg, sms_msg->dbus_path); +return 0 ; } -ofono_debug ("%s: %d: sms %p: MSG registered @ %s" , - __FILE__, __LINE__, sms_msg, sms_msg->dbus_path); }static void sms_msg_unregister (struct tx_queue_entry *sms_msg) { +if (sms_msg->dbus_path ==NULL ) +return ; +ofono_debug ("%s: %d: sms %p @ %s: MSG unregistered" , + __FILE__, __LINE__, sms_msg, sms_msg->dbus_path);g_dbus_unregister_interface (ofono_dbus_get_connection (), sms_msg->dbus_path,SMS_MSG_INTERFACE );g_free (sms_msg->dbus_path); -ofono_debug ("%s: %d: sms %p: MSG unregistered" , - __FILE__, __LINE__, sms_msg); }
@@ -801,7 +814,10 @@ struct tx_queue_entry *sms_msg_send( entry->name =g_strdup_printf (SMS_MSG_NAME_FMT , sms_path, msg_id_str, entry->num_pdus);ofono_debug ("sms/entry %p name %s\n" , entry, entry->name); -sms_msg_register (entry); +if (sms_msg_register (entry)) { +tx_queue_entry_destroy_free (entry,NULL ); +return NULL ; + }g_queue_push_tail (sms->txq, entry);ofono_sms_tx_state_set (entry,OFONO_SMS_TX_ST_QUEUED );
PATCH 15/23 SMS: split sms_send_message() into a D-Bus front end and an internal API
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
sms_send_message() is unfolded into:
- sms_msg_send(), a full C interface for SMS sending
- dbus_SendMessage(), which adapts sms_msg_send() to be a D-Bus call
This is done to allow plugins to use the same infrastructure as D-Bus clients to send SMS messages.
--- src/sms.c | 66 +++++++++++++++++++++++++++++++++++++++++--------------- src/smsutil.h | 6 +++++ 2 files changed, 54 insertions(+), 18 deletions(-)
src/sms.c +48 -18
@@ -466,6 +466,10 @@ static void tx_queue_entry_destroy(struct tx_queue_entry *entry)g_free (entry->pdus);g_free (entry->name); entry->state = __OFONO_SMS_TX_ST_INVALID; +if (entry->msg) { +dbus_message_unref (entry->msg); + entry->msg =NULL ; + } }static void tx_queue_entry_destroy_free (gpointer _entry,gpointer unused)
@@ -615,8 +619,9 @@ static struct tx_queue_entry *create_tx_queue_entry(GSList *msg_list)return entry; } +/* -* Pre-process a SMS text message and deliver it [D-Bus SendMessage()] +* Pre-process a SMS text message and deliver it * * @conn: D-Bus connection * @msg: message data (telephone number and text)
@@ -631,12 +636,10 @@ static struct tx_queue_entry *create_tx_queue_entry(GSList *msg_list)* @sms is the main SMS driver struct, @entry and @msg_list represent * the current message being processed. */ -static DBusMessage *sms_send_message (DBusConnection *conn,DBusMessage *msg, -void *data) +struct tx_queue_entry *sms_msg_send ( +struct ofono_sms *sms,const char *to,const char *text, +enum sms_msg_send_flags flags) { -struct ofono_sms *sms = data; -const char *to; -const char *text;GSList *msg_list;int ref_offset, ref;struct tx_queue_entry *entry;
@@ -647,13 +650,8 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,DECLARE_SMS_ADDR_STR (receiver_str);const char *sms_path =__ofono_atom_get_path (sms->atom); -if (!dbus_message_get_args (msg,NULL ,DBUS_TYPE_STRING , &to, -DBUS_TYPE_STRING , &text, -DBUS_TYPE_INVALID )) -return __ofono_error_invalid_args (msg); -if (valid_phone_number_format (to) == FALSE) -return __ofono_error_invalid_format (msg); +return NULL ;/* * Instead of using the telephone number/address we got from
@@ -664,13 +662,13 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,sms_address_from_string (&receiver, to);if (sms_assembly_encode_address (&receiver, receiver_str) == FALSE) -return __ofono_error_failed (msg); +return NULL ; msg_list =sms_text_prepare (text,0 , TRUE, &ref_offset, sms->use_delivery_reports);if (!msg_list) -return __ofono_error_invalid_format (msg); +return NULL ;sms_msg_id_init (&sms_msg_id);sms_msg_id_hash (&sms_msg_id, text,strlen (text));
@@ -688,7 +686,6 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,g_slist_foreach (msg_list, (GFunc )g_free,NULL );g_slist_free (msg_list); - entry->msg =dbus_message_ref (msg); entry->status_report = sms->use_delivery_reports; entry->name =g_strdup_printf (SMS_MSG_NAME_FMT , sms_path, msg_id_str, entry->num_pdus);
@@ -699,21 +696,54 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,ofono_sms_tx_state_set (entry,OFONO_SMS_TX_ST_QUEUED ); modem =__ofono_atom_get_modem (sms->atom); -__ofono_history_sms_send_pending (modem, entry->msg_id, to, -time (NULL ), text); +if (flags &SMS_MSG_SEND_HISTORY ) +__ofono_history_sms_send_pending (modem, entry->msg_id, to, +time (NULL ), text);if (g_queue_get_length (sms->txq) ==1 ) sms->tx_source =g_timeout_add (0 , tx_next, sms); +return entry; +} + + +/* +* D-Bus: Send a SMS text message +* +* @conn: D-Bus connection +* @msg: message data (telephone number and text) +* @data: SMS object to use for transmision +* +* Unwraps the arguments and sends the request to sms_msg_send() +*/ +static DBusMessage *dbus_SendMessage (DBusConnection *conn,DBusMessage *msg, +void *data) +{ +struct ofono_sms *sms = data; +const char *to; +const char *text; +struct tx_queue_entry *sms_msg; + +if (!dbus_message_get_args (msg,NULL ,DBUS_TYPE_STRING , &to, +DBUS_TYPE_STRING , &text, +DBUS_TYPE_INVALID )) +return __ofono_error_invalid_args (msg); + + sms_msg =sms_msg_send (sms, to, text,SMS_MSG_SEND_HISTORY ); +if (sms_msg ==NULL ) +return __ofono_error_invalid_format (msg); +/* Unreferenced upon tx_queue_entry_destroy() */ + sms_msg->msg =dbus_message_ref (msg);return NULL ; } +static GDBusMethodTable sms_manager_methods[] = { {"GetProperties" ,"" ,"a{sv}" , sms_get_properties,G_DBUS_METHOD_FLAG_ASYNC }, {"SetProperty" ,"sv" ,"" , sms_set_property,G_DBUS_METHOD_FLAG_ASYNC }, - {"SendMessage" ,"ss" ,"" , sms_send_message, + {"SendMessage" ,"ss" ,"" , dbus_SendMessage,G_DBUS_METHOD_FLAG_ASYNC }, { } };
src/smsutil.h +6 -0
@@ -217,6 +217,12 @@ enum cbs_geo_scope {CBS_GEO_SCOPE_CELL_NORMAL }; +/* Flags to sms_msg_send() */ +enum sms_msg_send_flags { +/* Record/dont this message to the history database */ +SMS_MSG_SEND_HISTORY =0x01 , +}; +struct sms_address {enum sms_number_type number_type;enum sms_numbering_plan numbering_plan;
Signature · 1 line
1.6.6.1
PATCH 16/23 SMS: introduce wait-for-ack state and infrastructure
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
This introduces basic infrastructure to support wait-for-ack in SMS messages. It consists of a state definition in the state transtition machine, a wait-for-ack queue (ofono_sms->wfaq) and a flag setting in which a message is tagged as 'requests wait for ack'.
This does not implement the network bits, just the basic infrastructure bits.
--- src/sms.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++------------- 1 files changed, 48 insertions(+), 13 deletions(-)
src/sms.c +48 -13
@@ -62,19 +62,21 @@ static GSList *g_drivers = NULL;* * Allowed transition table (Allowed, Not-allowed) from left to right: * -* UNINITIALIZED CANCELING FAILED -* | QUEUED DONE | CANCELLED EXPIRED -* UNINITIALIZED - A N N N N N -* QUEUED N - A A N A N -* DONE A N - N N N N -* CANCELING N N N - A A A -* CANCELLED A N N N - N N -* FAILED A N N N N - N -* EXPIRED A N N N N N - +* UNINITIALIZED CANCELING FAILED +* | QUEUED WFA DONE | CANCELLED EXPIRED +* UNINITIALIZED - A N N N N N N +* QUEUED N - A A A N A N +* WFA N N - A A N A A +* DONE A N N - N N N N +* CANCELING N N N N - A A A +* CANCELLED A N N N N - N N +* FAILED A N N N N N - N +* EXPIRED A N N N N N N - */ enum ofono_sms_tx_state {OFONO_SMS_TX_ST_UNINITIALIZED ,OFONO_SMS_TX_ST_QUEUED , +OFONO_SMS_TX_ST_WFA ,OFONO_SMS_TX_ST_DONE ,OFONO_SMS_TX_ST_CANCELING ,OFONO_SMS_TX_ST_CANCELLED ,
@@ -83,6 +85,12 @@ enum ofono_sms_tx_state { __OFONO_SMS_TX_ST_INVALID, }; + +/** +* @wfaq: Waiting-For-Acknoledgement queue; messages in this queue +* have been delivered but are waiting to be acknoledged by the +* network. +*/ struct ofono_sms {int flags;DBusMessage *pending;
@@ -90,6 +98,7 @@ struct ofono_sms {struct sms_assembly *assembly;guint ref;GQueue *txq; +GQueue *tx_wfaq;gint tx_source;struct ofono_message_waiting *mw;unsigned int mw_watch;
@@ -113,9 +122,13 @@ struct pending_pdu {/* * @name: Name for the SMS message object (used by D-Bus) * @dbus_path: D-Bus path for this object +* @sms_mgr: SMS manager / driver object * @state: Current state of the (in-transit) SMS +* @status_report: message reception/delivery should be acknoledged by +* the network. */ struct tx_queue_entry { +struct ofono_sms *sms_mgr;struct pending_pdu *pdus;unsigned char num_pdus;unsigned char cur_pdu;
@@ -375,9 +388,17 @@ static void __ofono_sms_tx_state_set(struct tx_queue_entry *entry,case OFONO_SMS_TX_ST_QUEUED :ofono_sms_tx_state_check ( file, line, entry, state_old, state_new, +1 <<OFONO_SMS_TX_ST_WFA + |1 <<OFONO_SMS_TX_ST_DONE + |1 <<OFONO_SMS_TX_ST_CANCELING ); +break ; +case OFONO_SMS_TX_ST_WFA : +ofono_sms_tx_state_check ( + file, line, entry, state_old, state_new,1 <<OFONO_SMS_TX_ST_DONE |1 <<OFONO_SMS_TX_ST_CANCELING - |1 <<OFONO_SMS_TX_ST_FAILED ); + |1 <<OFONO_SMS_TX_ST_FAILED + |1 <<OFONO_SMS_TX_ST_EXPIRED );break ;case OFONO_SMS_TX_ST_CANCELING : ofono_sms_tx_state_check(
@@ -543,9 +564,14 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data)time (NULL ), OFONO_HISTORY_SMS_STATUS_SUBMITTED); -ofono_sms_tx_state_set (entry,OFONO_SMS_TX_ST_DONE ); -tx_queue_entry_destroy (entry); -g_free (entry); +if (entry->status_report) { +ofono_sms_tx_state_set (entry,OFONO_SMS_TX_ST_WFA ); +g_queue_push_tail (sms->tx_wfaq, entry); + }else { +ofono_sms_tx_state_set (entry,OFONO_SMS_TX_ST_DONE ); +tx_queue_entry_destroy (entry); +g_free (entry); + }if (g_queue_peek_head (sms->txq)) {DBG ("Scheduling next" );
@@ -683,6 +709,7 @@ struct tx_queue_entry *sms_msg_send( entry, ref, ref_offset, msg_id_str); entry->msg_id = ref; entry->receiver = receiver; + entry->sms_mgr = sms;g_slist_foreach (msg_list, (GFunc )g_free,NULL );g_slist_free (msg_list);
@@ -1217,6 +1244,13 @@ static void sms_remove(struct ofono_atom *atom) sms->txq =NULL ; } +if (sms->tx_wfaq) { +g_queue_foreach (sms->tx_wfaq, + tx_queue_entry_destroy_free,NULL ); +g_queue_free (sms->tx_wfaq); + sms->tx_wfaq =NULL ; + } +if (sms->settings) {g_key_file_set_integer (sms->settings,SETTINGS_GROUP ,"NextReference" , sms->ref);
@@ -1269,6 +1303,7 @@ struct ofono_sms *ofono_sms_create(struct ofono_modem *modem, sms->sca.type =129 ; sms->ref =1 ; sms->txq =g_queue_new (); + sms->tx_wfaq =g_queue_new (); sms->atom =__ofono_modem_add_atom (modem,OFONO_ATOM_TYPE_SMS , sms_remove, sms);
Signature · 1 line
1.6.6.1
PATCH 17/23 SMS: introduce sms_msg_cancel and its D-Bus wrapper
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
This introduces the ability to cancel a pending SMS message, accessible via an internal API and over a D-Bus wrapper.
Sending a note to the network to cancel an in-transit message is not yet implemented.
--- src/sms.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/smsutil.h | 15 +++++++++++++++ 2 files changed, 72 insertions(+), 0 deletions(-)
src/sms.c +57 -0
@@ -426,6 +426,24 @@ static void __ofono_sms_tx_state_set(struct tx_queue_entry *entry, } +static DBusMessage *dbus_sms_msg_cancel ( +DBusConnection * conn,DBusMessage *msg,void *data) +{ +gboolean do_network_cancel = FALSE; +struct tx_queue_entry *sms_msg = data; +if (!dbus_message_get_args (msg,NULL , +DBUS_TYPE_BOOLEAN , &do_network_cancel)) +return __ofono_error_invalid_args (msg); +sms_msg_cancel (sms_msg, + do_network_cancel ?SMS_MSG_CANCEL_IN_NETWORK :0 ); + sms_msg->msg =dbus_message_ref (msg); +__ofono_dbus_pending_reply ( + &sms_msg->msg, +dbus_message_new_method_return (sms_msg->msg)); +return NULL ; +} + +/* * D-Bus SMS Message interface *
@@ -435,6 +453,8 @@ static void __ofono_sms_tx_state_set(struct tx_queue_entry *entry,static GDBusMethodTable sms_msg_methods[] = { + {"Cancel" ,DBUS_TYPE_BOOLEAN_AS_STRING ,"" , + dbus_sms_msg_cancel,G_DBUS_METHOD_FLAG_ASYNC }, { } };
@@ -734,6 +754,43 @@ struct tx_queue_entry *sms_msg_send( } +/** +* Cancel a pending SMS message +* +* @sms_msg: message to cancel +* @flags: modifies for the action +* +* This function cancels a message that is pending or being +* actively transmitted. +* +* \internal +* +* There is no need to cancel the calling of tx_next() by +* g_timeout_add() scheduled in sms_msg_send(). The rationale behind +* this is that the tx_next() function is scheduled to go over the +* list of messages in the @sms object, so it might have been +* scheduled for other messages also rather than just for this one +* @sms_msg. By the time it gets to run, it might see the list empty +* or see other messages, but @sms_msg won't be there. +*/ +void sms_msg_cancel (struct tx_queue_entry *sms_msg, +enum sms_msg_cancel_flags flags) +{ +struct ofono_sms *sms = sms_msg->sms_mgr; +DBG ("%s (%p)\n" , __func__, sms_msg); +if (sms_msg->state ==OFONO_SMS_TX_ST_QUEUED ) +g_queue_remove (sms->txq, sms_msg); +else if (sms_msg->state ==OFONO_SMS_TX_ST_WFA ) +g_queue_remove (sms->tx_wfaq, sms_msg); +ofono_sms_tx_state_set (sms_msg,OFONO_SMS_TX_ST_CANCELING ); +if (flags &SMS_MSG_CANCEL_IN_NETWORK ) +ofono_warn ("%s(): SMS_MSG_CANCEL_IN_NETWORK support " +"not yet implemented\n" , __func__); +ofono_sms_tx_state_set (sms_msg,OFONO_SMS_TX_ST_CANCELLED ); +tx_queue_entry_destroy (sms_msg); +} + +/* * D-Bus: Send a SMS text message *
src/smsutil.h +15 -0
@@ -559,6 +559,21 @@ gboolean cbs_topic_in_range(unsigned int topic, GSList *ranges);char *ussd_decode (int dcs,int len,const unsigned char *data); +/* +* SMS Message API +*/ + +enum sms_msg_cancel_flags { +/** +* The cancelled message will be reported to the network so it +* tries to cancel it if in transit. +*/ +SMS_MSG_CANCEL_IN_NETWORK =0x1 , +}; + +struct tx_queue_entry ; +void sms_msg_cancel (struct tx_queue_entry *sms_msg,enum sms_msg_cancel_flags ); +enum {/* Note this has to be a multiple of sizeof(guint32); * Compilation will assert-fail if this is not met. */
Signature · 1 line
1.6.6.1
PATCH 18/23 SMS: rename tx_queue_entry->msg to ->dbus_msg for clarity
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
'msg' gets too confusing: is it the SMS message, a D-Bus message?
--- src/sms.c | 25 +++++++++++++------------ 1 files changed, 13 insertions(+), 12 deletions(-)
src/sms.c +13 -12
@@ -134,7 +134,7 @@ struct tx_queue_entry {unsigned char cur_pdu;unsigned int msg_id;unsigned int retry; -DBusMessage *msg; +DBusMessage *dbus_msg;gboolean status_report;struct sms_address receiver;char *name;
@@ -436,10 +436,10 @@ static DBusMessage *dbus_sms_msg_cancel(return __ofono_error_invalid_args (msg);sms_msg_cancel (sms_msg, do_network_cancel ?SMS_MSG_CANCEL_IN_NETWORK :0 ); - sms_msg->msg =dbus_message_ref (msg); + sms_msg->dbus_msg =dbus_message_ref (msg);__ofono_dbus_pending_reply ( - &sms_msg->msg, -dbus_message_new_method_return (sms_msg->msg)); + &sms_msg->dbus_msg, +dbus_message_new_method_return (sms_msg->dbus_msg));return NULL ; }
@@ -507,9 +507,9 @@ static void tx_queue_entry_destroy(struct tx_queue_entry *entry)g_free (entry->pdus);g_free (entry->name); entry->state = __OFONO_SMS_TX_ST_INVALID; -if (entry->msg) { -dbus_message_unref (entry->msg); - entry->msg =NULL ; +if (entry->dbus_msg) { +dbus_message_unref (entry->dbus_msg); + entry->dbus_msg =NULL ; } }
@@ -543,8 +543,8 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data)ofono_sms_tx_state_set (entry,OFONO_SMS_TX_ST_CANCELING ); entry =g_queue_pop_head (sms->txq); -__ofono_dbus_pending_reply (&entry->msg, -__ofono_error_failed (entry->msg)); +__ofono_dbus_pending_reply (&entry->dbus_msg, +__ofono_error_failed (entry->dbus_msg)); __ofono_history_sms_send_status(modem, entry->msg_id, time(NULL),
@@ -578,8 +578,9 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data) } entry =g_queue_pop_head (sms->txq); -__ofono_dbus_pending_reply (&entry->msg, -dbus_message_new_method_return (entry->msg)); +__ofono_dbus_pending_reply ( + &entry->dbus_msg, +dbus_message_new_method_return (entry->dbus_msg));__ofono_history_sms_send_status (modem, entry->msg_id,time (NULL ),OFONO_HISTORY_SMS_STATUS_SUBMITTED );
@@ -817,7 +818,7 @@ static DBusMessage *dbus_SendMessage(DBusConnection *conn, DBusMessage *msg,if (sms_msg ==NULL )return __ofono_error_invalid_format (msg);/* Unreferenced upon tx_queue_entry_destroy() */ - sms_msg->msg =dbus_message_ref (msg); + sms_msg->dbus_msg =dbus_message_ref (msg);return NULL ; }
Signature · 1 line
1.6.6.1
PATCH 19/23 SMS: Implement D-Bus SMS-MSG::GetProperties
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
Currently this only returns the state of the SMS message.
--- src/sms.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 43 insertions(+), 0 deletions(-)
src/sms.c +43 -0
@@ -86,6 +86,24 @@ enum ofono_sms_tx_state { }; +static +const char *ofono_sms_tx_state_to_string (enum ofono_sms_tx_state status) +{ +switch (status) { +case OFONO_SMS_TX_ST_UNINITIALIZED :return "UNINITIALIZED" ; +case OFONO_SMS_TX_ST_QUEUED :return "QUEUED" ; +case OFONO_SMS_TX_ST_WFA :return "WFA" ; +case OFONO_SMS_TX_ST_DONE :return "DONE" ; +case OFONO_SMS_TX_ST_CANCELING :return "CANCELING" ; +case OFONO_SMS_TX_ST_CANCELLED :return "CANCELLED" ; +case OFONO_SMS_TX_ST_FAILED :return "FAILED" ; +case OFONO_SMS_TX_ST_EXPIRED :return "EXPIRED" ; +default : +return "INVALID" ; + } +} + +/** * @wfaq: Waiting-For-Acknoledgement queue; messages in this queue * have been delivered but are waiting to be acknoledged by the
@@ -426,6 +444,29 @@ static void __ofono_sms_tx_state_set(struct tx_queue_entry *entry, } +static DBusMessage *dbus_sms_msg_get_properties ( +DBusConnection * conn,DBusMessage *dbus_msg,void *_sms_msg) +{ +struct tx_queue_entry *sms_msg = _sms_msg; +DBusMessage *reply; +DBusMessageIter iter; +DBusMessageIter dict; +const char *str; + + reply =dbus_message_new_method_return (dbus_msg); +if (!reply) +return NULL ; +dbus_message_iter_init_append (reply, &iter); +dbus_message_iter_open_container (&iter,DBUS_TYPE_ARRAY , +OFONO_PROPERTIES_ARRAY_SIGNATURE , + &dict); + str =ofono_sms_tx_state_to_string (sms_msg->state); +ofono_dbus_dict_append (&dict,"TXState" ,DBUS_TYPE_STRING , &str); +dbus_message_iter_close_container (&iter, &dict); +return reply; +} + +static DBusMessage *dbus_sms_msg_cancel (DBusConnection * conn,DBusMessage *msg,void *data) {
@@ -453,6 +494,8 @@ static DBusMessage *dbus_sms_msg_cancel(static GDBusMethodTable sms_msg_methods[] = { + {"GetProperties" ,"" ,"a{sv}" , + dbus_sms_msg_get_properties,G_DBUS_METHOD_FLAG_ASYNC }, {"Cancel" ,DBUS_TYPE_BOOLEAN_AS_STRING ,"" , dbus_sms_msg_cancel,G_DBUS_METHOD_FLAG_ASYNC }, { }
Signature · 1 line
1.6.6.1
PATCH 20/23 SMS: send D-Bus SMS-MSG::ProperyChanged signals when message changes status
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
This hooks __sms_msg_state_change_dbus_signal() into __ofono_sms_tx_state_set(), so that a D-Bus signal will be emitted whenever an SMS Message transitions state. This allows a client to track, if desired, what is going on with the SMS Message it cares about.
With this, we can remove, in tx_finished() the asynchornous finalization of the call (via sms_msg->dbus_msg) and make the call synchronous (follow up commit).
The rest of the sites that were calling __ofono_sms_tx_state_set() get (by definition) to send the signal.
--- src/sms.c | 33 +++++++++++++++++++++++++++------ test/test-sms-msg-state-change | 24 ++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 6 deletions(-) create mode 100755 test/test-sms-msg-state-change
src/sms.c +27 -6
@@ -380,6 +380,32 @@ static void ofono_sms_tx_state_check(const char *file, unsigned line,/* +* Send a PropertyChange signal when the state changes +* +* D-Bus typing is "ss" for variable/value. +*/ +static +void __sms_msg_state_change_dbus_signal (struct tx_queue_entry *sms_msg) +{ +DBusConnection *dbus_conn =ofono_dbus_get_connection (); +DBusMessage *dbus_signal; +DBusMessageIter iter; +const char *property ="TXState" ; +const char *new_state_str; + + dbus_signal =dbus_message_new_signal ( + sms_msg->dbus_path,SMS_MSG_INTERFACE ,"PropertyChanged" ); +if (!dbus_signal) +return ; +dbus_message_iter_init_append (dbus_signal, &iter); +dbus_message_iter_append_basic (&iter,DBUS_TYPE_STRING , &property); + new_state_str =ofono_sms_tx_state_to_string (sms_msg->state); +dbus_message_iter_append_basic (&iter,DBUS_TYPE_STRING , &new_state_str); +g_dbus_send_message (dbus_conn, dbus_signal); +} + + +/* * Set a pending SMS's state * * This is just syntatic sugar that validates that the transition is
@@ -441,6 +467,7 @@ static void __ofono_sms_tx_state_set(struct tx_queue_entry *entry,ofono_debug ("%s:%d: SMS state change: %p %u -> %u\n" , file, line, entry, state_old, state_new); entry->state = state_new; +__sms_msg_state_change_dbus_signal (entry); }
@@ -584,10 +611,7 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data)DBG ("Max retries reached, giving up" ); -ofono_sms_tx_state_set (entry,OFONO_SMS_TX_ST_CANCELING ); entry =g_queue_pop_head (sms->txq); -__ofono_dbus_pending_reply (&entry->dbus_msg, -__ofono_error_failed (entry->dbus_msg)); __ofono_history_sms_send_status(modem, entry->msg_id, time(NULL),
@@ -621,9 +645,6 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data) } entry =g_queue_pop_head (sms->txq); -__ofono_dbus_pending_reply ( - &entry->dbus_msg, -dbus_message_new_method_return (entry->dbus_msg));__ofono_history_sms_send_status (modem, entry->msg_id,time (NULL ),OFONO_HISTORY_SMS_STATUS_SUBMITTED );
test/test-sms-msg-state-change new file +24 -0
@@ -0,0 +1,24 @@ +#!/usr/bin/python + +import gobject + +import dbus +import dbus.mainloop.glib + +def property_changed(name, value, path, interface): + if interface == "org.ofono.SMSMessage": + print "%s: name %s value %s interface %s" \ + % (path, name, value, interface) + +if __name__ == '__main__': + print "FIXME: this should test the whole state change transition machine" + dbus.mainloop.glib.DBusGMainLoop(set_as_default=True) + + bus = dbus.SystemBus() + + bus.add_signal_receiver( + property_changed, bus_name="org.ofono", + signal_name = "PropertyChanged", path_keyword="path", + interface_keyword="interface") + mainloop = gobject.MainLoop() + mainloop.run()
Signature · 1 line
1.6.6.1
PATCH 21/23 SMS: make D-Bus SendMessage and Cancel fully synchronous
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
Up until now, SMSManager::SendMessage() and SMSMessage::Cancel() where D-Bus asynchronous. They would perform the D-Bus method return reply inside the callbacks, as they needed to wait for the operation to complete.
Relying on previous commits that make the SMS message object send D-Bus updates when changing state, this makes Sendmessage and Cancel return inmediately. SendMessage() will return an string with the SMS Message object name and Cancel() will just return.
This renders all the D-Bus message bookeeping in 'struct tx_queue_entry' uneeded.
--- src/sms.c | 25 +++++++++++-------------- 1 files changed, 11 insertions(+), 14 deletions(-)
src/sms.c +11 -14
@@ -504,11 +504,7 @@ static DBusMessage *dbus_sms_msg_cancel(return __ofono_error_invalid_args (msg);sms_msg_cancel (sms_msg, do_network_cancel ?SMS_MSG_CANCEL_IN_NETWORK :0 ); - sms_msg->dbus_msg =dbus_message_ref (msg); -__ofono_dbus_pending_reply ( - &sms_msg->dbus_msg, -dbus_message_new_method_return (sms_msg->dbus_msg)); -return NULL ; +return dbus_message_new_method_return (msg); }
@@ -522,9 +518,9 @@ static DBusMessage *dbus_sms_msg_cancel(static GDBusMethodTable sms_msg_methods[] = { {"GetProperties" ,"" ,"a{sv}" , - dbus_sms_msg_get_properties,G_DBUS_METHOD_FLAG_ASYNC }, + dbus_sms_msg_get_properties,0 }, {"Cancel" ,DBUS_TYPE_BOOLEAN_AS_STRING ,"" , - dbus_sms_msg_cancel,G_DBUS_METHOD_FLAG_ASYNC }, + dbus_sms_msg_cancel,0 }, { } };
@@ -577,10 +573,6 @@ static void tx_queue_entry_destroy(struct tx_queue_entry *entry)g_free (entry->pdus);g_free (entry->name); entry->state = __OFONO_SMS_TX_ST_INVALID; -if (entry->dbus_msg) { -dbus_message_unref (entry->dbus_msg); - entry->dbus_msg =NULL ; - } }static void tx_queue_entry_destroy_free (gpointer _entry,gpointer unused)
@@ -872,6 +864,7 @@ static DBusMessage *dbus_SendMessage(DBusConnection *conn, DBusMessage *msg,const char *to;const char *text;struct tx_queue_entry *sms_msg; +DBusMessage *reply; if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &to, DBUS_TYPE_STRING, &text,
@@ -881,9 +874,13 @@ static DBusMessage *dbus_SendMessage(DBusConnection *conn, DBusMessage *msg, sms_msg =sms_msg_send (sms, to, text,SMS_MSG_SEND_HISTORY );if (sms_msg ==NULL )return __ofono_error_invalid_format (msg); -/* Unreferenced upon tx_queue_entry_destroy() */ - sms_msg->dbus_msg =dbus_message_ref (msg); -return NULL ; + reply =dbus_message_new_method_return (msg); +if (!reply) +return NULL ; +dbus_message_append_args (reply, +DBUS_TYPE_STRING , &sms_msg->dbus_path, +DBUS_TYPE_INVALID ); +return reply; }
Signature · 1 line
1.6.6.1
PATCH 22/23 SMS: set the SRR bit in outgoing PDUs if WFA is requested
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
When calling sms_text_prepare(), pass a new argument (srr) that denotes if the TP-SRR bit in the PDU should be set to request an Status Report from the Service Center.
--- src/sms.c | 13 ++++++++++--- src/smsutil.h | 2 ++ 2 files changed, 12 insertions(+), 3 deletions(-)
src/sms.c +10 -3
@@ -752,6 +752,7 @@ struct tx_queue_entry *sms_msg_send(struct sms_address receiver;DECLARE_SMS_ADDR_STR (receiver_str);const char *sms_path =__ofono_atom_get_path (sms->atom); +gboolean status_report;if (valid_phone_number_format (to) == FALSE)return NULL ;
@@ -767,8 +768,14 @@ struct tx_queue_entry *sms_msg_send( == FALSE)return NULL ; - msg_list =sms_text_prepare (text,0 , TRUE, &ref_offset, - sms->use_delivery_reports); +/* +* Request submit report? Set it in sms->submit for the +* encoding code to decide how to encode it, take the default +* from the SMS manager object. +*/ + status_report = flags &SMS_MSG_SEND_STATUS_REPORT ? + TRUE : sms->use_delivery_reports; + msg_list =sms_text_prepare (text,0 , TRUE, &ref_offset, status_report);if (!msg_list)return NULL ;
@@ -790,7 +797,7 @@ struct tx_queue_entry *sms_msg_send(g_slist_foreach (msg_list, (GFunc )g_free,NULL );g_slist_free (msg_list); - entry->status_report = sms->use_delivery_reports; + entry->status_report = status_report; entry->name =g_strdup_printf (SMS_MSG_NAME_FMT , sms_path, msg_id_str, entry->num_pdus);ofono_debug ("sms/entry %p name %s\n" , entry, entry->name);
src/smsutil.h +2 -0
@@ -569,6 +569,8 @@ enum sms_msg_cancel_flags {* tries to cancel it if in transit. */ SMS_MSG_CANCEL_IN_NETWORK =0x1 , +/* Request and wait for receipt acknowledgement from the network */ + SMS_MSG_SEND_STATUS_REPORT = 0x02, };struct tx_queue_entry ;
Signature · 1 line
1.6.6.1
Re: [SMS D-Bus 22/23] SMS: set the SRR bit in outgoing PDUs if WFA is requested
The final patch in this series for integrating the status report into D-Bus message state machine is added here -- don't want to resubmit the whole thing as this is still an RFC.
From 2a22f2cb4328c54fb5cb544056541de63667588c Mon Sep 17 00:00:00 2001
From: Inaky Perez-Gonzalez <[email protected]>
Date: Mon, 28 Jun 2010 16:18:27 -0700
Subject: [PATCH] SMS: hookup status reports to message status machine
When a message is waiting for a delivery report, it gets added to an special queue and it will show in D-Bus. Make it so that when the message's delivery is acknowledged, the message's status machine is updated, and thus, D-Bus signals for it are delivered.
--- src/sms.c | 18 ++++++++++++++++++ 1 files changed, 18 insertions(+), 0 deletions(-)
src/sms.c +18 -0
@@ -1148,6 +1148,21 @@ static void handle_deliver(struct ofono_sms *sms, const struct sms *incoming)g_slist_free (l); } + +static void __sms_find_destroy_by_msg_id (gpointer _sms_msg,gpointer _msg_id) +{ +unsigned msg_id = (unsigned ) _msg_id; +struct tx_queue_entry *sms_msg = _sms_msg; + +if (sms_msg->msg_id != msg_id) +return ; +ofono_debug ("SMS: ACKED %p msg_id match %x" , sms_msg, msg_id); +g_queue_remove (sms_msg->sms_mgr->tx_wfaq, sms_msg); +ofono_sms_tx_state_set (sms_msg,OFONO_SMS_TX_ST_DONE ); +tx_queue_entry_destroy_free (sms_msg,NULL ); +} + +static void handle_sms_status_report (struct ofono_sms *sms,const struct sms *incoming) {
@@ -1159,6 +1174,9 @@ static void handle_sms_status_report(struct ofono_sms *sms, &delivered) == FALSE)return ; +g_queue_foreach (sms->tx_wfaq, __sms_find_destroy_by_msg_id, + (void *) msg_id); +__ofono_history_sms_send_status (modem, msg_id,time (NULL ), delivered ?OFONO_HISTORY_SMS_STATUS_DELIVERED :OFONO_HISTORY_SMS_STATUS_DELIVER_FAILED );
PATCH 23/23 sms_text_prepare: document @use_delivery_reports
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
--- src/smsutil.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
src/smsutil.c +3 -0
@@ -2831,6 +2831,9 @@ static inline GSList *sms_list_append(GSList *l, const struct sms *in)* Returns a list of sms messages in order. If ref_offset is given, * then the ref_offset contains the reference number offset or 0 * if no concatenation took place. +* +* @use_delivery_reports: value for the Status-Report-Request field +* (23.040 3.2.9, 9.2.2.2) */ GSList *sms_text_prepare(const char *utf8, guint16 ref, gboolean use_16bit, int *ref_offset,
Signature · 1 line
1.6.6.1