All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/2] Added SQLite history plugin
@ 2010-04-06 10:43 Dario
  2010-04-06 17:56 ` Denis Kenzior
  0 siblings, 1 reply; 23+ messages in thread
From: Dario @ 2010-04-06 10:43 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 11515 bytes --]

---
 plugins/oFono_History_DB.sql |   27 ++++
 plugins/sqlite_history.c     |  301 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 328 insertions(+), 0 deletions(-)
 create mode 100644 plugins/oFono_History_DB.sql
 create mode 100644 plugins/sqlite_history.c

diff --git a/plugins/oFono_History_DB.sql b/plugins/oFono_History_DB.sql
new file mode 100644
index 0000000..7cfc3ba
--- /dev/null
+++ b/plugins/oFono_History_DB.sql
@@ -0,0 +1,27 @@
+CREATE TABLE IF NOT EXISTS "ofono_history_calls" (
+    "ohc_modem_path" TEXT, -- Modem path string i.e. "/modem0"
+    "ohc_type" INTEGER, -- Call type ( 0 = Call ended, 1 = Call missed )
+    "ohc_direction" INTEGER, -- Call direction ( 0 = Mobile Originated, 1 = Mobile Terminated )
+    "ohc_phone_number" TEXT, -- Other party phone number
+    "ohc_start_time" TEXT, -- Starting date/time
+    "ohc_end_time" TEXT -- Ending date/time
+);
+CREATE TABLE IF NOT EXISTS "ofono_history_incoming_messages" (
+    "ohim_modem_path" TEXT, -- Modem path string i.e. "/modem0"
+    "ohim_msg_id" INTEGER, -- oFono unique message id number
+    "ohim_sender" TEXT, -- Sender phone number
+    "ohim_text" TEXT, -- Message text
+    "ohim_local_date" TEXT, -- Local sent date/time
+    "ohim_remote_date" TEXT -- Remote sent date/time
+);
+CREATE TABLE IF NOT EXISTS "ofono_history_outgoing_messages" (
+    "ohom_modem_path" TEXT, -- Modem path string i.e. "/modem0"
+    "ohom_msg_id" INTEGER, -- oFono unique message id number
+    "ohom_recipient" TEXT, -- Recipient phone number
+    "ohom_text" TEXT, -- Message text
+    "ohom_creation_date" TEXT, -- Message creation date/time
+    "ohom_send_status" INTEGER, -- Sending status ( 0 = Pending, 1 = Submitted, 2 = Failed )
+    "ohom_status_update_date" TEXT -- Last row update date/time
+);
+CREATE UNIQUE INDEX IF NOT EXISTS "ohim_idx_msg_id" on ofono_history_incoming_messages (ohim_msg_id ASC);
+CREATE UNIQUE INDEX IF NOT EXISTS "ohom_idx_msg_id" on ofono_history_outgoing_messages (ohom_msg_id ASC);
diff --git a/plugins/sqlite_history.c b/plugins/sqlite_history.c
new file mode 100644
index 0000000..35dce71
--- /dev/null
+++ b/plugins/sqlite_history.c
@@ -0,0 +1,301 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2010 Dario 'Djdas' Conigliaro.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdlib.h>
+#include <string.h>
+#include <glib.h>
+
+#define OFONO_API_SUBJECT_TO_CHANGE
+#include <ofono/plugin.h>
+#include <ofono/log.h>
+#include <ofono/history.h>
+#include <ofono/types.h>
+#include <ofono/modem.h>
+
+#include "common.h"
+
+#include <sqlite3.h>
+
+#define SQL_HISTORY_DB_PATH STORAGEDIR "/ofono_history.sqlite"
+#define SQL_HISTORY_DB_SQL STORAGEDIR "/oFono_History_DB.sql"
+
+#define SELECT_CALLS "SELECT * FROM ofono_history_calls"
+#define INSERT_CALLS "INSERT INTO ofono_history_calls VALUES (?,?,?,?,?,?)"
+#define INSERT_IN_MSGS "INSERT INTO ofono_history_incoming_messages VALUES (?,?,?,?,?,?)"
+#define INSERT_OUT_MSGS "INSERT INTO ofono_history_outgoing_messages VALUES (?,?,?,?,?,?,?)"
+#define UPDATE_OUT_MSGS "UPDATE ofono_history_outgoing_messages SET \
+							ohom_send_status = ?, \
+							ohom_status_update_date = ? \
+							WHERE ohom_msg_id = ?"
+
+sqlite3 *db = NULL;
+
+static int sqlite_history_probe(struct ofono_history_context *context)
+{
+	char *execerror;
+
+	ofono_debug("SQLite History Probe for modem: %p", context->modem);
+	
+	if (sqlite3_open(SQL_HISTORY_DB_PATH, &db) != SQLITE_OK) {
+		ofono_debug("Error opening DB: %s", sqlite3_errmsg(db));
+		sqlite3_close(db);
+		return -1;
+	}
+
+	if (sqlite3_exec(db, SELECT_CALLS, NULL, NULL, &execerror) != SQLITE_OK) {
+		char *sqlscript;
+		GError *sqlerror = NULL;
+
+		ofono_debug("Creating DB");
+
+		g_file_get_contents(SQL_HISTORY_DB_SQL, &sqlscript, NULL, &sqlerror);
+	
+		if (sqlerror != NULL) {
+			ofono_debug("Error opening sql script: %s", sqlerror->message);
+			g_error_free(sqlerror);
+			return -1;
+		}
+	
+		if (sqlite3_exec(db, sqlscript, NULL, NULL, &execerror) != SQLITE_OK) {
+			ofono_debug("Error executing sql script: %s", execerror);
+			sqlite3_free(execerror);
+			g_free(sqlscript);
+			return -1;
+		}
+	
+		g_free(sqlscript);
+	}
+	
+	return 0;
+}
+
+static void sqlite_history_remove(struct ofono_history_context *context)
+{
+	ofono_debug("SQLite History Remove for modem: %p", context->modem);
+	
+	if (db != NULL)
+		sqlite3_close(db);
+}
+
+static void sqlite_history_call_ended(struct ofono_history_context *context,
+						const struct ofono_call *call,
+						time_t start, time_t end)
+{
+	const char *from = "Unknown";
+	char buf[128];
+	char buf1[128];
+	sqlite3_stmt *statement;
+
+	ofono_debug("Call Ended on modem: %p", context->modem);
+
+	if (call->type != 0)
+		return;
+
+	if (db == NULL)
+		return;
+
+	from = phone_number_to_string(&call->phone_number);
+	
+	strftime(buf, 127, "%Y-%m-%dT%H:%M:%S%z", localtime(&start));
+	buf[127] = '\0';
+	
+	sqlite3_prepare_v2(db, INSERT_CALLS, -1, &statement, NULL);
+	sqlite3_bind_text(statement, 1, ofono_modem_get_path(context->modem), 
+							-1, SQLITE_STATIC);
+	sqlite3_bind_int(statement, 2, 0);
+	sqlite3_bind_int(statement, 3, call->direction);
+	sqlite3_bind_text(statement, 4, from, -1, SQLITE_STATIC);
+	sqlite3_bind_text(statement, 5, buf, -1, SQLITE_STATIC);
+	
+	strftime(buf1, 127, "%Y-%m-%dT%H:%M:%S%z", localtime(&end));
+	buf1[127] = '\0';
+	
+	sqlite3_bind_text(statement, 6, buf1, -1, SQLITE_STATIC);
+
+	if (sqlite3_step(statement) != SQLITE_DONE)
+		ofono_debug("SQLite Error: %s", sqlite3_errmsg(db));
+
+	sqlite3_finalize(statement);
+}
+
+static void sqlite_history_call_missed(struct ofono_history_context *context,
+						const struct ofono_call *call,
+						time_t when)
+{
+	const char *from = "Unknown";
+	char buf[128];
+	sqlite3_stmt *statement;
+
+	ofono_debug("Call Missed on modem: %p", context->modem);
+
+	if (call->type != 0)
+		return;
+
+	if (db == NULL)
+		return;
+
+	from = phone_number_to_string(&call->phone_number);
+	
+	strftime(buf, 127, "%Y-%m-%dT%H:%M:%S%z", localtime(&when));
+	buf[127] = '\0';
+	
+	sqlite3_prepare_v2(db, INSERT_CALLS, -1, &statement, NULL);
+	sqlite3_bind_text(statement, 1, ofono_modem_get_path(context->modem), 
+							-1, SQLITE_STATIC);
+	sqlite3_bind_int(statement, 2, 1);
+	sqlite3_bind_int(statement, 3, call->direction);
+	sqlite3_bind_text(statement, 4, from, -1, SQLITE_STATIC);
+	sqlite3_bind_text(statement, 5, buf, -1, SQLITE_STATIC);
+	sqlite3_bind_text(statement, 6, buf, -1, SQLITE_STATIC);
+
+	if (sqlite3_step(statement) != SQLITE_DONE)
+		ofono_debug("SQLite Error: %s", sqlite3_errmsg(db));
+
+	sqlite3_finalize(statement);
+}
+
+static void sqlite_history_sms_received(struct ofono_history_context *context,
+					unsigned int msg_id, const char *from,
+					const struct tm *remote, const struct tm *local,
+					const char *text)
+{
+	char buf[128];
+	char buf1[128];
+	sqlite3_stmt *statement;
+
+	ofono_debug("Incoming SMS on modem: %p", context->modem);
+
+	if (db == NULL)
+		return;
+
+	strftime(buf, 127, "%Y-%m-%dT%H:%M:%S%z", local);
+	buf[127] = '\0';
+	
+	strftime(buf1, 127, "%Y-%m-%dT%H:%M:%S%z", remote);
+	buf1[127] = '\0';
+	
+	sqlite3_prepare_v2(db, INSERT_IN_MSGS, -1, &statement, NULL);
+	sqlite3_bind_text(statement, 1, ofono_modem_get_path(context->modem), 
+							-1, SQLITE_STATIC);
+	sqlite3_bind_int(statement, 2, msg_id);
+	sqlite3_bind_text(statement, 3, from, -1, SQLITE_STATIC);
+	sqlite3_bind_text(statement, 4, text, -1, SQLITE_STATIC);
+	sqlite3_bind_text(statement, 5, buf, -1, SQLITE_STATIC);
+	sqlite3_bind_text(statement, 6, buf1, -1, SQLITE_STATIC);
+
+	if (sqlite3_step(statement) != SQLITE_DONE)
+		ofono_debug("SQLite Error: %s", sqlite3_errmsg(db));
+
+	sqlite3_finalize(statement);
+}
+
+static void sqlite_history_sms_send_pending(struct ofono_history_context *context, 
+							unsigned int msg_id,
+							const char *to, time_t when,
+							const char *text)
+{
+	char buf[128];
+	char buf1[128];
+	sqlite3_stmt *statement;
+	time_t currtime;
+
+	ofono_debug("Sending SMS on modem: %p", context->modem);
+
+	if (db == NULL)
+		return;
+
+	strftime(buf, 127, "%Y-%m-%dT%H:%M:%S%z", localtime(&when));
+	buf[127] = '\0';
+	
+	currtime = time(NULL);
+	strftime(buf1, 127, "%Y-%m-%dT%H:%M:%S%z", localtime(&currtime));
+	buf1[127] = '\0';
+	
+	sqlite3_prepare_v2(db, INSERT_OUT_MSGS, -1, &statement, NULL);
+	sqlite3_bind_text(statement, 1, ofono_modem_get_path(context->modem), 
+							-1, SQLITE_STATIC);
+	sqlite3_bind_int(statement, 2, msg_id);
+	sqlite3_bind_text(statement, 3, to, -1, SQLITE_STATIC);
+	sqlite3_bind_text(statement, 4, text, -1, SQLITE_STATIC);
+	sqlite3_bind_text(statement, 5, buf, -1, SQLITE_STATIC);
+	sqlite3_bind_int(statement, 6, OFONO_HISTORY_SMS_STATUS_PENDING);
+	sqlite3_bind_text(statement, 7, buf1, -1, SQLITE_STATIC);
+
+	if (sqlite3_step(statement) != SQLITE_DONE)
+		ofono_debug("SQLite Error: %s", sqlite3_errmsg(db));
+
+	sqlite3_finalize(statement);
+}
+
+static void sqlite_history_sms_send_status(struct ofono_history_context *context, 
+						unsigned int msg_id, time_t when,
+						enum ofono_history_sms_status s)
+{
+	char buf[128];
+	sqlite3_stmt *statement;
+
+	ofono_debug("SMS status on modem: %p", context->modem);
+
+	if (db == NULL)
+		return;
+
+	strftime(buf, 127, "%Y-%m-%dT%H:%M:%S%z", localtime(&when));
+	buf[127] = '\0';
+	
+	sqlite3_prepare_v2(db, UPDATE_OUT_MSGS, -1, &statement, NULL);
+	sqlite3_bind_int(statement, 1, s);
+	sqlite3_bind_text(statement, 2, buf, -1, SQLITE_STATIC);
+	sqlite3_bind_int(statement, 3, msg_id);
+
+	if (sqlite3_step(statement) != SQLITE_DONE)
+		ofono_debug("SQLite Error: %s", sqlite3_errmsg(db));
+
+	sqlite3_finalize(statement);
+}
+
+static struct ofono_history_driver sqlite_driver = {
+	.name = "SQLite History",
+	.probe = sqlite_history_probe,
+	.remove = sqlite_history_remove,
+	.call_ended = sqlite_history_call_ended,
+	.call_missed = sqlite_history_call_missed,
+	.sms_received = sqlite_history_sms_received,
+	.sms_send_pending = sqlite_history_sms_send_pending,
+	.sms_send_status = sqlite_history_sms_send_status,
+};
+
+static int sqlite_history_init(void)
+{
+	return ofono_history_driver_register(&sqlite_driver);
+}
+
+static void sqlite_history_exit(void)
+{
+	ofono_history_driver_unregister(&sqlite_driver);
+}
+
+OFONO_PLUGIN_DEFINE(sqlite_history, "SQLite History Plugin",
+			VERSION, OFONO_PLUGIN_PRIORITY_DEFAULT,
+			sqlite_history_init, sqlite_history_exit)
-- 
1.6.3.3



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] Added SQLite history plugin
  2010-04-06 10:43 [PATCH 2/2] Added SQLite history plugin Dario
@ 2010-04-06 17:56 ` Denis Kenzior
  2010-04-07  8:50   ` Dario
  2010-04-07 11:47   ` Nicola Mfb
  0 siblings, 2 replies; 23+ messages in thread
