10:13 a.m.
Call History Plugin
Introduction:
Callhistory is a plugin loaded by ofono for persisting voice call history information in a
disk file. Plugin exports dbus methods and signals as org.ofono.callhistory interface.
Whenever there is a voice call in the system, ofono calls the plugin with the infomation
(lineid,calltype,start time, end time) , plugin writes the information into the file ,
increments the 'unread' field in the header and sends out dbus signal with the
number of unread records. Client (dialer) requests history records by calling method
"GetVoiceHistory" exposed on the callhistory interface. After receiving
information client calls the "SetVoiceHistoryRead" as an acknowledgement to
indicate end of transaction. The pointers in the header are adjusted appropriately
(explained clearly below).
Design : Plugin uses memory mapped file for high performance input/output operations. File
is used as cyclic queue for storing or reading records, it store s 50 records of
78bytes,hence fixed size file of 3916 bytes is used. File's first 16 bytes were used
for storing header, and next 3900 bytes for data. Header structure has head pointer, tail
pointer,unread and lastid. Head pointer points to next slot for writing record into,tail
for reading record, unread for number of history records unread by client, lastid for id
of the last record that is written.
Below is the file structure: (3916 bytes)
|Header|Data|
0 16
Header Structure: (16 bytes)
|head|tail|unread|lastid|
0 4 8 12 16
head : Points to next slot for writing
tail : Points to next record to be read
unread : Number of Unread records so far
lastid : Number of the record that is last written
Below is the structure of the history record:
History Record:(78 bytes)
|id|lineid|calltype|start time|end time|
id : Record id
lineid : phone Number
calltype : Outgoing/Incoming/Missed
start time: start time in UTC
end time: end time in UTC
When the plugin is loaded by the ofono, it opens or creates a disk file of 3916 bytes size
and maps it into fixed size memory space. Plugin reads the header of the file , if the
'unread' records is > 0 , a signal is sent out on the dbus with the information
of number of unread records.
Requesting data by client:
When the client calls "GetVoiceHistory" method,plugin uses temp_tail and
temp_unread variables to accomplish sending data to client. It copies tail pointer stored
in the header into temporary tail pointer ,starts to read history records one at a time
from the location pointed by the temp tail pointer , copies the into dbus structure,
increments the temp tail pointer by size of the history record (78 bytes) and reads data
from the next record location , this is repeated until it reaches head pointer. when all
the unread records are read then the dbus structure is sent to the client. Once the data
received, client needs to "SetVoiceHistoryRead" method to indicate that it
received the data. Plugin will update the tail pointer with the temp tail pointer and
unread records number is decremented by temp_unread. Using temp_tail and temp_unread
variables protect from modifying the header if there is any loss in data delivery. If
client doesnt call 'SetVoiceHistoryRead' then the actual header pointers wont be
updated in the file.
Writing record into the file:
When Ofono calls the plugin with call information (phone number, call type, call start
time , call end time), plugin increaments the lastid to create record id for this record
,writes the record in the location pointed by head, and the head pointer is incremented by
size of record (78 bytes), "unread" variable is incremented by 1. If the head
reaches end of file (3916) then it is reset it back to begining of the data portion of the
file (16 bytes). A dbus signal is sent out with number of current unread records. Plugin
keeps writing the records in circular queue.
Locking:
Accessing Memory mapped file pointer , all header fields (head,tail,unread,lastid),
temp_unread,temp_tail are synchronized by a mutex. writing into memory file and reading
from memory file happen asynchronously, so mutex is used to protect the memory mapped file
pointer , header, temp_unread, temp_tail.
Limitations:
History is limited to storing 50 records. The above design assumes that there is only one
client (either dialer or a history daemon) that reads data and sends acknowledgemnts.
10:13 a.m.
New subject: [PATCH] plugins: Adding implementation for persisting voice call history
---
Makefile.am | 3 +
plugins/callhistory.c | 594 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 597 insertions(+), 0 deletions(-)
create mode 100644 plugins/callhistory.c
diff --git a/Makefile.am b/Makefile.am
index aa66907..1b7f4d4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -261,6 +261,9 @@ builtin_sources += plugins/ste.c
builtin_modules += caif
builtin_sources += plugins/caif.c
+
+builtin_modules += callhistory
+builtin_sources += plugins/callhistory.c
endif
if MAINTAINER_MODE
diff --git a/plugins/callhistory.c b/plugins/callhistory.c
new file mode 100644
index 0000000..1579652
--- /dev/null
+++ b/plugins/callhistory.c
@@ -0,0 +1,594 @@
+/*
+ *
+ * oFono - Open Source Telephony
+ *
+ * Copyright (C) 2008-2010 Intel Corporation. All rights reserved.
+ *
+ * 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
+
+#define OFONO_API_SUBJECT_TO_CHANGE
+#include <string.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <glib.h>
+#include <time.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/dir.h>
+#include <sys/mman.h>
+#include <semaphore.h>
+#include <fcntl.h>
+#include <ofono/dbus.h>
+#include <dbus/dbus-glib.h>
+#include <ofono/types.h>
+#include <ofono/plugin.h>
+#include <ofono/log.h>
+#include <ofono/history.h>
+#include "gdbus.h"
+#include "common.h"
+
+#define HISTORY_FILE_PATH "/var/cache/callhistory/"
+#define HISTORY_FILE HISTORY_FILE_PATH"voicecallhistorydata"
+#define OFONO_MANAGER_PATH "/"
+#define PHONE_NUMBER_SIZE 64
+#define RECORD_SIZE 78
+#define HEADER_SIZE 16
+#define MAX_ITEMS 50
+
+#define TOTAL_SIZE (MAX_ITEMS * RECORD_SIZE + HEADER_SIZE)
+
+enum VOICE_CALL_TYPE {
+ OUTGOING = 0,
+ INCOMING,
+ MISSED
+};
+
+struct file_header {
+ int head;
+ int tail;
+ int unread;
+ unsigned int lastid;
+};
+
+struct voice_history {
+ int id;
+ char lineid[PHONE_NUMBER_SIZE];
+ short int calltype;
+ time_t starttime;
+ time_t endtime;
+};
+
+struct shared_info {
+ struct file_header header;
+ void *dataMap;
+ sem_t mutex;
+ int temp_unread;
+ int temp_tail;
+};
+
+static struct shared_info *shared_data;
+
+static int call_history_probe(struct ofono_history_context *context)
+{
+ DBG("History Probe for modem: %p", context->modem);
+ return 0;
+}
+
+static void call_history_remove(struct ofono_history_context *context)
+{
+ DBG("Example History Remove for modem: %p", context->modem);
+}
+
+static void clean_up()
+{
+ g_return_if_fail(shared_data != NULL);
+
+ sem_wait(&(shared_data->mutex));
+ /* unmap */
+ munmap(shared_data->dataMap, TOTAL_SIZE);
+ sem_post(&(shared_data->mutex));
+
+ /* remove semaphore */
+ if (sem_destroy(&(shared_data->mutex)) < 0)
+ perror("sem_destroy failed");
+
+ g_free(shared_data);
+}
+
+static int init_sem()
+{
+ unsigned int value = 1;
+ g_return_val_if_fail(shared_data != NULL, -1);
+
+ if (sem_init(&(shared_data->mutex), TRUE, value) < 0) {
+ perror("sem_init failed");
+ return -1;
+ }
+
+ return 0;
+}
+
+/* Start of DBus stuff */
+/* *************************************************************************
+ * Expose an interface, properties and signals for querying storage backend
+ * *************************************************************************/
+#define OFONO_CALL_HISTORY_INTERFACE OFONO_SERVICE".CallHistory"
+
+static void callhistory_emit_voice_history_changed(int userdata)
+{
+ int *valint = &userdata;
+ DBusConnection *conn = ofono_dbus_get_connection();
+ g_dbus_emit_signal(conn,
+ OFONO_MANAGER_PATH,
+ OFONO_CALL_HISTORY_INTERFACE,
+ "VoiceHistoryChanged",
+ DBUS_TYPE_UINT32,
+ valint,
+ DBUS_TYPE_INVALID);
+}
+
+static DBusMessage *call_history_set_voice_history_read(DBusConnection *conn,
+ DBusMessage *msg,
+ void *userdata)
+{
+ DBusMessage *reply;
+ DBusMessageIter iter;
+ g_return_val_if_fail((shared_data != NULL), NULL);
+
+ ofono_debug("Read ack received");
+
+ reply = dbus_message_new_method_return(msg);
+
+ if (!reply)
+ return NULL;
+
+ dbus_message_iter_init_append(reply, &iter);
+ sem_wait(&(shared_data->mutex));
+ shared_data->header.unread = (shared_data->header).unread -
+ shared_data->temp_unread;
+ shared_data->header.tail = shared_data->temp_tail;
+ /* write to file */
+ memcpy(shared_data->dataMap, &(shared_data->header), HEADER_SIZE);
+ sem_post(&(shared_data->mutex));
+ msync(shared_data->dataMap, TOTAL_SIZE, MS_ASYNC);
+ return reply;
+}
+
+static int init_header(void *dataPtr)
+{
+ int unread = 0;
+
+ g_return_val_if_fail((shared_data != NULL), -1);
+ g_return_val_if_fail((dataPtr != NULL), -1);
+ unread = (shared_data->header).unread;
+
+ sem_wait(&(shared_data->mutex));
+ memcpy(&(shared_data->header), (struct file_header *)(dataPtr),
+ HEADER_SIZE);
+
+ ofono_debug("Unread: %d, Header: %d, Tail: %d, serialno: %d\n",
+ unread,
+ (shared_data->header).head,
+ (shared_data->header).tail,
+ (shared_data->header).lastid);
+ sem_post(&(shared_data->mutex));
+ if (unread > 0)
+ callhistory_emit_voice_history_changed(unread);
+
+ return 0;
+}
+
+static void sync_mem_file(struct voice_history *vcall)
+{
+ void *writeMap = NULL;
+ int unread = 0;
+
+ g_return_if_fail(vcall != NULL);
+ g_return_if_fail(shared_data != NULL);
+ g_return_if_fail(shared_data->dataMap != NULL);
+
+ sem_wait(&(shared_data->mutex));
+
+ writeMap = shared_data->dataMap;
+ writeMap = writeMap + (shared_data->header).head;
+ vcall->id = (shared_data->header).lastid;
+ (shared_data->header).lastid++;
+ memcpy(writeMap, vcall, RECORD_SIZE);
+
+ /* update header */
+ ((shared_data->header).unread)++;
+ (shared_data->header).head += RECORD_SIZE;
+
+ if ((shared_data->header).lastid >= UINT_MAX)
+ (shared_data->header).lastid = 0;
+
+ unread = (shared_data->header).unread;
+
+ /* header reaches end of file */
+ if ((shared_data->header).head >= TOTAL_SIZE) {
+ /* reset head back to front */
+ (shared_data->header).head = HEADER_SIZE;
+
+ ofono_debug("Header = %d, Tail = %d",
+ (shared_data->header).head,
+ (shared_data->header).tail);
+
+ if (unread >= MAX_ITEMS)
+ ofono_error("No one reading history data");
+
+ }
+
+ memcpy(shared_data->dataMap, &(shared_data->header), HEADER_SIZE);
+ msync(shared_data->dataMap, TOTAL_SIZE, MS_ASYNC);
+
+ sem_post(&(shared_data->mutex));
+
+ callhistory_emit_voice_history_changed(unread);
+}
+
+/**
+ * Creates file "callhistory"
+ */
+static gboolean init_file()
+{
+ int historyFile;
+ struct stat statbuf;
+ DIR *dirptr;
+ char *fill = NULL;
+
+ g_return_val_if_fail((shared_data != NULL), FALSE);
+
+ dirptr = opendir(HISTORY_FILE_PATH);
+
+ if (!dirptr) {
+ ofono_debug("%s: doesnt exist", HISTORY_FILE_PATH);
+
+ if (mkdir(HISTORY_FILE_PATH,
+ S_IRWXU|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH) < 0) {
+ perror("Error creating callhistory dir");
+ return FALSE;
+ }
+
+ }
+
+ historyFile = open(HISTORY_FILE,
+ O_RDWR|O_CREAT|O_APPEND,
+ S_IRWXU);
+ if (historyFile < 0) {
+ perror("Open file failed");
+ return FALSE;
+ }
+
+ if (stat(HISTORY_FILE, &statbuf) < -1) {
+ perror("stat failed");
+ return FALSE;
+ }
+
+ ofono_debug("stat file: %d", (int) (statbuf.st_size));
+
+ if (statbuf.st_size == 0) {
+ int byteswritten = 0;
+ /* write header to the file */
+ (shared_data->header).head = HEADER_SIZE;
+ (shared_data->header).tail = (shared_data->header).head;
+ (shared_data->header).unread = 0;
+ ofono_debug("setting head: %d, tail: %d, unread: %d",
+ (shared_data->header).head,
+ (shared_data->header).tail,
+ (shared_data->header).unread);
+
+ /* fill the file with zeros */
+ ofono_debug("Trying to allocate %d size", TOTAL_SIZE);
+ fill = (char *) g_try_malloc0(TOTAL_SIZE);
+
+ if (!fill) {
+ ofono_debug("Error allocating init memory");
+ return FALSE;
+ }
+
+ /* Predefine file size and fill with some data */
+ bzero(fill, TOTAL_SIZE);
+ memcpy(fill, &(shared_data->header), HEADER_SIZE);
+
+ byteswritten = write(historyFile, fill, TOTAL_SIZE);
+
+ if (byteswritten < 0) {
+ ofono_debug("Error writing to file");
+ return FALSE;
+ }
+
+ g_free(fill);
+
+ statbuf.st_size = TOTAL_SIZE;
+ ofono_debug("History file %d size created\n", byteswritten);
+ }
+
+ /* create semaphor */
+ if (init_sem() < 0)
+ return FALSE;
+
+ sem_wait(&(shared_data->mutex));
+ shared_data->dataMap = mmap((caddr_t)NULL,
+ statbuf.st_size,
+ PROT_READ|PROT_WRITE|PROT_EXEC,
+ MAP_SHARED,
+ historyFile,
+ 0);
+ sem_post(&(shared_data->mutex));
+
+ if (shared_data->dataMap == (void *)(-1)) {
+ perror("mmap falied");
+ return FALSE;
+ }
+
+ close(historyFile);
+
+ init_header(shared_data->dataMap);
+
+ return TRUE;
+}
+
+/**
+ * call_history_call_ended:
+ * ofono calls this method with the call information
+ */
+static void call_history_call_ended(struct ofono_history_context *context,
+ const struct ofono_call *call,
+ time_t start,
+ time_t end)
+{
+ const char *from = "Unknown";
+ struct tm tmstart, tmend;
+ char sttime[128], endtime[128];
+ int fromlen = 0;
+ struct voice_history vcall;
+
+ ofono_debug("Call Ended on modem: %p", context->modem);
+
+ if (call->type != 0)
+ return;
+
+ ofono_debug("Voice Call, %s",
+ call->direction ? "Incoming" : "Outgoing");
+
+ if (call->direction)
+ vcall.calltype = INCOMING;
+ else
+ vcall.calltype = OUTGOING;
+
+ if (call->clip_validity == 0)
+ from = phone_number_to_string(&call->phone_number);
+
+ fromlen = strlen(from);
+ strcpy(vcall.lineid, from);
+
+ vcall.lineid[fromlen] = '\0';
+
+ vcall.starttime = start;
+ gmtime_r(&start, &tmstart);
+ strftime(sttime, 127, "%a, %d %b %Y %H:%M:%S %z", &tmstart);
+ sttime[127] = '\0';
+
+ ofono_debug("StartTime: %s", sttime);
+
+ vcall.endtime = end;
+ gmtime_r(&end, &tmend);
+ strftime(endtime, 127, "%a, %d %b %Y %H:%M:%S %z", &tmend);
+ endtime[127] = '\0';
+ ofono_debug("EndTime: %s", endtime);
+
+ sync_mem_file(&vcall);
+}
+/**
+ * call_history_call_missed:
+ * ofono calls this method with the call information
+ */
+static void call_history_call_missed(struct ofono_history_context *context,
+ const struct ofono_call *call,
+ time_t when)
+{
+ const char *from = "Unknown";
+ struct tm mtime;
+ char sttime[128];
+ struct voice_history vcall;
+ int fromlen = 0;
+
+ ofono_debug("Call Missed on modem: %p", context->modem);
+
+ if (call->type != 0)
+ return;
+
+ ofono_debug("Voice Call, Missed");
+ ofono_debug("Voice Call, %s",
+ call->direction ? "Incoming" : "Outgoing");
+
+ vcall.calltype = MISSED;
+
+ if (call->clip_validity == 0)
+ from = phone_number_to_string(&call->phone_number);
+
+ fromlen = strlen(from);
+ strncpy(vcall.lineid, from, fromlen);
+ vcall.lineid[fromlen] = '\0';
+
+ vcall.starttime = when;
+
+ gmtime_r(&when, &mtime);
+ ofono_debug("From: %s", vcall.lineid);
+ strftime(sttime, 127, "%a, %d %b %Y %H:%M:%S %z", &mtime);
+ sttime[127] = '\0';
+ ofono_debug("Missed Time: %s", sttime);
+
+ vcall.endtime = when;
+
+ sync_mem_file(&vcall);
+}
+
+static void read_data(struct voice_history *data, int temp)
+{
+ void *readMap = NULL;
+ g_return_if_fail(shared_data != NULL);
+ g_return_if_fail(shared_data->dataMap != NULL);
+
+ sem_wait(&(shared_data->mutex));
+ readMap = shared_data->dataMap+temp;
+
+ /* read a chunk into *data */
+ memcpy(data, readMap, RECORD_SIZE);
+ shared_data->temp_unread++;
+ sem_post(&(shared_data->mutex));
+}
+
+static DBusMessage *call_history_get_voice_history(DBusConnection *conn,
+ DBusMessage *msg,
+ void *userdata)
+{
+ DBusMessage *reply;
+ DBusMessageIter iter;
+ DBusMessageIter array, struct_s;
+ int i;
+ char *lineid;
+ struct voice_history data;
+ int unread = 0;
+
+ reply = dbus_message_new_method_return(msg);
+
+ if (!reply)
+ return NULL;
+
+ g_return_val_if_fail(shared_data != NULL, NULL);
+
+ dbus_message_iter_init_append(reply, &iter);
+
+ dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+ DBUS_STRUCT_BEGIN_CHAR_AS_STRING
+ DBUS_TYPE_UINT32_AS_STRING
+ DBUS_TYPE_STRING_AS_STRING
+ DBUS_TYPE_UINT16_AS_STRING
+ DBUS_TYPE_INT32_AS_STRING
+ DBUS_TYPE_INT32_AS_STRING
+ DBUS_STRUCT_END_CHAR_AS_STRING
+ , &array);
+
+ sem_wait(&(shared_data->mutex));
+ shared_data->temp_unread = 0;
+
+ shared_data->temp_tail = (shared_data->header).tail;
+ unread = (shared_data->header).unread;
+ sem_post(&(shared_data->mutex));
+
+ for (i = 0; i < unread; i++) {
+ read_data(&data, shared_data->temp_tail);
+ lineid = data.lineid;
+ dbus_message_iter_open_container(&array,
+ DBUS_TYPE_STRUCT,
+ NULL,
+ &struct_s);
+ dbus_message_iter_append_basic(&struct_s,
+ DBUS_TYPE_UINT32,
+ &(data.id));
+ dbus_message_iter_append_basic(&struct_s,
+ DBUS_TYPE_STRING,
+ &(lineid));
+ dbus_message_iter_append_basic(&struct_s,
+ DBUS_TYPE_UINT16,
+ &(data.calltype));
+ dbus_message_iter_append_basic(&struct_s,
+ DBUS_TYPE_INT32,
+ &(data.starttime));
+ dbus_message_iter_append_basic(&struct_s,
+ DBUS_TYPE_INT32,
+ &(data.endtime));
+ dbus_message_iter_close_container(&array, &struct_s);
+
+ shared_data->temp_tail = shared_data->temp_tail + RECORD_SIZE;
+
+ if (shared_data->temp_tail == TOTAL_SIZE) {
+ ofono_debug("End of Queue: %d",
+ shared_data->temp_tail);
+ /* reset back to front */
+ shared_data->temp_tail = HEADER_SIZE;
+ }
+
+ }
+ dbus_message_iter_close_container(&iter, &array);
+
+ return reply;
+}
+
+static GDBusMethodTable call_history_methods[] = {
+ { "GetVoiceHistory", "", "a(usqii)",
call_history_get_voice_history },
+ { "SetVoiceHistoryRead", "", "",
call_history_set_voice_history_read },
+ { }
+};
+
+static GDBusSignalTable call_history_signals[] = {
+ { "VoiceHistoryChanged", "u" },
+ {}
+};
+
+/* End of DBus stuff */
+static struct ofono_history_driver call_history_driver = {
+ .name = "callhistory",
+ .probe = call_history_probe,
+ .remove = call_history_remove,
+ .call_ended = call_history_call_ended,
+ .call_missed = call_history_call_missed,
+};
+
+static int call_history_init(void)
+{
+ DBusConnection *conn = ofono_dbus_get_connection();
+
+ if (!shared_data) {
+ shared_data = g_try_new0(struct shared_info, 1);
+
+ if (!shared_data)
+ return -ENOMEM;
+ }
+
+ if (!g_dbus_register_interface(conn,
+ OFONO_MANAGER_PATH,
+ OFONO_CALL_HISTORY_INTERFACE,
+ call_history_methods,
+ call_history_signals,
+ NULL, /* Properties */
+ shared_data, /* Userdata */
+ NULL)) /* Destroy func */
+ return -EIO;
+
+ if (!init_file())
+ return -ENOENT;
+
+ return ofono_history_driver_register(&call_history_driver);
+}
+
+static void call_history_exit(void)
+{
+ clean_up();
+ ofono_history_driver_unregister(&call_history_driver);
+}
+
+OFONO_PLUGIN_DEFINE(callhistory, "Call History Plugin",
+ OFONO_VERSION, OFONO_PLUGIN_PRIORITY_DEFAULT,
+ call_history_init, call_history_exit)
--
1.6.3.3
11:14 a.m.
New subject: [PATCH] plugins: Adding implementation for persisting voice call history
Hi Rajyalakshmi,
Makefile.am | 3 +
plugins/callhistory.c | 594 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 597 insertions(+), 0 deletions(-)
create mode 100644 plugins/callhistory.c
diff --git a/Makefile.am b/Makefile.am
index aa66907..1b7f4d4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -261,6 +261,9 @@ builtin_sources += plugins/ste.c
builtin_modules += caif
builtin_sources += plugins/caif.c
+
+builtin_modules += callhistory
+builtin_sources += plugins/callhistory.c
endif
if MAINTAINER_MODE
diff --git a/plugins/callhistory.c b/plugins/callhistory.c
new file mode 100644
index 0000000..1579652
--- /dev/null
+++ b/plugins/callhistory.c
@@ -0,0 +1,594 @@
+/*
+ *
+ * oFono - Open Source Telephony
+ *
+ * Copyright (C) 2008-2010 Intel Corporation. All rights reserved.
+ *
+ * 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
+ *
+ */
the indentation is broken here. Please fix that. The copyright notice
needs to be consistent throughout the whole source code.
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#define OFONO_API_SUBJECT_TO_CHANGE
+#include <string.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <glib.h>
+#include <time.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/dir.h>
+#include <sys/mman.h>
+#include <semaphore.h>
+#include <fcntl.h>
+#include <ofono/dbus.h>
+#include <dbus/dbus-glib.h>
+#include <ofono/types.h>
+#include <ofono/plugin.h>
+#include <ofono/log.h>
+#include <ofono/history.h>
+#include "gdbus.h"
+#include "common.h"
Yikes, please sort the includes properly like all other code pieces in
oFono do.
First the system includes, the OFONO_API_SUBJECT_TO_CHANGE and the
required oFono includes, then GLib and/or GDBus, then private ones.
Also please only include the ones that are actually needed.
+#define HISTORY_FILE_PATH "/var/cache/callhistory/"
+#define HISTORY_FILE HISTORY_FILE_PATH"voicecallhistorydata"
+#define OFONO_MANAGER_PATH "/"
What is this for?
+#define PHONE_NUMBER_SIZE 64
+#define RECORD_SIZE 78
+#define HEADER_SIZE 16
+#define MAX_ITEMS 50
+
+#define TOTAL_SIZE (MAX_ITEMS * RECORD_SIZE + HEADER_SIZE)
+
+enum VOICE_CALL_TYPE {
+ OUTGOING = 0,
+ INCOMING,
+ MISSED
+};
+
+struct file_header {
+ int head;
+ int tail;
+ int unread;
+ unsigned int lastid;
+};
No. If these are files that are written to disk or memory or wherever
please use explicit sized variables like uint16 or similar.
You code is currently not compatible with 32-bit / 64-bit differences
and that is a bad idea.
+struct voice_history {
+ int id;
+ char lineid[PHONE_NUMBER_SIZE];
+ short int calltype;
+ time_t starttime;
+ time_t endtime;
+};
+
+struct shared_info {
+ struct file_header header;
+ void *dataMap;
+ sem_t mutex;
+ int temp_unread;
+ int temp_tail;
+};
Are any of these stored on disk or in memory? If so they need to use
proper agnostic types.
+static struct shared_info *shared_data;
+
+static int call_history_probe(struct ofono_history_context *context)
+{
+ DBG("History Probe for modem: %p", context->modem);
+ return 0;
+}
+
+static void call_history_remove(struct ofono_history_context *context)
+{
+ DBG("Example History Remove for modem: %p", context->modem);
+}
+
+static void clean_up()
+{
+ g_return_if_fail(shared_data != NULL);
+
+ sem_wait(&(shared_data->mutex));
+ /* unmap */
+ munmap(shared_data->dataMap, TOTAL_SIZE);
+ sem_post(&(shared_data->mutex));
+
+ /* remove semaphore */
+ if (sem_destroy(&(shared_data->mutex)) < 0)
+ perror("sem_destroy failed");
+
+ g_free(shared_data);
+}
+
+static int init_sem()
+{
+ unsigned int value = 1;
+ g_return_val_if_fail(shared_data != NULL, -1);
+
+ if (sem_init(&(shared_data->mutex), TRUE, value) < 0) {
+ perror("sem_init failed");
+ return -1;
+ }
+
+ return 0;
+}
Why is this here and not next to the _init and _exit functions where it
is actually used.
Also no perror stuff. We have connman_error. In addition the val_if_fail
checks are rather pointless since the plugin _init and _exit is
protected and guaranteed to be only called once.
+
+/* Start of DBus stuff */
+/* *************************************************************************
+ * Expose an interface, properties and signals for querying storage backend
+ * *************************************************************************/
+#define OFONO_CALL_HISTORY_INTERFACE OFONO_SERVICE".CallHistory"
+
+static void callhistory_emit_voice_history_changed(int userdata)
+{
+ int *valint = &userdata;
+ DBusConnection *conn = ofono_dbus_get_connection();
+ g_dbus_emit_signal(conn,
+ OFONO_MANAGER_PATH,
+ OFONO_CALL_HISTORY_INTERFACE,
+ "VoiceHistoryChanged",
+ DBUS_TYPE_UINT32,
+ valint,
+ DBUS_TYPE_INVALID);
+}
What is this &valint hack here. That doesn't even look like it would
work properly. And even if it does, it is totally unneeded. Please use
the proper dbus_uint32_t types and just assign it. Later on you can just
use the &val value.
+static DBusMessage
*call_history_set_voice_history_read(DBusConnection *conn,
+ DBusMessage *msg,
+ void *userdata)
+{
+ DBusMessage *reply;
+ DBusMessageIter iter;
+ g_return_val_if_fail((shared_data != NULL), NULL);
+
+ ofono_debug("Read ack received");
No direct usage of ofono_debug. Use DBG() instead.
+
+ reply = dbus_message_new_method_return(msg);
+
+ if (!reply)
+ return NULL;
Remove the empty line between the new_method and its error check.
+
+ dbus_message_iter_init_append(reply, &iter);
+ sem_wait(&(shared_data->mutex));
+ shared_data->header.unread = (shared_data->header).unread -
+ shared_data->temp_unread;
+ shared_data->header.tail = shared_data->temp_tail;
+ /* write to file */
+ memcpy(shared_data->dataMap, &(shared_data->header), HEADER_SIZE);
The extra () around shared_data->header are confusing. Create a temp
variable and use the header directly actually. And in case of &
operation you don't even need them.
+ sem_post(&(shared_data->mutex));
+ msync(shared_data->dataMap, TOTAL_SIZE, MS_ASYNC);
+ return reply;
+}
+
+static int init_header(void *dataPtr)
+{
+ int unread = 0;
Don't initialize variables until really needed. In this case it is
clearly not needed.
+ g_return_val_if_fail((shared_data != NULL), -1);
+ g_return_val_if_fail((dataPtr != NULL), -1);
What are these for. How can shared_data be NULL. How can dataPtr be
NULL.
And while at it dataPtr is not a valid variable name according to our
coding style. Please fix that.
+ unread = (shared_data->header).unread;
A temporary variable for the header would make this a bit better.
+
+ sem_wait(&(shared_data->mutex));
+ memcpy(&(shared_data->header), (struct file_header *)(dataPtr),
+ HEADER_SIZE);
No need for () around dataPtr. This is (struct file_header *) dataPtr.
> +
> + ofono_debug("Unread: %d, Header: %d, Tail: %d, serialno: %d\n",
> + unread,
> + (shared_data->header).head,
> + (shared_data->header).tail,
> + (shared_data->header).lastid);
> + sem_post(&(shared_data->mutex));
> + if (unread > 0)
> + callhistory_emit_voice_history_changed(unread);
> +
> + return 0;
> +}
> +
> +static void sync_mem_file(struct voice_history *vcall)
> +{
> + void *writeMap = NULL;
> + int unread = 0;
> +
> + g_return_if_fail(vcall != NULL);
> + g_return_if_fail(shared_data != NULL);
> + g_return_if_fail(shared_data->dataMap != NULL);
> +
> + sem_wait(&(shared_data->mutex));
> +
> + writeMap = shared_data->dataMap;
> + writeMap = writeMap + (shared_data->header).head;
> + vcall->id = (shared_data->header).lastid;
> + (shared_data->header).lastid++;
> + memcpy(writeMap, vcall, RECORD_SIZE);
> +
> + /* update header */
> + ((shared_data->header).unread)++;
> + (shared_data->header).head += RECORD_SIZE;
> +
> + if ((shared_data->header).lastid >= UINT_MAX)
> + (shared_data->header).lastid = 0;
> +
+ unread = (shared_data->header).unread;
> +
> + /* header reaches end of file */
> + if ((shared_data->header).head >= TOTAL_SIZE) {
> + /* reset head back to front */
> + (shared_data->header).head = HEADER_SIZE;
> +
> + ofono_debug("Header = %d, Tail = %d",
> + (shared_data->header).head,
> + (shared_data->header).tail);
> +
> + if (unread >= MAX_ITEMS)
> + ofono_error("No one reading history data");
> +
> + }
> +
> + memcpy(shared_data->dataMap, &(shared_data->header), HEADER_SIZE);
> + msync(shared_data->dataMap, TOTAL_SIZE, MS_ASYNC);
> +
> + sem_post(&(shared_data->mutex));
> +
> + callhistory_emit_voice_history_changed(unread);
> +}
> +
> +/**
> + * Creates file "callhistory"
> + */
> +static gboolean init_file()
> +{
> + int historyFile;
> + struct stat statbuf;
> + DIR *dirptr;
> + char *fill = NULL;
> +
> + g_return_val_if_fail((shared_data != NULL), FALSE);
> +
> + dirptr = opendir(HISTORY_FILE_PATH);
> +
> + if (!dirptr) {
> + ofono_debug("%s: doesnt exist", HISTORY_FILE_PATH);
> +
> + if (mkdir(HISTORY_FILE_PATH,
> + S_IRWXU|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH) < 0) {
> + perror("Error creating callhistory dir");
> + return FALSE;
> + }
> +
> + }
See all my comments above for this one. This is rather too complex in
what it tries to achieve. And it uses wrong perror, ofono_debug etc.
+
+ historyFile = open(HISTORY_FILE,
+ O_RDWR|O_CREAT|O_APPEND,
+ S_IRWXU);
+ if (historyFile < 0) {
+ perror("Open file failed");
+ return FALSE;
+ }
+
+ if (stat(HISTORY_FILE, &statbuf) < -1) {
+ perror("stat failed");
+ return FALSE;
+ }
+
+ ofono_debug("stat file: %d", (int) (statbuf.st_size));
+
+ if (statbuf.st_size == 0) {
+ int byteswritten = 0;
+ /* write header to the file */
+ (shared_data->header).head = HEADER_SIZE;
+ (shared_data->header).tail = (shared_data->header).head;
+ (shared_data->header).unread = 0;
+ ofono_debug("setting head: %d, tail: %d, unread: %d",
+ (shared_data->header).head,
+ (shared_data->header).tail,
+ (shared_data->header).unread);
+
+ /* fill the file with zeros */
+ ofono_debug("Trying to allocate %d size", TOTAL_SIZE);
+ fill = (char *) g_try_malloc0(TOTAL_SIZE);
+
+ if (!fill) {
+ ofono_debug("Error allocating init memory");
+ return FALSE;
+ }
+
+ /* Predefine file size and fill with some data */
+ bzero(fill, TOTAL_SIZE);
+ memcpy(fill, &(shared_data->header), HEADER_SIZE);
+
+ byteswritten = write(historyFile, fill, TOTAL_SIZE);
+
+ if (byteswritten < 0) {
+ ofono_debug("Error writing to file");
+ return FALSE;
+ }
+
+ g_free(fill);
+
+ statbuf.st_size = TOTAL_SIZE;
+ ofono_debug("History file %d size created\n", byteswritten);
+ }
+
+ /* create semaphor */
+ if (init_sem() < 0)
+ return FALSE;
+
+ sem_wait(&(shared_data->mutex));
+ shared_data->dataMap = mmap((caddr_t)NULL,
+ statbuf.st_size,
+ PROT_READ|PROT_WRITE|PROT_EXEC,
+ MAP_SHARED,
+ historyFile,
+ 0);
+ sem_post(&(shared_data->mutex));
+
+ if (shared_data->dataMap == (void *)(-1)) {
+ perror("mmap falied");
+ return FALSE;
+ }
+
+ close(historyFile);
+
+ init_header(shared_data->dataMap);
+
+ return TRUE;
+}
This at least contains a file descriptor leak.
> +/**
> + * call_history_call_ended:
> + * ofono calls this method with the call information
> + */
> +static void call_history_call_ended(struct ofono_history_context *context,
> + const struct ofono_call *call,
> + time_t start,
> + time_t end)
> +{
> + const char *from = "Unknown";
> + struct tm tmstart, tmend;
> + char sttime[128], endtime[128];
> + int fromlen = 0;
> + struct voice_history vcall;
> +
> + ofono_debug("Call Ended on modem: %p", context->modem);
> +
> + if (call->type != 0)
> + return;
> +
> + ofono_debug("Voice Call, %s",
> + call->direction ? "Incoming" : "Outgoing");
> +
> + if (call->direction)
> + vcall.calltype = INCOMING;
> + else
> + vcall.calltype = OUTGOING;
> +
> + if (call->clip_validity == 0)
> + from = phone_number_to_string(&call->phone_number);
> +
> + fromlen = strlen(from);
> + strcpy(vcall.lineid, from);
> +
> + vcall.lineid[fromlen] = '\0';
> +
> + vcall.starttime = start;
> + gmtime_r(&start, &tmstart);
> + strftime(sttime, 127, "%a, %d %b %Y %H:%M:%S %z", &tmstart);
> + sttime[127] = '\0';
> +
> + ofono_debug("StartTime: %s", sttime);
> +
> + vcall.endtime = end;
> + gmtime_r(&end, &tmend);
> + strftime(endtime, 127, "%a, %d %b %Y %H:%M:%S %z", &tmend);
> + endtime[127] = '\0';
> + ofono_debug("EndTime: %s", endtime);
> +
> + sync_mem_file(&vcall);
> +}
> +/**
> + * call_history_call_missed:
> + * ofono calls this method with the call information
> + */
> +static void call_history_call_missed(struct ofono_history_context *context,
> + const struct ofono_call *call,
> + time_t when)
> +{
> + const char *from = "Unknown";
> + struct tm mtime;
> + char sttime[128];
> + struct voice_history vcall;
> + int fromlen = 0;
> +
> + ofono_debug("Call Missed on modem: %p", context->modem);
> +
> + if (call->type != 0)
> + return;
> +
> + ofono_debug("Voice Call, Missed");
> + ofono_debug("Voice Call, %s",
> + call->direction ? "Incoming" : "Outgoing");
> +
> + vcall.calltype = MISSED;
> +
> + if (call->clip_validity == 0)
> + from = phone_number_to_string(&call->phone_number);
> +
> + fromlen = strlen(from);
> + strncpy(vcall.lineid, from, fromlen);
> + vcall.lineid[fromlen] = '\0';
> +
> + vcall.starttime = when;
> +
> + gmtime_r(&when, &mtime);
> + ofono_debug("From: %s", vcall.lineid);
> + strftime(sttime, 127, "%a, %d %b %Y %H:%M:%S %z", &mtime);
> + sttime[127] = '\0';
> + ofono_debug("Missed Time: %s", sttime);
> +
> + vcall.endtime = when;
> +
> + sync_mem_file(&vcall);
> +}
> +
> +static void read_data(struct voice_history *data, int temp)
> +{
> + void *readMap = NULL;
> + g_return_if_fail(shared_data != NULL);
> + g_return_if_fail(shared_data->dataMap != NULL);
> +
> + sem_wait(&(shared_data->mutex));
> + readMap = shared_data->dataMap+temp;
> +
> + /* read a chunk into *data */
> + memcpy(data, readMap, RECORD_SIZE);
> + shared_data->temp_unread++;
> + sem_post(&(shared_data->mutex));
> +}
> +
> +static DBusMessage *call_history_get_voice_history(DBusConnection *conn,
> + DBusMessage *msg,
> + void *userdata)
> +{
> + DBusMessage *reply;
> + DBusMessageIter iter;
> + DBusMessageIter array, struct_s;
> + int i;
> + char *lineid;
> + struct voice_history data;
> + int unread = 0;
+
+ reply = dbus_message_new_method_return(msg);
+
+ if (!reply)
+ return NULL;
> +
> + g_return_val_if_fail(shared_data != NULL, NULL);
> +
> + dbus_message_iter_init_append(reply, &iter);
> +
> + dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
> + DBUS_STRUCT_BEGIN_CHAR_AS_STRING
> + DBUS_TYPE_UINT32_AS_STRING
> + DBUS_TYPE_STRING_AS_STRING
> + DBUS_TYPE_UINT16_AS_STRING
> + DBUS_TYPE_INT32_AS_STRING
> + DBUS_TYPE_INT32_AS_STRING
> + DBUS_STRUCT_END_CHAR_AS_STRING
> + , &array);
> +
> + sem_wait(&(shared_data->mutex));
> + shared_data->temp_unread = 0;
> +
> + shared_data->temp_tail = (shared_data->header).tail;
+ unread = (shared_data->header).unread;
>
+ sem_post(&(shared_data->mutex));
> +
> + for (i = 0; i < unread; i++) {
> + read_data(&data, shared_data->temp_tail);
> + lineid = data.lineid;
> + dbus_message_iter_open_container(&array,
> + DBUS_TYPE_STRUCT,
> + NULL,
> + &struct_s);
> + dbus_message_iter_append_basic(&struct_s,
> + DBUS_TYPE_UINT32,
> + &(data.id));
> + dbus_message_iter_append_basic(&struct_s,
> + DBUS_TYPE_STRING,
> + &(lineid));
> + dbus_message_iter_append_basic(&struct_s,
> + DBUS_TYPE_UINT16,
> + &(data.calltype));
> + dbus_message_iter_append_basic(&struct_s,
> + DBUS_TYPE_INT32,
> + &(data.starttime));
> + dbus_message_iter_append_basic(&struct_s,
> + DBUS_TYPE_INT32,
> + &(data.endtime));
> + dbus_message_iter_close_container(&array, &struct_s);
> +
> + shared_data->temp_tail = shared_data->temp_tail + RECORD_SIZE;
> +
> + if (shared_data->temp_tail == TOTAL_SIZE) {
> + ofono_debug("End of Queue: %d",
> + shared_data->temp_tail);
> + /* reset back to front */
> + shared_data->temp_tail = HEADER_SIZE;
> + }
> +
> + }
> + dbus_message_iter_close_container(&iter, &array);
> +
> + return reply;
> +}
> +
> +static GDBusMethodTable call_history_methods[] = {
> + { "GetVoiceHistory", "", "a(usqii)",
call_history_get_voice_history },
> + { "SetVoiceHistoryRead", "", "",
call_history_set_voice_history_read },
> + { }
> +};
> +
> +static GDBusSignalTable call_history_signals[] = {
> + { "VoiceHistoryChanged", "u" },
> + {}
> +};
> +
> +/* End of DBus stuff */
> +static struct ofono_history_driver call_history_driver = {
> + .name = "callhistory",
> + .probe = call_history_probe,
> + .remove = call_history_remove,
> + .call_ended = call_history_call_ended,
> + .call_missed = call_history_call_missed,
> +};
> +
> +static int call_history_init(void)
> +{
> + DBusConnection *conn = ofono_dbus_get_connection();
> +
> + if (!shared_data) {
> + shared_data = g_try_new0(struct shared_info, 1);
> +
> + if (!shared_data)
> + return -ENOMEM;
> + }
Double init of plugin is not possible. Also you are leaking conn here in
case of errors.
+ if (!g_dbus_register_interface(conn,
+ OFONO_MANAGER_PATH,
+ OFONO_CALL_HISTORY_INTERFACE,
+ call_history_methods,
+ call_history_signals,
+ NULL, /* Properties */
+ shared_data, /* Userdata */
+ NULL)) /* Destroy func */
+ return -EIO;
+
+ if (!init_file())
+ return -ENOENT;
+
+ return ofono_history_driver_register(&call_history_driver);
Interface registration should be done in the probe callback of the
history driver and not here. You are trying to work around race
conditions that you otherwise wouldn't have to begin with.
+}
+
+static void call_history_exit(void)
+{
+ clean_up();
+ ofono_history_driver_unregister(&call_history_driver);
+}
Where do you unregister the D-Bus interface?
Regards
Marcel
1:17 p.m.
Hi Raji,
Design : Plugin uses memory mapped file for high performance
input/output operations. File is used as cyclic queue for storing or reading records, it
store s 50 records of 78bytes,hence fixed size file of 3916 bytes is used. File's
first 16 bytes were used for storing header, and next 3900 bytes for data. Header
structure has head pointer, tail pointer,unread and lastid. Head pointer points to next
slot for writing record into,tail for reading record, unread for number of history records
unread by client, lastid for id of the last record that is written.
Just a couple quick points. Memory mapped files do not really give any
benefit over read/write/lseek for small files (e.g. less than a few
pages) with small record sizes (e.g. less than a 4K page.) In fact
they're probably slower in these cases.
Locking:
Accessing Memory mapped file pointer , all header fields (head,tail,unread,lastid),
temp_unread,temp_tail are synchronized by a mutex. writing into memory file and reading
from memory file happen asynchronously, so mutex is used to protect the memory mapped file
pointer , header, temp_unread, temp_tail.
I'm totally confused by this one; you use no threads in your plugin (and
neither does oFono) and you have no external clients accessing the same
shared memory region. It seems to me that locking is completely
unnecessary.
Am I missing something here?
Regards,
-Denis
4:32 p.m.
Denis,
Thanks for the feedback, I have given explanation for locking below.
Thanks,
Raji
-----Original Message-----
From: Denis Kenzior [mailto:denkenz@gmail.com]
Sent: Thursday, September 16, 2010 8:17 PM
To: ofono(a)ofono.org
Cc: Bommaraju, Rajyalakshmi
Subject: Re: Description of Voice call history plugin patch
Hi Raji,
Design : Plugin uses memory mapped file for high performance
input/output operations. File is used as cyclic queue for storing or reading records, it
store s 50 records of 78bytes,hence fixed size file of 3916 bytes is used. File's
first 16 bytes were used for storing header, and next 3900 bytes for data. Header
structure has head pointer, tail pointer,unread and lastid. Head pointer points to next
slot for writing record into,tail for reading record, unread for number of history records
unread by client, lastid for id of the last record that is written.
Just a couple quick points. Memory mapped files do not really give any
benefit over read/write/lseek for small files (e.g. less than a few
pages) with small record sizes (e.g. less than a 4K page.) In fact
they're probably slower in these cases.
Locking:
Accessing Memory mapped file pointer , all header fields (head,tail,unread,lastid),
temp_unread,temp_tail are synchronized by a mutex. writing into memory file and reading
from memory file happen asynchronously, so mutex is used to protect the memory mapped file
pointer , header, temp_unread, temp_tail.
I'm totally confused by this one; you use no threads in your plugin (and
neither does oFono) and you have no external clients accessing the same
shared memory region. It seems to me that locking is completely
unnecessary.
Raji > History reads are client driven, can happen during the write operation. Same
with writing, if there is any read operation in progress, write operation needs to wait.
So the mutex will lock the memory information until read/write operation is done.
6:07 p.m.
Hi Raji,
> Design : Plugin uses memory mapped file for high performance
input/output operations. File is used as cyclic queue for storing or reading records, it
store s 50 records of 78bytes,hence fixed size file of 3916 bytes is used. File's
first 16 bytes were used for storing header, and next 3900 bytes for data. Header
structure has head pointer, tail pointer,unread and lastid. Head pointer points to next
slot for writing record into,tail for reading record, unread for number of history records
unread by client, lastid for id of the last record that is written.
Just a couple quick points. Memory mapped files do not really give any
benefit over read/write/lseek for small files (e.g. less than a few
pages) with small record sizes (e.g. less than a 4K page.) In fact
they're probably slower in these cases.
> Locking:
> Accessing Memory mapped file pointer , all header fields (head,tail,unread,lastid),
temp_unread,temp_tail are synchronized by a mutex. writing into memory file and reading
from memory file happen asynchronously, so mutex is used to protect the memory mapped file
pointer , header, temp_unread, temp_tail.
I'm totally confused by this one; you use no threads in your plugin (and
neither does oFono) and you have no external clients accessing the same
shared memory region. It seems to me that locking is completely
unnecessary.
Raji > History reads are client driven, can happen during the write operation. Same
with writing, if there is any read operation in progress, write operation needs to wait.
So the mutex will lock the memory information until read/write operation is done.
who reads from this file. The mutex is local to ofonod. If any other
program reads from that memory you need a proper memory lock. The local
mutex is not helping since ofonod is (on purpose) single threaded and
thus no locking is actually needed since read and write operations from
ofonod can't happen at the same time.
Regards
Marcel
6:16 p.m.
Hi Raji,
Callhistory is a plugin loaded by ofono for persisting voice call
history information in a disk file. Plugin exports dbus methods and signals as
org.ofono.callhistory interface. Whenever there is a voice call in the system, ofono calls
the plugin with the infomation (lineid,calltype,start time, end time) , plugin writes the
information into the file , increments the 'unread' field in the header and sends
out dbus signal with the number of unread records. Client (dialer) requests history
records by calling method "GetVoiceHistory" exposed on the callhistory
interface. After receiving information client calls the "SetVoiceHistoryRead" as
an acknowledgement to indicate end of transaction. The pointers in the header are adjusted
appropriately (explained clearly below).
Design : Plugin uses memory mapped file for high performance input/output operations.
File is used as cyclic queue for storing or reading records, it store s 50 records of
78bytes,hence fixed size file of 3916 bytes is used. File's first 16 bytes were used
for storing header, and next 3900 bytes for data. Header structure has head pointer, tail
pointer,unread and lastid. Head pointer points to next slot for writing record into,tail
for reading record, unread for number of history records unread by client, lastid for id
of the last record that is written.
I am not buying into this performance argument at all. How many calls to
you expect per second.
When the plugin is loaded by the ofono, it opens or creates a disk
file of 3916 bytes size and maps it into fixed size memory space. Plugin reads the header
of the file , if the 'unread' records is > 0 , a signal is sent out on the dbus
with the information of number of unread records.
Requesting data by client:
When the client calls "GetVoiceHistory" method,plugin uses temp_tail and
temp_unread variables to accomplish sending data to client. It copies tail pointer stored
in the header into temporary tail pointer ,starts to read history records one at a time
from the location pointed by the temp tail pointer , copies the into dbus structure,
increments the temp tail pointer by size of the history record (78 bytes) and reads data
from the next record location , this is repeated until it reaches head pointer. when all
the unread records are read then the dbus structure is sent to the client. Once the data
received, client needs to "SetVoiceHistoryRead" method to indicate that it
received the data. Plugin will update the tail pointer with the temp tail pointer and
unread records number is decremented by temp_unread. Using temp_tail and temp_unread
variables protect from modifying the header if there is any loss in data delivery. If
client doesnt call 'SetVoiceHistoryRead' then the actua
l header pointers wont be updated in the file.
Writing record into the file:
When Ofono calls the plugin with call information (phone number, call type, call start
time , call end time), plugin increaments the lastid to create record id for this record
,writes the record in the location pointed by head, and the head pointer is incremented by
size of record (78 bytes), "unread" variable is incremented by 1. If the head
reaches end of file (3916) then it is reset it back to begining of the data portion of the
file (16 bytes). A dbus signal is sent out with number of current unread records. Plugin
keeps writing the records in circular queue.
This sounds all way too complicated. And the D-Bus API design is
actually racy since there is no clear access control on the history
information.
I think the proper solution is to create a call history plugin that
creates a D-Bus interface for a call history agent.
That way we know when an agent is present to consume history information
and only if none is present we have to cache it on disk. And once an
agent becomes available flush the cached information.
And the cache should be just be done with a simply appending to a file
and then truncating it after reading it.
Regards
Marcel
4 a.m.
Please see my comments below.
-----Original Message-----
From: ofono-bounces(a)ofono.org [mailto:ofono-bounces@ofono.org] On Behalf Of Marcel
Holtmann
Sent: Friday, September 17, 2010 1:17 AM
To: ofono(a)ofono.org
Subject: Re: Description of Voice call history plugin patch
Marcel,
Callhistory is a plugin loaded by ofono for persisting voice call
history information in a disk file. Plugin exports dbus methods and signals as
org.ofono.callhistory interface. Whenever there is a voice call in the system, ofono calls
the plugin with the infomation (lineid,calltype,start time, end time) , plugin writes the
information into the file , increments the 'unread' field in the header and sends
out dbus signal with the number of unread records. Client (dialer) requests history
records by calling method "GetVoiceHistory" exposed on the callhistory
interface. After receiving information client calls the "SetVoiceHistoryRead" as
an acknowledgement to indicate end of transaction. The pointers in the header are adjusted
appropriately (explained clearly below).
Design : Plugin uses memory mapped file for high performance input/output operations.
File is used as cyclic queue for storing or reading records, it store s 50 records of
78bytes,hence fixed size file of 3916 bytes is used. File's first 16 bytes were used
for storing header, and next 3900 bytes for data. Header structure has head pointer, tail
pointer,unread and lastid. Head pointer points to next slot for writing record into,tail
for reading record, unread for number of history records unread by client, lastid for id
of the last record that is written.
I am not buying into this performance argument at all. How many calls to
you expect per second.
Raji >> Even if there are not many calls, operating system takes care of flushing
the file to disk if there is any ofono crash.
When the plugin is loaded by the ofono, it opens or creates a disk
file of 3916 bytes size and maps it into fixed size memory space. Plugin reads the header
of the file , if the 'unread' records is > 0 , a signal is sent out on the dbus
with the information of number of unread records.
Requesting data by client:
When the client calls "GetVoiceHistory" method,plugin uses temp_tail and
temp_unread variables to accomplish sending data to client. It copies tail pointer stored
in the header into temporary tail pointer ,starts to read history records one at a time
from the location pointed by the temp tail pointer , copies the into dbus structure,
increments the temp tail pointer by size of the history record (78 bytes) and reads data
from the next record location , this is repeated until it reaches head pointer. when all
the unread records are read then the dbus structure is sent to the client. Once the data
received, client needs to "SetVoiceHistoryRead" method to indicate that it
received the data. Plugin will update the tail pointer with the temp tail pointer and
unread records number is decremented by temp_unread. Using temp_tail and temp_unread
variables protect from modifying the header if there is any loss in data delivery. If
client doesnt call 'SetVoiceHistoryRead' then the act
ua
l header pointers wont be updated in the file.
Writing record into the file:
When Ofono calls the plugin with call information (phone number, call type, call start
time , call end time), plugin increaments the lastid to create record id for this record
,writes the record in the location pointed by head, and the head pointer is incremented by
size of record (78 bytes), "unread" variable is incremented by 1. If the head
reaches end of file (3916) then it is reset it back to begining of the data portion of the
file (16 bytes). A dbus signal is sent out with number of current unread records. Plugin
keeps writing the records in circular queue.
This sounds all way too complicated. And the D-Bus API design is
actually racy since there is no clear access control on the history
information.
I think the proper solution is to create a call history plugin that
creates a D-Bus interface for a call history agent.
That way we know when an agent is present to consume history information
and only if none is present we have to cache it on disk. And once an
agent becomes available flush the cached information.
And the cache should be just be done with a simply appending to a file
and then truncating it after reading it.
Raji >> Marcel, James Ketrenos discussed this design with you back in april. I will
send you the thread on that. I was under impression that you both agreed on this design.
Thanks for feedback,
Raji
_______________________________________________
ofono mailing list
ofono(a)ofono.org
http://lists.ofono.org/listinfo/ofono
9:41 a.m.
Hi Raji,
> Callhistory is a plugin loaded by ofono for persisting voice
call history information in a disk file. Plugin exports dbus methods and signals as
org.ofono.callhistory interface. Whenever there is a voice call in the system, ofono calls
the plugin with the infomation (lineid,calltype,start time, end time) , plugin writes the
information into the file , increments the 'unread' field in the header and sends
out dbus signal with the number of unread records. Client (dialer) requests history
records by calling method "GetVoiceHistory" exposed on the callhistory
interface. After receiving information client calls the "SetVoiceHistoryRead" as
an acknowledgement to indicate end of transaction. The pointers in the header are adjusted
appropriately (explained clearly below).
>
> Design : Plugin uses memory mapped file for high performance input/output
operations. File is used as cyclic queue for storing or reading records, it store s 50
records of 78bytes,hence fixed size file of 3916 bytes is used. File's first 16 bytes
were used for storing header, and next 3900 bytes for data. Header structure has head
pointer, tail pointer,unread and lastid. Head pointer points to next slot for writing
record into,tail for reading record, unread for number of history records unread by
client, lastid for id of the last record that is written.
I am not buying into this performance argument at all. How many calls to
you expect per second.
Raji >> Even if there are not many calls, operating system takes care of flushing
the file to disk if there is any ofono crash.
what is the difference of calling msync() compared to fdatasync() here.
I highly doubt that we have any issue in either case. A Linux IO and
filesystem expert could answer this, but in the end both of them end in
IO activity. And once you write to it you need to sync to ensure they
are actually written to disk.
This is also not really direkt IO vs memory mapped file. I couldn't care
less in the end. However this is about the complexity that is needed for
maintaining a round-robin file. I am failing to see the need for it.
> When the plugin is loaded by the ofono, it opens or creates a
disk file of 3916 bytes size and maps it into fixed size memory space. Plugin reads the
header of the file , if the 'unread' records is > 0 , a signal is sent out on
the dbus with the information of number of unread records.
>
> Requesting data by client:
> When the client calls "GetVoiceHistory" method,plugin uses temp_tail and
temp_unread variables to accomplish sending data to client. It copies tail pointer stored
in the header into temporary tail pointer ,starts to read history records one at a time
from the location pointed by the temp tail pointer , copies the into dbus structure,
increments the temp tail pointer by size of the history record (78 bytes) and reads data
from the next record location , this is repeated until it reaches head pointer. when all
the unread records are read then the dbus structure is sent to the client. Once the data
received, client needs to "SetVoiceHistoryRead" method to indicate that it
received the data. Plugin will update the tail pointer with the temp tail pointer and
unread records number is decremented by temp_unread. Using temp_tail and temp_unread
variables protect from modifying the header if there is any loss in data delivery. If
client doesnt call 'SetVoiceHistoryRead' then the act
ua
> l header pointers wont be updated in the file.
>
> Writing record into the file:
> When Ofono calls the plugin with call information (phone number, call type, call
start time , call end time), plugin increaments the lastid to create record id for this
record ,writes the record in the location pointed by head, and the head pointer is
incremented by size of record (78 bytes), "unread" variable is incremented by 1.
If the head reaches end of file (3916) then it is reset it back to begining of the data
portion of the file (16 bytes). A dbus signal is sent out with number of current unread
records. Plugin keeps writing the records in circular queue.
This sounds all way too complicated. And the D-Bus API design is
actually racy since there is no clear access control on the history
information.
I think the proper solution is to create a call history plugin that
creates a D-Bus interface for a call history agent.
That way we know when an agent is present to consume history information
and only if none is present we have to cache it on disk. And once an
agent becomes available flush the cached information.
And the cache should be just be done with a simply appending to a file
and then truncating it after reading it.
Raji >> Marcel, James Ketrenos discussed this design with you back in april. I will
send you the thread on that. I was under impression that you both agreed on this design.
Just get one thing straight here. Design is design and implementation is
something totally different.
If we use a memory mapped file aside, the whole D-Bus API is pretty much
racy in my eyes since you can not track the history consumer application
use. You don't know when it is running or not. You don't know what to
cache or not. You can not handle crashing of the history consumer
application.
This current implementation is rather complicated to what it is suppose
to achieve. We have two things here, one is the general questions about
on how the D-Bus API of this should actually work. And second about your
current implementation.
The current API has limitations as listed above. These needs to be
addressed and clear way forward seems an agent based model.
The implementation has some flaws. For starters the locking that you
still haven't explained why it is correct. And I agree with Denis here
that I don't see what it is doing. Second is the file system format that
can't work properly on different architectures. You need to use properly
sized variable and ensure the structures memory layout is the same. I
even forgot to mention big endian vs little endian.
Without these fixed, I am not even considering this for upstream.
Regards
Marcel
4:52 a.m.
What is the call history agent idea? Can you explain clearly?
When the client is up, is the history plug-in sends data instead of any signal? How about
acknowledgement for data delivery especially when we are sending data cached on disk?
If we forefeit the memory mapped idea, is the plugin writing & reading directly from
the file?
I will send out a simple algorithm once I understand what you are proprosing and once we
are on the same page I will implement that and send you the patch.
Can I send the patch with is developer tested code? Does it need to be tested by QA?
Thanks & regards,
Raji
-----Original Message-----
From: ofono-bounces(a)ofono.org [mailto:ofono-bounces@ofono.org] On Behalf Of Marcel
Holtmann
Sent: Friday, September 17, 2010 1:17 AM
To: ofono(a)ofono.org
Subject: Re: Description of Voice call history plugin patch
Hi Raji,
Callhistory is a plugin loaded by ofono for persisting voice call
history information in a disk file. Plugin exports dbus methods and signals as
org.ofono.callhistory interface. Whenever there is a voice call in the system, ofono calls
the plugin with the infomation (lineid,calltype,start time, end time) , plugin writes the
information into the file , increments the 'unread' field in the header and sends
out dbus signal with the number of unread records. Client (dialer) requests history
records by calling method "GetVoiceHistory" exposed on the callhistory
interface. After receiving information client calls the "SetVoiceHistoryRead" as
an acknowledgement to indicate end of transaction. The pointers in the header are adjusted
appropriately (explained clearly below).
Design : Plugin uses memory mapped file for high performance input/output operations.
File is used as cyclic queue for storing or reading records, it store s 50 records of
78bytes,hence fixed size file of 3916 bytes is used. File's first 16 bytes were used
for storing header, and next 3900 bytes for data. Header structure has head pointer, tail
pointer,unread and lastid. Head pointer points to next slot for writing record into,tail
for reading record, unread for number of history records unread by client, lastid for id
of the last record that is written.
I am not buying into this performance argument at all. How many calls to
you expect per second.
When the plugin is loaded by the ofono, it opens or creates a disk
file of 3916 bytes size and maps it into fixed size memory space. Plugin reads the header
of the file , if the 'unread' records is > 0 , a signal is sent out on the dbus
with the information of number of unread records.
Requesting data by client:
When the client calls "GetVoiceHistory" method,plugin uses temp_tail and
temp_unread variables to accomplish sending data to client. It copies tail pointer stored
in the header into temporary tail pointer ,starts to read history records one at a time
from the location pointed by the temp tail pointer , copies the into dbus structure,
increments the temp tail pointer by size of the history record (78 bytes) and reads data
from the next record location , this is repeated until it reaches head pointer. when all
the unread records are read then the dbus structure is sent to the client. Once the data
received, client needs to "SetVoiceHistoryRead" method to indicate that it
received the data. Plugin will update the tail pointer with the temp tail pointer and
unread records number is decremented by temp_unread. Using temp_tail and temp_unread
variables protect from modifying the header if there is any loss in data delivery. If
client doesnt call 'SetVoiceHistoryRead' then the act
ua
l header pointers wont be updated in the file.
Writing record into the file:
When Ofono calls the plugin with call information (phone number, call type, call start
time , call end time), plugin increaments the lastid to create record id for this record
,writes the record in the location pointed by head, and the head pointer is incremented by
size of record (78 bytes), "unread" variable is incremented by 1. If the head
reaches end of file (3916) then it is reset it back to begining of the data portion of the
file (16 bytes). A dbus signal is sent out with number of current unread records. Plugin
keeps writing the records in circular queue.
This sounds all way too complicated. And the D-Bus API design is
actually racy since there is no clear access control on the history
information.
I think the proper solution is to create a call history plugin that
creates a D-Bus interface for a call history agent.
That way we know when an agent is present to consume history information
and only if none is present we have to cache it on disk. And once an
agent becomes available flush the cached information.
And the cache should be just be done with a simply appending to a file
and then truncating it after reading it.
Raji > What is the call history agent idea? Can you explain clearly?
When the client is up, is the history plug-in sends data instead of any signal? How about
acknowledgement for data delivery especially when we are sending data cached on disk?
If we forefeit the memory mapped idea, is the plugin writing & reading directly from
the file?
I will send out a simple algorithm once I understand what you are proprosing and once we
are on the same page I will implement that and send you the patch.
Can I send the patch with is developer tested code? Does it need to be tested by QA?
Thanks & regards,
Raji
_______________________________________________
ofono mailing list
ofono(a)ofono.org
http://lists.ofono.org/listinfo/ofono
12:37 p.m.
Hi Raji,
just a friendly reminder here to not do top posting. We are not allowing
this on an open source mailing list. It is a good way of actually
getting ignored ;)
What is the call history agent idea? Can you explain clearly?
It is like the SIM Toolkit agent, the PIN agent in BlueZ. Read the
documentation and the examples. It is really simple.
In the end an agent is a callback interface inside the UI or an
application outside of oFono. It is were oFono asks for further
information or were it relays information to. In this case the call
history information.
The agent design has the advantage the oFono knows when an agent is
present and when not. So until no agent is present it can spool the
history to disk. And once an agent becomes available replay it to the
agent.
When the client is up, is the history plug-in sends data instead of
any signal? How about acknowledgement for data delivery especially when we are sending
data cached on disk?
The agent are like callbacks. They are D-Bus method calls. The
acknowledgment is implicit here with the method return or error in case
of problems.
If we forefeit the memory mapped idea, is the plugin writing &
reading directly from the file?
I personally would just write to the file. However if you wanna do
memory mapped file in round-robin fashion, then that works as well.
However in case of round-robin files you need to think about the memory
layout of your file.
I will send out a simple algorithm once I understand what you are
proprosing and once we are on the same page I will implement that and send you the patch.
Send a D-Bus API document first. See the SIM Toolkit API for an example.
Can I send the patch with is developer tested code? Does it need to
be tested by QA?
You can even send non-working or non-complete code for review at any
time. Just mark them as RFC (request for comments).
Regards
Marcel
4380
days inactive
4384
days old
10 comments
4 participants
participants (4)
-
Bommaraju, Rajyalakshmi -
Denis Kenzior -
Marcel Holtmann -
Rajyalakshmi Bommaraju