pull request
cover pull request
From: Inaky Perez-Gonzalez <[email protected]>
- UUID code reworked, simplified to Dennis' suggestion
- UUID code integrated -- still kind of incomplete because we need to see what to do when the message contents and destination number is the same.
- BUG_ON* removed, not used anymore
- removed 'struct sms_msg_dbus_data', pass around 'dbus_path' as private data.
- D-Bus:
number of PDUs not longer part of the D-Bus name
added 'To' to the D-Bus properties; ignored for now submittion/delivery times as it will complicate the interface -- we need another callback for property change specification. Is it worth it?
Fixed GetProperties method mistakenly marked ASYNC
Interface renamed to SmsManager
Cleaned up doc/sms-api.txt according to feedback
- unfolded tx_queue_entry_destroy_free into tx_queue_entry_free_foreach() and tx_queue_entry_free() according to feedback.
- broke up multi-directory commits
- cleaned up style violations
- add a fix to generation of symlinks for headers, it broke in VPATH builds.
The following changes since commit 6f1ab8b6794333673fefc3d52f9b0388526a56cd:
Kristen Carlson Accardi (1):
test-stkutil: unit test for img to xpm converter
are available in the git repository at:
git://gitorious.org/~inakypg/ofono/ofono-inakypg.git master
Patches follow for reviewing convenience.
Inaky Perez-Gonzalez (19):
write_file: make transaction-safe
sms: introduce message ID API
sms: implement SHA256-based message IDs [incomplete]
sms: document the org.ofono.SmsMessage interface
sms: document handle_sms_status_report() struct tx_queue_entry: add a destructor
sms: introduce bare state machine and transitions
sms: introduce the Wait-for-Status-Report state
sms: introduce a state change callback for messages
sms: export outgoing messages over D-Bus
sms: send PropertyChanged signals on state change
sms: introduce sms_msg_cancel and its D-Bus wrapper
sms: Implement D-Bus SMS-MSG::GetProperties
sms: document SMS Message's D-Bus 'To' property
sms: test code for message's D-Bus GetProperties
automake: fix generation of symlinks for headers
sms: document variable usage in sms_msg_send()
sms: add test case for message cancel
sms: add test case for state change signals
Makefile.am | 2 +- doc/sms-api.txt | 54 ++++- src/ofono.h | 45 +++- src/sms.c | 581 ++++++++++++++++++++++++++++++++------ src/smsutil.c | 78 +++++ src/smsutil.h | 8 + src/stk.c | 22 ++- src/storage.c | 47 +++- test/test-sms-msg-cancel | 173 +++++++++++ test/test-sms-msg-get-properties | 26 ++ test/test-sms-msg-state-change | 24 ++ 11 files changed, 951 insertions(+), 109 deletions(-) create mode 100755 test/test-sms-msg-cancel create mode 100755 test/test-sms-msg-get-properties create mode 100755 test/test-sms-msg-state-change
PATCH 1/19 write_file: make transaction-safe
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.
--- src/storage.c | 47 ++++++++++++++++++++++++++++++++++++----------- 1 files changed, 36 insertions(+), 11 deletions(-)
src/storage.c +36 -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,41 @@ 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 01/19] 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.
This patch has now been applied.
Regards,
-Denis
PATCH 2/19 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. Will be used by follow up commits.
The ID is not generic, but specifc to SMS messages (due to having to dig inside 'struct sms') and thus generates directly 16-bit IDs.
--- src/smsutil.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/smsutil.h | 2 + 2 files changed, 80 insertions(+), 0 deletions(-)
src/smsutil.c +78 -0
@@ -3983,3 +3983,81 @@ char *ussd_decode(int dcs, int len, const unsigned char *data)return utf8; } + +static +int __sms_uuid_from_pdu (GChecksum *checksum,const struct sms *sms_pdu) +{ +const guint8 *buf; +size_t buf_len; + +switch (sms_pdu->type) { +case SMS_TYPE_DELIVER : + buf = sms_pdu->deliver.ud; + buf_len = sms_pdu->deliver.udl; +break ; +case SMS_TYPE_DELIVER_REPORT_ACK : + buf = sms_pdu->deliver_ack_report.ud; + buf_len = sms_pdu->deliver_ack_report.udl; +break ; +case SMS_TYPE_DELIVER_REPORT_ERROR : + buf = sms_pdu->deliver_err_report.ud; + buf_len = sms_pdu->deliver_err_report.udl; +break ; +case SMS_TYPE_STATUS_REPORT : + buf = sms_pdu->status_report.ud; + buf_len = sms_pdu->status_report.udl; +break ; +case SMS_TYPE_SUBMIT : + buf = sms_pdu->submit.ud; + buf_len = sms_pdu->submit.udl; +break ; +case SMS_TYPE_SUBMIT_REPORT_ACK : + buf = sms_pdu->submit_ack_report.ud; + buf_len = sms_pdu->submit_ack_report.udl; +break ; +case SMS_TYPE_SUBMIT_REPORT_ERROR : + buf = sms_pdu->submit_err_report.ud; + buf_len = sms_pdu->submit_err_report.udl; +break ; +case SMS_TYPE_COMMAND : + buf = sms_pdu->command.cd; + buf_len = sms_pdu->command.cdl; +break ; +default : +return 1 ; + } +g_checksum_update (checksum, buf, buf_len); +return 0 ; +} + +/** +* Generate a UUID from an SMS PDU List +* +* @param sms_pdu_list GSlist containing 'struct sms' nodes to +* generate the UUID from. +* @return 0 in error (no memory or serious code inconsistency in the +* input data structures), otherwise the SMS UUID. +*/ +guint16 sms_uuid_from_pdu_list (const GSList *sms_pdu_list) +{ +guint16 uuid =0 ; +GChecksum *checksum; +const GSList *node; +gsize uuid_size =g_checksum_type_get_length (G_CHECKSUM_SHA256 ); +guint8 data[uuid_size]; + + checksum =g_checksum_new (G_CHECKSUM_SHA256 ); +if (checksum ==NULL ) + goto error_new; +for (node = sms_pdu_list; node; node = node->next) { +struct sms *sms_pdu = node->data; +if (__sms_uuid_from_pdu (checksum, sms_pdu)) + goto error_pdu; + } +g_checksum_get_digest (checksum, data, &uuid_size); + uuid = data[0 ] | data[1 ] <<8 ; +error_pdu: +g_checksum_free (checksum); +error_new: +return uuid; +}
src/smsutil.h +2 -0
@@ -547,3 +547,5 @@ 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); + +guint16 sms_uuid_from_pdu_list (const GSList *sms_pdu_list);
Signature · 1 line
1.6.6.1
Re: [SMS D-Bus 02/19] sms: introduce message ID API
Hi Inaky,
Quoted 17 lines · Inaky Perez-Gonzalez
From: Inaky Perez-Gonzalez <[email protected]>
This adds a simple API to use for generating unique IDs for SMS messages. Will be used by follow up commits.
The ID is not generic, but specifc to SMS messages (due to having to dig inside 'struct sms') and thus generates directly 16-bit IDs.
--- src/smsutil.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/smsutil.h | 2 + 2 files changed, 80 insertions(+), 0 deletions(-)src/smsutil.c +0
-0
src/smsutil.c char *ussd_decode(int dcs, int len, const unsigned char *data) · line 3988
Quoted 6 lines
src/smsutil.c +3
-0@@ -3983,3 +3983,81 @@ char *ussd_decode(int dcs, int len, const unsigned char *data)return utf8; } + +static +int __sms_uuid_from_pdu (GChecksum *checksum,const struct sms *sms_pdu)
Since this is a static function, it is preferable not to prefix it with __
Quoted 27 lines
src/smsutil.c +27
-0+{ +const guint8 *buf; +size_t buf_len; + +switch (sms_pdu->type) { +case SMS_TYPE_DELIVER : + buf = sms_pdu->deliver.ud; + buf_len = sms_pdu->deliver.udl; +break ; +case SMS_TYPE_DELIVER_REPORT_ACK : + buf = sms_pdu->deliver_ack_report.ud; + buf_len = sms_pdu->deliver_ack_report.udl; +break ; +case SMS_TYPE_DELIVER_REPORT_ERROR : + buf = sms_pdu->deliver_err_report.ud; + buf_len = sms_pdu->deliver_err_report.udl; +break ; +case SMS_TYPE_STATUS_REPORT : + buf = sms_pdu->status_report.ud; + buf_len = sms_pdu->status_report.udl; +break ; +case SMS_TYPE_SUBMIT : + buf = sms_pdu->submit.ud; + buf_len = sms_pdu->submit.udl; +break ; +case SMS_TYPE_SUBMIT_REPORT_ACK : + buf = sms_pdu->submit_ack_report.ud;
src/smsutil.c char *ussd_decode(int dcs, int len, const unsigned char *data) · line 4025
Quoted 10 lines
src/smsutil.c +10
-0+ buf_len = sms_pdu->submit_ack_report.udl; +break ; +case SMS_TYPE_SUBMIT_REPORT_ERROR : + buf = sms_pdu->submit_err_report.ud; + buf_len = sms_pdu->submit_err_report.udl; +break ; +case SMS_TYPE_COMMAND : + buf = sms_pdu->command.cd; + buf_len = sms_pdu->command.cdl; +break ;
This part is wrong, the user-data (ud) is not the entirety of the PDU. It is just the message payload. Hashing over it only is not enough.
src/smsutil.c char *ussd_decode(int dcs, int len, const unsigned char *data) · line 4027
Quoted 2 lines
src/smsutil.c +2
-0+default : +return 1 ;
You might want to make this function return TRUE/FALSE...
src/smsutil.c char *ussd_decode(int dcs, int len, const unsigned char *data) · line 4029
Quoted 2 lines
src/smsutil.c +2
-0+ } +g_checksum_update (checksum, buf, buf_len);
An empty line before g_checksum_update()
Quoted 19 lines
src/smsutil.c +19
-0+return 0 ; +} + +/** +* Generate a UUID from an SMS PDU List +* +* @param sms_pdu_list GSlist containing 'struct sms' nodes to +* generate the UUID from. +* @return 0 in error (no memory or serious code inconsistency in the +* input data structures), otherwise the SMS UUID. +*/ +guint16 sms_uuid_from_pdu_list (const GSList *sms_pdu_list) +{ +guint16 uuid =0 ; +GChecksum *checksum; +const GSList *node; +gsize uuid_size =g_checksum_type_get_length (G_CHECKSUM_SHA256 ); +guint8 data[uuid_size]; +
src/smsutil.c char *ussd_decode(int dcs, int len, const unsigned char *data) · line 4058
Quoted 10 lines
src/smsutil.c +10
-0+ checksum =g_checksum_new (G_CHECKSUM_SHA256 ); +if (checksum ==NULL ) + goto error_new; +for (node = sms_pdu_list; node; node = node->next) { +struct sms *sms_pdu = node->data; +if (__sms_uuid_from_pdu (checksum, sms_pdu)) + goto error_pdu; + } +g_checksum_get_digest (checksum, data, &uuid_size); + uuid = data[0 ] | data[1 ] <<8 ;
The current message id is 32 bits. Why are we stopping@16? We could go all the way to 64 if needed...
Also, we need to take time and some random value into account. Perhaps including time() output and a simple static counter into the hashing function would make it unique for those 'same sender, same contents messages'
Quoted 10 lines
src/smsutil.c +5
-0+error_pdu: +g_checksum_free (checksum); +error_new: +return uuid; +}src/smsutil.h +0
-0
src/smsutil.h GSList *cbs_optimize_ranges(GSList *ranges); · line 551
Quoted 5 lines
src/smsutil.h +2
-0@@ -547,3 +547,5 @@ 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); + +guint16 sms_uuid_from_pdu_list (const GSList *sms_pdu_list);
Regards,
-Denis
Re: [SMS D-Bus 02/19] sms: introduce message ID API
Quoted 21 lines · Denis Kenzior
Hi Inaky,
Quoted 18 lines · Inaky Perez-Gonzalez
From: Inaky Perez-Gonzalez <[email protected]>
This adds a simple API to use for generating unique IDs for SMS messages. Will be used by follow up commits.
The ID is not generic, but specifc to SMS messages (due to having to dig inside 'struct sms') and thus generates directly 16-bit IDs.
--- src/smsutil.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/smsutil.h | 2 + 2 files changed, 80 insertions(+), 0 deletions(-)src/smsutil.c +0
-0@@ -3983,3 +3983,81 @@ char *ussd_decode(int dcs, int len, const unsigned char *data)
Quoted 7 lines
src/smsutil.c char *ussd_decode(int dcs, int len, const unsigned char *data) · line 3988
Quoted 5 lines
src/smsutil.c +3
-0return utf8; } + +static +int __sms_uuid_from_pdu (GChecksum *checksum,const struct sms *sms_pdu)Since this is a static function, it is preferable not to prefix it with __
Will do
Quoted 32 lines
Quoted 32 lines
src/smsutil.c +32
-0+{ +const guint8 *buf; +size_t buf_len; + +switch (sms_pdu->type) { +case SMS_TYPE_DELIVER : + buf = sms_pdu->deliver.ud; + buf_len = sms_pdu->deliver.udl; +break ; +case SMS_TYPE_DELIVER_REPORT_ACK : + buf = sms_pdu->deliver_ack_report.ud; + buf_len = sms_pdu->deliver_ack_report.udl; +break ; +case SMS_TYPE_DELIVER_REPORT_ERROR : + buf = sms_pdu->deliver_err_report.ud; + buf_len = sms_pdu->deliver_err_report.udl; +break ; +case SMS_TYPE_STATUS_REPORT : + buf = sms_pdu->status_report.ud; + buf_len = sms_pdu->status_report.udl; +break ; +case SMS_TYPE_SUBMIT : + buf = sms_pdu->submit.ud; + buf_len = sms_pdu->submit.udl; +break ; +case SMS_TYPE_SUBMIT_REPORT_ACK : + buf = sms_pdu->submit_ack_report.ud; + buf_len = sms_pdu->submit_ack_report.udl; +break ; +case SMS_TYPE_SUBMIT_REPORT_ERROR : + buf = sms_pdu->submit_err_report.ud; + buf_len = sms_pdu->submit_err_report.udl;
Quoted 8 lines
src/smsutil.c char *ussd_decode(int dcs, int len, const unsigned char *data) · line 4025
Quoted 5 lines
src/smsutil.c +5
-0+break ; +case SMS_TYPE_COMMAND : + buf = sms_pdu->command.cd; + buf_len = sms_pdu->command.cdl; +break ;This part is wrong, the user-data (ud) is not the entirety of the PDU. It is just the message payload. Hashing over it only is not enough.
So which other fields for each type in the union are to be taken into account?
From our discussions, I was under the impression that we wanted to hash only on the contents of the message and destination -- this actually led to the problem of the same sender and data issue below...[more on it below].
Quoted 4 lines
src/smsutil.c char *ussd_decode(int dcs, int len, const unsigned char *data) · line 4027
Quoted 2 lines
src/smsutil.c +2
-0+default : +return 1 ;You might want to make this function return TRUE/FALSE...
Ok
Quoted 4 lines
src/smsutil.c char *ussd_decode(int dcs, int len, const unsigned char *data) · line 4029
Quoted 2 lines
src/smsutil.c +2
-0+ } +g_checksum_update (checksum, buf, buf_len);An empty line before
g_checksum_update()
k
Quoted 24 lines
Quoted 24 lines
src/smsutil.c +24
-0+return 0 ; +} + +/** +* Generate a UUID from an SMS PDU List +* +* @param sms_pdu_list GSlist containing 'struct sms' nodes to +* generate the UUID from. +* @return 0 in error (no memory or serious code inconsistency in the +* input data structures), otherwise the SMS UUID. +*/ +guint16 sms_uuid_from_pdu_list (const GSList *sms_pdu_list) +{ +guint16 uuid =0 ; +GChecksum *checksum; +const GSList *node; +gsize uuid_size =g_checksum_type_get_length (G_CHECKSUM_SHA256 ); +guint8 data[uuid_size]; + + checksum =g_checksum_new (G_CHECKSUM_SHA256 ); +if (checksum ==NULL ) + goto error_new; +for (node = sms_pdu_list; node; node = node->next) { +struct sms *sms_pdu = node->data;
Quoted 8 lines
src/smsutil.c char *ussd_decode(int dcs, int len, const unsigned char *data) · line 4058
Quoted 5 lines
src/smsutil.c +5
-0+if (__sms_uuid_from_pdu (checksum, sms_pdu)) + goto error_pdu; + } +g_checksum_get_digest (checksum, data, &uuid_size); + uuid = data[0 ] | data[1 ] <<8 ;The current message id is 32 bits. Why are we stopping at 16? We could go all the way to 64 if needed...
Well, at the end what gets sent to the network is 16 bits right?
Quoted 4 lines
Also, we need to take time and some random value into account. Perhaps including
time()output and a simple static counter into the hashing function would make it unique for those 'same sender, same contents messages'
So then we really don't care about being able to regenerate the hash ID from the contents?
The design discussions we held about this involved being able to, with the content of the message and destination number, generating a hash that could be uniquely reproduced by rehashing it again. You had a reason for it and I can't remember which one it was -- I should have written it down. But that brought the problem of 'somebody sending the same message to the same number' which yields the same ID.
However, if to fix that we are adding randomness in the form of time(), why bother with the contents? let's just do a random ID from the ground up and save us the pain of hashing.
Re: [SMS D-Bus 02/19] sms: introduce message ID API
Hi Inaky,
Quoted 4 lines
Quoted 2 lines
The current message id is 32 bits. Why are we stopping at 16? We could go all the way to 64 if needed...
Well, at the end what gets sent to the network is 16 bits right?
So I think we're all confused here. This ID is not being sent to the network. It is only used for uniquely identifying the message over D-Bus and to message history. This number can be as small or as big as we want.
Quoted 16 lines
Quoted 5 lines
Also, we need to take time and some random value into account. Perhaps including
time()output and a simple static counter into the hashing function would make it unique for those 'same sender, same contents messages'So then we really don't care about being able to regenerate the hash ID from the contents?
The design discussions we held about this involved being able to, with the content of the message and destination number, generating a hash that could be uniquely reproduced by rehashing it again. You had a reason for it and I can't remember which one it was -- I should have written it down. But that brought the problem of 'somebody sending the same message to the same number' which yields the same ID.
Quoted 4 lines
However, if to fix that we are adding randomness in the form of
time(), why bother with the contents? let's just do a random ID from the ground up and save us the pain of hashing.
I don't remember this discussion any more. There might be a case where an old message is re-delivered because oFono crashed after delivery but before deletion.
At least to me it seems that generating a random int is really not guaranteed to be unique over reboots. Hashing seems way better. However, I'm not a math/crypto geek, so feel free to prove me wrong.
Regards,
-Denis
Re: [SMS D-Bus 02/19] sms: introduce message ID API
Quoted 11 lines · Denis Kenzior
Hi Inaky,
Quoted 4 lines
Quoted 2 lines
The current message id is 32 bits. Why are we stopping at 16? We could go all the way to 64 if needed...
Well, at the end what gets sent to the network is 16 bits right?
So I think we're all confused here. This ID is not being sent to the network. It is only used for uniquely identifying the message over D-Bus and to message history. This number can be as small or as big as we want.
I was under the impression that you wanted to use this for the ref also, and thus it is imprinted into the PDUs by set_ref_and_to(). Can you confirm this?
Quoted 20 lines
Quoted 15 lines
Quoted 4 lines
Also, we need to take time and some random value into account. Perhaps including
time()output and a simple static counter into the hashing function would make it unique for those 'same sender, same contents messages'So then we really don't care about being able to regenerate the hash ID from the contents?
The design discussions we held about this involved being able to, with the content of the message and destination number, generating a hash that could be uniquely reproduced by rehashing it again. You had a reason for it and I can't remember which one it was -- I should have written it down. But that brought the problem of 'somebody sending the same message to the same number' which yields the same ID.
Quoted 4 lines
However, if to fix that we are adding randomness in the form of
time(), why bother with the contents? let's just do a random ID from the ground up and save us the pain of hashing.
Quoted 7 lines
I don't remember this discussion any more. There might be a case where an old message is re-delivered because oFono crashed after delivery but before deletion.
At least to me it seems that generating a random int is really not guaranteed to be unique over reboots. Hashing seems way better. However, I'm not a math/crypto geek, so feel free to prove me wrong.
Ok, I see your concern. Proper seeding of the RNG should take care of that case, but either way works. If you want hashing + time() seeding that can be done easily.
I still could use clarification on which other fields of the 'struct sms' union you want to have hashed other than the PDU payloads.
Re: [SMS D-Bus 02/19] sms: introduce message ID API
Hi Inaky,
Quoted 8 lines · Inaky Perez-Gonzalez
Quoted 7 lines · Denis Kenzior
Hi Inaky,
Quoted 4 lines
Quoted 2 lines
The current message id is 32 bits. Why are we stopping at 16? We could go all the way to 64 if needed...
Well, at the end what gets sent to the network is 16 bits right?
Quoted 9 lines
Quoted 4 lines
So I think we're all confused here. This ID is not being sent to the network. It is only used for uniquely identifying the message over D-Bus and to message history. This number can be as small or as big as we want.
I was under the impression that you wanted to use this for the ref also, and thus it is imprinted into the PDUs by
set_ref_and_to(). Can you confirm this?
No, you're totally confused ;) Ref is only sent on messages that are multi-fragment. It is not used for single-fragment messages. The logic is currently implemented per spec and I never had the intention to do it any other way.
Quoted 7 lines
Ok, I see your concern. Proper seeding of the RNG should take care of that case, but either way works. If you want hashing +
time()seeding that can be done easily.I still could use clarification on which other fields of the 'struct sms' union you want to have hashed other than the PDU payloads.
All of them. Again, it might be much simpler to do this over the fragments when they are in true PDU form. This can be done in sms_assembly and tx_queue_entry_new since they deal with fragments already.
Alternatively handling the individual structure fields can be done as well.
Regards,
-Denis
Re: [SMS D-Bus 02/19] sms: introduce message ID API
Quoted 21 lines · Denis Kenzior
Hi Inaky,
Quoted 8 lines · Inaky Perez-Gonzalez
Quoted 7 lines · Denis Kenzior
Hi Inaky,
Quoted 4 lines
Quoted 2 lines
The current message id is 32 bits. Why are we stopping at 16? We could go all the way to 64 if needed...
Well, at the end what gets sent to the network is 16 bits right?
Quoted 9 lines
Quoted 4 lines
So I think we're all confused here. This ID is not being sent to the network. It is only used for uniquely identifying the message over D-Bus and to message history. This number can be as small or as big as we want.
I was under the impression that you wanted to use this for the ref also, and thus it is imprinted into the PDUs by
set_ref_and_to(). Can you confirm this?
Quoted 4 lines
No, you're totally confused ;) Ref is only sent on messages that are multi-fragment. It is not used for single-fragment messages. The logic is currently implemented per spec and I never had the intention to do it any other way.
Okay, so then this means that all the current ref infrastructure (monotonically increased) is left in place and the message ID stuff is used only internally for D-Bus and whichever other needs there are. I'll up it to 64-bits.
Quoted 8 lines
Quoted 7 lines
Ok, I see your concern. Proper seeding of the RNG should take care of that case, but either way works. If you want hashing +
time()seeding that can be done easily.I still could use clarification on which other fields of the 'struct sms' union you want to have hashed other than the PDU payloads.
Quoted 5 lines
All of them. Again, it might be much simpler to do this over the fragments when they are in true PDU form. This can be done in
sms_assemblyandtx_queue_entry_newsince they deal with fragments already.Alternatively handling the individual structure fields can be done as well.
I'll look into it.
Re: [SMS D-Bus 02/19] sms: introduce message ID API
In reply to Denis KenziorQuoted 9 lines · Denis Kenzior
Quoted 3 lines
I still could use clarification on which other fields of the 'struct sms' union you want to have hashed other than the PDU payloads.
All of them. Again, it might be much simpler to do this over the fragments when they are in true PDU form. This can be done in
sms_assemblyandtx_queue_entry_newsince they deal with fragments already.Alternatively handling the individual structure fields can be done as well.
On the RX path, it is not going to be easy to use the raw PDUs to generate a UUID:
whoever code gets the PDU
ofono_sms_deliver_notify(PDU)
PDU decoded into a 'struct sms'
handle_deliver(struct sms)
added to fragment list in assembly
sms_dispatch(fragment list of 'struct sms')
so if we also want to generate a UUID of this based on RAW PDUs, either we keep the raw PDU around or we base the UUID generation from the struct sms fields. We could really only generate this at sms_dispatch() or similar, where we have the fragments in the proper order.
Thoughts?
PATCH 3/19 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 different or the same numbers also have a different ID.
--- src/ofono.h | 9 +++++---- src/sms.c | 55 +++++++++++++++++++++++-------------------------------- src/stk.c | 5 +++-- 3 files changed, 31 insertions(+), 38 deletions(-)
src/ofono.h +5 -4
@@ -185,10 +185,11 @@ enum ofono_sms_submit_flag {typedef void (*ofono_sms_txq_submit_cb_t )(gboolean ok,void *data); -unsigned int __ofono_sms_txq_submit (struct ofono_sms *sms,GSList *list, -unsigned int flags, -ofono_sms_txq_submit_cb_t cb, -void *data,ofono_destroy_func destroy); +struct tx_queue_entry *__ofono_sms_txq_submit ( +struct ofono_sms *sms,GSList *list, +unsigned int flags,unsigned ref, +ofono_sms_txq_submit_cb_t cb, +void *data,ofono_destroy_func destroy);#include <ofono/sim.h> #include <ofono/stk.h>
src/sms.c +23 -32
138 lines not on this page · raw message
src/stk.c +3 -2
@@ -323,6 +323,7 @@ static gboolean handle_command_send_sms(const struct stk_command *cmd,struct ofono_atom *sms_atom;struct ofono_sms *sms;GSList msg_list; +guint16 msg_id; sms_atom =__ofono_modem_find_atom (modem,OFONO_ATOM_TYPE_SMS );
@@ -338,8 +339,8 @@ static gboolean handle_command_send_sms(const struct stk_command *cmd, msg_list.data = (void *) &cmd->send_sms.gsm_sms; msg_list.next =NULL ; - -__ofono_sms_txq_submit (sms, &msg_list,0 , send_sms_submit_cb, + msg_id =sms_uuid_from_pdu_list (&msg_list); +__ofono_sms_txq_submit (sms, &msg_list,0 , msg_id, send_sms_submit_cb, stk->sms_submit_req, g_free); stk->cancel_cmd = send_sms_cancel;
Signature · 1 line
1.6.6.1
Re: [SMS D-Bus 03/19] sms: implement SHA256-based message IDs [incomplete]
Hi Inaky,
Quoted 21 lines · Inaky Perez-Gonzalez
From: 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 different or the same numbers also have a different ID.
--- src/ofono.h | 9 +++++---- src/sms.c | 55 +++++++++++++++++++++++-------------------------------- src/stk.c | 5 +++-- 3 files changed, 31 insertions(+), 38 deletions(-)src/ofono.h +0
-0@@ -185,10 +185,11 @@ enum ofono_sms_submit_flag {typedef void (*ofono_sms_txq_submit_cb_t )(gboolean ok,void *data);
src/ofono.h enum ofono_sms_submit_flag { · line 192
Quoted 10 lines
src/ofono.h +5
-4-unsigned int __ofono_sms_txq_submit (struct ofono_sms *sms,GSList *list, -unsigned int flags, -ofono_sms_txq_submit_cb_t cb, -void *data,ofono_destroy_func destroy); +struct tx_queue_entry *__ofono_sms_txq_submit ( +struct ofono_sms *sms,GSList *list, +unsigned int flags,unsigned ref, +ofono_sms_txq_submit_cb_t cb, +void *data,ofono_destroy_func destroy);
As I said before, please don't mess with this function.
Quoted 29 lines
src/ofono.h +0
-0#include <ofono/sim.h> #include <ofono/stk.h> src/sms.c +2
-2@@ -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;@@ -589,7 +588,8 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,int ref_offset;struct ofono_modem *modem;unsigned int flags; -unsigned int msg_id; +guint16 msg_id; +struct tx_queue_entry *sms_msg; if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &to, DBUS_TYPE_STRING, &text,@@ -605,24 +605,19 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,if (!msg_list)return __ofono_error_invalid_format (msg);
src/sms.c static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg, · line 608
Quoted 10 lines
src/sms.c +1
-9-set_ref_and_to (msg_list, sms->ref, ref_offset, to); -DBG ("ref: %d, offset: %d" , sms->ref, ref_offset); - -if (ref_offset !=0 ) { -if (sms->ref ==65536 ) - sms->ref =1 ; -else - sms->ref = sms->ref +1 ; - } + msg_id =sms_uuid_from_pdu_list (msg_list);
You're calculating the ID before setting the ref and to? That is pretty silly ;)
Quoted 5 lines
src/sms.c +2
-0+set_ref_and_to (msg_list, msg_id, ref_offset, to); +DBG ("SMS ID: 0x%04x" , msg_id); flags =OFONO_SMS_SUBMIT_FLAG_RECORD_HISTORY ; flags |=OFONO_SMS_SUBMIT_FLAG_RETRY ;
src/sms.c static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg, · line 620
Quoted 10 lines
src/sms.c +4
-3if (sms->use_delivery_reports) flags |=OFONO_SMS_SUBMIT_FLAG_REQUEST_SR ; - msg_id =__ofono_sms_txq_submit (sms, msg_list, flags, send_message_cb, -dbus_message_ref (msg), - send_message_destroy); + sms_msg =__ofono_sms_txq_submit (sms, msg_list, flags, msg_id, + send_message_cb, +dbus_message_ref (msg), + send_message_destroy);
Make the ID calculation happen inside tx_queue_entry_new. That function already converts all the SMS objects into a PDU. So simply hashing the PDUs there would be very easy.
Quoted 117 lines
src/sms.c +14
-18g_slist_foreach (msg_list, (GFunc )g_free,NULL );g_slist_free (msg_list);@@ -659,7 +654,7 @@ static void dispatch_app_datagram(struct ofono_sms *sms, int dst, int src, }static void dispatch_text_message (struct ofono_sms *sms, -const char *message, +const char *message,guint16 msg_id,enum sms_class cls,const struct sms_address *addr,const struct sms_scts *scts)@@ -717,11 +712,9 @@ 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, +if (cls !=SMS_CLASS_0 ) +__ofono_history_sms_received (modem, msg_id, str, &remote, &local, message); - sms->next_msg_id +=1 ; - } }static void sms_dispatch (struct ofono_sms *sms,GSList *sms_list)@@ -732,6 +725,7 @@ static void sms_dispatch(struct ofono_sms *sms, GSList *sms_list)enum sms_class cls;int srcport =-1 ;int dstport =-1 ; +guint16 msg_id;if (sms_list ==NULL )return ;@@ -749,6 +743,7 @@ static void sms_dispatch(struct ofono_sms *sms, GSList *sms_list)* used, the addresses are the same across all segments. */ + msg_id =sms_uuid_from_pdu_list (sms_list);for (l = sms_list; l; l = l->next) {guint8 dcs;gboolean comp = FALSE;@@ -825,8 +820,8 @@ static void sms_dispatch(struct ofono_sms *sms, GSList *sms_list) s = sms_list->data; -dispatch_text_message (sms, message, cls, &s->deliver.oaddr, - &s->deliver.scts); +dispatch_text_message (sms, message, msg_id, cls, + &s->deliver.oaddr, &s->deliver.scts);g_free (message); } }@@ -1104,8 +1099,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",@@ -1201,9 +1194,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 );if (sms->ref >=65536 )@@ -1306,10 +1296,11 @@ void *ofono_sms_get_data(struct ofono_sms *sms)return sms->driver_data; } -unsigned int __ofono_sms_txq_submit (struct ofono_sms *sms,GSList *list, -unsigned int flags, -ofono_sms_txq_submit_cb_t cb, -void *data,ofono_destroy_func destroy) +struct tx_queue_entry *__ofono_sms_txq_submit ( +struct ofono_sms *sms,GSList *list, +unsigned int flags,unsigned msg_id, +ofono_sms_txq_submit_cb_t cb, +void *data,ofono_destroy_func destroy) {struct tx_queue_entry *entry =tx_queue_entry_new (list);@@ -1320,7 +1311,7 @@ unsigned int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list, sizeof(entry->receiver)); } - entry->msg_id = sms->next_msg_id++; + entry->msg_id = msg_id; entry->flags = flags; entry->cb = cb; entry->data = data;@@ -1331,5 +1322,5 @@ unsigned int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list,if (g_queue_get_length (sms->txq) ==1 ) sms->tx_source =g_timeout_add (0 , tx_next, sms); -return entry->msg_id; +return entry; }src/stk.c +1
-0@@ -323,6 +323,7 @@ static gboolean handle_command_send_sms(const struct stk_command *cmd,struct ofono_atom *sms_atom;struct ofono_sms *sms;GSList msg_list; +guint16 msg_id; sms_atom =__ofono_modem_find_atom (modem,OFONO_ATOM_TYPE_SMS );
src/stk.c static gboolean handle_command_send_sms(const struct stk_command *cmd, · line 346
Quoted 10 lines
src/stk.c +2
-2@@ -338,8 +339,8 @@ static gboolean handle_command_send_sms(const struct stk_command *cmd, msg_list.data = (void *) &cmd->send_sms.gsm_sms; msg_list.next =NULL ; - -__ofono_sms_txq_submit (sms, &msg_list,0 , send_sms_submit_cb, + msg_id =sms_uuid_from_pdu_list (&msg_list); +__ofono_sms_txq_submit (sms, &msg_list,0 , msg_id, send_sms_submit_cb, stk->sms_submit_req, g_free); stk->cancel_cmd = send_sms_cancel;
Regards,
-Denis
Re: [SMS D-Bus 03/19] sms: implement SHA256-based message IDs [incomplete]
Quoted 25 lines · Denis Kenzior
Hi Inaky,
Quoted 22 lines · Inaky Perez-Gonzalez
From: 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 different or the same numbers also have a different ID.
--- src/ofono.h | 9 +++++---- src/sms.c | 55 +++++++++++++++++++++++-------------------------------- src/stk.c | 5 +++-- 3 files changed, 31 insertions(+), 38 deletions(-)src/ofono.h +0
-0@@ -185,10 +185,11 @@ enum ofono_sms_submit_flag {typedef void (*ofono_sms_txq_submit_cb_t )(gboolean ok,void *data);
Quoted 11 lines
src/ofono.h enum ofono_sms_submit_flag { · line 192
Quoted 9 lines
src/ofono.h +5
-4-unsigned int __ofono_sms_txq_submit (struct ofono_sms *sms,GSList *list, -unsigned int flags, -ofono_sms_txq_submit_cb_t cb, -void *data,ofono_destroy_func destroy); +struct tx_queue_entry *__ofono_sms_txq_submit ( +struct ofono_sms *sms,GSList *list, +unsigned int flags,unsigned ref, +ofono_sms_txq_submit_cb_t cb, +void *data,ofono_destroy_func destroy);As I said before, please don't mess with this function.
Once we settle on the ref / msg_id part, that one will sort out by itself.
On later commits, the callback has to change in order to be able to implement PropertChanged signals. Unless you have a better way to do it that doesn't involve poking with 'struct tx_queue_entry' internals. A helper to set the callback after the fact is another option, but it is way dirtier.
Likewise with the return value (msg id vs the struct itself). You suggested poking at the head of the queue upon return. That is peeking on internals and you have adamantly (and rightfully) argued against it in other cases.
Quoted 29 lines
Quoted 29 lines
src/ofono.h +0
-0#include <ofono/sim.h> #include <ofono/stk.h> src/sms.c +2
-2@@ -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;@@ -589,7 +588,8 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,int ref_offset;struct ofono_modem *modem;unsigned int flags; -unsigned int msg_id; +guint16 msg_id; +struct tx_queue_entry *sms_msg; if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &to, DBUS_TYPE_STRING, &text,@@ -605,24 +605,19 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,if (!msg_list)return __ofono_error_invalid_format (msg);
Quoted 13 lines
src/sms.c static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg, · line 608
Quoted 10 lines
src/sms.c +1
-9-set_ref_and_to (msg_list, sms->ref, ref_offset, to); -DBG ("ref: %d, offset: %d" , sms->ref, ref_offset); - -if (ref_offset !=0 ) { -if (sms->ref ==65536 ) - sms->ref =1 ; -else - sms->ref = sms->ref +1 ; - } + msg_id =sms_uuid_from_pdu_list (msg_list);You're calculating the ID before setting the ref and to? That is pretty silly ;)
the ref was supposed to be generated off the msg ID according to your initial design decisions, so this is following spec. We need to clarify this (and this touches with the other email).
Quoted 8 lines
Quoted 8 lines
src/sms.c +2
-0+set_ref_and_to (msg_list, msg_id, ref_offset, to); +DBG ("SMS ID: 0x%04x" , msg_id); flags =OFONO_SMS_SUBMIT_FLAG_RECORD_HISTORY ; flags |=OFONO_SMS_SUBMIT_FLAG_RETRY ;if (sms->use_delivery_reports) flags |=OFONO_SMS_SUBMIT_FLAG_REQUEST_SR ;
Quoted 11 lines
src/sms.c static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg, · line 620
Quoted 7 lines
src/sms.c +4
-3- msg_id =__ofono_sms_txq_submit (sms, msg_list, flags, send_message_cb, -dbus_message_ref (msg), - send_message_destroy); + sms_msg =__ofono_sms_txq_submit (sms, msg_list, flags, msg_id, + send_message_cb, +dbus_message_ref (msg), + send_message_destroy);Make the ID calculation happen inside
tx_queue_entry_new. That function already converts all the SMS objects into a PDU. So simply hashing the PDUs there would be very easy.
Ok, holding until we confirm what relationship there will be between ref and msg_id.
PATCH 4/19 sms: document the org.ofono.SmsMessage interface
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
--- doc/sms-api.txt | 49 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 47 insertions(+), 2 deletions(-)
doc/sms-api.txt +47 -2
@@ -22,9 +22,27 @@ Methods dict GetProperties() Possible Errors: [service].Error.InvalidArguments [service].Error.DoesNotExist - void SendMessage(string to, string text) + object SendMessage(string to, string text) - Send the message in text to the number in to. + 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. + + Returns the name of the D-Bus object that + represents said SMS Message. + + Possible Errors: [service].Error.InvalidArguments + + array{object} Messages() + + Returns a list of SMS Messages that have been + submitted for delivery and that are still + pending. + + FIXME: currently not implemented Signals PropertyChanged(string name, variant value)
@@ -64,3 +82,30 @@ Properties string ServiceCenterAddress "ps-preferred" - Use CS if PS is unavailable By default oFono uses "cs-preferred" setting. + + +SMS / Messaging interface +========================= + +Service org.ofono +Interface org.ofono.SmsMessage +Object path [variable prefix]/{modem0,modem1...}/{message01,message02...} + +Methods dict GetProperties() + + Returns a dictionary with the current + properties of a message. + + void Cancel() + + Cancels a pending message's delivery. + +Signals PropertyChanged(string name, variant value) + + This signal indicates a changed value of the given + property. + +Properties string State + + Current transmission state {queued, wsr, done, + canceling, cancelled, failed, expired}
Signature · 1 line
1.6.6.1
PATCH 5/19 sms: document handle_sms_status_report()
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
--- src/sms.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
src/sms.c +9 -0
@@ -859,6 +859,15 @@ static void handle_deliver(struct ofono_sms *sms, const struct sms *incoming)g_slist_free (l); } + +/* +* Handle a delivery/status report has been received for an SMS +* apparently sent from here. +* +* We need to find the message in the SMS manager's +* waiting-for-acknoledge queue (sms->tx_wfaq) and remove it. As well, +* fill out history for it. +*/ static void handle_sms_status_report (struct ofono_sms *sms,const struct sms *incoming) {
Signature · 1 line
1.6.6.1
PATCH 6/19 struct tx_queue_entry: add a destructor
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
Introduce a destructor function to encapsulate all the release steps for this data type, as more are to be added later.
--- src/sms.c | 28 ++++++++++++++++++++++------ 1 files changed, 22 insertions(+), 6 deletions(-)
src/sms.c +22 -6
@@ -405,6 +405,26 @@ 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 releases resources allocated *inside* @entry and @entry +* itself. +*/ +static void tx_queue_entry_destroy (struct tx_queue_entry *entry) +{ +if (entry->destroy) + entry->destroy (entry->data); + +g_free (entry->pdus); +g_free (entry); +} + +static void tx_queue_entry_destroy_foreach (gpointer _entry,gpointer unused) +{ +tx_queue_entry_destroy (_entry); +} +static void tx_finished (const struct ofono_error *error,int mr,void *data) {struct ofono_sms *sms = data;
@@ -465,11 +485,7 @@ next_q: time(NULL), hs); } -if (entry->destroy) - entry->destroy (entry->data); - -g_free (entry->pdus); -g_free (entry); +tx_queue_entry_destroy (entry);if (g_queue_peek_head (sms->txq)) {DBG ("Scheduling next" );
@@ -1101,7 +1117,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_foreach,NULL );g_queue_free (sms->txq); sms->txq =NULL ; }
Signature · 1 line
1.6.6.1
Re: [SMS D-Bus 06/19] struct tx_queue_entry: add a destructor
Hi Inaky,
Quoted 4 lines · Inaky Perez-Gonzalez
From: Inaky Perez-Gonzalez <[email protected]>
Introduce a destructor function to encapsulate all the release steps for this data type, as more are to be added later.
This patch has been applied. Thanks.
Regards,
-Denis
PATCH 7/19 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/ofono.h | 33 +++++++++++++++++ src/sms.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 148 insertions(+), 2 deletions(-)
src/ofono.h +33 -0
@@ -183,6 +183,39 @@ enum ofono_sms_submit_flag { OFONO_SMS_SUBMIT_FLAG_RETRY = 0x4, }; +/* +* 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 WSR DONE | CANCELLED EXPIRED +* UNINITIALIZED - A N N N N N N +* QUEUED N - A A A N A N +* WSR 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_WSR , +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, +}; + +const char *ofono_sms_tx_state_to_string (enum ofono_sms_tx_state ); +typedef void (*ofono_sms_txq_submit_cb_t )(gboolean ok,void *data); struct tx_queue_entry *__ofono_sms_txq_submit(
src/sms.c +115 -2
179 lines not on this page · raw message
Signature · 1 line
1.6.6.1
PATCH 8/19 sms: introduce the Wait-for-Status-Report state
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
A WSR (Waiting for Status Report) state definition is added to the state transtition machine; as well, a queue (ofono_sms->tx_wfaq) where messages waiting for an status report are queued. When the message's delivery is acknowledged, the message is removed from the queue and the message's status machine is updated, which can trigger things such as D-Bus signals.
--- src/sms.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 73 insertions(+), 7 deletions(-)
src/sms.c +73 -7
@@ -66,6 +66,11 @@ const char *ofono_sms_tx_state_to_string(enum ofono_sms_tx_state status) } } +/** +* @tx_wsrq: Waiting-for-Status-Report queue; messages in this queue +* have been delivered but are waiting to be acknoledged by the +* network. +*/ struct ofono_sms {int flags;DBusMessage *pending;
@@ -73,6 +78,7 @@ struct ofono_sms {struct sms_assembly *assembly;guint ref;GQueue *txq; +GQueue *tx_wsrq;gint tx_source;struct ofono_message_waiting *mw;unsigned int mw_watch;
@@ -473,10 +479,19 @@ 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_WSR + |1 <<OFONO_SMS_TX_ST_DONE + |1 <<OFONO_SMS_TX_ST_CANCELING ); +g_queue_remove (entry->sms_mgr->txq, entry); +break ; +case OFONO_SMS_TX_ST_WSR : +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 ); -g_queue_remove (entry->sms_mgr->txq, entry); + |1 <<OFONO_SMS_TX_ST_FAILED + |1 <<OFONO_SMS_TX_ST_EXPIRED ); +g_queue_remove (entry->sms_mgr->tx_wsrq, entry);break ;case OFONO_SMS_TX_ST_CANCELING : ofono_sms_tx_state_check(
@@ -519,6 +534,8 @@ static void tx_queue_entry_destroy(struct tx_queue_entry *entry)if (entry->state ==OFONO_SMS_TX_ST_QUEUED )g_queue_remove (entry->sms_mgr->txq, entry); +else if (entry->state ==OFONO_SMS_TX_ST_WSR ) +g_queue_remove (entry->sms_mgr->tx_wsrq, entry); entry->state = __OFONO_SMS_TX_ST_INVALID;g_free (entry);
@@ -591,12 +608,16 @@ next_q:__ofono_history_sms_send_status (modem, entry->msg_id,time (NULL ), hs); } -if (ok) +if (ok && entry->flags &OFONO_SMS_SUBMIT_FLAG_REQUEST_SR ) { +ofono_sms_tx_state_set (entry,OFONO_SMS_TX_ST_WSR ); +g_queue_push_tail (sms->tx_wsrq, entry); + }else if (ok && !(entry->flags &OFONO_SMS_SUBMIT_FLAG_REQUEST_SR )) {ofono_sms_tx_state_set (entry,OFONO_SMS_TX_ST_DONE ); -else +tx_queue_entry_destroy (entry); + }else {ofono_sms_tx_state_set (entry,OFONO_SMS_TX_ST_FAILED ); - -tx_queue_entry_destroy (entry); +tx_queue_entry_destroy (entry); + }if (g_queue_peek_head (sms->txq)) {DBG ("Scheduling next" );
@@ -986,13 +1007,45 @@ static void handle_deliver(struct ofono_sms *sms, const struct sms *incoming)g_slist_free (l); } +/* +* Pack information needed to locate a message from a Status Report +* and decide if it was succesfully delivered. +*/ +struct sms_msg_sr_locator { +guint16 msg_id; +gboolean delivered; +}; + +/* +* This function is a g_queue_foreach() functor called by +* handle_sms_status_report() to destroy the message that matches the +* reported MSG-ID in the SMS manager's list of messages waiting for +* acknowledgement. +*/ +static void __sms_find_destroy_by_msg_id (gpointer _sms_msg,gpointer _locator) +{ +const struct sms_msg_sr_locator *locator = _locator; +struct tx_queue_entry *sms_msg = _sms_msg; + +if (sms_msg->msg_id != locator->msg_id) +return ; +ofono_debug ("SMS: ACKED %p msg_id match %x" , sms_msg, locator->msg_id); +g_queue_remove (sms_msg->sms_mgr->tx_wsrq, sms_msg); + +if (locator->delivered) +ofono_sms_tx_state_set (sms_msg,OFONO_SMS_TX_ST_DONE ); +else +ofono_sms_tx_state_set (sms_msg,OFONO_SMS_TX_ST_FAILED ); + +tx_queue_entry_destroy (sms_msg); +}/* * Handle a delivery/status report has been received for an SMS * apparently sent from here. * * We need to find the message in the SMS manager's -* waiting-for-acknoledge queue (sms->tx_wfaq) and remove it. As well, +* waiting-for-status report queue (sms->tx_wsrq) and remove it. As well, * fill out history for it. */ static void handle_sms_status_report(struct ofono_sms *sms,
@@ -1001,11 +1054,16 @@ static void handle_sms_status_report(struct ofono_sms *sms,struct ofono_modem *modem =__ofono_atom_get_modem (sms->atom);gboolean delivered;unsigned int msg_id; +struct sms_msg_sr_locator locator;if (status_report_assembly_report (sms->sr_assembly, incoming, &msg_id, &delivered) == FALSE)return ; + locator.msg_id = msg_id; + locator.delivered = delivered; +g_queue_foreach (sms->tx_wsrq, __sms_find_destroy_by_msg_id, &locator); +__ofono_history_sms_send_status (modem, msg_id,time (NULL ), delivered ?OFONO_HISTORY_SMS_STATUS_DELIVERED :OFONO_HISTORY_SMS_STATUS_DELIVER_FAILED );
@@ -1233,6 +1291,13 @@ static void sms_remove(struct ofono_atom *atom) sms->txq =NULL ; } +if (sms->tx_wsrq) { +g_queue_foreach (sms->tx_wsrq, + tx_queue_entry_destroy_foreach,NULL ); +g_queue_free (sms->tx_wsrq); + sms->tx_wsrq =NULL ; + } +if (sms->settings) {g_key_file_set_integer (sms->settings,SETTINGS_GROUP ,"NextReference" , sms->ref);
@@ -1287,6 +1352,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_wsrq =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 9/19 sms: introduce a state change callback for messages
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
When the SMS message going through the TX hoops changes states, call the given callback function (if any). This will be used later to hook up the D-Bus propagation of property changed signals.
Note this will do away with the ofono_sms_txq_submit_cb callback, as it can be implemented in terms of a state change callback.
--- src/ofono.h | 5 +++-- src/sms.c | 30 ++++++++++++++++++++++-------- src/stk.c | 19 +++++++++++++++++-- 3 files changed, 42 insertions(+), 12 deletions(-)
src/ofono.h +3 -2
@@ -216,12 +216,13 @@ enum ofono_sms_tx_state {const char *ofono_sms_tx_state_to_string (enum ofono_sms_tx_state ); -typedef void (*ofono_sms_txq_submit_cb_t )(gboolean ok,void *data); +typedef void (*ofono_sms_msg_stch_cb_t )(void *data, +enum ofono_sms_tx_state new_state);struct tx_queue_entry *__ofono_sms_txq_submit (struct ofono_sms *sms,GSList *list,unsigned int flags,unsigned ref, -ofono_sms_txq_submit_cb_t cb, +ofono_sms_msg_stch_cb_t stch_cb,void *data,ofono_destroy_func destroy);#include <ofono/sim.h>
src/sms.c +22 -8
@@ -113,7 +113,7 @@ struct tx_queue_entry {unsigned int msg_id;unsigned int retry;unsigned int flags; -ofono_sms_txq_submit_cb_t cb; +ofono_sms_msg_stch_cb_t stch_cb;void *data;ofono_destroy_func destroy; };
@@ -516,6 +516,8 @@ 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; +if (entry->stch_cb) + entry->stch_cb (entry->data, state_new); }
@@ -594,9 +596,6 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data) next_q: entry =g_queue_peek_head (sms->txq); -if (entry->cb) - entry->cb (ok, entry->data); -if (entry->flags &OFONO_SMS_SUBMIT_FLAG_RECORD_HISTORY ) {enum ofono_history_sms_status hs;
@@ -692,11 +691,26 @@ static struct tx_queue_entry *tx_queue_entry_new(GSList *msg_list)return entry; } -static void send_message_cb (gboolean ok,void *data) +static void send_message_stch_cb (void *data,enum ofono_sms_tx_state new_state) {DBusConnection *conn =ofono_dbus_get_connection ();DBusMessage *msg = data;DBusMessage *reply; +gboolean ok; + +/* +* We care about only the final states +* (DONE/CANCELLED/FAILED/EXPIRED), the rest are just +* ignored. +*/ +if (new_state ==OFONO_SMS_TX_ST_DONE ) + ok = TRUE; +else if (new_state ==OFONO_SMS_TX_ST_CANCELLED + || new_state ==OFONO_SMS_TX_ST_FAILED + || new_state ==OFONO_SMS_TX_ST_EXPIRED ) + ok = FALSE; +else +return ;if (ok) reply =dbus_message_new_method_return (msg);
@@ -763,7 +777,7 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg, flags |=OFONO_SMS_SUBMIT_FLAG_REQUEST_SR ; sms_msg =__ofono_sms_txq_submit (sms, msg_list, flags, msg_id, - send_message_cb, + send_message_stch_cb,dbus_message_ref (msg), send_message_destroy);
@@ -1501,7 +1515,7 @@ void *ofono_sms_get_data(struct ofono_sms *sms)struct tx_queue_entry *__ofono_sms_txq_submit (struct ofono_sms *sms,GSList *list,unsigned int flags,unsigned msg_id, -ofono_sms_txq_submit_cb_t cb, +ofono_sms_msg_stch_cb_t stch_cb,void *data,ofono_destroy_func destroy) {struct tx_queue_entry *entry =tx_queue_entry_new (list);
@@ -1515,7 +1529,7 @@ struct tx_queue_entry *__ofono_sms_txq_submit( entry->msg_id = msg_id; entry->flags = flags; - entry->cb = cb; + entry->stch_cb = stch_cb; entry->data = data; entry->destroy = destroy; entry->sms_mgr = sms;
src/stk.c +17 -2
@@ -287,12 +287,27 @@ static void send_sms_cancel(struct ofono_stk *stk)stk_alpha_id_unset (stk); } -static void send_sms_submit_cb (gboolean ok,void *data) +static void send_sms_stch_cb (void *data,enum ofono_sms_tx_state new_state) {struct sms_submit_req *req = data;struct ofono_stk *stk = req->stk;struct ofono_error failure = { .type =OFONO_ERROR_TYPE_FAILURE };struct stk_response rsp; +gboolean ok; + +/* +* We care about only the final states +* (DONE/CANCELLED/FAILED/EXPIRED), the rest are just +* ignored. +*/ +if (new_state ==OFONO_SMS_TX_ST_DONE ) + ok = TRUE; +else if (new_state ==OFONO_SMS_TX_ST_CANCELLED + || new_state ==OFONO_SMS_TX_ST_FAILED + || new_state ==OFONO_SMS_TX_ST_EXPIRED ) + ok = FALSE; +else +return ;ofono_debug ("SMS submission %s" , ok ?"successful" :"failed" );
@@ -340,7 +355,7 @@ static gboolean handle_command_send_sms(const struct stk_command *cmd, msg_list.data = (void *) &cmd->send_sms.gsm_sms; msg_list.next =NULL ; msg_id =sms_uuid_from_pdu_list (&msg_list); -__ofono_sms_txq_submit (sms, &msg_list,0 , msg_id, send_sms_submit_cb, +__ofono_sms_txq_submit (sms, &msg_list,0 , msg_id, send_sms_stch_cb, stk->sms_submit_req, g_free); stk->cancel_cmd = send_sms_cancel;
Signature · 1 line
1.6.6.1
PATCH 10/19 sms: export outgoing messages over D-Bus
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
Splits sms_send_message() into a D-Bus front end and an internal API:
- sms_msg_send(), a full C interface for SMS sending
- dbus_sms_msg_send(): adapts sms_msg_send() to be a D-Bus call, exporting the object on the bus, returning its name and setting up all the callbacks and D-Bus specific data. The call is synchronous.
This allows internal code to use the same infrastructure as D-Bus clients to send SMS messages.
--- src/sms.c | 183 +++++++++++++++++++++++++++++++++++++++++---------------- src/smsutil.h | 6 ++ 2 files changed, 137 insertions(+), 52 deletions(-)
src/sms.c +131 -52
@@ -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"
@@ -522,6 +526,24 @@ 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" }, + { } +}; + +/* * Destroy/release the contents of a 'struct tx_queue_entry' * * This releases resources allocated *inside* @entry and @entry
@@ -541,6 +563,7 @@ static void tx_queue_entry_destroy(struct tx_queue_entry *entry) entry->state = __OFONO_SMS_TX_ST_INVALID;g_free (entry); +ofono_debug ("%s():%d: sms_msg %p freed" , __func__, __LINE__, entry); }static void tx_queue_entry_destroy_foreach (gpointer _entry,gpointer unused)
@@ -687,52 +710,34 @@ static struct tx_queue_entry *tx_queue_entry_new(GSList *msg_list)DBG ("pdu_len: %d, tpdu_len: %d" , pdu->pdu_len, pdu->tpdu_len); } -return entry; } -static void send_message_stch_cb (void *data,enum ofono_sms_tx_state new_state) + +static void sms_msg_dbus_destroy (void *_dbus_path) { -DBusConnection *conn =ofono_dbus_get_connection (); -DBusMessage *msg = data; -DBusMessage *reply; -gboolean ok; +char *dbus_path = _dbus_path; -/* -* We care about only the final states -* (DONE/CANCELLED/FAILED/EXPIRED), the rest are just -* ignored. -*/ -if (new_state ==OFONO_SMS_TX_ST_DONE ) - ok = TRUE; -else if (new_state ==OFONO_SMS_TX_ST_CANCELLED - || new_state ==OFONO_SMS_TX_ST_FAILED - || new_state ==OFONO_SMS_TX_ST_EXPIRED ) - ok = FALSE; -else +if (dbus_path ==NULL )return ; -if (ok) - reply =dbus_message_new_method_return (msg); -else - reply =__ofono_error_failed (msg); - -g_dbus_send_message (conn, reply); +g_dbus_unregister_interface (ofono_dbus_get_connection (), + dbus_path,SMS_MSG_INTERFACE ); +g_free (dbus_path); } -static void send_message_destroy (void *data) -{ -DBusMessage *msg = data; - -dbus_message_unref (msg); -}/* -* 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) * @data: SMS object to use for transmision +* @send_flags: flags that manipulate how the message is sent. +* @send_cb: function called when the message is sent +* @destroy_cb: function called when the message structure is +* being released +* @priv: pointer to pass to the @data and @send_cb functions * * An alphabet is chosen for the text and it (might be) segmented in * fragments by sms_text_prepare() into @msg_list. A queue list @entry
@@ -740,12 +745,13 @@ static void send_message_destroy(void *data)* appends that entry to the SMS transmit queue. Then the tx_next() * function is scheduled to run to process the queue. */ -static DBusMessage *sms_send_message (DBusConnection *conn,DBusMessage *msg, -void *data) +static +struct tx_queue_entry *sms_msg_send ( +struct ofono_sms *sms,const char *to,const char *text, +enum sms_msg_send_flags send_flags, +ofono_sms_msg_stch_cb_t stch_cb, +void (*destroy_cb)(void *),void *priv) { -struct ofono_sms *sms = data; -const char *to; -const char *text;GSList *msg_list;int ref_offset;struct ofono_modem *modem;
@@ -753,41 +759,115 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,guint16 msg_id;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); -if (valid_phone_number_format (to) == FALSE) -return __ofono_error_invalid_format (msg); +return NULL ; msg_list =sms_text_prepare (text,0 , TRUE, &ref_offset, - sms->use_delivery_reports); + sms->use_delivery_reports);if (!msg_list) -return __ofono_error_invalid_format (msg); +return NULL ; msg_id =sms_uuid_from_pdu_list (msg_list);set_ref_and_to (msg_list, msg_id, ref_offset, to);DBG ("SMS ID: 0x%04x" , msg_id); - flags =OFONO_SMS_SUBMIT_FLAG_RECORD_HISTORY ; +if (send_flags &SMS_MSG_SEND_HISTORY ) + flags =OFONO_SMS_SUBMIT_FLAG_RECORD_HISTORY ; flags |=OFONO_SMS_SUBMIT_FLAG_RETRY ;if (sms->use_delivery_reports) flags |=OFONO_SMS_SUBMIT_FLAG_REQUEST_SR ; sms_msg =__ofono_sms_txq_submit (sms, msg_list, flags, msg_id, - send_message_stch_cb, -dbus_message_ref (msg), - send_message_destroy); + stch_cb, priv, destroy_cb);g_slist_foreach (msg_list, (GFunc )g_free,NULL );g_slist_free (msg_list); modem =__ofono_atom_get_modem (sms->atom); -__ofono_history_sms_send_pending (modem, msg_id, to,time (NULL ), text); -return NULL ; +return sms_msg; +} + +/* +* 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_sms_msg_send (DBusConnection *conn,DBusMessage *msg, +void *data) +{ +struct ofono_sms *sms = data; +const char *to; +const char *text; +struct tx_queue_entry *sms_msg; +DBusMessage *reply; +char *dbus_path; +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); + +/* +* This is released by @ sms_msg_dbus_destroy() -- first we +* allocate and then once we have the MSG ID, we re-write it. +*/ + dbus_path =g_strdup_printf ("%s/%04x" , sms_path,0 ); + sms_msg =sms_msg_send (sms, to, text,SMS_MSG_SEND_HISTORY , +NULL , + sms_msg_dbus_destroy, + dbus_path); + +if (sms_msg ==NULL ) + goto error_sms_msg_send; + +/* Set the MSG ID in the D-Bus path now that we have it. */ +sprintf (dbus_path,"%s/%04x" , sms_path, sms_msg->msg_id &0xffff ); + +if (!g_dbus_register_interface (ofono_dbus_get_connection (), + dbus_path,SMS_MSG_INTERFACE , + sms_msg_methods, sms_msg_signals, +NULL , sms_msg,NULL )) { +ofono_error ("%s():%d: %s: Could not create %s interface" , + __func__, __LINE__, dbus_path,SMS_MSG_INTERFACE ); + goto error_dbus_register_interface; + } + +ofono_debug ("%s():%d: sms %p @ %s: MSG registered" , + __func__, __LINE__, sms_msg, dbus_path); + + reply =dbus_message_new_method_return (msg); + +if (!reply) + goto error_dbus_new_method_return; + +dbus_message_append_args (reply, +DBUS_TYPE_STRING , &dbus_path, +DBUS_TYPE_INVALID ); +return reply; + + +error_dbus_new_method_return: +g_dbus_unregister_interface (ofono_dbus_get_connection (), + dbus_path,SMS_MSG_INTERFACE ); +error_dbus_register_interface: +/* +* Note we don't want the destructor called, as we do it's +* steps manually here when falling through. Kind of layering +* violation, but it is the cleaner way as things are +* currently laid out. +*/ + sms_msg->destroy =NULL ; +tx_queue_entry_destroy (sms_msg); +error_sms_msg_send: +g_free (dbus_path); +return __ofono_error_invalid_format (msg); }static GDBusMethodTable sms_manager_methods[] = {
@@ -795,8 +875,7 @@ static GDBusMethodTable sms_manager_methods[] = { G_DBUS_METHOD_FLAG_ASYNC }, {"SetProperty" ,"sv" ,"" , sms_set_property, G_DBUS_METHOD_FLAG_ASYNC }, - { "SendMessage", "ss", "", sms_send_message, - G_DBUS_METHOD_FLAG_ASYNC }, + { "SendMessage", "ss", "", dbus_sms_msg_send, 0 }, { } };
src/smsutil.h +6 -0
@@ -209,6 +209,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 11/19 sms: send PropertyChanged signals on state change
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
This hooks sms_msg_stch_dbus_cb() into the SMS state change callback 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.
--- src/sms.c | 36 +++++++++++++++++++++++++++++++++++- 1 files changed, 35 insertions(+), 1 deletions(-)
src/sms.c +35 -1
@@ -790,6 +790,40 @@ struct tx_queue_entry *sms_msg_send( }/* +* Send a PropertyChange signal when the state changes +* +* In D-Bus we don't care about the initial state transition to +* _QUEUED, as the object path hasn't even been published yet in the +* bus. +*/ +static +void sms_msg_stch_dbus_cb (void *_dbus_path, +enum ofono_sms_tx_state new_state) +{ +char *dbus_path = _dbus_path; +DBusConnection *dbus_conn =ofono_dbus_get_connection (); +DBusMessage *dbus_signal; +DBusMessageIter iter; +const char *property ="State" ; +const char *new_state_str; + +if (new_state ==OFONO_SMS_TX_ST_QUEUED ) +return ; + + dbus_signal =dbus_message_new_signal ( + 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 (new_state); +dbus_message_iter_append_basic (&iter,DBUS_TYPE_STRING , &new_state_str); +g_dbus_send_message (dbus_conn, dbus_signal); +} + +/* * D-Bus: Send a SMS text message * * @conn: D-Bus connection
@@ -820,7 +854,7 @@ static DBusMessage *dbus_sms_msg_send(DBusConnection *conn, DBusMessage *msg,*/ dbus_path =g_strdup_printf ("%s/%04x" , sms_path,0 ); sms_msg =sms_msg_send (sms, to, text,SMS_MSG_SEND_HISTORY , -NULL , + sms_msg_stch_dbus_cb, sms_msg_dbus_destroy, dbus_path);
Signature · 1 line
1.6.6.1
PATCH 12/19 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.
Note the test case code requires follow up commits that propagate the message's state changes over D-Bus.
--- src/sms.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 55 insertions(+), 0 deletions(-)
src/sms.c +55 -0
@@ -525,6 +525,20 @@ static void __ofono_sms_tx_state_set(struct tx_queue_entry *entry, } + +static void sms_msg_cancel (struct tx_queue_entry *); +/* +* Note that the D-Bus specific cleanups are taken care by the +* sms_msg_dbus_destroy() callback passed in dbus_sms_msg_send(). +*/ +static DBusMessage *dbus_sms_msg_cancel ( +DBusConnection * conn,DBusMessage *msg,void *data) +{ +struct tx_queue_entry *sms_msg = data; +sms_msg_cancel (sms_msg); +return dbus_message_new_method_return (msg); +} +/* * D-Bus SMS Message interface *
@@ -534,6 +548,8 @@ static void __ofono_sms_tx_state_set(struct tx_queue_entry *entry,static GDBusMethodTable sms_msg_methods[] = { + {"Cancel" ,"" ,"" , + dbus_sms_msg_cancel,0 }, { } };
@@ -579,6 +595,11 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data)gboolean ok = error->type ==OFONO_ERROR_TYPE_NO_ERROR ;DBG ("tx_finished" ); +/* +* Queue is empty? Messages might have been cancelled. +*/ +if (entry ==NULL ) +return ;if (ok == FALSE) {if (!(entry->flags &OFONO_SMS_SUBMIT_FLAG_RETRY )) {
@@ -789,6 +810,40 @@ struct tx_queue_entry *sms_msg_send(return sms_msg; } +/** +* Cancel a pending SMS message +* +* @sms_msg: message to cancel +* +* This function cancels a message that is pending or being +* actively transmitted. Note that after this function returns, the +* @sms_msg handle is no longer valid. +* +* \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. +*/ +static void sms_msg_cancel (struct tx_queue_entry *sms_msg) +{ +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_WSR ) +g_queue_remove (sms->tx_wsrq, sms_msg); + +ofono_sms_tx_state_set (sms_msg,OFONO_SMS_TX_ST_CANCELING ); +ofono_sms_tx_state_set (sms_msg,OFONO_SMS_TX_ST_CANCELLED ); +tx_queue_entry_destroy (sms_msg); +} +/* * Send a PropertyChange signal when the state changes *
Signature · 1 line
1.6.6.1
PATCH 13/19 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 and the destination number. Note we make the @receiver field in 'struct tx_queue_entry' be always filled out in __ofono_sms_txq_submit() as now it is always needed for the properties, not only when a Status Report is requested.
--- src/sms.c | 39 +++++++++++++++++++++++++++++++++------ 1 files changed, 33 insertions(+), 6 deletions(-)
src/sms.c +33 -6
@@ -524,7 +524,35 @@ static void __ofono_sms_tx_state_set(struct tx_queue_entry *entry, entry->stch_cb (entry->data, state_new); } +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,"State" ,DBUS_TYPE_STRING , &str); + + str =sms_address_to_string (&sms_msg->receiver); +ofono_dbus_dict_append (&dict,"To" ,DBUS_TYPE_STRING , &str); + +dbus_message_iter_close_container (&iter, &dict); + +return reply; +}static void sms_msg_cancel (struct tx_queue_entry *);/*
@@ -548,6 +576,8 @@ static DBusMessage *dbus_sms_msg_cancel(static GDBusMethodTable sms_msg_methods[] = { + {"GetProperties" ,"" ,"a{sv}" , + dbus_sms_msg_get_properties,0 }, {"Cancel" ,"" ,"" , dbus_sms_msg_cancel,0 }, { }
@@ -1687,13 +1717,10 @@ struct tx_queue_entry *__ofono_sms_txq_submit( void *data, ofono_destroy_func destroy) {struct tx_queue_entry *entry =tx_queue_entry_new (list); +struct sms *head = list->data; -if (flags &OFONO_SMS_SUBMIT_FLAG_REQUEST_SR ) { -struct sms *head = list->data; - -memcpy (&entry->receiver, &head->submit.daddr, -sizeof (entry->receiver)); - } +memcpy (&entry->receiver, &head->submit.daddr, +sizeof (entry->receiver)); entry->msg_id = msg_id; entry->flags = flags;
Signature · 1 line
1.6.6.1
PATCH 14/19 sms: document SMS Message's D-Bus 'To' property
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
--- doc/sms-api.txt | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
doc/sms-api.txt +5 -0
@@ -109,3 +109,8 @@ Properties string State Current transmission state {queued, wsr, done, canceling, cancelled, failed, expired} + + string To + + Destination number to which the message is + being sent.
Signature · 1 line
1.6.6.1
PATCH 15/19 sms: test code for message's D-Bus GetProperties
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
--- test/test-sms-msg-get-properties | 26 ++++++++++++++++++++++++++ 1 files changed, 26 insertions(+), 0 deletions(-) create mode 100755 test/test-sms-msg-get-properties
test/test-sms-msg-get-properties new file +26 -0
@@ -0,0 +1,26 @@ +#!/usr/bin/python + +import sys +import dbus + +if len(sys.argv) != 2: + print "Error: missing argument (SMS message D-Bus object name)" + sys.exit(1) + + +bus = dbus.SystemBus() + +manager = dbus.Interface(bus.get_object('org.ofono', '/'), + 'org.ofono.Manager') + +properties = manager.GetProperties() + +path = properties["Modems"][0] + +manager = dbus.Interface(bus.get_object('org.ofono', path), + 'org.ofono.SmsManager') + +sms_msg = dbus.Interface(bus.get_object('org.ofono', sys.argv[1]), + 'org.ofono.SmsMessage') +properties = sms_msg.GetProperties() +print "To: %s\nState: %s\n" % (properties["To"], properties["State"])
Signature · 1 line
1.6.6.1
PATCH 16/19 automake: fix generation of symlinks for headers
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
When running 'make distcheck' from a vpath build directory (ie: one that is not where the source lives), the target headers in include/ofono are not generated properly due to a typo in the Makefile.am, that is using _srcdir where _buildir should be being used.
--- Makefile.am | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
Makefile.am +1 -1
@@ -444,7 +444,7 @@ include/ofono/version.h: include/version.h include/ofono/%.h: include/%.h $(AM_V_at)$(MKDIR_P) include/ofono - $(AM_V_GEN)$(LN_S) $(abs_top_srcdir)/$< $@ + $(AM_V_GEN)$(LN_S) $(abs_top_builddir)/$< $@ clean-local: @$(RM) -rf include/ofono
Signature · 1 line
1.6.6.1
PATCH 17/19 sms: document variable usage in sms_msg_send()
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
--- src/sms.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
src/sms.c +3 -0
@@ -795,6 +795,9 @@ static void sms_msg_dbus_destroy(void *_dbus_path)* is created by tx_queue_entry_new() 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 struct tx_queue_entry *sms_msg_send(
Signature · 1 line
1.6.6.1
PATCH 18/19 sms: add test case for message cancel
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
--- test/test-sms-msg-cancel | 173 ++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 173 insertions(+), 0 deletions(-) create mode 100755 test/test-sms-msg-cancel
test/test-sms-msg-cancel new file +173 -0
@@ -0,0 +1,173 @@ +#!/usr/bin/python +# +# Sends a message and inmediately cancels it once the state moves to +# "queued"; fails if the message doesn't move to the CANCELLED state. +# +# Arguments are: DEST-PHONE-NUMBER MSG-TEXT BOOLEAN +# +# BOOLEAN: request delivery confirmation / no +# +# Notes: accessing the globals result and reason using 'globals' +# + +import gobject +import sys +import dbus +import dbus.mainloop.glib + +dbus.mainloop.glib.DBusGMainLoop(set_as_default=True) + +class sms_msg_cancel_test: + number = "" + text = "" + wsr = "" + target_state = "" + + result = "" + reason = "" + + sms_msg_path = "" + + # FIXME + def __init__(self, number, text, wsr): + self.number = number + self.text = text + self.wsr = wsr + self.mainloop = gobject.MainLoop() + + self.dbus = dbus.SystemBus() + ofono_manager = dbus.Interface(self.dbus.get_object('org.ofono', '/'), + 'org.ofono.Manager') + path = ofono_manager.GetProperties()["Modems"][0] + self.sms_manager = dbus.Interface(self.dbus.get_object('org.ofono', path), + 'org.ofono.SmsManager') + self.sms_manager.SetProperty("UseDeliveryReports", dbus.Boolean(int(self.wsr))) + + # Connect the signal *before* creating the message, otherwise we miss + # the transition - this also means we can't use sms_msg.connect_to_signal() + self.dbus.add_signal_receiver(self.property_changed, bus_name="org.ofono", + signal_name = "PropertyChanged", + path_keyword="path", + interface_keyword="interface") + self.result = "FAILED" + self.reason = "didn't see CANCELING state" + + # Cancel the message and update test state + def msg_cancel(self): + print "I: %s[%s]: SMS message: canceling because " \ + "target state was reached" \ + % (self.sms_msg_path, self.target_state) + sms_msg = dbus.Interface(self.dbus.get_object('org.ofono', self.sms_msg_path), + 'org.ofono.SmsMessage') + try: + sms_msg.Cancel() + except dbus.DBusException as dbus_ex: + if dbus_ex.get_dbus_name() != 'org.freedesktop.DBus.Error.UnknownMethod': + pass + # Object has been cancelled already--this is + # why we get the D-Bus exception, so we can + # consider this a win + print "I: %s[%s]: SMS message: ok, D-Bus object already removed" \ + % (self.sms_msg_path, self.target_state) + self.result = "SUCCESS" + self.reason = "object dissapeared from the bus, meaning it was cancelled" + self.mainloop.quit() + return + + # Run the actual test FIXME + def run(self, target_state): + self.target_state = target_state + + self.result = "FAILED" + self.reason = "didn't see %s state to allow cancelling" \ + % target_state + + print "I: [%s]: starting test" % self.target_state + self.sms_msg_path = self.sms_manager.SendMessage(number, text) + print "I: %s[%s]: SMS message: D-Bus object created" \ + % (self.sms_msg_path, self.target_state) + + # wait for property changed signals, property_changed() will be called + # when there is a transition and the thing cancelled. + gobject.timeout_add(20000, self.timeout_fail) + # If the target state to cancel on is QUEUED, schedule a + # cancellation inmediately -- note we need to schedule it in + # the mainloop to ensure property_changed() signals are + # properly propagated. + if target_state == "queued" \ + or target_state == "canceling" \ + or target_state == "cancelled": + gobject.idle_add(self.msg_cancel) + self.mainloop.run() + if self.result == "SUCCESS": + letter = "I" + result = 0 + else: + letter = "E" + result = 1 + print "%s: %s[%s]: %s: %s" % (letter, self.sms_msg_path, + self.target_state, + self.result, self.reason) + return result + + def timeout_fail(self): + self.result = "FAILED" + self.reason = "%s (timedout)" % self.reason + self.mainloop.quit() + # Don't rearm the timeout ... doesn't really matter as we + # quit the mainloop. + return 0 + + + # When the message's state (the one we created) switches to QUEUED, cancel + def property_changed(self, property_name, property_value, + path, interface): + if interface != "org.ofono.SmsMessage": + return + if self.sms_msg_path != path: + return + if property_name != "State": + return + print "I: %s[%s]: SMS message: transitioning to %s state" \ + % (self.sms_msg_path, self.target_state, property_value) + if property_value == "canceling": + print "I: %s[%s]: SMS message: ok, saw canceling state" \ + % (self.sms_msg_path, self.target_state) + self.result = "FAIL" + self.reason = "didn't see the CANCELLED state" + if property_value == "cancelled": + print "I: %s[%s]: SMS message: ok, saw cancelled state, quiting" \ + % (self.sms_msg_path, self.target_state) + self.result = "SUCCESS" + self.reason = "saw CANCELLED state" + self.mainloop.quit() + # If we get to the state that is the target state for + # the test, cancel the message -- we need to do this + # after evaluating propery_value or we might enter + # into conflicts (as self.msg_cancel() will change + # self.{result,reason} if it turns out the operation + # is complete). + if self.target_state == property_value: + # Cancel the message + self.msg_cancel() + + +# Send the message +if len(sys.argv) == 4: + wsr = int(sys.argv[3]) +elif len(sys.argv) == 3: + wsr = 0 +else: + raise NameError("E: Bad number of arguments (expected NUMBER TEXT [BOOLEAN])") + +number = sys.argv[1] +text = sys.argv[2] +global test + +test = sms_msg_cancel_test(number, text, wsr) + +test.run('queued') +test.run('wsr') +test.run('canceling') +test.run('cancelled') +test.run('done')
Signature · 1 line
1.6.6.1
PATCH 19/19 sms: add test case for state change signals
In reply to Inaky Perez-GonzalezFrom: Inaky Perez-Gonzalez <[email protected]>
--- test/test-sms-msg-state-change | 24 ++++++++++++++++++++++++ 1 files changed, 24 insertions(+), 0 deletions(-) create mode 100755 test/test-sms-msg-state-change
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