From: Denis Kenzior @ 2010-04-06 17:56 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 2549 bytes --]

Hi Dario,

> +#define SQL_HISTORY_DB_PATH STORAGEDIR "/ofono_history.sqlite"
> +#define SQL_HISTORY_DB_SQL STORAGEDIR "/oFono_History_DB.sql"

So I have my concerns about storing this in /.  This should be in a proper 
directory, like /var/lib/ofono or something like that.

> +static int sqlite_history_probe(struct ofono_history_context *context)
> +{
> +	char *execerror;
> +
> +	ofono_debug("SQLite History Probe for modem: %p", context->modem);
> +
> +	if (sqlite3_open(SQL_HISTORY_DB_PATH, &db) != SQLITE_OK) {
> +		ofono_debug("Error opening DB: %s", sqlite3_errmsg(db));

Please use DBG macro instead of ofono_debug.

> +		sqlite3_close(db);
> +		return -1;
> +	}
> +
> +	if (sqlite3_exec(db, SELECT_CALLS, NULL, NULL, &execerror) != SQLITE_OK)
>  { +		char *sqlscript;

Should be

if (sqlite3_exec == SQLITE_OK)
	return 0;

Then the rest of the if statement follows unnested.
> +		GError *sqlerror = NULL;
> +
> +		ofono_debug("Creating DB");
> +
> +		g_file_get_contents(SQL_HISTORY_DB_SQL, &sqlscript, NULL, &sqlerror);
> +
> +		if (sqlerror != NULL) {
> +			ofono_debug("Error opening sql script: %s", sqlerror->message);
> +			g_error_free(sqlerror);
> +			return -1;
> +		}
> +
> +		if (sqlite3_exec(db, sqlscript, NULL, NULL, &execerror) != SQLITE_OK) {
> +			ofono_debug("Error executing sql script: %s", execerror);
> +			sqlite3_free(execerror);
> +			g_free(sqlscript);
> +			return -1;
> +		}
> +
> +		g_free(sqlscript);
> +	}
> +
> +	return 0;
> +}
> +
> +static void sqlite_history_remove(struct ofono_history_context *context)
> +{
> +	ofono_debug("SQLite History Remove for modem: %p", context->modem);
> +
> +	if (db != NULL)
> +		sqlite3_close(db);
> +}
> +

