12:01 a.m.
Hello again,
this is the new version of SMS status report. It is implemented
according to the latest comments which you gave about my previous
patches. These patches include concatenated status report
assembling and status report result printing via history plugin.
Next I am going to take a new look at the saving of the pending
status reports to imsi file.
Br,
Pasi Miettinen
12:01 a.m.
New subject: [RFC PATCH 1/3] smsutil: Status Report assembly
---
src/smsutil.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
src/smsutil.h | 28 ++++++++++++
2 files changed, 163 insertions(+), 0 deletions(-)
diff --git a/src/smsutil.c b/src/smsutil.c
index 278d335..04247fe 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -2625,6 +2625,141 @@ void sms_assembly_expire(struct sms_assembly *assembly, time_t
before)
}
}
+struct status_report_assembly *status_report_assembly_new(const char *imsi)
+{
+ struct status_report_assembly *ret =
+ g_new0(struct status_report_assembly, 1);
+
+ ret->assembly_table = g_hash_table_new_full(g_str_hash, g_str_equal,
+ g_free, (GDestroyNotify)g_hash_table_destroy);
+
+ if (imsi)
+ ret->imsi = imsi;
+
+ return ret;
+}
+
+void status_report_assembly_free(struct status_report_assembly *assembly)
+{
+ g_hash_table_destroy(assembly->assembly_table);
+}
+
+enum ofono_history_sms_status status_report_assembly_report(
+ struct status_report_assembly *assembly,
+ const struct sms *status_report,
+ unsigned int *msg_id)
+{
+ unsigned int offset = status_report->status_report.mr / 32;
+ unsigned int bit = 1 << (status_report->status_report.mr % 32);
+ GHashTable *id_table;
+ struct id_table_node *node = NULL;
+ gpointer key;
+ gpointer value;
+ GHashTableIter iter;
+ int i;
+
+ id_table = g_hash_table_lookup(assembly->assembly_table,
+ status_report->status_report.raddr.address);
+
+ /* ERROR, key (receiver address) does not exist in assembly*/
+ if (!id_table)
+ return OFONO_HISTORY_SMS_STATUS_ERROR;
+
+ g_hash_table_iter_init(&iter, id_table);
+ while (g_hash_table_iter_next(&iter, &key, &value)) {
+ node = value;
+
+ if (!(node->mrs[offset] & bit))
+ continue;
+
+ /* mr belongs to this node */
+ node->mrs[offset] ^= bit;
+ *msg_id = node->ofono_msg_id;
+
+ for (i = 0; i < 8; i++) {
+ /* There are still pending mr(s). */
+ if (node->mrs[i] != 0)
+ return OFONO_HISTORY_SMS_STATUS_PENDING;
+ }
+ /* No more pending mrs for this node. Remove node from id_table
+ * and free it.
+ */
+ g_hash_table_iter_remove(&iter);
+ /* id_list empty, remove key and value from assembly_table */
+ if (g_hash_table_size(id_table) == 0)
+ g_hash_table_remove(assembly->assembly_table,
+ status_report->status_report.raddr.address);
+
+ return OFONO_HISTORY_SMS_STATUS_DELIVERED;
+ }
+ /* mr not found. */
+ return OFONO_HISTORY_SMS_STATUS_ERROR;
+}
+
+void status_report_assembly_add_fragment(struct status_report_assembly
+ *assembly, unsigned int *msg_id,
+ struct sms_address *to,
+ unsigned char mr, time_t expiration)
+{
+ unsigned int offset = mr / 32;
+ unsigned int bit = 1 << (mr % 32);
+ GHashTable *id_table;
+ struct id_table_node *node;
+ char *assembly_table_key;
+ unsigned int *id_table_key;
+
+ id_table = g_hash_table_lookup(assembly->assembly_table, to->address);
+
+ if (id_table) {
+ node = g_hash_table_lookup(id_table, msg_id);
+
+ if (node) {
+ node->mrs[offset] |= bit;
+ node->expiration = expiration;
+ } else {
+ id_table_key = g_new0(unsigned int, 1);
+ node = g_new0(struct id_table_node, 1);
+ node->to = *to;
+ node->ofono_msg_id = *msg_id;
+ node->mrs[offset] |= bit;
+ node->expiration = expiration;
+
+ *id_table_key = *msg_id;
+ g_hash_table_insert(id_table, id_table_key, node);
+ }
+ } else {
+ id_table = g_hash_table_new_full(g_int_hash, g_int_equal,
+ g_free, g_free);
+ id_table_key = g_new0(unsigned int, 1);
+
+ node = g_new0(struct id_table_node, 1);
+ node->to = *to;
+ node->ofono_msg_id = *msg_id;
+ node->mrs[offset] |= bit;
+ node->expiration = expiration;
+
+ *id_table_key = *msg_id;
+ g_hash_table_insert(id_table, id_table_key, node);
+
+ assembly_table_key = g_try_malloc(sizeof(to->address));
+
+ if (assembly_table_key == NULL)
+ return;
+
+ g_strlcpy(assembly_table_key, to->address, sizeof(to->address));
+
+ g_hash_table_insert(assembly->assembly_table,
+ assembly_table_key, id_table);
+ }
+}
+
+void status_report_assembly_expire(struct status_report_assembly *assembly,
+ time_t before, GFunc foreach_func,
+ gpointer data)
+{
+ /*TODO*/
+}
+
static inline GSList *sms_list_append(GSList *l, const struct sms *in)
{
struct sms *sms;
diff --git a/src/smsutil.h b/src/smsutil.h
index a36a9d3..14a01dc 100644
--- a/src/smsutil.h
+++ b/src/smsutil.h
@@ -19,6 +19,8 @@
*
*/
+#include "history.h"
+
#define CBS_MAX_GSM_CHARS 93
enum sms_type {
@@ -364,6 +366,18 @@ struct sms_assembly {
GSList *assembly_list;
};
+struct id_table_node {
+ struct sms_address to;
+ unsigned int ofono_msg_id;
+ unsigned int mrs[8];
+ time_t expiration;
+};
+
+struct status_report_assembly {
+ const char *imsi;
+ GHashTable *assembly_table;
+};
+
struct cbs {
enum cbs_geo_scope gs; /* 2 bits */
guint16 message_code; /* 10 bits */
@@ -481,6 +495,20 @@ GSList *sms_assembly_add_fragment(struct sms_assembly *assembly,
guint16 ref, guint8 max, guint8 seq);
void sms_assembly_expire(struct sms_assembly *assembly, time_t before);
+struct status_report_assembly *status_report_assembly_new(const char *imsi);
+void status_report_assembly_free(struct status_report_assembly *assembly);
+enum ofono_history_sms_status status_report_assembly_report(
+ struct status_report_assembly *assembly,
+ const struct sms *status_report,
+ unsigned int *msg_id);
+void status_report_assembly_add_fragment(struct status_report_assembly
+ *assembly, unsigned int *msg_id,
+ struct sms_address *to,
+ unsigned char mr, time_t expiration);
+void status_report_assembly_expire(struct status_report_assembly *assembly,
+ time_t before, GFunc foreach_func,
+ gpointer data);
+
GSList *sms_text_prepare(const char *utf8, guint16 ref,
gboolean use_16bit, int *ref_offset);
--
1.6.0.4
12:01 a.m.
New subject: [RFC PATCH 2/3] history: print SMS status
---
src/history.c | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/src/history.c b/src/history.c
index f868ca2..0eef130 100644
--- a/src/history.c
+++ b/src/history.c
@@ -238,6 +238,8 @@ void __ofono_history_sms_send_status(struct ofono_modem *modem,
enum ofono_history_sms_status status)
{
struct history_sms_foreach_data hfd;
+ struct tm *ts;
+ char buf[80];
hfd.msg_id = msg_id;
hfd.address = NULL;
@@ -245,6 +247,15 @@ void __ofono_history_sms_send_status(struct ofono_modem *modem,
hfd.when = when;
hfd.status = status;
+ /* Format time, "ddd yyyy-mm-dd hh:mm:ss zzz" */
+ ts = localtime(&when);
+ strftime(buf, sizeof(buf), "%a %Y-%m-%d %H:%M:%S %Z", ts);
+
+ if (status == OFONO_HISTORY_SMS_STATUS_DELIVERED)
+ DBG("SMS delivered, msg_id: %i, time: %s", msg_id, buf);
+ else if (status == OFONO_HISTORY_SMS_STATUS_DELIVER_FAILED)
+ DBG("SMS undeliverable, msg_id: %i, time: %s", msg_id, buf);
+
__ofono_modem_foreach_atom(modem, OFONO_ATOM_TYPE_HISTORY,
history_sms_send_status, &hfd);
}
--
1.6.0.4
12:01 a.m.
New subject: [RFC PATCH 3/3] sms: status report notify
---
include/history.h | 3 ++
src/sms.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 61 insertions(+), 1 deletions(-)
diff --git a/include/history.h b/include/history.h
index 300a4fb..61a0dea 100644
--- a/include/history.h
+++ b/include/history.h
@@ -33,6 +33,9 @@ enum ofono_history_sms_status {
OFONO_HISTORY_SMS_STATUS_PENDING,
OFONO_HISTORY_SMS_STATUS_SUBMITTED,
OFONO_HISTORY_SMS_STATUS_SUBMIT_FAILED,
+ OFONO_HISTORY_SMS_STATUS_DELIVERED,
+ OFONO_HISTORY_SMS_STATUS_DELIVER_FAILED,
+ OFONO_HISTORY_SMS_STATUS_ERROR,
};
struct ofono_history_context {
diff --git a/src/sms.c b/src/sms.c
index c0e9fc4..1e78b9e 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -67,6 +67,8 @@ struct ofono_sms {
const struct ofono_sms_driver *driver;
void *driver_data;
struct ofono_atom *atom;
+ ofono_bool_t use_delivery_reports;
+ struct status_report_assembly *sr_assembly;
};
struct pending_pdu {
@@ -82,6 +84,8 @@ struct tx_queue_entry {
unsigned int msg_id;
unsigned int retry;
DBusMessage *msg;
+ gboolean status_report;
+ struct sms_address receiver;
};
static void set_sca(struct ofono_sms *sms,
@@ -306,6 +310,12 @@ static void tx_finished(const struct ofono_error *error, int mr, void
*data)
entry->cur_pdu += 1;
entry->retry = 0;
+ if (entry->status_report)
+ status_report_assembly_add_fragment(sms->sr_assembly,
+ &entry->msg_id,
+ &entry->receiver,
+ mr, time(NULL));
+
if (entry->cur_pdu < entry->num_pdus) {
sms->tx_source = g_timeout_add(0, tx_next, sms);
return;
@@ -436,6 +446,8 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage
*msg,
set_ref_and_to(msg_list, sms->ref, ref_offset, to);
entry = create_tx_queue_entry(msg_list);
+ sms_address_from_string(&entry->receiver, to);
+
g_slist_foreach(msg_list, (GFunc)g_free, NULL);
g_slist_free(msg_list);
@@ -448,6 +460,7 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage
*msg,
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);
@@ -692,6 +705,24 @@ static void handle_deliver(struct ofono_sms *sms, const struct sms
*incoming)
g_slist_free(l);
}
+static void handle_sms_status_report(struct ofono_sms *sms,
+ const struct sms *incoming)
+{
+ unsigned int msg_id;
+ struct ofono_modem *modem = __ofono_atom_get_modem(sms->atom);
+
+ if (incoming->status_report.st == SMS_ST_COMPLETED_RECEIVED) {
+
+ if (status_report_assembly_report(sms->sr_assembly,
+ incoming, &msg_id) == OFONO_HISTORY_SMS_STATUS_DELIVERED)
+ __ofono_history_sms_send_status(modem, msg_id,
+ time(NULL), OFONO_HISTORY_SMS_STATUS_DELIVERED);
+ } else
+ __ofono_history_sms_send_status(modem, msg_id, time(NULL),
+ OFONO_HISTORY_SMS_STATUS_DELIVER_FAILED);
+}
+
+
static inline gboolean handle_mwi(struct ofono_sms *sms, struct sms *s)
{
gboolean discard;
@@ -823,7 +854,25 @@ out:
void ofono_sms_status_notify(struct ofono_sms *sms, unsigned char *pdu,
int len, int tpdu_len)
{
- ofono_error("SMS Status-Report not yet handled");
+ struct sms s;
+ enum sms_class cls;
+
+ if (!sms_decode(pdu, len, FALSE, tpdu_len, &s)) {
+ ofono_error("Unable to decode PDU");
+ return;
+ }
+
+ if (s.type != SMS_TYPE_STATUS_REPORT) {
+ ofono_error("Expecting a STATUS REPORT pdu");
+ return;
+ }
+
+ if (!sms_dcs_decode(s.deliver.dcs, &cls, NULL, NULL, NULL)) {
+ ofono_error("Unknown / Reserved DCS. Ignoring");
+ return;
+ }
+
+ handle_sms_status_report(sms, &s);
}
int ofono_sms_driver_register(const struct ofono_sms_driver *d)
@@ -903,6 +952,11 @@ static void sms_remove(struct ofono_atom *atom)
sms->settings = NULL;
}
+ if (sms->sr_assembly) {
+ status_report_assembly_free(sms->sr_assembly);
+ sms->sr_assembly = NULL;
+ }
+
g_free(sms);
}
@@ -1038,9 +1092,12 @@ void ofono_sms_register(struct ofono_sms *sms)
imsi = ofono_sim_get_imsi(sms->sim);
sms->assembly = sms_assembly_new(imsi);
+ sms->sr_assembly = status_report_assembly_new(imsi);
+
sms_load_settings(sms, imsi);
} else {
sms->assembly = sms_assembly_new(NULL);
+ sms->sr_assembly = status_report_assembly_new(NULL);
}
__ofono_atom_register(sms->atom, sms_unregister);
--
1.6.0.4
9:50 a.m.
Hi Pasi
On Wed, 2010-06-09 at 17:01 +0300, Pasi Miettinen wrote:
Hello again,
this is the new version of SMS status report. It is implemented
according to the latest comments which you gave about my previous
patches. These patches include concatenated status report
assembling and status report result printing via history plugin.
Next I am going to take a new look at the saving of the pending
status reports to imsi file.
I found out right before going offline for last week that you were
working on status reports -- good to know I am not alone.
I'd like to coordinate with your work. I had started tackling the issue
from the infrastructure of the whole SMS code and had recently started
work on the actual status-report part. Once your code started showing
up, I've ditched part of my code and rebased on top of what you had.
Could you please take a look at the smsdbus branch in my repo and tell
me if there is anything there that will conflict with the way you are
implementing things?
git://gitorious.org/~inakypg/ofono/ofono-inakypg.git
there is a set of patches there which lay out changes in the SMS code
infrastructure to do the following:
- export of SMS messages to D-Bus
- each message is an object in the bus
- messages can be canceled, properties queried, etc
- in transit SMS messages get an state machine describing at which phase
they are. D-Bus signals are sent in transitions
- messages waiting for an status report are queued in a separate queue
- a unique ID is generated based on message contents and destination
nunmber to reduce the chances of collision -- need still to apply it to
received messages so it's easier to gather status reports from different
fragments
- provide an internal C API for managing messages (D-Bus's maps on top
of this)
FIXMEs/TODOs/RFCs:
still need to cleanup some more to adapt to your code
plenty of whatifs/FIXMEs litered around the code still
remove the 'request-status-report' from D-Bus::SendMessage
message id needs more uniquedness fed into it
Note the branch depends on some more commits that are still not in
mainline; those are in my master branch and pending a resubmit to
upstream.
Thanks!
6:30 p.m.
New subject: VS: SMS status report
Hello Inaky
Hi Pasi
|
there is a set of patches there which lay out changes in the SMS code
infrastructure to do the following:
- export of SMS messages to D-Bus
- each message is an object in the bus
- messages can be canceled, properties queried, etc
If a message that waits for a status report is canceled, status_report_assembly
data structure must be updated (msg_id and mr-number removed).
- in transit SMS messages get an state machine describing at which phase
they are. D-Bus signals are sent in transitions
This could become topical when we decide how to react to different status
report results. Currently it is just "delivered"/"undelivered".
- messages waiting for an status report are queued in a separate queue
I need to know the the lifespan of these messages that are waiting for status
reports. When a message that requests for status report is created, it must
be added to the status_report_assembly data structure. And when the
status report arrives, the data structure must be updated accordingly. I
have implemented functions for these actions and they are going to be
in smsutil file.
- a unique ID is generated based on message contents and destination
nunmber to reduce the chances of collision -- need still to apply it to
received messages so it's easier to gather status reports from different
fragments
I think this is one place where we need to keep track of what we are doing,
because at least for now I am using this msg_id as a key in a hashmap
which I use to keep track of pending status reports.
- provide an internal C API for managing messages (D-Bus's maps on top
of this)
FIXMEs/TODOs/RFCs:
still need to cleanup some more to adapt to your code
You can also suggest to me if you would like me to change something in my
code. Mainly I have been going under Denis's guidance to get up to speed
here in my start for the project.
plenty of whatifs/FIXMEs litered around the code still
remove the 'request-status-report' from D-Bus::SendMessage
This is currently done via SetProperty. ofono_sms has use_delivery_reports
boolean variable.
message id needs more uniquedness fed into it
|
Thanks!
Thank you :)
-Pasi
9:45 a.m.
New subject: VS: SMS status report
On Thu, 2010-06-10 at 11:30 +0300, Miettinen Pasi wrote:
Hello Inaky
> Hi Pasi
|
>
> there is a set of patches there which lay out changes in the SMS code
> infrastructure to do the following:
>
> - export of SMS messages to D-Bus
>
> - each message is an object in the bus
>
> - messages can be canceled, properties queried, etc
If a message that waits for a status report is canceled, status_report_assembly
data structure must be updated (msg_id and mr-number removed).
so I guess here we'd have to unify whichever infrastructure is there to
keep track of pending guys. The way I had it laid out was an extra queue
in the smsmanager object (sms->tx_wfaq) where those were kept, and then
the msg_id/ref was contained in there. That's going to be kind of like
the same like what you have, I have no personal preference on which one
to use.
>
> - in transit SMS messages get an state machine describing at which phase
> they are. D-Bus signals are sent in transitions
This could become topical when we decide how to react to different status
report results. Currently it is just "delivered"/"undelivered".
This one Denis and me discussed long and hard, as we wanted to move the
hole SMS interface to be fully asynchronous (you send the message, get
an object name back and then track it by signals reporting status
changes). That whole code is also implemented in the commit set; I am
decently satisfied with how it was working, I think it fits the bill. I
am just thinking of adding a last state (destroyed) so listeners can
tell when a message disappears from oFono's knowledge (it gets delivered
or fails completely or whatever).
>
> - messages waiting for an status report are queued in a separate queue
I need to know the the lifespan of these messages that are waiting for status
reports. When a message that requests for status report is created, it must
be added to the status_report_assembly data structure. And when the
status report arrives, the data structure must be updated accordingly. I
have implemented functions for these actions and they are going to be
in smsutil file.
Ok.
>
> - a unique ID is generated based on message contents and destination
> nunmber to reduce the chances of collision -- need still to apply it to
> received messages so it's easier to gather status reports from different
> fragments
I think this is one place where we need to keep track of what we are doing,
because at least for now I am using this msg_id as a key in a hashmap
which I use to keep track of pending status reports.
Yep, so I built some infrastructure for it to build a hash just piping
data to it -- those commits are kind of independent. I need to clean up
further my commits to fully isolate how it gets inserted into the actual
code, but at the end the idea is that the 16-bit message id gets
generated from this hash instead of having a monotonic increasing one.
This is for the TX side; for the RX side I still hadn't started thinking
about it and probably you went further than me.
> - provide an internal C API for managing messages (D-Bus's
maps on top
> of this)
>
>
> FIXMEs/TODOs/RFCs:
>
> still need to cleanup some more to adapt to your code
You can also suggest to me if you would like me to change something in my
code. Mainly I have been going under Denis's guidance to get up to speed
here in my start for the project.
Likewise -- in any case, most of your stuff in the SRR side seems way
more advanced than were I was. Most of my progress was in reorganizing
the SMS & D-BUS code, the status machine and propagation. I had to
wiggle to force things into the changes you had done but that is mostly
done already.
So I guess the only collision part is now how to keep track of the
messages that are pending acknowledgement. Take your pick, after all
your code in that are is more advanced. Just please take a look at the
commits:
SMS-introduce-wait-for-ack-state-and-infrastructure
this is what creates the queue I was planning on using. Other commits do
the msg_id thingie, but I still need to cleanup the commits more so it
looks cleaner -- right now it is spread around a few commits.
[I just repushed a rebase of my smsdbus branch with more cleanups, so
you might want to force pull if you want to look at the commits]
>
> plenty of whatifs/FIXMEs litered around the code still
>
> remove the 'request-status-report' from D-Bus::SendMessage
This is currently done via SetProperty. ofono_sms has use_delivery_reports
boolean variable.
Yes, this is something I still need to clean up, so feel free to ignore
it.
4482
days inactive
4484
days old
6 comments
3 participants
participants (3)
-
Inaky Perez-Gonzalez -
Miettinen Pasi -
Pasi Miettinen