So remember that the history plugin is instantiated for each and every modem, 
so you will be sqlite_open/sqlite_closing the db in each instance.  Not really 
sure if this is what you want, or whether you want a reference-counted 
database connection here.

Another thing to consider is that you might want to store the IMSI of the 
modem along with the history information for every call / sms event.  That way 
when a SIM card is changed, the user can be shown a different set of call / sms  
history.

Finally, you might want to set limits on the number of entries in the 
database, and expire them as the limit is reached.   Otherwise you'd need to 
expire them periodically from e.g. cron, but would need to release control of 
the database during that time.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] Added SQLite history plugin
  2010-04-06 17:56 ` Denis Kenzior
@ 2010-04-07  8:50   ` Dario
  2010-04-07 17:01     ` Denis Kenzior
  2010-04-07 11:47   ` Nicola Mfb
  1 sibling, 1 reply; 23+ messages in thread
From: Dario @ 2010-04-07  8:50 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 3837 bytes --]

Hi Denis,
> Hi Dario,
>
>   
>> +#define SQL_HISTORY_DB_PATH STORAGEDIR "/ofono_history.sqlite"
>> +#define SQL_HISTORY_DB_SQL STORAGEDIR "/oFono_History_DB.sql"
>>     
>
> So I have my concerns about storing this in /.  This should be in a proper 
> directory, like /var/lib/ofono or something like that.
>
>   
STORAGEDIR is a macro expanded by Autoconf to ${storagedir} (please see 
rows 172-178 of configure.ac) which is normally /var/lib/ofono so the DB 
and the scripts are already stored inside that dir.
> So remember that the history plugin is instantiated for each and every modem, 
> so you will be sqlite_open/sqlite_closing the db in each instance.  Not really 
> sure if this is what you want, or whether you want a reference-counted 
> database connection here.
>   
Database connection is handled by sqlite3_* API in a transparent manner 
so you could have different open connections at the same time without 
any problem, each SQL statement is transactional so the DB is every time 
consistent, but if you prefer I can rethink of it (maybe a DB for every 
modem could be a possibility but this would overload the plugin because 
we have to open and close the DB in every callback given that we have to 
filter the files access by modem).
> Another thing to consider is that you might want to store the IMSI of the 
> modem along with the history information for every call / sms event.  That way 
> when a SIM card is changed, the user can be shown a different set of call / sms  
> history.
>
>   
Thank you for this suggestion, didn't think about this :) I have one 
question: what's the best way to get that info?
I see only one function to get the imsi information: const char 
*ofono_sim_get_imsi(struct ofono_sim *sim) which needs a struct 
ofono_sim pointer, but I don't have it in history as it's a member of 
the struct ofono_modem which is blind for me. Can you please point me at 
the function to achieve this?
> Finally, you might want to set limits on the number of entries in the 
> database, and expire them as the limit is reached.   Otherwise you'd need to 
> expire them periodically from e.g. cron, but would need to release control of 
> the database during that time.
>
>   
Well this is a more complex topic because needs to point at the purpose 
of the history plugins (in general); I thought at the plugin as a simple 
"event logger" in which an "event" could be a call or a SMS; the 
question is: must the plugin handle all the logic and possibly interface 
with the applications (for example using DBus methods/signals) or is it 
only a mere storage daemon letting applications handle the informations?
Talking about this with one of my team members (Nicola.mfb, which should 
explain better than me applications point of view in a next email) we 
arrived at the conclusion that the history plugin should be able to talk 
directly to the applications (tipically sending DBus signals and 
exporting methods) to mask the storage type of the history data.
Also in this case there could be some possible issues to handle: who is 
aware of the msg_id? Only the plugin or even the application? In the 
former case must the application provide to the plugin its own msg_id to 
recover SMS data?
Another question is: what if we had different history plugins at the 
same time, like an EDS one, a plain text one, a Berkley DB one and an 
SQLite one? There should be only one plugin active per ofonod instance 
or per modem? In case different plugins are active at the same time, the 
"history DBus interface" is global for all the plugins (i.e. a method to 
delete an incoming sms from modem /modem0 with msg_id = 123 must delete 
it on all backends?) or each plugin has its own?
Thank you very much in advance, best regards,
Dario

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] Added SQLite history plugin
  2010-04-06 17:56 ` Denis Kenzior
  2010-04-07  8:50   ` Dario
@ 2010-04-07 11:47   ` Nicola Mfb
  2010-04-07 17:28     ` Bastian, Waldo
  2010-04-07 19:28     ` Denis Kenzior
  1 sibling, 2 replies; 23+ messages in thread
From: Nicola Mfb @ 2010-04-07 11:47 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 2778 bytes --]

On Tue, Apr 6, 2010 at 7:56 PM, Denis Kenzior <denkenz@gmail.com> wrote:

[...]

Hi Denis,

Actually our apps listen for the IncomingMessage signal, and store the
SMS in an internal database.
At some point by design, a restart, a crash or other reasons, it may
happen that apps are down while ofono and the modem are up, in that
case we lose incoming messages.

To solve this we tought to use the history, so, for example, clients
at startup query the plugin, processes "lost" messages and further
continue listening for the IncomingMessage signal.

We have a problem here, as the IncomingMessage lacks for the sms id
property, we generate an internal id that is out of sync respect of
ther history id, so at the next app restart we cannot determine what
messages were "lost".

The second problem is that we may use several clients at the same
time, thinks for a phone app, and a desktop widget showing the number
of incoming/unread etc. messages.
The two clients needs to access the same informations in a consistent way.

For that reason, and due the necessity to abstract the db access with
a dbus api, we thought the natural way is to use the history plugin as
a "permanent storage" plugin too, improving the dbus interface to have
clients cooperating in a consistent manner, and avoiding duplicate
sms/calls data in different places.

The phone app may request to delete a "message", and a
"HistoryMessageDeleted" signal should be emitted, and so on.

Here we may have an HistoryIncomingMessage signal that provides the id
field too and fix the first issue (if you do not prefer to add the id
field in the IncomingMessage signal directly).

The plugin may be used to store further metadata providing signals to
inform of their changes, that my be useful in all clients (if a
message was read in the phone app, the desktop widget should take care
of that and decrease the showed number of new messages).

Finally apps may decide with the right dbus calls if sms listing has
to be filtered by sim or not, or when and how expire the history.

But thats impacts a lot on the general perspective of the history
plugin usage and further developing.

Infact if a complete dbus api is going to be defined, it would be nice
to share it for all history plugins, adding for example an
"org.ofono.HistoryManager" interface to the modem, while if the
history plugin has to be used in a different manner, or ofono does not
care of that by design, we have to implement the dbus interface in the
sqlite plugin directly, forcing our apps to not be generally oFono
compliants (at least when not using the sqlite plugin).

So we are asking to you what's are your plans and how may we proceed.

Thanks for your support!

    Niko

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] Added SQLite history plugin
  2010-04-07  8:50   ` Dario
@ 2010-04-07 17:01     ` Denis Kenzior
  2010-04-07 17:54       ` Nicola Mfb
  0 siblings, 1 reply; 23+ messages in thread
From: Denis Kenzior @ 2010-04-07 17:01 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 5552 bytes --]

Hi Dario,

> >> +#define SQL_HISTORY_DB_PATH STORAGEDIR "/ofono_history.sqlite"
> >> +#define SQL_HISTORY_DB_SQL STORAGEDIR "/oFono_History_DB.sql"
> >
> > So I have my concerns about storing this in /.  This should be in a
> > proper directory, like /var/lib/ofono or something like that.
> 
> STORAGEDIR is a macro expanded by Autoconf to ${storagedir} (please see
> rows 172-178 of configure.ac) which is normally /var/lib/ofono so the DB
> and the scripts are already stored inside that dir.

Whooops, I missed the space between PATH/SQL and STORAGEDIR.  Nevermind what I 
said ;)

> 
> > So remember that the history plugin is instantiated for each and every
> > modem, so you will be sqlite_open/sqlite_closing the db in each instance.
> >  Not really sure if this is what you want, or whether you want a
> > reference-counted database connection here.
> 
> Database connection is handled by sqlite3_* API in a transparent manner
> so you could have different open connections at the same time without
> any problem, each SQL statement is transactional so the DB is every time
> consistent, but if you prefer I can rethink of it (maybe a DB for every
> modem could be a possibility but this would overload the plugin because
> we have to open and close the DB in every callback given that we have to
> filter the files access by modem).

My concern here was that multiple open connections to the same database might 
require locking or other overhead that could be avoided with a single shared 
connection.  It might be there is no overhead and your original approach is 
fine.

> 
> > Another thing to consider is that you might want to store the IMSI of the
> > modem along with the history information for every call / sms event. 
> > That way when a SIM card is changed, the user can be shown a different
> > set of call / sms history.
> 
> Thank you for this suggestion, didn't think about this :) I have one
> question: what's the best way to get that info?
> I see only one function to get the imsi information: const char
> *ofono_sim_get_imsi(struct ofono_sim *sim) which needs a struct
> ofono_sim pointer, but I don't have it in history as it's a member of
> the struct ofono_modem which is blind for me. Can you please point me at
> the function to achieve this?

For now you'd have to use the private ofono atom API which is accessible to 
you since you're part of the tree:
sim_atom = __ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_SIM);
sim = __ofono_atom_get_data(sim);
imsi = ofono_sim_get_imsi(sim);

> 
> > Finally, you might want to set limits on the number of entries in the
> > database, and expire them as the limit is reached.   Otherwise you'd need
> > to expire them periodically from e.g. cron, but would need to release
> > control of the database during that time.
> 
> Well this is a more complex topic because needs to point at the purpose
> of the history plugins (in general); I thought at the plugin as a simple
> "event logger" in which an "event" could be a call or a SMS; the
> question is: must the plugin handle all the logic and possibly interface
> with the applications (for example using DBus methods/signals) or is it
> only a mere storage daemon letting applications handle the informations?

That is purely an implementation detail and completely up to the implementer.  
The choice of tools can affect this decision though; for instance I remember 
SQLite had issues with multiple applications having simultaneous write access 
to the database.  Maybe this has been solved and my information is outdated.

> Talking about this with one of my team members (Nicola.mfb, which should
> explain better than me applications point of view in a next email) we
> arrived at the conclusion that the history plugin should be able to talk
> directly to the applications (tipically sending DBus signals and
> exporting methods) to mask the storage type of the history data.
> Also in this case there could be some possible issues to handle: who is
> aware of the msg_id? Only the plugin or even the application? In the
> former case must the application provide to the plugin its own msg_id to
> recover SMS data?

We're working on improving the SMS capabilities, and might eventually expose 
the message id.  However, the message id really has no meaning, so this is not 
something we really want to expose.  I'd like to keep it private between the 
history plugin and the core.  Are you sure you really require it to be 
exposed, or can you do without it?

> Another question is: what if we had different history plugins at the
> same time, like an EDS one, a plain text one, a Berkley DB one and an
> SQLite one? There should be only one plugin active per ofonod instance
> or per modem? In case different plugins are active at the same time, the
> "history DBus interface" is global for all the plugins (i.e. a method to
> delete an incoming sms from modem /modem0 with msg_id = 123 must delete
> it on all backends?) or each plugin has its own?

Really there should be no limitations here and there are cases when several 
plugins might be logging the same information.  For instance, certain SIMs 
provide area to log call history information onto the SIM (as opposed to 
storing it in the ME/filesystem).  For those devices you might want to write a 
plugin that writes to the SIM in addition to writing to EDS/SQLite/etc.

> Thank you very much in advance, best regards,
> Dario

Regards,
-Denis

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [PATCH 2/2] Added SQLite history plugin
  2010-04-07 11:47   ` Nicola Mfb
@ 2010-04-07 17:28     ` Bastian, Waldo
  2010-04-07 17:33       ` Denis Kenzior
  2010-04-07 19:28     ` Denis Kenzior
  1 sibling, 1 reply; 23+ messages in thread
From: Bastian, Waldo @ 2010-04-07 17:28 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 633 bytes --]

> Hi Denis,
> 
> Actually our apps listen for the IncomingMessage signal, and store the
> SMS in an internal database.
> At some point by design, a restart, a crash or other reasons, it may
> happen that apps are down while ofono and the modem are up, in that
> case we lose incoming messages.

A similar lack of robustness exists at the modem API side as well, if oFono crashes/runs out of battery after receiving the message from the modem driver the message is lost. There is no provision in the SMS modem API to signal back to the modem driver that the message has been successfully stored on the APE.

Cheers,
Waldo

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] Added SQLite history plugin
  2010-04-07 17:28     ` Bastian, Waldo
@ 2010-04-07 17:33       ` Denis Kenzior
  2010-04-07 17:43         ` Nicola Mfb
  2010-04-07 18:03         ` Bastian, Waldo
  0 siblings, 2 replies; 23+ messages in thread
From: Denis Kenzior @ 2010-04-07 17:33 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 815 bytes --]

Hi Waldo,

> > Hi Denis,
> >
> > Actually our apps listen for the IncomingMessage signal, and store the
> > SMS in an internal database.
> > At some point by design, a restart, a crash or other reasons, it may
> > happen that apps are down while ofono and the modem are up, in that
> > case we lose incoming messages.
> 
> A similar lack of robustness exists at the modem API side as well, if oFono
>  crashes/runs out of battery after receiving the message from the modem
>  driver the message is lost. There is no provision in the SMS modem API to
>  signal back to the modem driver that the message has been successfully
>  stored on the APE.

If oFono crashes/runs out of battery here the driver is gone along with oFono.  
Notifying it of anything is not going to help ;)

Regards,
-Denis

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] Added SQLite history plugin
  2010-04-07 17:33       ` Denis Kenzior
@ 2010-04-07 17:43         ` Nicola Mfb
  2010-04-07 17:55           ` Denis Kenzior
  2010-04-07 18:03         ` Bastian, Waldo
  1 sibling, 1 reply; 23+ messages in thread
From: Nicola Mfb @ 2010-04-07 17:43 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 867 bytes --]

On Wed, Apr 7, 2010 at 7:33 PM, Denis Kenzior <denkenz@gmail.com> wrote:
[...]
>> A similar lack of robustness exists at the modem API side as well, if oFono
>>  crashes/runs out of battery after receiving the message from the modem
>>  driver the message is lost. There is no provision in the SMS modem API to
>>  signal back to the modem driver that the message has been successfully
>>  stored on the APE.
>
> If oFono crashes/runs out of battery here the driver is gone along with oFono.
> Notifying it of anything is not going to help ;)

Yes, anyway I'm thinking about the case when the storage system is
full, or an other error occours while trying to store the message.
In this case the driver should notify the network with a NACK, while a
signal should notify the user that the storage is full or there is
another problem.

      Niko

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] Added SQLite history plugin
  2010-04-07 17:01     ` Denis Kenzior
@ 2010-04-07 17:54       ` Nicola Mfb
  2010-04-07 18:05         ` Denis Kenzior
  0 siblings, 1 reply; 23+ messages in thread
From: Nicola Mfb @ 2010-04-07 17:54 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 1588 bytes --]

On Wed, Apr 7, 2010 at 7:01 PM, Denis Kenzior <denkenz@gmail.com> wrote:
[...]
>> Database connection is handled by sqlite3_* API in a transparent manner
>> so you could have different open connections at the same time without
>> any problem, each SQL statement is transactional so the DB is every time
>> consistent, but if you prefer I can rethink of it (maybe a DB for every
>> modem could be a possibility but this would overload the plugin because
>> we have to open and close the DB in every callback given that we have to
>> filter the files access by modem).
>
> My concern here was that multiple open connections to the same database might
> require locking or other overhead that could be avoided with a single shared
> connection.  It might be there is no overhead and your original approach is
> fine.

What's about a dbfile for each modem? we'll have one connection per
database, and may avoid the "modem" column on all the tables, saving
storage space.

[...]

> We're working on improving the SMS capabilities, and might eventually expose
> the message id.  However, the message id really has no meaning, so this is not
> something we really want to expose.  I'd like to keep it private between the
> history plugin and the core.  Are you sure you really require it to be
> exposed, or can you do without it?

We prefer to have the id to easily sync with the history plugin, as
said in previous mail, if you prefer to not expose the id on
IncomingMessage, we may emit an HistoryIncomingMessage signal with the
id.

Regards

     Niko

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] Added SQLite history plugin
  2010-04-07 17:43         ` Nicola Mfb
@ 2010-04-07 17:55           ` Denis Kenzior
  2010-04-07 18:09             ` Nicola Mfb
  0 siblings, 1 reply; 23+ messages in thread
From: Denis Kenzior @ 2010-04-07 17:55 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 1256 bytes --]

Hi Niko,

> On Wed, Apr 7, 2010 at 7:33 PM, Denis Kenzior <denkenz@gmail.com> wrote:
> [...]
> 
> >> A similar lack of robustness exists at the modem API side as well, if
> >> oFono crashes/runs out of battery after receiving the message from the
> >> modem driver the message is lost. There is no provision in the SMS modem
> >> API to signal back to the modem driver that the message has been
> >> successfully stored on the APE.
> >
> > If oFono crashes/runs out of battery here the driver is gone along with
> > oFono. Notifying it of anything is not going to help ;)
> 
> Yes, anyway I'm thinking about the case when the storage system is
> full, or an other error occours while trying to store the message.
> In this case the driver should notify the network with a NACK, while a
> signal should notify the user that the storage is full or there is
> another problem.

If your storage is full then you have other problems.  Adding nack/ack 
capability would increate the complexity of the driver/core about 10 fold, so 
this is not something we're willing to consider right now.  If this is an 
issue, then the user should be notified well in advance to rectify the 
situation or face possible data loss.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [PATCH 2/2] Added SQLite history plugin
  2010-04-07 17:33       ` Denis Kenzior
  2010-04-07 17:43         ` Nicola Mfb
@ 2010-04-07 18:03         ` Bastian, Waldo
  2010-04-07 18:27           ` Denis Kenzior
  1 sibling, 1 reply; 23+ messages in thread
From: Bastian, Waldo @ 2010-04-07 18:03 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 1709 bytes --]

> Hi Waldo,
> 
> > > Hi Denis,
> > >
> > > Actually our apps listen for the IncomingMessage signal, and store the
> > > SMS in an internal database.
> > > At some point by design, a restart, a crash or other reasons, it may
> > > happen that apps are down while ofono and the modem are up, in that
> > > case we lose incoming messages.
> >
> > A similar lack of robustness exists at the modem API side as well, if
> oFono
> >  crashes/runs out of battery after receiving the message from the modem
> >  driver the message is lost. There is no provision in the SMS modem API
> to
> >  signal back to the modem driver that the message has been successfully
> >  stored on the APE.
> 
> If oFono crashes/runs out of battery here the driver is gone along with
> oFono.
> Notifying it of anything is not going to help ;)

Currently the modem driver acknowledges the PDU right away. If there was a notification back to the modem driver, the PDU acknowledgement could be delayed till that point in time. If something bad happens in between, either the modem or the network can hold on to the PDU and redeliver once oFono is up and running again.

Looking at at_cmt_notify() the current implementation might have that affect already, as AT+CNMA is only send after calling ofono_sms_deliver_notify(). So as long as ofono_sms_deliver_notify() is handled synchronously and ensures that the PDU is stored one way or another before returning, it will work in case of a crash. (Would like to see assumptions like that called out in the code) That said, if your storage has issues (disk full?) there is currently no way to let the modem driver know that delivery was unsuccessful.

Cheers,
Waldo

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] Added SQLite history plugin
  2010-04-07 17:54       ` Nicola Mfb
@ 2010-04-07 18:05         ` Denis Kenzior
  2010-04-07 19:20           ` Nicola Mfb
  0 siblings, 1 reply; 23+ messages in thread
From: Denis Kenzior @ 2010-04-07 18:05 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 2074 bytes --]

Hi Niko,

> On Wed, Apr 7, 2010 at 7:01 PM, Denis Kenzior <denkenz@gmail.com> wrote:
> [...]
> 
> >> Database connection is handled by sqlite3_* API in a transparent manner
> >> so you could have different open connections at the same time without
> >> any problem, each SQL statement is transactional so the DB is every time
> >> consistent, but if you prefer I can rethink of it (maybe a DB for every
> >> modem could be a possibility but this would overload the plugin because
> >> we have to open and close the DB in every callback given that we have to
> >> filter the files access by modem).
> >
> > My concern here was that multiple open connections to the same database
> > might require locking or other overhead that could be avoided with a
> > single shared connection.  It might be there is no overhead and your
> > original approach is fine.
> 
> What's about a dbfile for each modem? we'll have one connection per
> database, and may avoid the "modem" column on all the tables, saving
> storage space.

This is possible, or you can do 1 db for each IMSI.  Do note that some drivers 
do not have IMSIs (e.g. making calls via HFP driver).  So it is up to the 
implementation to decide whether to log those calls or not.

> 
> [...]
> 
> > We're working on improving the SMS capabilities, and might eventually
> > expose the message id.  However, the message id really has no meaning, so
> > this is not something we really want to expose.  I'd like to keep it
> > private between the history plugin and the core.  Are you sure you really
> > require it to be exposed, or can you do without it?
> 
> We prefer to have the id to easily sync with the history plugin, as
> said in previous mail, if you prefer to not expose the id on
> IncomingMessage, we may emit an HistoryIncomingMessage signal with the
> id.

Your plugin is free to do anything it wants.  The real question is whether you 
truly need this id or you can simply update the application state based on the 
IncomingMessage signal only.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] Added SQLite history plugin
  2010-04-07 17:55           ` Denis Kenzior
@ 2010-04-07 18:09             ` Nicola Mfb
  2010-04-07 18:20               ` Denis Kenzior
  0 siblings, 1 reply; 23+ messages in thread
From: Nicola Mfb @ 2010-04-07 18:09 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 1296 bytes --]

On Wed, Apr 7, 2010 at 7:55 PM, Denis Kenzior <denkenz@gmail.com> wrote:
[...]
>> Yes, anyway I'm thinking about the case when the storage system is
>> full, or an other error occours while trying to store the message.
>> In this case the driver should notify the network with a NACK, while a
>> signal should notify the user that the storage is full or there is
>> another problem.
>
> If your storage is full then you have other problems.  Adding nack/ack
> capability would increate the complexity of the driver/core about 10 fold, so
> this is not something we're willing to consider right now.  If this is an
> issue, then the user should be notified well in advance to rectify the
> situation or face possible data loss.

That's not always possibile, for example in the case of
filesystem/block device faults.
I have a real case here, on the freerunner sometimes using the WiFi
will broke the access to the SD media card, I know it's not a perfect
device but similiar problems may happen on different hardware too.

Anyway I was aware of such complexity, my old phones nacks when the
sim/me/fs storage is full, hoping that will be addressed in some
future.

For now we will notify the user in advance and power down the modem.

Thanks and regards

      Niko

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] Added SQLite history plugin
  2010-04-07 18:09             ` Nicola Mfb
@ 2010-04-07 18:20               ` Denis Kenzior
  0 siblings, 0 replies; 23+ messages in thread
From: Denis Kenzior @ 2010-04-07 18:20 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 1609 bytes --]

Hi Niko,

> On Wed, Apr 7, 2010 at 7:55 PM, Denis Kenzior <denkenz@gmail.com> wrote:
> [...]
> 
> >> Yes, anyway I'm thinking about the case when the storage system is
> >> full, or an other error occours while trying to store the message.
> >> In this case the driver should notify the network with a NACK, while a
> >> signal should notify the user that the storage is full or there is
> >> another problem.
> >
> > If your storage is full then you have other problems.  Adding nack/ack
> > capability would increate the complexity of the driver/core about 10
> > fold, so this is not something we're willing to consider right now.  If
> > this is an issue, then the user should be notified well in advance to
> > rectify the situation or face possible data loss.
> 
> That's not always possibile, for example in the case of
> filesystem/block device faults.
> I have a real case here, on the freerunner sometimes using the WiFi
> will broke the access to the SD media card, I know it's not a perfect
> device but similiar problems may happen on different hardware too.
> 
> Anyway I was aware of such complexity, my old phones nacks when the
> sim/me/fs storage is full, hoping that will be addressed in some
> future.

This is certainly possible to add, however the priority is rather low because 
only truly bizarre conditions result in loss of data.  Namely disk full and 
physical device corruption.  In both cases you have other more important 
information than SMS that will be lost.

If this is important to you, you can always submit a patch :)

Regards,
-Denis

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] Added SQLite history plugin
  2010-04-07 18:03         ` Bastian, Waldo
@ 2010-04-07 18:27           ` Denis Kenzior
  0 siblings, 0 replies; 23+ messages in thread
From: Denis Kenzior @ 2010-04-07 18:27 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 1715 bytes --]

Hi Waldo,

> 
> Currently the modem driver acknowledges the PDU right away. If there was a
>  notification back to the modem driver, the PDU acknowledgement could be
>  delayed till that point in time. If something bad happens in between,
>  either the modem or the network can hold on to the PDU and redeliver once
>  oFono is up and running again.
> 
> Looking at at_cmt_notify() the current implementation might have that
>  affect already, as AT+CNMA is only send after calling
>  ofono_sms_deliver_notify(). So as long as ofono_sms_deliver_notify() is
>  handled synchronously and ensures that the PDU is stored one way or
>  another before returning, it will work in case of a crash. (Would like to
>  see assumptions like that called out in the code) That said, if your
>  storage has issues (disk full?) there is currently no way to let the modem
>  driver know that delivery was unsuccessful.

You described the current implementation perfectly.  The driver can assume 
that the fragment has been stored after the call to sms_deliver_notify has 
returned.  For its part, the history plugin should ensure the storage is 
synced when any of the callbacks are called.

I freely admit the lack of documentation, and if this is a comment you want to 
add in the driver definition file I would gladly accept it.  However, up to this 
point we've only had a single driver for SMS and it gets this part right.

The disk full case we're aware of.  Again, this can easily be avoided or 
minimized by notifying the user of such conditions in advance.  This was an 
important use case for devices with 512K of storage, much less so for devices 
with 64GB of storage.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] Added SQLite history plugin
  2010-04-07 18:05         ` Denis Kenzior
@ 2010-04-07 19:20           ` Nicola Mfb
  2010-04-07 19:33             ` Denis Kenzior
  0 siblings, 1 reply; 23+ messages in thread
From: Nicola Mfb @ 2010-04-07 19:20 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 1591 bytes --]

On Wed, Apr 7, 2010 at 8:05 PM, Denis Kenzior <denkenz@gmail.com> wrote:
[...]
>> We prefer to have the id to easily sync with the history plugin, as
>> said in previous mail, if you prefer to not expose the id on
>> IncomingMessage, we may emit an HistoryIncomingMessage signal with the
>> id.
>
> Your plugin is free to do anything it wants.  The real question is whether you
> truly need this id or you can simply update the application state based on the
> IncomingMessage signal only.

An use case may explain that better.

I have an app that listen for incoming messages and forwards them by mail.

After sending X messages the app goes down for some reason, when comes
up again there are Y messages in the history plugin becouse Y-X new
messages arrived.

Well, I want my app to be able to recover from the downtime using the
history plugin, and want to avoid to resend the first X messages.

A simple way to achieve that is adding the id in the IncomingMessage
signal, so the app is able to store it somewhere every time it
receives a new message.

When the app start, it simply query the history plugin and start
processing only messages where id > lastsavedid.

Of course this is only an easy way to achieve that, I may use the
timestamps, but really I do not trust them, it may be I'm missing
somethingh here, is there an easy/elegant alternative?

A real case may be to powerup the modem at device startup to get
already a network registration while the gui comes later, or the user
volontary restarts the gui, etc.

Regards

       Niko

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] Added SQLite history plugin
  2010-04-07 11:47   ` Nicola Mfb
  2010-04-07 17:28     ` Bastian, Waldo
@ 2010-04-07 19:28     ` Denis Kenzior
  1 sibling, 0 replies; 23+ messages in thread
From: Denis Kenzior @ 2010-04-07 19:28 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 1212 bytes --]

Hi Niko,

> Infact if a complete dbus api is going to be defined, it would be nice
> to share it for all history plugins, adding for example an
> "org.ofono.HistoryManager" interface to the modem, while if the
> history plugin has to be used in a different manner, or ofono does not
> care of that by design, we have to implement the dbus interface in the
> sqlite plugin directly, forcing our apps to not be generally oFono
> compliants (at least when not using the sqlite plugin).

So today I would consider this to be out-of-scope for the oFono project.  We 
really want to concentrate on being the telephony stack and leave the 
peripheral aspects to integrators.  However, I do realize this is still 
largely uncharted territory, so any suggestions you have on how oFono core can 
make your life easier in this space are definitely welcome.

In the end some decision has to be made regarding the storage backend.  
Whether it is MySQL, SQLite, Tracker, EDS, portable black holes, etc.  I 
prefer to leave this to each distribution / platform instead of mandating a 
solution.  In the end we're telephony experts, none of us is an expert with 
contacts or databases.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] Added SQLite history plugin
  2010-04-07 19:20           ` Nicola Mfb
@ 2010-04-07 19:33             ` Denis Kenzior
  2010-04-07 21:26               ` Nicola Mfb
  0 siblings, 1 reply; 23+ messages in thread
From: Denis Kenzior @ 2010-04-07 19:33 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 945 bytes --]

Hi Niko,
Hi Niko,
> After sending X messages the app goes down for some reason, when comes
> up again there are Y messages in the history plugin becouse Y-X new
> messages arrived.
> 
> Well, I want my app to be able to recover from the downtime using the
> history plugin, and want to avoid to resend the first X messages.
> 
> A simple way to achieve that is adding the id in the IncomingMessage
> signal, so the app is able to store it somewhere every time it
> receives a new message.

I suggest storing this information directly in the history database.  E.g. 
Read / Unread flag.  Ideally the application can access the database 
concurrently to mark the message as Read / Unread.  If not, then some sort of 
DBus API will be required.

> 
> When the app start, it simply query the history plugin and start
> processing only messages where id > lastsavedid.

Why not query the database directly?

Regards,
-Denis

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] Added SQLite history plugin
  2010-04-07 19:33             ` Denis Kenzior
@ 2010-04-07 21:26               ` Nicola Mfb
  0 siblings, 0 replies; 23+ messages in thread
From: Nicola Mfb @ 2010-04-07 21:26 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 1671 bytes --]

On Wed, Apr 7, 2010 at 9:33 PM, Denis Kenzior <denkenz@gmail.com> wrote:
[...]
> I suggest storing this information directly in the history database.  E.g.
> Read / Unread flag.  Ideally the application can access the database
> concurrently to mark the message as Read / Unread.  If not, then some sort of
> DBus API will be required.

At first impact we thinked on this solution, but there are two
different use cases, for example from user perspective a message may
be read/unread, and that's an information that has a global sense and
has to stay in the history db (remember my first mail on this topic
when I talked about metadata), but from application perpective a
message may be "processed" or not and IMHO that's an information that
should manage the application itself.

>> When the app start, it simply query the history plugin and start
>> processing only messages where id > lastsavedid.
>
> Why not query the database directly?

Yes, but apps cannot determine the last processed id, I know we may
query the db every time an IncomingMessage arrives and do some magic
(and assumptions, for example that the signal is emitted always after
the message reach the history plugin, or doing periodically a cpu
wasting query).
But thats seems quite weird, so I think finally we'll go providing a
dbus interface to the history plugin with a signal giving the needed
id when a new message arrives.

I understand all that may be out of the ofono scope, but if
ineterested we'll keep you updated on our further development and
provide patches that may be interesting for other users.

Keep up the good work and regards!

     Niko

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [PATCH 2/2] Added SQLite history plugin
  2010-04-06 10:55   ` Dario
@ 2010-04-06 16:12     ` Bastian, Waldo
  0 siblings, 0 replies; 23+ messages in thread
From: Bastian, Waldo @ 2010-04-06 16:12 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 566 bytes --]

> Bastian, Waldo ha scritto:
> > The message handling in this patch seems to be vulnerable to SQL
> injection attacks. See http://en.wikipedia.org/wiki/SQL_injection
> >
> > Cheers,
> > Waldo
> 
> Hi Waldo,
> I didn't think of a message carrying an SQL injection :)
> Honestly I would use prepared statement since start of the job but I
> didn't manage how to do them in SQLite but I agree with you they're more
> secure and the code is cleaner, so I converted the source to them after
> studying their use.

Thanks, much better :-)

Cheers,
Waldo

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] Added SQLite history plugin
  2010-04-05  2:54 ` Bastian, Waldo
@ 2010-04-06 10:55   ` Dario
  2010-04-06 16:12     ` Bastian, Waldo
  0 siblings, 1 reply; 23+ messages in thread
From: Dario @ 2010-04-06 10:55 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 554 bytes --]

Bastian, Waldo ha scritto:
> The message handling in this patch seems to be vulnerable to SQL injection attacks. See http://en.wikipedia.org/wiki/SQL_injection
>
> Cheers,
> Waldo

Hi Waldo,
I didn't think of a message carrying an SQL injection :)
Honestly I would use prepared statement since start of the job but I 
didn't manage how to do them in SQLite but I agree with you they're more 
secure and the code is cleaner, so I converted the source to them after 
studying their use.
Thank you for your suggestion.
Best Regards,
Dario.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [PATCH 2/2] Added SQLite history plugin
  2010-04-04 21:51 Dario
@ 2010-04-05  2:54 ` Bastian, Waldo
  2010-04-06 10:55   ` Dario
  0 siblings, 1 reply; 23+ messages in thread
From: Bastian, Waldo @ 2010-04-05  2:54 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 1045 bytes --]

> +    tmpparams = g_strdup_printf("\"%s\",%d,\"%s\",\"%s\",\"%s\",",
> +                    ofono_modem_get_path(context->modem),
> +                    msg_id, from, text, buf);
> +
> +    strftime(buf, 127, "\"%Y-%m-%dT%H:%M:%S%z\"", remote);
> +    buf[127] = '\0';
> +    params = g_strconcat(tmpparams, buf, NULL);
> +
> +    query = g_strdup_printf(INSERT_IN_MSGS, params);
[Snip]
> +    tmpparams = g_strdup_printf("\"%s\",%d,\"%s\",\"%s\",\"%s\",%d,",
> +                    ofono_modem_get_path(context->modem),
> +                    msg_id, to, text, buf,
> +                    OFONO_HISTORY_SMS_STATUS_PENDING);
> +
> +    currtime = time(NULL);
> +    strftime(buf, 127, "\"%Y-%m-%dT%H:%M:%S%z\"", localtime(&currtime));
> +    buf[127] = '\0';
> +    params = g_strconcat(tmpparams, buf, NULL);
> +
> +    query = g_strdup_printf(INSERT_OUT_MSGS, params);

The message handling in this patch seems to be vulnerable to SQL injection attacks. See http://en.wikipedia.org/wiki/SQL_injection

Cheers,
Waldo

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 2/2] Added SQLite history plugin
@ 2010-04-04 21:51 Dario
  2010-04-05  2:54 ` Bastian, Waldo
  0 siblings, 1 reply; 23+ messages in thread
From: Dario @ 2010-04-04 21:51 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 12025 bytes --]


---
 plugins/oFono_History_DB.sql |   27 ++++
 plugins/sqlite_history.c     |  324 
++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 351 insertions(+), 0 deletions(-)
 create mode 100644 plugins/oFono_History_DB.sql
 create mode 100644 plugins/sqlite_history.c

diff --git a/plugins/oFono_History_DB.sql b/plugins/oFono_History_DB.sql
new file mode 100644
index 0000000..cf3180a
--- /dev/null
+++ b/plugins/oFono_History_DB.sql
@@ -0,0 +1,27 @@
+CREATE TABLE "ofono_history_calls" (
+    "ohc_modem_path" TEXT, -- Modem path string i.e. "/modem0"
+    "ohc_type" INTEGER, -- Call type ( 0 = Call ended, 1 = Call missed )
+    "ohc_direction" INTEGER, -- Call direction ( 0 = Mobile Originated, 
1 = Mobile Terminated )
+    "ohc_phone_number" TEXT, -- Other party phone number
+    "ohc_start_time" TEXT, -- Starting date/time
+    "ohc_end_time" TEXT -- Ending date/time
+);
+CREATE TABLE "ofono_history_incoming_messages" (
+    "ohim_modem_path" TEXT, -- Modem path string i.e. "/modem0"
+    "ohim_msg_id" INTEGER, -- oFono unique message id number
+    "ohim_sender" TEXT, -- Sender phone number
+    "ohim_text" TEXT, -- Message text
+    "ohim_local_date" TEXT, -- Local sent date/time
+    "ohim_remote_date" TEXT -- Remote sent date/time
+);
+CREATE TABLE "ofono_history_outgoing_messages" (
+    "ohom_modem_path" TEXT, -- Modem path string i.e. "/modem0"
+    "ohom_msg_id" INTEGER, -- oFono unique message id number
+    "ohom_recipient" TEXT, -- Recipient phone number
+    "ohom_text" TEXT, -- Message text
+    "ohom_creation_date" TEXT, -- Message creation date/time
+    "ohom_send_status" INTEGER, -- Sending status ( 0 = Pending, 1 = 
Submitted, 2 = Failed )
+    "ohom_status_update_date" TEXT -- Last row update date/time
+);
+CREATE UNIQUE INDEX "ohim_idx_msg_id" on 
ofono_history_incoming_messages (ohim_msg_id ASC);
+CREATE UNIQUE INDEX "ohom_idx_msg_id" on 
ofono_history_outgoing_messages (ohom_msg_id ASC);
diff --git a/plugins/sqlite_history.c b/plugins/sqlite_history.c
new file mode 100644
index 0000000..9015403
--- /dev/null
+++ b/plugins/sqlite_history.c
@@ -0,0 +1,324 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2010 Dario 'Djdas' Conigliaro.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  
02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdlib.h>
+#include <string.h>
+#include <glib.h>
+
+#define OFONO_API_SUBJECT_TO_CHANGE
+#include <ofono/plugin.h>
+#include <ofono/log.h>
+#include <ofono/history.h>
+#include <ofono/types.h>
+#include <ofono/modem.h>
+
+#include "common.h"
+
+#include <sqlite3.h>
+
+#define SQL_HISTORY_DB_PATH STORAGEDIR "/ofono_history.sqlite"
+#define SQL_HISTORY_DB_SQL STORAGEDIR "/oFono_History_DB.sql"
+
+#define INSERT_CALLS "INSERT INTO ofono_history_calls VALUES (%s)"
+#define INSERT_IN_MSGS "INSERT INTO ofono_history_incoming_messages 
VALUES (%s)"
+#define UPDATE_IN_MSGS "UPDATE ofono_history_incoming_messages SET %s 
WHERE ohim_msg_id = %d"
+#define INSERT_OUT_MSGS "INSERT INTO ofono_history_outgoing_messages 
VALUES (%s)"
+#define UPDATE_OUT_MSGS "UPDATE ofono_history_outgoing_messages SET %s 
WHERE ohom_msg_id = %d"
+
+sqlite3 *db = NULL;
+
+int check_db()
+{
+    int ret_val = 0;
+    
+    ofono_debug("Checking if DB is empty");
+        
+    if (sqlite3_table_column_metadata(db, NULL, "ofono_history_calls",
+                      "ohc_modem_path", NULL, NULL, NULL, NULL,
+                      NULL) != SQLITE_OK) {
+        char *systemcmd;
+        systemcmd = g_strdup_printf("sqlite3 -batch %s < %s", 
SQL_HISTORY_DB_PATH, SQL_HISTORY_DB_SQL);
+        
+        ofono_debug("Initializing DB: %s", systemcmd);
+        
+        ret_val = system(systemcmd);
+        g_free(systemcmd);        
+    }
+    return ret_val;
+}
+
+static int sqlite_history_probe(struct ofono_history_context *context)
+{
+    ofono_debug("SQLite History Probe for modem: %p", context->modem);
+    if (sqlite3_open(SQL_HISTORY_DB_PATH, &db) != SQLITE_OK) {
+        ofono_debug("Error opening DB: %s", sqlite3_errmsg(db));
+        sqlite3_close(db);
+        return -1;
+    }
+
+    if (check_db() != 0)
+        return -1;
+
+    return 0;
+}
+
+static void sqlite_history_remove(struct ofono_history_context *context)
+{
+    ofono_debug("SQLite History Remove for modem: %p", context->modem);
+    if (db != NULL)
+        sqlite3_close(db);
+}
+
+static void sqlite_history_call_ended(struct ofono_history_context 
*context,
+                      const struct ofono_call *call,
+                      time_t start, time_t end)
+{
+    const char *from = "Unknown";
+    char buf[128];
+    char *query;
+    char *params;
+    char *tmpparams;
+    char *errormsg;
+
+    ofono_debug("Call Ended on modem: %p", context->modem);
+
+    if (call->type != 0)
+        return;
+
+    if (db == NULL)
+        return;
+
+    from = phone_number_to_string(&call->phone_number);
+
+    strftime(buf, 127, "%Y-%m-%dT%H:%M:%S%z", localtime(&start));
+    buf[127] = '\0';
+    tmpparams = g_strdup_printf("\"%s\",0,%d,\"%s\",\"%s\",",
+                    ofono_modem_get_path(context->modem),
+                    call->direction, from, buf);
+
+    strftime(buf, 127, "\"%Y-%m-%dT%H:%M:%S%z\"", localtime(&end));
+    buf[127] = '\0';
+    params = g_strconcat(tmpparams, buf, NULL);
+
+    query = g_strdup_printf(INSERT_CALLS, params);
+
+    ofono_debug("Executing query: %s", query);
+
+    if (sqlite3_exec(db, query, NULL, NULL, &errormsg) != SQLITE_OK) {
+        ofono_debug("Error: %s", errormsg);
+        sqlite3_free(errormsg);
+    }
+
+    g_free(tmpparams);
+    g_free(params);
+    g_free(query);
+}
+
+static void sqlite_history_call_missed(struct ofono_history_context 
*context,
+                       const struct ofono_call *call,
+                       time_t when)
+{
+    const char *from = "Unknown";
+    char buf[128];
+    char *query;
+    char *params;
+    char *errormsg;
+
+    ofono_debug("Call Missed on modem: %p", context->modem);
+
+    if (call->type != 0)
+        return;
+
+    if (db == NULL)
+        return;
+
+    from = phone_number_to_string(&call->phone_number);
+
+    strftime(buf, 127, "%Y-%m-%dT%H:%M:%S%z", localtime(&when));
+    buf[127] = '\0';
+
+    params = g_strdup_printf("\"%s\",1,%d,\"%s\",\"%s\",\"%s\"",
+                 ofono_modem_get_path(context->modem),
+                 call->direction, from, buf, buf);
+
+    query = g_strdup_printf(INSERT_CALLS, params);
+
+    ofono_debug("Executing query: %s", query);
+
+    if (sqlite3_exec(db, query, NULL, NULL, &errormsg) != SQLITE_OK) {
+        ofono_debug("Error: %s", errormsg);
+        sqlite3_free(errormsg);
+    }
+
+    g_free(params);
+    g_free(query);
+}
+
+static void sqlite_history_sms_received(struct ofono_history_context 
*context,
+                    unsigned int msg_id,
+                    const char *from,
+                    const struct tm *remote,
+                    const struct tm *local,
+                    const char *text)
+{
+    char buf[128];
+    char *query;
+    char *params;
+    char *tmpparams;
+    char *errormsg;
+
+    ofono_debug("Incoming SMS on modem: %p", context->modem);
+
+    if (db == NULL)
+        return;
+
+    strftime(buf, 127, "%Y-%m-%dT%H:%M:%S%z", local);
+    buf[127] = '\0';
+
+    tmpparams = g_strdup_printf("\"%s\",%d,\"%s\",\"%s\",\"%s\",",
+                    ofono_modem_get_path(context->modem),
+                    msg_id, from, text, buf);
+
+    strftime(buf, 127, "\"%Y-%m-%dT%H:%M:%S%z\"", remote);
+    buf[127] = '\0';
+    params = g_strconcat(tmpparams, buf, NULL);
+
+    query = g_strdup_printf(INSERT_IN_MSGS, params);
+
+    ofono_debug("Executing query: %s", query);
+
+    if (sqlite3_exec(db, query, NULL, NULL, &errormsg) != SQLITE_OK) {
+        ofono_debug("Error: %s", errormsg);
+        sqlite3_free(errormsg);
+    }
+
+    g_free(tmpparams);
+    g_free(params);
+    g_free(query);
+}
+
+static void sqlite_history_sms_send_pending(struct ofono_history_context
+                        *context, unsigned int msg_id,
+                        const char *to, time_t when,
+                        const char *text)
+{
+    char buf[128];
+    char *query;
+    char *params;
+    char *tmpparams;
+    char *errormsg;
+    time_t currtime;
+
+    ofono_debug("Sending SMS on modem: %p", context->modem);
+
+    if (db == NULL)
+        return;
+
+    strftime(buf, 127, "%Y-%m-%dT%H:%M:%S%z", localtime(&when));
+    buf[127] = '\0';
+
+    tmpparams = g_strdup_printf("\"%s\",%d,\"%s\",\"%s\",\"%s\",%d,",
+                    ofono_modem_get_path(context->modem),
+                    msg_id, to, text, buf,
+                    OFONO_HISTORY_SMS_STATUS_PENDING);
+
+    currtime = time(NULL);
+    strftime(buf, 127, "\"%Y-%m-%dT%H:%M:%S%z\"", localtime(&currtime));
+    buf[127] = '\0';
+    params = g_strconcat(tmpparams, buf, NULL);
+
+    query = g_strdup_printf(INSERT_OUT_MSGS, params);
+
+    ofono_debug("Executing query: %s", query);
+
+    if (sqlite3_exec(db, query, NULL, NULL, &errormsg) != SQLITE_OK) {
+        ofono_debug("Error: %s", errormsg);
+        sqlite3_free(errormsg);
+    }
+
+    g_free(tmpparams);
+    g_free(params);
+    g_free(query);
+
+}
+
+static void sqlite_history_sms_send_status(struct ofono_history_context
+                       *context, unsigned int msg_id,
+                       time_t when,
+                       enum ofono_history_sms_status s)
+{
+    char buf[128];
+    char *query;
+    char *setclause;
+    char *errormsg;
+
+    ofono_debug("SMS status on modem: %p", context->modem);
+
+    if (db == NULL)
+        return;
+
+    strftime(buf, 127, "%Y-%m-%dT%H:%M:%S%z", localtime(&when));
+    buf[127] = '\0';
+
+    setclause =
+        g_strdup_printf
+        ("ohom_send_status=%d, ohom_status_update_date=\"%s\"", s, buf);
+    query = g_strdup_printf(UPDATE_OUT_MSGS, setclause, msg_id);
+
+    ofono_debug("Executing query: %s", query);
+
+    if (sqlite3_exec(db, query, NULL, NULL, &errormsg) != SQLITE_OK) {
+        ofono_debug("Error: %s", errormsg);
+        sqlite3_free(errormsg);
+    }
+
+    g_free(setclause);
+    g_free(query);
+}
+
+static struct ofono_history_driver sqlite_driver = {
+    .name = "SQLite History",
+    .probe = sqlite_history_probe,
+    .remove = sqlite_history_remove,
+    .call_ended = sqlite_history_call_ended,
+    .call_missed = sqlite_history_call_missed,
+    .sms_received = sqlite_history_sms_received,
+    .sms_send_pending = sqlite_history_sms_send_pending,
+    .sms_send_status = sqlite_history_sms_send_status,
+};
+
+static int sqlite_history_init(void)
+{
+    return ofono_history_driver_register(&sqlite_driver);
+}
+
+static void sqlite_history_exit(void)
+{
+    ofono_history_driver_unregister(&sqlite_driver);
+}
+
+OFONO_PLUGIN_DEFINE(sqlite_history, "SQLite History Plugin",
+            VERSION, OFONO_PLUGIN_PRIORITY_DEFAULT,
+            sqlite_history_init, sqlite_history_exit)
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2010-04-07 21:26 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-06 10:43 [PATCH 2/2] Added SQLite history plugin Dario
2010-04-06 17:56 ` Denis Kenzior
2010-04-07  8:50   ` Dario
2010-04-07 17:01     ` Denis Kenzior
2010-04-07 17:54       ` Nicola Mfb
2010-04-07 18:05         ` Denis Kenzior
2010-04-07 19:20           ` Nicola Mfb
2010-04-07 19:33             ` Denis Kenzior
2010-04-07 21:26               ` Nicola Mfb
2010-04-07 11:47   ` Nicola Mfb
2010-04-07 17:28     ` Bastian, Waldo
2010-04-07 17:33       ` Denis Kenzior
2010-04-07 17:43         ` Nicola Mfb
2010-04-07 17:55           ` Denis Kenzior
2010-04-07 18:09             ` Nicola Mfb
2010-04-07 18:20               ` Denis Kenzior
2010-04-07 18:03         ` Bastian, Waldo
2010-04-07 18:27           ` Denis Kenzior
2010-04-07 19:28     ` Denis Kenzior
  -- strict thread matches above, loose matches on Subject: below --
2010-04-04 21:51 Dario
2010-04-05  2:54 ` Bastian, Waldo
2010-04-06 10:55   ` Dario
2010-04-06 16:12     ` Bastian, Waldo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.