Re: [PATCH 2/3] hdlc: handle wrapped buffers

2010-04-17 Thread Marcel Holtmann
Hi Kristen,

 ---
  gatchat/gathdlc.c |   75 
  1 files changed, 40 insertions(+), 35 deletions(-)
 
 diff --git a/gatchat/gathdlc.c b/gatchat/gathdlc.c
 index c5c02cf..13e15a1 100644
 --- a/gatchat/gathdlc.c
 +++ b/gatchat/gathdlc.c
 @@ -59,11 +59,12 @@ static void new_bytes(GAtHDLC *hdlc)
  {
   unsigned int len = ring_buffer_len(hdlc-read_buffer);
   unsigned char *buf = ring_buffer_read_ptr(hdlc-read_buffer, 0);
 + unsigned int wrap = ring_buffer_len_no_wrap(hdlc-read_buffer);
   unsigned char val;
   unsigned int pos = 0;
  
   while (pos  len) {
 - if (buf[pos] == 0x7e) {
 + if (*buf == 0x7e) {
   if (hdlc-receive_func  hdlc-decode_offset  2 
   hdlc-decode_fcs == 0xf0b8) {
   hdlc-receive_func(hdlc-decode_buffer,

please do this patch fist and send it separately. Intermixing it with
the ACCM support makes it hard to review.

So I like to see the patch series this way:

1) Handle ringbuffer wrapping in HDLC
2) Add recording support for HDLC
3) Add ACCM support to HDLC
4) Port PPP to use HDLC

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH] ppp: implement MRU option

2010-04-22 Thread Marcel Holtmann
Hi Kristen,

 If the peer requests a MRU option, set the mtu for the network
 phase.  When we are in link establishment phase, we should
 continue to behave as if no option has been set and the peer
 should use the default MRU.
 
 This option is required for the Huawei E160G modem.

can you also send a pppdump from this modem setup with a few IP packets
in for reference. I like to see what the Huawei modem does on LCP or
IPCP setup.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 1/3] atmodem: add signal strength support for huawei

2010-04-23 Thread Marcel Holtmann
Hi Kalle,

 Huawei doesn't support CIND indications, so use CSQ instead. But naturally
 the response from modem is not according to standard:

is this true for all Huawei modems?

 ofonod[6401]:  \r\n^BOOT:38645652,0,0,0,87\r\n
 ofonod[6401]:  \r\n^RSSI:23\r\n
 
 Support for this format is not yet implemented.

You see these on the same TTY? Normally they happen on the second TTY.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 2/3] atmodem: add a huawei hack to check attachment status

2010-04-23 Thread Marcel Holtmann
Hi Kalle,

 Huawei doesn't seem to send CGREG notifications (or I wasn't able to
 properly configure the modem to do it), so add an ugly to hack poll the
 state with CGATT.

shouldn't be polling AT+CGREG? the better way to do this?

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 3/3] huawei: add gprs context

2010-04-23 Thread Marcel Holtmann
Hi Kalle,

 Tested with Huawei E1552 HSDPA USB stick using a finnish Saunalahti prepaid
 SIM.
 ---
 
  plugins/huawei.c |   10 ++
  1 files changed, 10 insertions(+), 0 deletions(-)
 
 diff --git a/plugins/huawei.c b/plugins/huawei.c
 index 90fdcf0..fd1528f 100644
 --- a/plugins/huawei.c
 +++ b/plugins/huawei.c
 @@ -41,6 +41,8 @@
  #include ofono/gprs.h
  #include ofono/voicecall.h
  #include ofono/log.h
 +#include ofono/gprs.h
 +#include ofono/gprs-context.h
  
  #include drivers/atmodem/vendor.h
  
 @@ -178,11 +180,19 @@ static void huawei_pre_sim(struct ofono_modem *modem)
  static void huawei_post_sim(struct ofono_modem *modem)
  {
   struct huawei_data *data = ofono_modem_get_data(modem);
 + struct ofono_gprs_context *gc;
 + struct ofono_gprs *gprs;
  
   DBG(%p, modem);
  
   ofono_netreg_create(modem, OFONO_VENDOR_HUAWEI, atmodem, data-chat);
   ofono_sms_create(modem, OFONO_VENDOR_QUALCOMM_MSM, atmodem, 
 data-chat);
 +
 + gprs = ofono_gprs_create(modem, 0, atmodem, data-chat);
 + gc = ofono_gprs_context_create(modem, 0, atmodem, data-chat);
 +
 + if (gprs  gc)
 + ofono_gprs_add_context(gprs, gc);
  }

this patch is obviously correct if the PPP part and generic GPRS context
support is working correct. I haven't had time to verify that yet. So
far we only used gsmdial for testing.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH] Fix: transaction id usage in gisi/server.c

2010-04-23 Thread Marcel Holtmann
Hi Remi,

  Casting to or from void is not necessary.
  
  Oh, it is. If a struct sockaddr_pn structure is casted directly to
  struct sockaddr, gcc will issue alignment warning on ARM.
 
 The cast-align warning is a can of false positives. First that warning
 should probably not be enabled. It does nothing on x86, and it's a waste of
 time with ARM and other pointer-picky platforms.

I don't have an ARM platform easy available to test this, but this can
be fixed by just memcpy the data structures.

 Most importantly, using -Werror _by_default_ is a _damn_idiotic_ idea. Been
 there, done that. One cannot know what weird warning any given compiler or
 version thereof will ever return. And then I don't need to mention bugs
 in header files from underlying components. Even changing GCC optimization
 levels can yield different conflicting warnings. That's known to happen for
 uninitialized values detection, which relies on optimization-dependent code
 analysis as an example.

The rant about -Werror doesn't help. We are using it to catch nasty
constructs that might fail on any given platform. We have used -Werror
inside BlueZ and ConnMan for a long time now. Yes, sometimes new GCC
warnings come up and we fix that, but it was always good to have these.
And yes, it is true that you need to use -O2 to enable a bunch of these
warnings.

So in conclusion, the -Werror stays and we need to fix the code to
compile cleanly on any given platform. It is possible, so lets get this
done. Feel free to post the warnings on the mailing list and I am more
than happy to look into it.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: DUN client for oFono and BlueZ

2010-04-26 Thread Marcel Holtmann
Hi Gustavo,

 I'm starting the DUN Client implementation for the Linux Stack. DUN is
 the Bluetooth dial-up network profile. It makes possible share internet
 connection between two Bluetooth devices. That is my Google Summer of Code
 project for this year.
 
 Here follows a simple, and possible not complete, roadmap:
 
 1. Put send_method_call() and send_method_call_with_reply() on gdbus: those
 functions are very useful for DBus operations. You can send a DBus message
 just calling them with the right parameters. Patch for that work already 
 on the mailing list.
 
 2. Create a bluetooth.c file with the bluetooth helpers. oFono plugins for 
 HFP,
 DUN and SAP will be able to share the same bluetooth code to access BlueZ
 stuff. This is a ongoing work.
 
 3. plugin/dun.c prototype. A simple prototype for the DUN client.
 
 4. Agent server on BlueZ. This one is very similar to the HFP Agent server. At
 the end of the DUN agent project I plan to merge the both agent servers. SAP
 will take advantage of that merge too.
 
 5. oFono DUN agent. Implement the agent handling for DUN.
 
 6. AT command parser and PPP stack integration with DUN. The biggest task,
 where the core of the project is.

this is pretty clear. oFono carries the GAtChat library that comes with
AT command and PPP support. So oFono should carry the DUN client.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH] Add send_method_call to g_dbus

2010-04-26 Thread Marcel Holtmann
Hi Gustavo,

 Puting send_method_call and send_method_call_with_reply on g_dbus will
 avoid some code duplication and will make things easier mainly for the
 Bluetooth plugins (HFP, DUN, SAP) inside oFono.
 ---
  gdbus/gdbus.h  |   12 
  gdbus/object.c |   81 +
  plugins/hfp.c  |  154 +--
  3 files changed, 130 insertions(+), 117 deletions(-)

first of all, we don't intermix gdbus patches with other changes.
Remember that these commits have to be applied to BlueZ and ConnMan as
well.

 diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
 index 47e18cf..ac488c5 100644
 --- a/gdbus/gdbus.h
 +++ b/gdbus/gdbus.h
 @@ -106,12 +106,24 @@ DBusMessage *g_dbus_create_error_valist(DBusMessage 
 *message, const char *name,
  DBusMessage *g_dbus_create_reply(DBusMessage *message, int type, ...);
  DBusMessage *g_dbus_create_reply_valist(DBusMessage *message,
   int type, va_list args);
 +DBusMessage *g_dbus_create_method_call(const char *dest, const char *path,
 + const char *interface, const char *method,
 + int type, va_list args);
  
  gboolean g_dbus_send_message(DBusConnection *connection, DBusMessage 
 *message);
  gboolean g_dbus_send_reply(DBusConnection *connection,
   DBusMessage *message, int type, ...);
  gboolean g_dbus_send_reply_valist(DBusConnection *connection,
   DBusMessage *message, int type, va_list args);
 +gboolean g_dbus_send_method_call(DBusConnection *connection, const char 
 *dest,
 + const char *path, const char *interface,
 + const char *method, int type, ...);
 +gboolean g_dbus_send_method_call_with_reply(DBusConnection *connection,
 + const char *dest, const char *path,
 + const char *interface, const char *method,
 + DBusPendingCallNotifyFunction cb,
 + void *user_data, DBusFreeFunction free_func,
 + int timeout, int type, ...);

Is there any real reason to export send_method_call and
create_method_call? What are these good for and what are your expected
users of these?

I think that a g_dbus_method_call which is always async and take a
pending call notifier is just enough.

All the other helpers are more for callbacks that have to respond to
messages than for client usage.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 2/3] hdlc: handle wrapped buffers

2010-04-26 Thread Marcel Holtmann
Hi Kristen,

 So I like to see the patch series this way:
 
 1) Handle ringbuffer wrapping in HDLC
 2) Add recording support for HDLC
 3) Add ACCM support to HDLC
 4) Port PPP to use HDLC

so I have done 1) now in a way that it is efficient and doesn't become
the bottle-neck for PPP. What is the status of the other 3 tasks?

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 3/4] gatchat: Emit notification when command is sent to modem.

2010-04-29 Thread Marcel Holtmann
Hi Denis,

  +/*!
  + * Same as g_at_chat_send but with an ability to return a notification the
  + * moment the command finally leaves the queue and is submitted to lower
  + * layer.
  + *
  + * This is useful for cases where the modem's response time needs to be
  + * measured, assuming that the lower layers processing time is shorter
  + * than the minimum accuracy needed.
  + */
  +guint g_at_chat_send_with_callback(GAtChat *chat, const char *cmd,
  +   const char **valid_resp,
  +   GAtSubmitNotifyFunc sent,
  +   GAtResultFunc func,
  +   gpointer user_data,
  +   GDestroyNotify notify);
  +
 
 So I'm fine with the implementation but the name needs work.  Can we use 
 g_at_chat_send_with_submit_notify? Or maybe g_at_chat_send_full, similar to 
 how GLib does it.
 
 Perhaps enabling submit_notification for a given command after it has been 
 submitted with g_at_chat_send?
 
 e.g. g_at_chat_set_submit_notify(GAtChat *chat, guint command, 
 GAtSubmitNotifyFunc sent, gpointer user_data, GDestroyNotify notify);

I am not a huge fan of the _full() stuff, but it is actually pretty nice
for the cases where 99% of users don't care. And this seems to be one of
them. The send_with_submit_notify() is way too long.

Maybe g_at_chat_send_and_notify() is an acceptable simple version for
this or just g_at_chat_submit() and g_at_chat_send() to keep these
versions apart.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH v2 1/6] atmodem: create struct atmodem_gprs

2010-04-30 Thread Marcel Holtmann
Hi Kalle,

 This is need to to provide more chat channels to atmodem gprs module.
 ---
 
  drivers/atmodem/gprs.c |7 ---
  drivers/atmodem/gprs.h |   29 +
  plugins/hso.c  |8 +++-
  plugins/mbm.c  |7 ++-
  plugins/palmpre.c  |8 +++-
  plugins/phonesim.c |7 ++-
  plugins/ste.c  |7 ++-
  7 files changed, 65 insertions(+), 8 deletions(-)
  create mode 100644 drivers/atmodem/gprs.h

I don't really like this. You only need to catch the CGREG anyway so we
might just wanna add some indication method. Similar to what we have for
network registration with ofono_netreg_status_notify.

Then the messed up CGREG from Huawei modem doesn't dictate the whole
simple one GAtChat approach. It is really their fault with no sending
these on the proper AT command channel. No need to make other drivers
suffer.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH v2 5/6] atmodem: add signal strength support for huawei

2010-04-30 Thread Marcel Holtmann
Hi Kalle,

 Huawei doesn't support CIND indications, instead it uses some non-standard
 RSSI messages:
 
 ofonod[6401]:  \r\n^RSSI:23\r\n
 
 Add support for this to atmodem.

please use ofono_netreg_strength_notify for this.

The Huawei specific details with this stupid extra channel need to be
handled inside the Huawei plugin. Cluttering the core with it is not a
good idea.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 3/4] gatchat: Emit notification when command is sent to modem.

2010-04-30 Thread Marcel Holtmann
Hi Denis,

+/*!
+ * Same as g_at_chat_send but with an ability to return a notification
the + * moment the command finally leaves the queue and is submitted to
lower + * layer.
+ *
+ * This is useful for cases where the modem's response time needs to
be + * measured, assuming that the lower layers processing time is
shorter + * than the minimum accuracy needed.
+ */
+guint g_at_chat_send_with_callback(GAtChat *chat, const char *cmd,
+   const char **valid_resp,
+   GAtSubmitNotifyFunc sent,
+   GAtResultFunc func,
+   gpointer user_data,
+   GDestroyNotify notify);
+
  
   So I'm fine with the implementation but the name needs work.  Can we use
   g_at_chat_send_with_submit_notify? Or maybe g_at_chat_send_full, similar
   to how GLib does it.
  
   Perhaps enabling submit_notification for a given command after it has
   been submitted with g_at_chat_send?
  
   e.g. g_at_chat_set_submit_notify(GAtChat *chat, guint command,
   GAtSubmitNotifyFunc sent, gpointer user_data, GDestroyNotify notify);
  
  I am not a huge fan of the _full() stuff, but it is actually pretty nice
  for the cases where 99% of users don't care. And this seems to be one of
  them. The send_with_submit_notify() is way too long.
 
 I'm not a fan of _full either, however it is a precedent, so might as well be 
 a candidate.
 
  
  Maybe g_at_chat_send_and_notify() is an acceptable simple version for
  this or just g_at_chat_submit() and g_at_chat_send() to keep these
  versions apart.
 
 In my opinion send_and_notify does not convey enough information about what 
 the function is trying to do.  _submit is even less clear.  API should be 
 very 
 clear on its intent just from the function name without needing to consult 
 documentation.
 
 Out of all these so far my vote is on send_full just because it is familiar 
 to 
 folks using GLib...  We might have to cut down some of the parameters to 
 _send 
 as well (like GDestroyNotify argument) if we introduce send_full.

then lets do send_full() and move the destroy function parameter to the
full version.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 3/4] gatchat: Emit notification when command is sent to modem.

2010-05-04 Thread Marcel Holtmann
Hi Andrew,

  So I'm fine with the implementation but the name needs work.  Can we use
  g_at_chat_send_with_submit_notify? Or maybe g_at_chat_send_full, similar to
  how GLib does it.
 
  Perhaps enabling submit_notification for a given command after it has been
  submitted with g_at_chat_send?
 
  e.g. g_at_chat_set_submit_notify(GAtChat *chat, guint command,
  GAtSubmitNotifyFunc sent, gpointer user_data, GDestroyNotify notify);
 
  I am not a huge fan of the _full() stuff, but it is actually pretty nice
  for the cases where 99% of users don't care. And this seems to be one of
  them. The send_with_submit_notify() is way too long.
 
  Maybe g_at_chat_send_and_notify() is an acceptable simple version for
  this or just g_at_chat_submit() and g_at_chat_send() to keep these
  versions apart.
 
 Here's a patch to add a g_at_chat_set_submit_notify function that
 modifies an already submitted command.  Removing the destroy callback
 from g_at_chat_send would require changing all the many uses of it.

the conclusion was to have g_at_chat_send_full with submit and destroy
notifier. And g_at_chat_send without the destroy notifier.

Yes, we might have to touch a lot of code, but that is fine. That is one
of the reasons why GAtChat is not a separate (yet).

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH] add vid/pid for Dell 5541 and 5542

2010-05-04 Thread Marcel Holtmann
Hi Torgny,

 ---
  plugins/ofono.rules |4 
  1 files changed, 4 insertions(+), 0 deletions(-)

patch has been applied. Thanks.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: New TODO process

2010-05-19 Thread Marcel Holtmann
Hi Kalle,

  oFono project has been getting frequent requests for project status and 
  roadmap details recently.  In an effort to add some visibility to these 
  aspects 
  we decided to implement a new process.
 
  The idea is quite simple: identify the gaps in oFono features and try to 
  estimate the relative complexity and priority of implementing these 
  features.  
  The list can be used to quickly understand the relative status of oFono 
  (what 
  works, what doesn't work.)  In addition, people interested in contributing 
  to 
  the oFono project can use this list for an idea which tasks/features they 
  can 
  contribute.
 
 I'm sure all managers are excited about this :)
 
 I also think it's a good idea. Just keeping the document up-to-date is
 the challenging part.
 
 What about hardware/modem support, any plans to document that part?
 That's my main concern right now. 
 
 Is there a list containing various types of modems and models available
 somewhere? Naturally it is bound to be incomplete, but at least that
 would be a good start.

just send patches against this file. Potentially I can contain whatever
we want.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 2/9] Remove hack around hfp agent interface

2010-05-25 Thread Marcel Holtmann
Hi Gustavo,

  In the past we did this dirty hack to quickly fix HFP for the release, now
  fixing it in a proper way.
  ---
   drivers/hfpmodem/hfpmodem.h |1 -
   plugins/hfp.c   |   12 +++-
   2 files changed, 3 insertions(+), 10 deletions(-)
  
 
 
 The first two were refused. The following patches still apply cleanly from
 here.

send the debug option change separately (and also for ConnMan of course)
and then the rest as its own series. Intermixing multiple patches of
different types is a bad idea since everybody gets lost at some point.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH] huawei: follow sim state change notifications

2010-05-25 Thread Marcel Holtmann
Hi Kalle,

 With Huawei E1552 I got sim busy errors when I plugged in the modem
 and ofono was already running:
 
 May 24 17:02:04 tukki ofonod[7619]:  AT+CRC=1\r
 May 24 17:02:04 tukki ofonod[7619]:  \r\n+CME ERROR: SIM busy\r\n
 May 24 17:02:04 tukki ofonod[7619]:  AT+CLIP=1\r
 May 24 17:02:04 tukki ofonod[7619]:  \r\n+CME ERROR: SIM busy\r\n
 
 Fix this by following sim state changes with ^SIMST notification and
 only enable modem after SIM is ready. In case SIM is already ready
 and we miss the notification for some reason, also use AT^SYSINFO
 to check the state during enable phase.
 
 Also change huawei_enable() to return -EINPROGRESS to make sure that
 ofono modem is not powered too early. I believe this was a bug.
 ---
 
  plugins/huawei.c |   99 
 +++---
  1 files changed, 93 insertions(+), 6 deletions(-)
 
 diff --git a/plugins/huawei.c b/plugins/huawei.c
 index e1408bd..7d59a26 100644
 --- a/plugins/huawei.c
 +++ b/plugins/huawei.c
 @@ -47,9 +47,18 @@
  #include drivers/atmodem/atutil.h
  #include drivers/atmodem/vendor.h
  
 +enum huawei_state {
 + HUAWEI_DISABLED,
 + HUAWEI_DISABLE,
 + HUAWEI_ENABLE,
 + HUAWEI_ENABLED,
 +};
 +

is this really needed? I don't see why you want it. Where is the race
condition that you are trying to fix with it.

  struct huawei_data {
   GAtChat *chat;
   GAtChat *event;
 + gint sim_state;
 + enum huawei_state state;
  };

I think the sim_state should be just enough.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH v2] atmodem: fix crash during context deactivation

2010-05-26 Thread Marcel Holtmann
Hi Kalle,

 Ofono either crashed or busy looped with my Huawei E1552 3G modem when I
 tried to deactivate GPRS context. The reason was that gcd-chat was
 unreferenced already in setup_ppp() but the chat was still accessed
 later in at_gprs_deactivate_primary().
 
 To fix the problem, change the logic instead to suspend chat session
 for PPP and resume when PPP has disconnected. Now it doesn't crash
 anymore.
 
 Also remove the CGACT=0 command during deactivation, it's enough to shutdown
 ppp.
 
 Deactivation still doesn't work properly with Huawei E1552, and most
 probably with other Huawei modems, because the modem hangs up the chat
 line after PPP deactivation. This needs to be fixed separately. The
 workaround is to reboot the modem, for example physically unplug and plug
 it in again.
 ---
 
  drivers/atmodem/gprs-context.c |   56 
 +---
  1 files changed, 18 insertions(+), 38 deletions(-)
 
 diff --git a/drivers/atmodem/gprs-context.c b/drivers/atmodem/gprs-context.c
 index ba5f0c0..794db8a 100644
 --- a/drivers/atmodem/gprs-context.c
 +++ b/drivers/atmodem/gprs-context.c
 @@ -65,22 +65,6 @@ struct gprs_context_data {
   void *cb_data;  /* Callback data */
  };
  
 -static void at_cgact_down_cb(gboolean ok, GAtResult *result, gpointer 
 user_data)
 -{
 - struct cb_data *cbd = user_data;
 - ofono_gprs_context_cb_t cb = cbd-cb;
 - struct ofono_gprs_context *gc = cbd-user;
 - struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 - struct ofono_error error;
 -
 - if (ok)
 - gcd-active_context = 0;
 -
 - decode_at_error(error, g_at_result_final_response(result));
 -
 - cb(error, cbd-data);
 -}
 -
  static void ppp_connect(const char *interface, const char *ip,
   const char *dns1, const char *dns2,
   gpointer user_data)
 @@ -104,13 +88,19 @@ static void ppp_disconnect(GAtPPPDisconnectReason 
 reason, gpointer user_data)
   struct ofono_gprs_context *gc = user_data;
   struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
  
 + DBG();
 +
 + g_at_chat_resume(gcd-chat);
 +
   if (gcd-state == STATE_ENABLING) {
   CALLBACK_WITH_FAILURE(gcd-up_cb, NULL, FALSE, NULL,
   NULL, NULL, NULL, gcd-cb_data);
 - return;
 + } else if (gcd-state == STATE_DISABLING) {
 + CALLBACK_WITH_SUCCESS(gcd-down_cb, gcd-cb_data);
 + } else {
 + ofono_gprs_context_deactivated(gc, gcd-active_context);
   }

the easier to read coding style here is noew clearly a switch statement.

 - ofono_gprs_context_deactivated(gc, gcd-active_context);
   gcd-active_context = 0;
   gcd-state = STATE_IDLE;
  }
 @@ -118,13 +108,14 @@ static void ppp_disconnect(GAtPPPDisconnectReason 
 reason, gpointer user_data)
  static gboolean setup_ppp(struct ofono_gprs_context *gc)
  {
   struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 - GIOChannel *channel;
 + GAtIO *io;
 +
 + io = g_at_chat_get_io(gcd-chat);
  
 - channel = g_at_chat_get_channel(gcd-chat);
 - g_at_chat_unref(gcd-chat);
 + g_at_chat_suspend(gcd-chat);
  
   /* open ppp */
 - gcd-ppp = g_at_ppp_new(channel);
 + gcd-ppp = g_at_ppp_new_from_io(io);
  
   if (gcd-ppp == NULL)
   return FALSE;
 @@ -227,25 +218,14 @@ static void at_gprs_deactivate_primary(struct 
 ofono_gprs_context *gc,
   ofono_gprs_context_cb_t cb, void *data)
  {
   struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 - struct cb_data *cbd = cb_data_new(cb, data);
 - char buf[64];
 -
 - if (!cbd)
 - goto error;
 -
 - cbd-user = gc;
  
 - snprintf(buf, sizeof(buf), AT+CGACT=0,%u, id);
 + DBG();
  
 - if (g_at_chat_send(gcd-chat, buf, none_prefix,
 - at_cgact_down_cb, cbd, g_free)  0)
 - return;
 -
 -error:
 - if (cbd)
 - g_free(cbd);
 + gcd-state = STATE_DISABLING;
 + gcd-down_cb = cb;
 + gcd-cb_data = data;
  
 - CALLBACK_WITH_FAILURE(cb, data);
 + g_at_ppp_shutdown(gcd-ppp);
  }
  
  static int at_gprs_context_probe(struct ofono_gprs_context *gc,

The rest looks fine to me. However maybe splitting this into one patch
that removes the CGACT and another that adds suspend/resume calls seems
a bit cleaner to me.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH v4 1/2] atmodem: remove CGACT command

2010-05-26 Thread Marcel Holtmann
Hi Kalle,

 It's enough that we shutdown PPP, no need to send CGACT after that.
 Recommended by Denis.
 ---
 
  drivers/atmodem/gprs-context.c |   51 
 +---
  1 files changed, 16 insertions(+), 35 deletions(-)
 
 diff --git a/drivers/atmodem/gprs-context.c b/drivers/atmodem/gprs-context.c
 index ba5f0c0..9b32d59 100644
 --- a/drivers/atmodem/gprs-context.c
 +++ b/drivers/atmodem/gprs-context.c
 @@ -65,22 +65,6 @@ struct gprs_context_data {
   void *cb_data;  /* Callback data */
  };
  
 -static void at_cgact_down_cb(gboolean ok, GAtResult *result, gpointer 
 user_data)
 -{
 - struct cb_data *cbd = user_data;
 - ofono_gprs_context_cb_t cb = cbd-cb;
 - struct ofono_gprs_context *gc = cbd-user;
 - struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 - struct ofono_error error;
 -
 - if (ok)
 - gcd-active_context = 0;
 -
 - decode_at_error(error, g_at_result_final_response(result));
 -
 - cb(error, cbd-data);
 -}
 -
  static void ppp_connect(const char *interface, const char *ip,
   const char *dns1, const char *dns2,
   gpointer user_data)
 @@ -104,13 +88,21 @@ static void ppp_disconnect(GAtPPPDisconnectReason 
 reason, gpointer user_data)
   struct ofono_gprs_context *gc = user_data;
   struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
  
 - if (gcd-state == STATE_ENABLING) {
 + DBG();
 +
 + switch (gcd-state) {
 + case STATE_ENABLING:
   CALLBACK_WITH_FAILURE(gcd-up_cb, NULL, FALSE, NULL,
   NULL, NULL, NULL, gcd-cb_data);
 - return;
 + break;

just double-checking that you really wanna have break; and not return;
here. So that gcd-state gets reset to STATE_IDLE.

 + case STATE_DISABLING:
 + CALLBACK_WITH_SUCCESS(gcd-down_cb, gcd-cb_data);
 + break;
 + default:
 + ofono_gprs_context_deactivated(gc, gcd-active_context);
 + break;
   }
  
 - ofono_gprs_context_deactivated(gc, gcd-active_context);
   gcd-active_context = 0;
   gcd-state = STATE_IDLE;
  }

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 06/11] Made it possible to ask for status report via SendMessage method parameters. True=status report on, false=off.

2010-05-27 Thread Marcel Holtmann
Hi Denis,

   - { SendMessage,ss,   , sms_send_message,
   + { SendMessage,ssb,  , sms_send_message,
 
   G_DBUS_METHOD_FLAG_ASYNC },
 { }
};
  
  I don't like this being an argument to SendMessage(). I think it needs
  to be exposed, but as a property instead. Is there a use case for
  setting this per message? I think majority of current phones either
  provide a global setting for this, or set it on by default.
  
 
 I agree, we should expose this as the 'UseDeliveryReports' property on 
 SmsManager or alternatively use a global oFono setting read at startup.

I think just a global setting in /etc/ofono/main.conf would be better. I
don't really see the point in making this an option for the user. My
personal opinion here is to always request delivery reports by default
and handle them internally to acknowledge SMS messages.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH]enable-modem.c

2010-05-27 Thread Marcel Holtmann
Hi Daniele,

 here is the first simple example in c for enabling modem. This is my first 
 time using glib, dbus and git so I hope not to have done too many mistakes... 
 If you find it useful I can continue submitting the others I have,

 test/enable-modem.c |  116 +++
 1 files changed, 116 insertions(+), 0 deletions(-)
 
 diff --git a/test/enable-modem.c b/test/enable-modem.c
 new file mode 100644
 index 000..610b933
 --- /dev/null
 +++ b/test/enable-modem.c
 @@ -0,0 +1,116 @@
 +/*
 + * enable-modem.c
 + *
 + *  Created on: 25/mag/2010
 + *  Author: Daniele Palmas daniele.pal...@telit.com
 + *
 + *  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
 + */
 +
 +#include stdlib.h
 +#include string.h
 +#include dbus/dbus-glib.h

so we have a strict no gobject and no dbus-glib policy within the oFono
source code. I would prefer if you re-write it just using the low-level
libdbus. I know that dictionary parsing can be tricky, but it is not
that complicated. Check the connman/client/main.c for some simple
example on just to use libdbus.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [RFC patches 02/13] sms_send_message: add a short roadmap

2010-05-28 Thread Marcel Holtmann
Hi Inaky,

 From: Inaky Perez-Gonzalez inaky.perez-gonza...@intel.com
 
 ---
  src/sms.c |   13 +
  1 files changed, 13 insertions(+), 0 deletions(-)
 
 diff --git a/src/sms.c b/src/sms.c
 index 3a1cff0..594481e 100644
 --- a/src/sms.c
 +++ b/src/sms.c
 @@ -398,6 +398,19 @@ static struct tx_queue_entry 
 *create_tx_queue_entry(GSList *msg_list)
   return entry;
  }
  
 +/*
 + * Pre-process a SMS text message and deliver it [D-BUS' SendMessage()]
 + *
 + * @conn: D-BUS connection
 + * @msg: message data (telephone number and text)
 + * @data: SMS object to use for transmision
 + *
 + * An alphabet is chosen for the text and it (might be) segmented in
 + * fragments by sms_text_prepare() into @msg_list. A queue list @entry
 + * is created by create_tx_queue_entry() and g_queue_push_tail()
 + * appends that entry to the SMS transmit queue. Then the tx_next()
 + * function is scheduled to run to process the queue.
 + */
  static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,
   void *data)
  {

small comments here. So it is D-Bus. That is the official naming in
documentation we settled on some time ago. Half of the documentation
might be still using it wrongly, but we might wanna set a good example
here.

I don't know how picky gtk-doc is about not having the : for title and
the empty line after the title. However this is nicely readable inside
the source code, so I am fine with it.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [RFC patches 03/13] documentation: add note about referencing standards

2010-05-28 Thread Marcel Holtmann
Hi Inaky,

 From: Inaky Perez-Gonzalez inaky.perez-gonza...@intel.com
 
 ---
  doc/standards.txt |8 
  1 files changed, 8 insertions(+), 0 deletions(-)
  create mode 100644 doc/standards.txt
 
 diff --git a/doc/standards.txt b/doc/standards.txt
 new file mode 100644
 index 000..c4b68eb
 --- /dev/null
 +++ b/doc/standards.txt
 @@ -0,0 +1,8 @@
 +Referencing standards in the source
 +===
 +
 +When referencing standard documents use raw numbers xx.xxx for 3GPP
 +documents or xxx.xxx for ETSI document (eg: 23.040). If needing to
 +point to an specific section/subsection, explicitly say Section foo
 +
 +3GPP specs can be found in http://3gpp.org/ftp/Specs.

would this not better fit into HACKING document. I don't really have a
strong opinion here. Just asking.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [RFC patches 05/13] util.h: Add BUILD_BUG_ON() and friends for compile-time assert checking

2010-05-28 Thread Marcel Holtmann
Hi Inaky,

 From: Inaky Perez-Gonzalez inaky.perez-gonza...@intel.com
 
 These have been stolen from the Linux kernel source; come pretty handy
 to make build-time consistency checks and thus avoid run-time
 surprises.
 ---
  src/util.h |   29 +
  1 files changed, 29 insertions(+), 0 deletions(-)
 
 diff --git a/src/util.h b/src/util.h
 index 2835b76..cf34b67 100644
 --- a/src/util.h
 +++ b/src/util.h
 @@ -77,3 +77,32 @@ unsigned char *pack_7bit(const unsigned char *in, long 
 len, int byte_offset,
   long *items_written, unsigned char terminator);
  
  char *sim_string_to_utf8(const unsigned char *buffer, int length);
 +
 +/*
 + * Build time consistency checks
 + *
 + * Stolen from the Linux kernel 2.6.35-rc; as most taken from the
 + * Linux kernel, this is licensed GPL v2 or newer.
 + */
 +
 +/* Force a compilation error if condition is true */
 +#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
 +
 +/* Force a compilation error if condition is constant and true */
 +#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
 +
 +/* Force a compilation error if a constant expression is not a power of 2 */
 +#define BUILD_BUG_ON_NOT_POWER_OF_2(n)   \
 + BUILD_BUG_ON((n) == 0 || (((n)  ((n) - 1)) != 0))
 +
 +/* Force a compilation error if condition is true, but also produce a
 +   result (of value 0 and type size_t), so the expression can be used
 +   e.g. in a structure initializer (or where-ever else comma expressions
 +   aren't permitted). */
 +#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
 +#define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
 +
 +/*
 + * End of code stolen from the Linux kernel 2.6.35-rc. From now, code
 + * is GPL v2 only.
 + */

sounds like a nice addition, but src/util.h is the wrong file. I prefer
we make this compatible so that it can be applied to ConnMan and BlueZ
as well.

I think something like include/bug.h is better.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [RFC patches 06/13] smutil.h: add missing header file dependencies

2010-05-28 Thread Marcel Holtmann
Hi Inaky,

 diff --git a/src/smsutil.h b/src/smsutil.h
 index 469a49e..356ec5d 100644
 --- a/src/smsutil.h
 +++ b/src/smsutil.h
 @@ -18,6 +18,14 @@
   *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  
 USA
   *
   */
 +#ifndef __smsutil_h__
 +#define __smsutil_h__

please leave the circular dependency protection out here. I really want
it to fail if we go so crazy and would require it.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [RFC patches 10/13] doc: explain debugging options to -d, add a pointer in -h to manpage

2010-05-28 Thread Marcel Holtmann
Hi Inaky,

 From: Inaky Perez-Gonzalez inaky.perez-gonza...@intel.com
 
 Modified HACKING and man page to have more formation on what are the
 debugging options and how to enable them.
 ---
  HACKING  |   10 ++
  doc/ofonod.8 |5 -
  src/main.c   |4 +++-
  3 files changed, 17 insertions(+), 2 deletions(-)
 
 diff --git a/HACKING b/HACKING
 index ae420aa..e825185 100644
 --- a/HACKING
 +++ b/HACKING
 @@ -81,3 +81,13 @@ automatically includes this option.
  
  For production installations or distribution packaging it is important that
  the --enable-maintainer-mode option is NOT used.
 +
 +Note multiple arguments to -d can be specified, colon, comma or space
 +separated. The arguments are relative source code filenames for which
 +debugging output should be enabled; output shell-style globs are
 +accepted (e.g.: 'plugins/*:src/main.c').
 +
 +Other debugging settings that can be toggled:
 +
 + - Environment variable OFONO_AT_DEBUG (set to 1): enable AT commands
 +   debugging
 diff --git a/doc/ofonod.8 b/doc/ofonod.8
 index 474d7fb..7bb908c 100644
 --- a/doc/ofonod.8
 +++ b/doc/ofonod.8
 @@ -18,7 +18,10 @@ is used to manage \fID-Bus\fP permissions for oFono.
  .SH OPTIONS
  .TP
  .B --debug, -d
 -Enable debug information output.
 +Enable debug information output. Note multiple arguments to -d can be
 +specified, colon, comma or space separated. The arguments are relative
 +source code filenames for which debugging output should be enabled;
 +output shell-style globs are accepted (e.g.: plugins/*:src/main.c).
  .TP
  .B --nodetach, -n
  Don't run as daemon in background.

you need to hook this up to automake :)

 diff --git a/src/main.c b/src/main.c
 index 8e686ac..c5791be 100644
 --- a/src/main.c
 +++ b/src/main.c
 @@ -98,7 +98,9 @@ static gboolean option_version = FALSE;
  
  static GOptionEntry options[] = {
   { debug, 'd', 0, G_OPTION_ARG_STRING, option_debug,
 - Specify debug options to enable, DEBUG },
 +Specify debug options to enable (see the 
 +man page for ofonod(8) for more information).,
 +DEBUG },
   { nodetach, 'n', G_OPTION_FLAG_REVERSE,
   G_OPTION_ARG_NONE, option_detach,
   Don't run as daemon in background },

Please leave this out. If we have a man page and man ofonod works, then
this is not needed.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [RFC patches 11/13] automake: fix installation of udev rules in VPATH builds

2010-05-28 Thread Marcel Holtmann
Hi Inaky,

 When the build directory is different than the source directory, we
 need to specify the source prefix to the original file we are
 copying.
 ---
  Makefile.am |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/Makefile.am b/Makefile.am
 index ed13346..31c157c 100644
 --- a/Makefile.am
 +++ b/Makefile.am
 @@ -418,7 +418,7 @@ src/ofono.ver: src/ofono.exp
   $(AM_V_at)echo local: *; };  $@
  
  plugins/%.rules:
 - $(AM_V_GEN)cp $(subst 97-,,$@) $@
 + $(AM_V_GEN)cp $(srcdir)/$(subst 97-,,$@) $@
  
  $(src_ofonod_OBJECTS) $(unit_objects): $(local_headers)

I tested it within my release build system and it looks fine to me. At
least nothing breaks. I leave it up to Denis to apply the patch, but
please also submit similar patches for BlueZ and ConnMan since I am
pretty sure I missed it there as well.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [RFC patches 12/13] SMS: introduce message ID API

2010-05-28 Thread Marcel Holtmann
Hi Inaky,

 +/**
 + * Create a random UUID
 + *
 + * \param msg_id Descriptor
 + *
 + * \internal
 + *
 + * Crams the msg_id ID buffer with random 32-bit numbers; because we
 + * use a byte based buffer, we play the cast to guint32 trick (as that
 + * is what g_random_int() returns).
 + */

can we just stick with gtk-doc like you had in your previous patches. I
think intermixing it with doxygen is a bad idea.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 1/1] Huawei E176: Mark primary and secondary device at ofono.rules TODO: Fix sim_add detection, add E1552 idProduct to ofono.rules

2010-05-31 Thread Marcel Holtmann
Hi Kalle,

 I personally would prefer a solution which would dynamically probe the
 ports and choose them based on results. I believe modemmanager does
 something like this, but I haven't looked in detail.

we should do something like auto-detect at some point, but there are
limits in it.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


RE: [PATCH] Introduction of HACKING file in phonesim

2010-06-01 Thread Marcel Holtmann
Hi Yang,

 +When using ./configure --enable-maintainer-mode the automake scripts will
 +use the plugins directly from within the repository. This removes the need
 +to use make install when testing phonesim. The bootstrap-configure
 +automatically includes this option.
 +
 +  Run phonesim in foreground using the following options
 +# ./src/phonesim -p 12345 -gui xml/default.xml
 
 The xml directory has been removed. So here should be src/default.xml.

good catch. I fixed that up. Thanks.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: Huawei E1552 broken

2010-06-08 Thread Marcel Holtmann
Hi Kalle,

  I mentioned this briefly on irc, but I'll inform it here as well. This
  patch broke ofono support for my Huawei E1552:
  
  commit 6ab0b6f29acc8f6b2a5f3b94c02c405ee7318244
  Author: Marcel Holtmann mar...@holtmann.org
  Date:   Sun Jun 6 15:51:36 2010 -0700
  
 Fix detection of Huawei E220 and E270 modems
  
  Huawei plugin uses /dev/ttyUSB1 for the event channel but for my modem
  it should be /dev/ttyUSB2. We need to solve this problem somehow, even
  if we need to add usb ids to ofono.rules.
 
  Denis has already updated udev.c by commit 71ea72adf and now it
  detects interface number by ofono.rules.
 
 Oh damn, I forgot to do git update this morning and missed this
 altogether. Thank you for pointing out this.
 
  So I think you should add one more E1552 specific item in ofono.rules
  with your product id and interface number, etc.
 
 Actually Denis already added them. Excellent, thank you Denis! My modem
 works again :)
 
 But now connman doesn't show the operator name Saunalahti anymore, the
 name property in connman Service is just empty:
 
 ./cmcc show cellular_244053111242822_huawei0_primarycontext1
 AutoConnect: true
 Name: 
 Nameservers: { 193.229.0.40 193.229.0.42 }
 Proxy: { Method=direct }

this might be a bug in the oFono plugin in ConnMan. I have encountered
this as well. I think ConnMan forgets to listen to name changes and to
update the name. Some times you get the name other times you don't.

Regards

Marcel



___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [RFC patches 10/13] doc: explain debugging options to -d, add a pointer in -h to manpage

2010-06-08 Thread Marcel Holtmann
Hi Inaky,

   From: Inaky Perez-Gonzalez inaky.perez-gonza...@intel.com
   
   Modified HACKING and man page to have more formation on what are the
   debugging options and how to enable them.
   ---
HACKING  |   10 ++
doc/ofonod.8 |5 -
src/main.c   |4 +++-
3 files changed, 17 insertions(+), 2 deletions(-)
   
   diff --git a/HACKING b/HACKING
   index ae420aa..e825185 100644
   --- a/HACKING
   +++ b/HACKING
   @@ -81,3 +81,13 @@ automatically includes this option.

For production installations or distribution packaging it is important 
   that
the --enable-maintainer-mode option is NOT used.
   +
   +Note multiple arguments to -d can be specified, colon, comma or space
   +separated. The arguments are relative source code filenames for which
   +debugging output should be enabled; output shell-style globs are
   +accepted (e.g.: 'plugins/*:src/main.c').
   +
   +Other debugging settings that can be toggled:
   +
   + - Environment variable OFONO_AT_DEBUG (set to 1): enable AT commands
   +   debugging
   diff --git a/doc/ofonod.8 b/doc/ofonod.8
   index 474d7fb..7bb908c 100644
   --- a/doc/ofonod.8
   +++ b/doc/ofonod.8
   @@ -18,7 +18,10 @@ is used to manage \fID-Bus\fP permissions for oFono.
.SH OPTIONS
.TP
.B --debug, -d
   -Enable debug information output.
   +Enable debug information output. Note multiple arguments to -d can be
   +specified, colon, comma or space separated. The arguments are relative
   +source code filenames for which debugging output should be enabled;
   +output shell-style globs are accepted (e.g.: plugins/*:src/main.c).
.TP
.B --nodetach, -n
Don't run as daemon in background.
  
  you need to hook this up to automake :)
 
 Can you clarify, please?

we do wanna install the man pages, right? Then this needs man_MANS and
EXTRA_DIST magic to get included and installed.

   diff --git a/src/main.c b/src/main.c
   index 8e686ac..c5791be 100644
   --- a/src/main.c
   +++ b/src/main.c
   @@ -98,7 +98,9 @@ static gboolean option_version = FALSE;

static GOptionEntry options[] = {
 { debug, 'd', 0, G_OPTION_ARG_STRING, option_debug,
   - Specify debug options to enable, DEBUG },
   +Specify debug options to enable (see the 
   +man page for ofonod(8) for more information).,
   +DEBUG },
 { nodetach, 'n', G_OPTION_FLAG_REVERSE,
 G_OPTION_ARG_NONE, option_detach,
 Don't run as daemon in background },
  
  Please leave this out. If we have a man page and man ofonod works, then
  this is not needed.
 
 Denis and me, AFAIR, agreed on this wording as it just gives the quick
 pointer to where the extra information is found. Again, not critical,
 but confusing feedback.

Maybe I am too old school Linux here ;)

If we have an installed man page and I don't understand the usage
information, then it is clear to just do man ofonod. I don't need the
usage tell me that.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [RFC patches 12/13] SMS: introduce message ID API

2010-06-08 Thread Marcel Holtmann
Hi Inaky,

   +/**
   + * Create a random UUID
   + *
   + * \param msg_id Descriptor
   + *
   + * \internal
   + *
   + * Crams the msg_id ID buffer with random 32-bit numbers; because we
   + * use a byte based buffer, we play the cast to guint32 trick (as that
   + * is what g_random_int() returns).
   + */
  
  can we just stick with gtk-doc like you had in your previous patches. I
  think intermixing it with doxygen is a bad idea.
 
 Yep, my bad, sorry.
 
 In any case, picking this up from earlier:
 
 I have my doubts on gtk-doc vs doxygen. I remember there was a thread
 somewhere (email? irc? voice?) on which system to settle on which never
 got finalized. I am not too satisfied with gtk-doc's limitations, I'd
 rather do doxygen. Does anyone have any preference on once vs the other?
 
 In any case, when we settle on one, I'll have to go and cleanup all
 these to conform to it. I'll also write a quick summary with examples on
 how to use it in doc/.

my preference with gtk-doc is that it generates nicer documentation and
also makes devhelp files available.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH] TODO: CBS CMAS

2010-06-09 Thread Marcel Holtmann
Hi Waldo,

 +Cell Broadcast System
 +=
 +
 +- Support for Commercial Mobile Alert System (CMAS) according to 3GPP
 +  23.041, 3GPP 22.268 and ATIS/TIA J-STD-100 in addition to Earthquake
 +  and Tsunami Warning Service (ETWS). oFono already reports ETWS messages
 +  with CBS message identifiers 4352-4356 as EmergencyBroadcast DBUS signals.
 +  oFono should be extended with support for CMAS messages which have CBS
 +  message identifiers in the range of 4370-4382.
 +
 +  Priority: Medium
 +  Complexity: C1
 +
 +

do we expect this to be always enabled? If so, then just send a patch
for it. Should be pretty much straight forward.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 06/11] Made it possible to ask for status report via SendMessage method parameters. True=status report on, false=off.

2010-06-09 Thread Marcel Holtmann
Hi Inaky,

  I don't like this being an argument to SendMessage(). I think it
  needs to be exposed, but as a property instead. Is there a use case
  for setting this per message? I think majority of current phones
  either provide a global setting for this, or set it on by default.

 our idea is actually that every new SMS has its own object path for 
 its
 lifetime. So we can have then properties easily on them.
   
Sure, but there should still be a property in SmsManager to control
whether srr is to be set on outgoing messages.
   
Another property in the actual SmsMessage (residing on its own object
path) could then indicate whether srr *was* set for that particular
message when it was sent.
   
   I tend to disagree with that; by making it an SmsManager property, you
   are creating an API that is not reentrant. If more than one application
   is sending SMS's at the same time and they have different requirements
   wrt to status-reports, they would be trumping each other:
  
  While you're 100% correct about a possible race condition, the reality is 
  that 
  nobody actually uses it this way.  It is just a setting buried somewhere in 
  the Settings UI that the user maybe changes once in his lifetime.
 
 If you are confident this will never be a problem then *shrug*; I am
 just not comfortable with generic unconstrained APIs that operate like
 that.

please keep in mind that we are on purpose not going to expose anything
just for the sake of having an API for it. The API must make sense from
an UI use case point of view. oFono is suppose to do all the heavy
lifting and this also means that we are not exposing every single detail
or possibility of GSM/UMTS.

I would even go one step further in this case and always request status
reports and if the user doesn't want them, then it can throw this kind
of information away.

So there might be cases where we are wrong in the first place, but that
is fine as well. This is a learning experience that we have to go
through to create proper APIs. This API design by committee is not
working out anyway. It just creates clutter and extra shim layers since
everything gets way too complicated. The idea is to have a really simple
API first and then only if needed introduce complexity.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 17/20] stkutil: Add the Event Download envelope builder

2010-06-09 Thread Marcel Holtmann
Hi Denis,

  /* Network Byte Order */
  -   unsigned int ipv4;
  +   guint32 ipv4;
 
 Why?

actually unsigned int on 64-bit has a different size than on 32-bit.
However I prefer we use uint32_t and not the guint32 types.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: Connman operator name empty with Huawei E1552

2010-06-10 Thread Marcel Holtmann
Hi Kalle,

  But now connman doesn't show the operator name Saunalahti anymore, the
  name property in connman Service is just empty:
  
  ./cmcc show cellular_244053111242822_huawei0_primarycontext1
  AutoConnect: true
  Name: 
  Nameservers: { 193.229.0.40 193.229.0.42 }
  Proxy: { Method=direct }
 
  this might be a bug in the oFono plugin in ConnMan. I have encountered
  this as well. I think ConnMan forgets to listen to name changes and to
  update the name. Some times you get the name other times you don't.
 
  Thanks, that's good to know. I have been testing connman and ofono for
  some time now, but I saw this problem first time only yesterday. But I
  might have been just lucky earlier.
 
 I did more testing now. I see this a lot now, at least 90% of the time
 operator name in connman is empty. Seems to be a race condition of some
 sort:
 
 (I added extra debug messages)
 
 Jun 10 08:43:44 tukki connmand[23976]: plugins/ofono.c:set_network_name() 
 path /huawei0
 Jun 10 08:43:44 tukki connmand[23976]: 
 plugins/ofono.c:set_network_name_reply() network 0x87d2b0
 Jun 10 08:43:44 tukki connmand[23976]: 
 plugins/ofono.c:set_network_name_reply() key Status
 Jun 10 08:43:44 tukki connmand[23976]: 
 plugins/ofono.c:set_network_name_reply() key Mode
 Jun 10 08:43:44 tukki connmand[23976]: 
 plugins/ofono.c:set_network_name_reply() key LocationAreaCode
 Jun 10 08:43:44 tukki connmand[23976]: 
 plugins/ofono.c:set_network_name_reply() key CellId
 Jun 10 08:43:44 tukki connmand[23976]: 
 plugins/ofono.c:set_network_name_reply() key Name
 Jun 10 08:43:44 tukki connmand[23976]: 
 src/network.c:connman_network_set_name() network 0x87d2b0 name 
 Jun 10 08:43:44 tukki connmand[23976]: src/element.c:set_static_property() 
 element 0x87d2b0 name huawei0_primarycontext1
 Jun 10 08:43:44 tukki connmand[23976]: src/element.c:set_static_property() 
 name Name type 115 value 0x7fffc931f1e8
 Jun 10 08:43:44 tukki connmand[23976]: 
 plugins/ofono.c:set_network_name_reply() name ''
 
 The last debug line is printed here:
 
   if (g_str_equal(key, Operator) == TRUE ||
   g_str_equal(key, Name) == TRUE) {
   const char *name;
 
   dbus_message_iter_get_basic(value, name);
   connman_network_set_name(network, name);
   DBG(name '%s', name);
   create_service(network);
   }
 
 And then a bit later in the log:
 
 
 Jun 10 08:43:44 tukki ofonod[23999]: Pcui: \r\n+CRSM: 144,0,\r\n\r\nOK\r\n
 Jun 10 08:43:44 tukki ofonod[23999]: drivers/atmodem/sim.c:at_crsm_info_cb() 
 crsm_info_cb: 90, 00, 0
 Jun 10 08:43:44 tukki ofonod[23999]: Pcui: AT+COPS=3,0\r
 Jun 10 08:43:44 tukki ofonod[23999]: Pcui: \r\nOK\r\n
 Jun 10 08:43:44 tukki ofonod[23999]: Pcui: AT+COPS?\r
 Jun 10 08:43:44 tukki ofonod[23999]: Pcui: \r\n+COPS: 
 0,0,Saunalahti,2\r\n\r\nOK\r\n
 Jun 10 08:43:44 tukki ofonod[23999]: 
 drivers/atmodem/network-registration.c:cops_cb() cops_cb: Saunalahti, 244 05 2
 Jun 10 08:43:44 tukki ofonod[23999]: 
 src/network.c:current_operator_callback() 0xaa5a50, (nil)
 Jun 10 08:43:44 tukki ofonod[23999]: src/gprs.c:netreg_status_changed() 1
 Jun 10 08:43:44 tukki ofonod[23999]: src/cbs.c:cbs_location_changed() 1, 
 1450, 21880, -1, 24405
 Jun 10 08:43:44 tukki ofonod[23999]: src/cbs.c:cbs_location_changed() 1, 0, 0
 
 Then I run test/list-modems after initialisation, I did see the operator
 name correctly:
 
 [ org.ofono.NetworkRegistration ]
 Status = registered
 Strength = 67
 Name = Saunalahti
 
 So connman uses the Name property from org.ofono.NetworkRegistration
 even before ofono has set it and doesn't follow name changes.
 
 Also CCed the connman list because I don't yet know which one is buggy,
 connman or ofono. I vote for connman's ofono plugin. Thoughts?

it is the ConnMan plugin. It needs to follow name changes. Especially
since the name can change at any time anyway. Most common case would
actually be roaming, but also operator names can be updates over the air
at any time (even if that happens rarely).

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH] stkutil: convert text attributes to html

2010-06-16 Thread Marcel Holtmann
Hi Kristen,

 ---
  src/stkutil.c |  195 
 +
  src/stkutil.h |   21 ++
  2 files changed, 216 insertions(+), 0 deletions(-)
 
 diff --git a/src/stkutil.c b/src/stkutil.c
 index 8ac1dba..6244d03 100644
 --- a/src/stkutil.c
 +++ b/src/stkutil.c
 @@ -4307,3 +4307,198 @@ const unsigned char *stk_pdu_from_envelope(const 
 struct stk_envelope *envelope,
  
   return pdu;
  }
 +
 +static void map_color_to_html(GString *string, guint8 color)
 +{
 + int fg = color  0x0f;
 + int bg = (color  4)  0x0f;
 + static const char *html_colors[] = {
 + #00, /* Black */
 + #808080, /* Dark Grey */
 + #C11B17, /* Dark Red */
 + #FBB117, /* Dark Yellow */
 + #347235, /* Dark Green */
 + #307D7E, /* Dark Cyan */
 + #A0, /* Dark Blue */
 + #C031C7, /* Dark Magenta */
 + #C0C0C0, /* Grey */
 + #FF, /* White */
 + #FF, /* Bright Red */
 + #00, /* Bright Yellow */
 + #00FF00, /* Bright Green */
 + #00, /* Bright Cyan */
 + #FF, /* Bright Blue */
 + #FF00FF, /* Bright Magenta */
 + };
 +
 + if (color == 0)
 + return;
 +
 + if (fg)
 + g_string_append_printf(string, color: %s;, html_colors[fg]);
 + if (bg)
 + g_string_append_printf(string,
 + background-color: %s;, html_colors[bg]);

so here is where g_string_append_printf() makes is a lot more readable.
You clearly see what is the key and what the value you are setting. It
is not split over multiple lines.

 +
 + return;
 +}

We don't add return; statements that are not needed. So please remove
this.

 +
 +static void end_format(GString *string, guint8 code)
 +{
 + g_string_append_printf(string, /span);
 +
 + if ((code  STK_TEXT_FORMAT_ALIGN_MASK) != STK_TEXT_FORMAT_NO_ALIGN)
 + g_string_append_printf(string, /div);

Here the g_string_append_printf() is not useful. Just using
g_string_append() is enough.

It is actually pretty simple. If you have variable parameters in your
string, then use ...append_printf() if just pure string, do
just ..._append().

 +}
 +
 +static void map_format_to_html(GString *string, guint8 code, guint8 color)
 +{
 + guint8 align = code  STK_TEXT_FORMAT_ALIGN_MASK;
 + guint8 font = code  STK_TEXT_FORMAT_FONT_MASK;
 + guint8 style = code  STK_TEXT_FORMAT_STYLE_MASK;
 +
 + /* align formatting applies to a block of test */
 + if (align != STK_TEXT_FORMAT_NO_ALIGN)
 + g_string_append_printf(string, div style=\);
 +
 + if (align == STK_TEXT_FORMAT_RIGHT_ALIGN)
 + g_string_append_printf(string, text-align: right;\);
 + else if (align == STK_TEXT_FORMAT_CENTER_ALIGN)
 + g_string_append_printf(string, text-align: center;\);
 + else if (align == STK_TEXT_FORMAT_LEFT_ALIGN)
 + g_string_append_printf(string, text-align: left;\);
 +
 + /* font, style, and color are inline */
 + g_string_append_printf(string, span style=\);
 +
 + if (font == STK_TEXT_FORMAT_FONT_SIZE_LARGE)
 + g_string_append_printf(string, font-size: big;);
 + else if (font == STK_TEXT_FORMAT_FONT_SIZE_SMALL)
 + g_string_append_printf(string, font-size: small;);
 +
 + if (style == STK_TEXT_FORMAT_STYLE_BOLD)
 + g_string_append_printf(string, font-weight: bold;);
 + else if (style == STK_TEXT_FORMAT_STYLE_ITALIC)
 + g_string_append_printf(string, font-style: italic;);
 + else if (style == STK_TEXT_FORMAT_STYLE_UNDERLINED)
 + g_string_append_printf(string, text-decoration: underline;);
 + else if (style == STK_TEXT_FORMAT_STYLE_STRIKETHROUGH)
 + g_string_append_printf(string,
 + text-decoration: line-through;);
 +
 + /* add any color */
 + map_color_to_html(string, color);

Can we just make the color array a global static variable and then just
add the colors here directly instead of calling map_to_color. The
compiler will optimize it anyway, but that way also the reader doesn't
have to jump while reading this function.

 +
 + g_string_append_printf(string, \);
 +}
 +
 +static gboolean is_special_char(char c)
 +{
 + return (c == '\n' || c == '\r' || c == '' || c == '' || c == '');
 +}

How many times are you using is_special_char? If it is only one, I
prefer you just code this directly in the place where you are using
this.

 +
 +static gboolean map_char_to_html(char *s, int pos, int len, GString *string)
 +{
 + gboolean rval = FALSE;
 + unsigned char c = s[pos];

Why are you bothering with these two variables?

Just use s[pos] in the switch statement directly. Makes is a lot clearer
than having to look up where c is coming. And since c never changes I
don't see a point 

Re: commit a7690918 huawei: Import huawei rules broke Huawei E1552

2010-06-22 Thread Marcel Holtmann
Hi Kalle,

 after commit a7690918 huawei: Import huawei rules my Huawei E1552
 modem doesn't work anymore. The commit has this change:
 
 -SUBSYSTEMS==usb, ATTRS{bInterfaceNumber}==?*, 
 ENV{OFONO_IFACE_NUM}=$attr{bInterfaceNumber}
 +SUBSYSTEMS==usb, ATTRS{bInterfaceNumber}==ff, 
 ENV{OFONO_IFACE_NUM}=$attr{bInterfaceNumber}
 
 After I change ff back to ?* my modem works again. What's the reason
 behind this change? Can we revert it back?

I have a similar problem with an E1800 I bought in Italy a few days ago.
Might be an issue with a specific kernel version, but I didn't look into
the details so far.

Regards

Marcel



___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: Ofono and PPP

2010-06-22 Thread Marcel Holtmann
Hi Arun,

 I have managed to put together ofono and conn man for an at modem
 interface. 
 
 The atmodem is working and here is the detail of a pdp context i
 created.
 
 [ /generic ]
 [ /generic/primarycontext1 ]
 Username = 
 Name = Internet access
 Settings = { Interface=ppp0 Netmask=255.255.255.255
 Method=static DomainNameServers=192.89.123.231,192.89.123.230,
 Address=109.240.193.169 }
 Active = 1
 AccessPointName = internet
 Password = 
 Type = internet
 
 
 the ifconfig:
 
 ppp0  Link encap:UNSPEC  HWaddr
 00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00  
   UP POINTOPOINT RUNNING NOARP MULTICAST  MTU:1500  Metric:1
   RX packets:0 errors:0 dropped:0 overruns:0 frame:0
   TX packets:32 errors:0 dropped:0 overruns:0 carrier:0
   collisions:0 txqueuelen:500 
   RX bytes:0 (0.0 B)  TX bytes:10944 (10.9 KB)
 --
 
 The ifconfig doesn't seem to have all the required information from
 the ppp link created. why this is so? Is there a way to put these in
 sync?

try the latest versions of ConnMan and oFono. I have this actually
working perfectly fine with Novatel and Huawei devices. Maybe the
atmodem driver has an issue, but in general this works fine.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 1/2] stkutil: display text attributes as html

2010-06-24 Thread Marcel Holtmann
Hi Kristen,

  src/stkutil.c |  167 
 +
  src/stkutil.h |   32 +++
  2 files changed, 199 insertions(+), 0 deletions(-)
 
 diff --git a/src/stkutil.c b/src/stkutil.c
 index 6f072e7..73449e2 100644
 --- a/src/stkutil.c
 +++ b/src/stkutil.c
 @@ -26,6 +26,7 @@
  #include string.h
  #include stdlib.h
  #include stdint.h
 +#include stdio.h
  
  #include glib.h
  
 @@ -5819,3 +5820,169 @@ const unsigned char *stk_pdu_from_envelope(const 
 struct stk_envelope *envelope,
  
   return pdu;
  }
 +
 +static const char *html_colors[] = {
 + #00, /* Black */
 + #808080, /* Dark Grey */
 + #C11B17, /* Dark Red */
 + #FBB117, /* Dark Yellow */
 + #347235, /* Dark Green */
 + #307D7E, /* Dark Cyan */
 + #A0, /* Dark Blue */
 + #C031C7, /* Dark Magenta */
 + #C0C0C0, /* Grey */
 + #FF, /* White */
 + #FF, /* Bright Red */
 + #00, /* Bright Yellow */
 + #00FF00, /* Bright Green */
 + #00, /* Bright Cyan */
 + #FF, /* Bright Blue */
 + #FF00FF, /* Bright Magenta */
 +};
 +
 +static void end_format(GString *string, guint8 code)
 +{
 + g_string_append(string, /span);
 +
 + if ((code  STK_TEXT_FORMAT_ALIGN_MASK) != STK_TEXT_FORMAT_NO_ALIGN)
 + g_string_append(string, /div);
 +}
 +
 +static void start_format(GString *string, guint8 code, guint8 color)
 +{
 + guint8 align = code  STK_TEXT_FORMAT_ALIGN_MASK;
 + guint8 font = code  STK_TEXT_FORMAT_FONT_MASK;
 + guint8 style = code  STK_TEXT_FORMAT_STYLE_MASK;
 + int fg = color  0x0f;
 + int bg = (color  4)  0x0f;
 +
 + /* align formatting applies to a block of test */
 + if (align != STK_TEXT_FORMAT_NO_ALIGN)
 + g_string_append(string, div style=\);
 +
 + if (align == STK_TEXT_FORMAT_RIGHT_ALIGN)
 + g_string_append(string, text-align: right;\);
 + else if (align == STK_TEXT_FORMAT_CENTER_ALIGN)
 + g_string_append(string, text-align: center;\);
 + else if (align == STK_TEXT_FORMAT_LEFT_ALIGN)
 + g_string_append(string, text-align: left;\);

using a switch {} statement seems more appropriate here.

 +
 + /* font, style, and color are inline */
 + g_string_append(string, span style=\);
 +
 + if (font == STK_TEXT_FORMAT_FONT_SIZE_LARGE)
 + g_string_append(string, font-size: big;);
 + else if (font == STK_TEXT_FORMAT_FONT_SIZE_SMALL)
 + g_string_append(string, font-size: small;);

Same here. I prefer switch {} since it is more readable.

 +
 + if (style == STK_TEXT_FORMAT_STYLE_BOLD)
 + g_string_append(string, font-weight: bold;);
 + else if (style == STK_TEXT_FORMAT_STYLE_ITALIC)
 + g_string_append(string, font-style: italic;);
 + else if (style == STK_TEXT_FORMAT_STYLE_UNDERLINED)
 + g_string_append(string, text-decoration: underline;);
 + else if (style == STK_TEXT_FORMAT_STYLE_STRIKETHROUGH)
 + g_string_append(string, text-decoration: line-through;);

And also here please.

 +
 + /* add any color */
 + if (fg)
 + g_string_append_printf(string, color: %s;, html_colors[fg]);
 + if (bg)
 + g_string_append_printf(string, background-color: %s;,
 + html_colors[bg]);
 + g_string_append(string, \);
 +}
 +
 +char *stk_text_to_html(char *text, int text_len,
 + const unsigned char *attrs, int attrs_len)
 +{
 + GString *string = g_string_sized_new(text_len + 1);
 + gint formats[257];  /* maximum number of chars in text + 1 */
 + int pos = 0, attr, i, j, prev_attr;
 + guint8 start, end, code, color, len, align;
 +
 + /* we will need formatting at the position beyond the last char */
 + for (i = 0; i = text_len; i++)
 + formats[i] = STK_TEXT_FORMAT_INIT;
 +
 + i = 0;
 +
 + while (i  attrs_len) {
 + start = attrs[i++];
 + len = attrs[i++];
 + code = attrs[i++];
 +
 + if (i  attrs_len)
 + color = attrs[i++];
 + else
 + color = 0;
 +
 + if (len == 0)
 + end = text_len;
 + else
 + end = start + len;
 +
 + /* sanity check values */
 + if (start  end || end  text_len)
 + continue;
 +
 + /*
 +  * if the alignment is the same as either the default
 +  * or the last alignment used, don't set any alignment
 +  * value.
 +  */
 + if (start == 0)
 + align = STK_DEFAULT_TEXT_ALIGNMENT;
 + else {
 + align = get_align(formats[start - 1]);
 + if (align == STK_TEXT_FORMAT_NO_ALIGN)
 + align = STK_DEFAULT_TEXT_ALIGNMENT;
 

Re: Required suggestion to use third part code in oFono

2010-06-25 Thread Marcel Holtmann
Hi Satya,

 I have a third part OEM layer. 
 According to the oFono architecture and standards can any one suggest
 the good approach to use the third part OEM layer @ oFono.

oFono is licensed under GPL v2 and as long as your OEM layer is
compatible with GPL v2 you can use it.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [SMS D-Bus 03/23] smutil.h: add missing header file dependencies

2010-06-25 Thread Marcel Holtmann
Hi Inaky,

  src/smsutil.h |9 +
  1 files changed, 9 insertions(+), 0 deletions(-)
 
 diff --git a/src/smsutil.h b/src/smsutil.h
 index 66ef6f8..baa3eca 100644
 --- a/src/smsutil.h
 +++ b/src/smsutil.h
 @@ -18,6 +18,14 @@
   *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  
 USA
   *
   */
 +#ifndef __smsutil_h__
 +#define __smsutil_h__

for internal headers I don't want the circular inclusion protection. It
buys us nothing. And leaving it out will warn us when we do circular
inclusion. That is a clear indication that something is a bit too
complicated if it is needed.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: Required suggestion to use third part code in oFono

2010-06-28 Thread Marcel Holtmann
Hi Satya,

a gently reminder that this mailing list doesn't like top posting.

 In this case is it required to make the third part OEM layer as OPEN-SOURCE.

Yes, every plugin, driver or extension of oFono has to be released as
open source under a GPL compatible license. As in general you have to be
compatible with the license of the project. If in doubt ask a lawyer.

Only the client applications, like the dialers etc., are except from
this. They use D-Bus as communication protocol.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: How about offering a raw way for SMS

2010-07-01 Thread Marcel Holtmann
Hi Caiwen,

please don't piggy-back on other threads. For new questions/discussion,
please start a brand new thread.

 Ofono stack encodes SMS before send it.  User may need send some
 special SMS. Such as send a v-card via SMS. 
 
 It need to add user data header to indicate it is a v-card. Ofono
 currently does not support this feature. How about 
 
 offering an API for user to sending raw PDU directly?  such as:
 
  void SendRawMessage(string pdu)
 
 So,  user can encode it by himself and send the PDU. It can also
 applied to sending message with validity period/status report 
 
 requirement and so on.
 
  
 
 Similarly,  how about sending out the raw PDU
 when dispatch incoming message?  There are many kinds of
 
 SMS, such as voice mail notification, MMS notification, push mail
 notification etc.  How about sending out the raw 
 
 PDU to upper layer and depending on upper layer to decode and decide
 how to process it? 

this is exactly what we NOT wanna do. There will be one entity that does
PDU encoding and decoding and this will be oFono. The reason behind this
is that keeping copies of PDU encoder/decoder around in every single
application is pretty much a really bad idea. We want it in one central
place with a large set of unit tests and proper long term testing. This
helps improving quality and ensures security and stability.

Having upper layers dealing with raw PDU is a design mistake that has
been made in the past way too many times. And this stops with oFono now.

So if you wanna send vCards or something similar, then you need to
extend oFono with interfaces to send vCards. Same goes for receiving
functionality.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 3/3] huawei: Remove call meter support for EM770

2010-07-01 Thread Marcel Holtmann
Hi Zhenhua,

 EM770W returns COMMAND NOT SUPPORT when we send AT+CCWE=1 to
 initialize call meter atom. So disable it.

when re-doing patch 2/3 then please fix this properly so that this extra
patch is not needed anymore.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: Atmodem PPP Link establishment issues

2010-07-06 Thread Marcel Holtmann
Hi Arun,

please try to compose proper emails and not just forward or reply to
your own message to send it again.

 We are using atmodem plugin and are trying to get the PPP up and
 running.  I am able to create and activate context, but the PPP link
 establishment seems to have issues.
 
 The list-context script returns:
 
 [ /generic ] 
 [ /generic/primarycontext3 ] 
 Username = 
 Name = Internet access 
 Settings = { Interface=ppp0 Netmask=255.255.255.255
 Method=static DomainNameServers=192.89.123.231,192.89.123.230,
 Address=109.240.91.14 }
 
 Active = 1 
 AccessPointName = internet 
 Password = 
 Type = internet
 
 But for a PPP link, there needs to be two ip addresses, one local and
 one of the peer. Here in the settings you see only one IP address.

No they don't have to be. Who says so. The second address of PPP is
totally irrelevant since it is a point-to-point link and we do interface
routing (route add default dev ppp0).

 I collected the logs during the PPP link establishment, (frames and a
 brief detail about it)
 
 Sent from  Ofono: 
 Frame: 0x7e 0xff   0x7d 0x23 0xc0 0x21 0x7d 0x21 0x7d 0x21 0x7d 0x20
 0x7d 0x2a 0x7d 0x22 0x7d 0x26 0x7d 0x20 0x7d 0x20 0x7d 0x20 0x7d 0x20
 0x58 0x7b 0x7e 
 
 03 c021 1 (conf req) 1 (id) 0x000a (length) 2 (accm) 6 (length) 
 fcs
 
 Recv from Modem: 
 Frame: 0x7e 0xff 0x7d 0x23 0xc0 0x21 0x7d 0x21 0x7d 0x21 0x7d 0x20
 0x7d 0x34 0x7d 0x22 0x7d 0x26 0x7d 0x20 0x7d 0x20 0x7d 0x20 0x7d 0x20
 0x7d 0x25 0x7d 0x26 0xd2 0x8a 0xde 0xd2 0x7d 0x27 0x7d 0x22 0x7d 0x28
 0x7d 0x22 0xbf 0x7d 0x35 0x7e
 
 3 c021 1 (conf req) 1 (id) 0x0034 2 6 0 0 0 0 5(magic num) 6 d2 8a de
 d2 7(protocol compression) 2 (id) 8 (addr, control compres) 2 fcs
 
 
 Frame: 0x7e 0xff 0x7d 0x23 0xc0 0x21 0x7d 0x22 0x7d 0x21 0x7d 0x20
 0x7d 0x2a 0x7d 0x22 0x7d 0x26 0x7d 0x20 0x7d 0x20 0x7d 0x20 0x7d 0x20
 0x31 0x7d 0x2f 0x7e
 
 3 c021 2 (conf ack) 1 (id) 0x000a 2 6 0 0 0 0 fcs
 
 Sent from  ofono: 
 Frame: 0x7e 0xff 0x7d 0x23 0xc0 0x21 0x7d 0x22 0x7d 0x21 0x7d 0x20
 0x7d 0x34 0x7d 0x22 0x7d 0x26 0x7d 0x20 0x7d 0x20 0x7d 0x20 0x7d 0x20
 0x7d 0x25 0x7d 0x26 0xd2 0x8a 0xde 0xd2 0x7d 0x27 0x7d 0x22 0x7d 0x28
 0x7d 0x22 0x54 0x7c 0x7e
 
 3 c021 2 (conf ack) 1 0x0034 2 6  5 6 d2 8a de d2 7 2 8 2 fcs
 
 
 Frame: 0x7e 0xff 0x3 0x80 0x21 0x1 0x1 0x0 0x22 0x3 0x6 0x0 0x0 0x0
 0x0 0x81 0x6 0x0 0x0 0x0 0x0 0x83 0x6 0x0 0x0 0x0 0x0 0x82 0x6 0x0 0x0
 0x0 0x0 0x84 0x6 0x0 0x0 0x0 0x0 0x80 0x52 0x7e
 
 3 8021 1 (conf req) 1 (id) 0x0022 (length)  3 (ip) 6  81(dns1) 6
  83(nbns1) 6  82(dns2) 6  84 (nbns2) 6  fcs
 
 Recv from Modem: 
 Frame: 0x7e 0xff 0x3 0x80 0x21 0x3 0x1 0x0 0x10 0x81 0x6 0xa 0xb 0xc
 0xd 0x83 0x6 0xa 0xb 0xc 0xe 0x4d 0x4d 0x7e 
 3 8021 3(conf nak) 1 (id) 0x010 81 (dns1) 6 abcd 83 (dns2) 6 abce fcs
 
 Sent from ofono: 
 Frame: 0x7e 0xff 0x3 0x80 0x21 0x1 0x2 0x0 0x22 0x3 0x6 0x0 0x0 0x0
 0x0 0x81 0x6 0xa 0xb 0xc 0xd 0x83 0x6 0xa 0xb 0xc 0xe 0x82 0x6 0x0 0x0
 0x0 0x0 0x84 0x6 0x0 0x0 0x0 0x0 0xad 0x7a 0x7e
 
 3 8021 1 (conf ack) 2 (id) 0x0022 3 6  81 6 abcd 83 6 abce 82 6
  84 6  fcs
 
 Recv from Modem: 
 Frame: 0x7e 0xff 0x3 0x80 0x21 0x3 0x2 0x0 0x10 0x81 0x6 0xa 0xb 0xc
 0xd 0x83 0x6 0xa 0xb 0xc 0xe 0x33 0x95 0x7e 
 3 8021 3(conf nak) 2 (id) 0x010 81 (dns1) 6 abcd 83 (dns2) 6 abce fcs
 
 Sent from ofono: 
 Frame: 0x7e 0xff 0x3 0x80 0x21 0x1 0x3 0x0 0x22 0x3 0x6 0x0 0x0 0x0
 0x0 0x81 0x6 0xa 0xb 0xc 0xd 0x83 0x6 0xa 0xb 0xc 0xe 0x82 0x6 0x0 0x0
 0x0 0x0 0x84 0x6 0x0 0x0 0x0 0x0 0xce 0x3a 0x7e
 
 3 8021 1 (conf req) 3 (id) 0x0022 3 6  81 6 abcd 83 6 abce 82 6
  84 6  fcs
 
 Recv from Modem: 
 Frame: 0x7e 0xff 0x3 0x80 0x21 0x1 0x1 0x0 0x4 0x0 0xb7 0x7e 
 3 8021 1 (conf req) 1 (id) 0 4  0 fcs
 
 Frame: 0x7e 0xff 0x3 0x80 0x21 0x4 0x3 0x0 0x10 0x82 0x6 0x0 0x0 0x0
 0x0 0x84 0x6 0x0 0x0 0x0 0x0 0x34 0xcc 0x7e 
 3 8021 4 (conf reject) 3(id) 0x0010 82 6  84 6  fcs
 
 Sent from ofono: 
 Frame: 0xff 0x3 0x80 0x21 0x2 0x1 0x0 0x4 0xcd 0x92 0x7e 
 3 8021 2 (conf ack) 1 (id) 0 4 
 
 Frame: 0xff 0x3 0x80 0x21 0x1 0x4 0x0 0x16 0x3 0x6 0x0 0x0 0x0 0x0
 0x81 0x6 0xa 0xb 0xc 0xd 0x83 0x6 0xa 0xb 0xc 0xe 0xe3 0x9 0x7e 
 
 3 8021 1 (conf req)  4 (id) 0x0016 3 6   81 6 abcd 83 6 abce fcs
 
 Recv from modem: 
 Frame: 0x7e 0xff 0x3 0x80 0x21 0x3 0x4 0x0 0x16 0x3 0x6 0x6d 0xf0 0x40
 0xbe 0x81 0x6 0xc0 0x59 0x7b 0xe7 0x83 0x6 0xc0 0x59 0x7b 0xe6 0xc5
 0xf1 0x7e
 
 3 8021 3 (conf nak) 4 (id) 0x0016 3 (ip) 6 6d f0 40 be 81 (dns1) 6 c0
 59 7b e7 83 (dns2) 6 c0 59 7b e6 fcs
 
 Sent from ofono: 
 Frame: 0x7e 0xff 0x3 0x80 0x21 0x1 0x5 0x0 0x16 0x3 0x6 0x6d 0xf0 0x40
 0xbe 0x81 0x6 0xc0 0x59 0x7b 0xe7 0x83 0x6 0xc0 0x59 0x7b 0xe6 0x2
 0x9c 0x7e
 
 3 8021 1 (conf req) 5 (id) 0x0016 3 6 6d f0 40 be 81 6 c0 59 7b e7 83
 6 c0 59 7b e6 fcs
 
 Recv from Modem: 
 Frame: 0x7e 0xff 0x3 0x80 0x21 0x2 0x5 0x0 0x16 0x3 0x6 0x6d 0xf0 0x40
 0xbe 0x81 0x6 0xc0 0x59 0x7b 0xe7 0x83 0x6 0xc0 0x59 0x7b 0xe6 0xf4
 0x6f 0x7e
 

Re: [PATCH 1/2] gatppp: Check ppp instance before unref it

2010-07-06 Thread Marcel Holtmann
Hi Zhenhua,

 ---
  gatchat/gatppp.c |3 +++
  1 files changed, 3 insertions(+), 0 deletions(-)
 
 diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
 index 1d41ded..d9b1627 100644
 --- a/gatchat/gatppp.c
 +++ b/gatchat/gatppp.c
 @@ -446,6 +446,9 @@ void g_at_ppp_unref(GAtPPP *ppp)
  {
   gboolean is_zero;
  
 + if (ppp == NULL)
 + return;
 +
   is_zero = g_atomic_int_dec_and_test(ppp-ref_count);
  
   if (is_zero == FALSE)

since we have been safe-guarding this in other unref functions as well,
I also applied this patch.

In general the calling code is doing something wrong here if it tries to
unref a NULL pointer.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH] test-server: Use cfmakeraw to set TTY raw mode

2010-07-06 Thread Marcel Holtmann
Hi Zhenhua,

 Use cfmakeraw to disable echoing and special characters processing. If
 we don't turn off ICRNL, TTY layer translates \r\n to \n\n.
 ---
  gatchat/test-server.c |8 +++-
  1 files changed, 3 insertions(+), 5 deletions(-)
 
 diff --git a/gatchat/test-server.c b/gatchat/test-server.c
 index 25a1192..2911978 100644
 --- a/gatchat/test-server.c
 +++ b/gatchat/test-server.c
 @@ -848,12 +848,10 @@ static void set_raw_mode(int fd)
  {
   struct termios options;
  
 + memset(options, 0, sizeof(struct termios));
   tcgetattr(fd, options);
 -
 - /* Set TTY as raw mode to disable echo back of input characters
 -  * when they are received from Modem to avoid feedback loop */
 - options.c_lflag = ~(ICANON | ECHO | ECHOE | ISIG);
 -
 + tcflush(fd, TCIOFLUSH);
 + cfmakeraw(options);
   tcsetattr(fd, TCSANOW, options);
  }

I am fine with using cfmakeraw. So patch has been applied.

Minor comment here that sizeof(options) would be better then referencing
the struct itself. And in general we have used ti as variable name for
termios options. Don't ask me really why. Just have done that in the
patch. So feel free to send an cleanup patch.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 11/15] emulator: Register mandatory AT command handlers

2010-07-07 Thread Marcel Holtmann
Hi Remi,

  +static void cfun_cb(GAtServerRequestType type, GAtResult *cmd,
  +   gpointer user_data)
  +{
  +   struct ofono_emulator *e = user_data;
  +   char buf[50];
  +
  +   switch (type) {
  +   case G_AT_SERVER_REQUEST_TYPE_SUPPORT:
  +   ofono_emulator_send_info(e, +CFUN: (0-1), TRUE);
  +   ofono_emulator_send_final(e, G_AT_SERVER_RESULT_OK);
  +   break;
  +   case G_AT_SERVER_REQUEST_TYPE_QUERY:
  +   snprintf(buf, sizeof(buf), +CFUN: %d, e-modem_mode);
  +   ofono_emulator_send_info(e, buf, TRUE);
  +   ofono_emulator_send_final(e, G_AT_SERVER_RESULT_OK);
  +   break;
 
 Does not make much sense. If mode is 0, the device has functionality
 minimum, meaning usually power off or close to that. oFono does not run in
 such stage, so it won't be there to answer CFUN: 0.

this is actually more fake than real. We are not mapping all commands
1:1 to the real modem. It is mainly to just make some dialers and things
like Windows happy ;)

  +   case G_AT_SERVER_REQUEST_TYPE_SET:
  +   {
  +   GAtResultIter iter;
  +   int mode;
  +
  +   g_at_result_iter_init(iter, cmd);
  +   g_at_result_iter_next(iter, +CFUN=);
  +
  +   if (g_at_result_iter_next_number(iter, mode) == FALSE)
  +   goto error;
  +
  +   if (mode != 0  mode != 1)
  +   goto error;
  +
  +   DBG(set CFUN to %d, mode);
  +
  +   e-modem_mode = mode;
  +   g_timeout_add_seconds(1, send_ok, e);
  +   break;
  +   }
 
 I might be missing something, but it does not look like AT+CFUN=0 will
 power off the system, which is more or less what it is expected to do.

Look again, it won't. It is an arbitrary delay for testing. If we would
follow the Bluetooth DUN specification literally, then only the magic
AT*99# syntax would be needed, but there are other commands that are
expected to work. So in most cases we just fake them.

 +static void cops_cb(GAtServerRequestType type, GAtResult *result,
  +   gpointer user_data)
  +{
  +   struct ofono_emulator *e = user_data;
  +   char buf[50];
  +
  +   switch (type) {
  +   case G_AT_SERVER_REQUEST_TYPE_SUPPORT:
  +   ofono_emulator_send_final(e, G_AT_SERVER_RESULT_OK);
  +   break;
  +   case G_AT_SERVER_REQUEST_TYPE_QUERY:
  +   snprintf(buf, sizeof(buf), +COPS: %d, e-modem_cops);
  +   ofono_emulator_send_info(e, buf, TRUE);
  +   ofono_emulator_send_final(e, G_AT_SERVER_RESULT_OK);
  +   break;
  +   case G_AT_SERVER_REQUEST_TYPE_SET:
  +   {
  +   GAtResultIter iter;
  +   int mode;
  +
  +   g_at_result_iter_init(iter, result);
  +   g_at_result_iter_next(iter, +COPS=);
  +
  +   if (g_at_result_iter_next_number(iter, mode) == FALSE)
  +   goto error;
  +
  +   if (mode  0 || mode  2)
  +   goto error;
  +
  +   DBG(set GPRS cops status %d, mode);
  +
  +   e-modem_cops = mode;
  +   ofono_emulator_send_final(e, G_AT_SERVER_RESULT_OK);
 
 You won't get away with this. You're supposed to register/unregister/etc
 from the network here.
 Similarly, AT+CGATT is wrong, GPRS should be attached/detached. Yes, that
 means you may have to tear down someone else's GPRS context, which may seem
 evil.

Same here actually. We will fake COPS and CGATT for the client. Compared
to the CFUN code this needs a bit more work since we should return an
error when the real modem is not ready yet or GPRS is not attached.

That said, there has to be done some work for CFUN as well if our modem
is offline. However my take here is to just disable DUN service when the
modems goes offline.

 Last, AT+CPIN should at the very least return PIN READY when appropriate,
 if I read the spec right.

We will fake this again and always return READY here. If the physical
modem is not ready (sim_post state), then we will not provide DUN server
support.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


RE: [PATCH 11/15] emulator: Register mandatory AT command handlers

2010-07-08 Thread Marcel Holtmann
Hi Remi,

  Marcel is right. Here I fake most AT commands like +CFUN to make DUN
  client happy. We don't need to power on/off real modem at all. See
  gatchat/test-server.c for similar implementation.
 
 That depends much what you're trying to achieve. If you only care about
 Dial-Up Networking, that might work. But if you care about passing various
 some kinds of certification testing, AT commands _must_ work _correctly_.
 
 So I doubt oFono will get away with this in the medium term.

we are clearly and on purpose splitting Bluetooth Dialup support from
generic Serial Port Profile support.

For the serial port for certification testing you will NOT be able to
use dialup channel. Also I doubt that you wanna do the certification
testing over Bluetooth. I assume that will most likely be done via USB
with the device in some special RD mode.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 1/1] ppp: Add MAX_IPCP_FAILURE to avoid timeout quickly

2010-07-09 Thread Marcel Holtmann
Hi Zhenhua,

 We use IPCP NAK response to stall the progress of acquiring the client
 IP address from DHCP server. So we need to increase the max failure of
 NAKs in IPCP handshaking.
 ---
  gatchat/ppp_cp.c   |8 ++--
  gatchat/ppp_cp.h   |2 +-
  gatchat/ppp_ipcp.c |   10 +-
  gatchat/ppp_lcp.c  |2 +-
  4 files changed, 17 insertions(+), 5 deletions(-)
 
 diff --git a/gatchat/ppp_cp.c b/gatchat/ppp_cp.c
 index 647e241..e013283 100644
 --- a/gatchat/ppp_cp.c
 +++ b/gatchat/ppp_cp.c
 @@ -997,7 +997,7 @@ void pppcp_set_local_options(struct pppcp_data *pppcp,
  }
  
  struct pppcp_data *pppcp_new(GAtPPP *ppp, const struct pppcp_proto *proto,
 - gboolean dormant)
 + gboolean dormant, int max_failure)

since the value is actually unsigned here, please also use guint and not
int for this parameter.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 1/1] ppp: Add MAX_IPCP_FAILURE to avoid timeout quickly

2010-07-09 Thread Marcel Holtmann
Hi Zhenhua,

 We use IPCP NAK response to stall the progress of acquiring the client
 IP address from DHCP server. So we need to increase the max failure of
 NAKs in IPCP handshaking.
 ---
  gatchat/ppp_cp.c   |8 ++--
  gatchat/ppp_cp.h   |2 +-
  gatchat/ppp_ipcp.c |   10 +-
  gatchat/ppp_lcp.c  |2 +-
  4 files changed, 17 insertions(+), 5 deletions(-)

patch has been applied. Thanks.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [SMS D-Bus 11/23] struct tx_queue_entry: add fields and destructor

2010-07-10 Thread Marcel Holtmann
Hi Inaky,

  Maybe we're not talking about the same thing.  My workflow is
  
  git commit / git am
  git fetch
  git rebase
  git push
 
 gee, didn't see this -- well, if the rebase is changing the commit IDs,
 which it probably does, then you are breaking the commit history. Would
 depend on the details.
 
 Anyway, I am not so sure that is the problem. Spent the whole day going
 around it and I still couldn't figure the sucker out. Will keep trying.

the ofono.git master tree never got re-based. All commit ids are fully
atomic. So that is not the problem.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 4/4] make bluetooth.{c,h} a static library

2010-07-18 Thread Marcel Holtmann
Hi Gustavo,

  Makefile.am |7 +++
  plugins/bluetooth.c |3 ---
  2 files changed, 3 insertions(+), 7 deletions(-)

why are we doing this exactly? If I missed it, please explain it again
since I am not sure that I agree.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 4/4] make bluetooth.{c,h} a static library

2010-07-18 Thread Marcel Holtmann
Hi Gustavo,

Makefile.am |7 +++
plugins/bluetooth.c |3 ---
2 files changed, 3 insertions(+), 7 deletions(-)
  
  why are we doing this exactly? If I missed it, please explain it again
  since I am not sure that I agree.
 
 We are doing this for the DUN daemon. As you guys said to me it will be
 separated daemon in oFono sources, so we need bluetooth.c as a static
 library to reuse it in the DUN daemon.

we need to discuss this again. I am not sure it is the best idea to have
this as a separate daemon. I argue with myself forth and back on this
idea. One option is to do this as an oFono atom, another one is to do
this as a separate daemon. Currently the atom idea is winning.

Also there is no need to create a static library for doing this. That is
just wrong. We can do the build magic with autofoo properly and without
having to use a hack with a static library.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 4/4] make bluetooth.{c,h} a static library

2010-07-18 Thread Marcel Holtmann
Hi Gustavo,

  Makefile.am |7 +++
  plugins/bluetooth.c |3 ---
  2 files changed, 3 insertions(+), 7 deletions(-)

why are we doing this exactly? If I missed it, please explain it again
since I am not sure that I agree.
   
   We are doing this for the DUN daemon. As you guys said to me it will be
   separated daemon in oFono sources, so we need bluetooth.c as a static
   library to reuse it in the DUN daemon.
  
  we need to discuss this again. I am not sure it is the best idea to have
  this as a separate daemon. I argue with myself forth and back on this
  idea. One option is to do this as an oFono atom, another one is to do
  this as a separate daemon. Currently the atom idea is winning.
 
 Ok, so I'll wait to continue my implementation of the DUN client. I
 don't know the oFono internal too much to help on that decision.

I was talking about the server and not the client. For the client we
have to have a chat about it. I might have missed parts of the initial
discussion.
 
  Also there is no need to create a static library for doing this. That is
  just wrong. We can do the build magic with autofoo properly and without
  having to use a hack with a static library.
 
 I meant build statically here like gatchat, without any hack with static
 library. ;)

Actually gatchat is not build statically. It is build as part of out AT
modem driver. That it is in separate directory is purely cosmetic.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: Reg. MMS Support in oFono

2010-07-20 Thread Marcel Holtmann
Hi Satya,

 I have one doubt regarding MMS features. 
 
 
 In previous mail list ofono team mention that MMS is not supported by
 oFono, these belong in a separate daemon. Related link is as follows
  :  http://lists.ofono.org/pipermail/ofono/2010-June/002672.html
 
 
 I thought it is ConnMan.  
 
 
 But in ConnMan mailing list it is saying that oFono will export these
 via a Socks5 or HTTP proxy to the MMS application without having
 ConnMan involved. Related link is as
 follows : http://lists.connman.net/pipermail/connman/2010-June/001515.html
 
 
 May I know what is that daemon that supports MMS features (upload /
 download, MMS read receipt indication, Delivery report indication).

this is all true. The MMS application/daemon will talk over a Socks5
proxy to oFono to get access to the MMSC. And will use oFono's D-Bus
interface to get MMS push notifications.

For MMS support we are on purpose trying to avoid using ConnMan and will
handle all TCP connections that the MMS application/daemon needs via a
proxy. This is important to not interfere with the Internet connections.
In general the contexts for MMS are just for MMS only and so we handle
them internally in oFono.

I might look rather complicated and complex right now, but once the
basic pieces are getting merged into oFono, I hope this becomes a lot
clearer.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: About the TDS-CDMA support of oFono

2010-07-21 Thread Marcel Holtmann
Hi Zheng,

quick reminder that it is NOT okay to do top posting. Please use proper
mailing list netiquette. Top posting emails will be otherwise ignored in
the future.

 OK, Thank you. We are going to buy Huawei ET128 to use oFono in the
 TD-SCDMA environment n China. I will share some info to discuss with
 everyone later.OK, Thank you. We are going to buy Huawei ET128 to use
 oFono in the TD-SCDMA environment n China. I will share some info to
 discuss with everyone later.

If this is for pure data connections, then the modifications to support
CDMA should be limited. And maybe can be done quickly. If we are talking
about full blown CDMA support, that is a different story. Not sure if
the ET128 is actually voice capable or not.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: Howto about Qt QML and Ofono

2010-07-22 Thread Marcel Holtmann
Hi Joao,

  http://matgnt.wordpress.com/2010/07/19/qt-ofono-d-bus-and-qml-part-1/
  http://matgnt.wordpress.com/2010/07/19/qt-ofono-d-bus-and-qml-part-2/
  http://matgnt.wordpress.com/2010/07/19/qt-ofono-d-bus-and-qml-part-3/
 
  I hope this helps people out there to create cool new phone
  applications. You're welcome to leave a note here or on the howto 
  directly.
 
 
  Your lovely program gives a good demo on how to writing an application 
  based on
 oFono. oFono does claim it will help on building application in a 
 convenient way, so
 the prosperity of application will definitely bring feedback to oFono and 
 make oFono
 grow better.
  I have two comments on your first part: 1) configure is not in the git 
  tree of oFono, so
 we will use bootstrap-configure instead. 2) I'm sure your phonesim works 
 well. But we
 also have another phonesim derived from your repo, which we had some 
 modification
 and enhancement in. The repo is
 git://git.kernel.org/pub/scm/network/ofono/phonesim.git. The building 
 command is
 same as oFono, and the execution command is ./phonesim -p 12345 -gui
 src/default.xml.
 
 Thanks Yang, I've added the ./bootstrap command to the listing. I also
 wrote down a hint to the ofono branch of phonesim.
 
  bootstrap-configure is a single script, rather than two different ones.
 
 
 There is a bootstrap script also, and bootstrap-configure basically
 runs it and then runs configure with a pre-set of switches.

and more important with a pre-set of switches that every developer
should use since it enables proper debug and maintainer mode. With these
switches the compiler becomes your best test tool before submitting
patches.

However bootstrap and bootstrap-configure is not intended for packaging.
And that is why you only find these in the ofono.git repository and not
in the tarballs.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 1/2] stkutil: convert img to xpm

2010-07-22 Thread Marcel Holtmann
Hi Kristen,

  Also, XPM is pretty flexible here, there's no need to use 0-9 digits
  only.  For instance, you can do two look up tables, one for images with
  LUTs up to 64 entries and another for LUTs with more.  E.g.
 
  a-za-z...@$ and 'aa ab ac .. pp' or something like that.
  
  I realize this - this was an implementation choice to keep the code
  less complex, thinking that it would be better to favor that vs.
  saving some bytes on the xpm.
 
 The wording on my part was poor.  Of course you knew this.  My intent
 was to encourage the optimization of the xpm format.  Remember these
 images are up to 64k in size, so a difference between transferring /
 using up 128k and 192k respectively is quite significant.  The added
 (minor) code complexity is definitely worth it.

we need such optimization in this case. We are already accepting a
penalty hit by having to transfer the images over D-Bus. So every single
byte we can save here is a win.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH] watch: Free service data in service_reply

2010-07-23 Thread Marcel Holtmann
Hi Zhenhua,

 Avoid the memory leak of server_data.
 ---
  gdbus/watch.c |5 -
  1 files changed, 4 insertions(+), 1 deletions(-)

do me a favor and post this to linux-blueto...@vger.kernel.org as well
for the Bluetooth guys to review. After that I can take of applying it
to all trees.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: CDMA support in Ofono

2010-07-25 Thread Marcel Holtmann
Hi Steven,

 Currently, Ofono only support GSM/UTMS, is there any plan to support 
 CDMA/EVDO and when will Ofono support CDMA?
 
 Any suggestions or ideas about supporting CDMA will be appreciated .

so CDMA and also EVDO is on the long-term feature list of oFono. Right
now the focus is clearly on GSM/UMTS. One reason might be that CDMA in
Europe is not really existent or that more documentation and/or hardware
is available.

I foresee that EVDO integration for data connections with oFono might be
a lot simpler to integrate than a full CDMA stack with voice and text
messaging features. However we are more than happy to see CDMA and EVDO
contributions from other individuals or companies.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: USSD support in ofono-0.25

2010-07-30 Thread Marcel Holtmann
Hi,

 Is USSD support is available and functional in ofono latest version?

yes it is.

 I found the support in SupplementaryServices interface and I am able
 to read the properties from this interface, but I am getting error
 when try to call Initiate method,
 from the definition of Initiate method in ussd.c found that out
 arguments are sv. below is the test code.
 
 gchar *str=NULL;
 GValue val={0,};
 g_value_init(val,G_TYPE_STRING);
 dbus_g_proxy_call (proxy,
 Initiate,error,G_TYPE_STRING,*#43#,G_TYPE_INVALID,G_TYPE_STRING,str,G_TYPE_VALUE,val,
  G_TYPE_INVALID);
 
 if above code is executed  , the error is 
  GLib-GObject-WARNING **: gvalue.c:185: cannot initialize GValue with
 type `GValueArray_gchararray+GHashTable_gchararray+GValue__', the
 value has already been initialized as `gchararray'
 
 Couldn't convert argument, expected GValue 
 
 
 and in generate_cw_ss_query_reply callback , message signature is
 (sa{sv})
 so tried with 
 
 GArray *list;
 GHashTable *OUT_arg0=NULL;
 dbus_g_proxy_call (proxy,
 Initiate,error,G_TYPE_STRING,*#43#,G_TYPE_INVALID,
 dbus_g_type_get_struct (GValueArray,
 G_TYPE_STRING,
 dbus_g_type_get_map (GHashTable,
 G_TYPE_STRING, G_TYPE_VALUE),G_TYPE_INVALID),
 OUT_arg0, G_TYPE_INVALID);
 
 but it is returning error  Expected D-BUS struct, got type code 's' 
 
 can any body please tell me the correct return types.
 
 in both cases ofono is getting response from phonesim.
 
 and no proper documentation is available for USSD support .

Look at doc/supplementaryservices-api.txt for a detailed description of
the API. And test/test-ussd is an example in Python on how to use it.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: USSD support in ofono-0.25

2010-07-30 Thread Marcel Holtmann
Hi,

a gentle reminder that we are not allowing top-posting on this mailing
list. Don't do it.

  supplementaryservices-api.txt is not available in doc
 folder in ofono-0.25.tar.gz ,is there any other location to find it?

it is in the GIT repository, but was never included in the tarballs. I
fixed that now.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: Add CDMA/EVDO into TODO

2010-08-02 Thread Marcel Holtmann
Hi Penghuo,

I am not fully against submitting patches as attachments, but if you do
expect reviews, then we do prefer inline patches. I would advise to
start using git send-email for patch submission.

 From 9af61f4d2546ff28a8b1ed9546c41f5f3c945f5a Mon Sep 17 00:00:00 2001
 From: Pengzhuo wang pengzhuo.wa...@windriver.com
 Date: Tue, 3 Aug 2010 19:53:32 +0800
 Subject: [PATCH] Update TODO to add CDMA/EVDO support
 
 ---
  TODO |   25 +
  1 files changed, 25 insertions(+), 0 deletions(-)
 
 diff --git a/TODO b/TODO
 index 5221324..b9130d3 100644
 --- a/TODO
 +++ b/TODO
 @@ -567,3 +567,28 @@ Miscellaneous
  
Priority: High
Complexity: C1
 +
 +CDMA/EVDO
 +=
 +
 +- Extend modem interface with a 'Type' property to indicate the modem
 type. 
 +
 +  Priority: High
 +  Complexity: C1

We need to discuss this with Denis. Either we do it here on the mailing
list or on #ofono IRC channel on Freenode.

 +- Disable CDMA unsupport feature for CDMA modem. Include USSD,
 supplementary services, 
 +  SSN, PDP context.  
 +  
 +  Priority: High
 +  Complexity: C2

Actually this is not a TODO item. That is a modem plugin specific
detail. If your modem (CDMA or not) doesn't supports such features, then
don't enable the atoms for it.

So I say that this already supported ;)

 +- Add call forwarding, call waiting, network registration
 implementation for CDMA.
 +  
 +  Priority: High
 +  Complexity: C4

This is more like add CDMA specific atom drivers for netreg etc. That
these are CDMA specific is more a minor detail.

I like to hear Denis comment on his, but first guess would be that this
should be done as drivers/cdmamodem/...

 +- Change radio setting, SIM manager, SMS manager, voice call manager,
 Data Connection
 +  Manager to make them support both GSM and CDMA
 +  
 +  Priority: High
 +  Complexity: C8

I think you have to be a bit more specific here. Maybe it is a good idea
to split them into separate items.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [SMS D-Bus v5 14/17] automake: fix generation of symlinks for headers

2010-08-05 Thread Marcel Holtmann
Hi Inaky,

 When running 'make distcheck' from a vpath build directory (ie: one
 that is not where the source lives), the target headers in
 include/ofono are not generated properly due to a typo in the
 Makefile.am, that is using _srcdir where _buildir should be being
 used.
 ---
  Makefile.am |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/Makefile.am b/Makefile.am
 index b64ce8e..28d1e77 100644
 --- a/Makefile.am
 +++ b/Makefile.am
 @@ -446,7 +446,7 @@ include/ofono/version.h: include/version.h
  
  include/ofono/%.h: include/%.h
   $(AM_V_at)$(MKDIR_P) include/ofono
 - $(AM_V_GEN)$(LN_S) $(abs_top_srcdir)/$ $@
 + $(AM_V_GEN)$(LN_S) $(abs_top_builddir)/$ $@

are we having the same problem in ConnMan. I am seeing only builddir be
used for the version.h generation.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH] add default property setter for modemconf

2010-08-09 Thread Marcel Holtmann
Hi Leaf,

 I was trying to run ofono for a huawei usb modem on my meego netbook,
 but the udev on netbook seems not work for ofono, so that I've to
 mannualy write settings in modem.conf.
 
 Huawei driver needs two device, pcui  modem, and I can't specify them
 by either Interface  Device in modem.conf.
 
 Then I wrote this patch, It will load all Key=Value pairs in
 modem.conf file( Except Driver=xxx pair ), and store them to modem
 properties.
 
 Then I add Pcui=  Modem= entry in modem.conf file, it works, I can
 use ofono to send  recieve message by huawei stick now.

my question is why you wanna do this at all? oFono should auto-detect
any Huawei modem via udev. So this is not needed.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH v2 1/1] sim: Read EFust and EFest

2010-08-11 Thread Marcel Holtmann
Hi Yang,

 ---
  src/sim.c |   83 ++-
  src/simutil.h |  108 
 +
  2 files changed, 189 insertions(+), 2 deletions(-)
 
 diff --git a/src/sim.c b/src/sim.c
 index d2ed780..9fc1bc6 100644
 --- a/src/sim.c
 +++ b/src/sim.c
 @@ -95,6 +95,8 @@ struct ofono_sim {
   void *driver_data;
   struct ofono_atom *atom;
   DBusMessage *pending;
 + unsigned char service_ust[(SIM_UST_SERVICE_MAX - 1) / 8 + 1];
 + unsigned char service_est[(SIM_EST_SERVICE_MAX - 1) / 8 + 1];
  };
  
  struct msisdn_set_request {
 @@ -128,6 +130,18 @@ static const char *const passwd_name[] = {
   [OFONO_SIM_PASSWORD_PHCORP_PUK] = corppuk,
  };
  
 +inline gboolean sim_get_ust(unsigned char *service_table,
 + enum sim_ust_service index)
 +{
 + return (service_table[index / 8]  (index % 8))  1;
 +}
 +
 +inline gboolean sim_get_est(unsigned char *service_table,
 + enum sim_est_service index)
 +{
 + return (service_table[index / 8]  (index % 8))  1;
 +}
 +
  static const char *sim_passwd_name(enum ofono_sim_password_type type)
  {
   return passwd_name[type];
 @@ -1076,6 +1090,71 @@ static void sim_retrieve_imsi(struct ofono_sim *sim)
   sim-driver-read_imsi(sim, sim_imsi_cb, sim);
  }
  
 +static void sim_efest_read_cb(int ok, int length, int record,
 + const unsigned char *data,
 + int record_length, void *userdata)
 +{
 + struct ofono_sim *sim = userdata;
 + int i;
 + int size;
 +
 + if (!ok)
 + goto out;
 +
 + if (length  1) {
 + ofono_error(EFest shall contain at least one byte);
 + goto out;
 + }
 +
 + size = sizeof(sim-service_est) / sizeof(*sim-service_est);
 + for (i = 0; i  size; i++) {
 + if (i = length)
 + sim-service_est[i] = 0;
 + else
 + sim-service_est[i] = data[i];
 + }
 +
 +out:
 + sim_retrieve_imsi(sim);
 +}
 +
 +static void sim_efust_read_cb(int ok, int length, int record,
 + const unsigned char *data,
 + int record_length, void *userdata)
 +{
 + struct ofono_sim *sim = userdata;
 + int i;
 + int size;
 +
 + if (!ok)
 + goto out;
 +
 + if (length  1) {
 + ofono_error(EFust shall contain at least one byte);
 + goto out;
 + }
 +
 + size = sizeof(sim-service_ust) / sizeof(*sim-service_ust);
 + for (i = 0; i  size; i++) {
 + if (i = length)
 + sim-service_ust[i] = 0;
 + else
 + sim-service_ust[i] = data[i];
 + }
 +
 +out:
 + ofono_sim_read(sim, SIM_EFEST_FILEID,
 + OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
 + sim_efest_read_cb, sim);
 +}
 +
 +static inline void sim_retrieve_efust(struct ofono_sim *sim)
 +{
 + ofono_sim_read(sim, SIM_EFUST_FILEID,
 + OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
 + sim_efust_read_cb, sim);
 +}
 +
  static void sim_pin_query_cb(const struct ofono_error *error,
   enum ofono_sim_password_type pin_type,
   void *data)
 @@ -1110,13 +1189,13 @@ static void sim_pin_query_cb(const struct ofono_error 
 *error,
  
  checkdone:
   if (pin_type == OFONO_SIM_PASSWORD_NONE)
 - sim_retrieve_imsi(sim);
 + sim_retrieve_efust(sim);
  }
  
  static void sim_pin_check(struct ofono_sim *sim)
  {
   if (!sim-driver-query_passwd_state) {
 - sim_retrieve_imsi(sim);
 + sim_retrieve_efust(sim);
   return;
   }
  
 diff --git a/src/simutil.h b/src/simutil.h
 index 29194ca..0ff372c 100644
 --- a/src/simutil.h
 +++ b/src/simutil.h
 @@ -26,9 +26,11 @@ enum sim_fileid {
   SIM_EF_CPHS_MWIS_FILEID = 0x6f11,
   SIM_EF_CPHS_INFORMATION_FILEID = 0x6f16,
   SIM_EF_CPHS_MBDN_FILEID = 0x6f17,
 + SIM_EFUST_FILEID = 0x6f38,
   SIM_EFMSISDN_FILEID = 0x6f40,
   SIM_EFSPN_FILEID = 0x6f46,
   SIM_EFSDN_FILEID = 0x6f49,
 + SIM_EFEST_FILEID = 0x6f56,
   SIM_EFAD_FILEID = 0x6fad,
   SIM_EFPHASE_FILEID = 0x6fae,
   SIM_EFPNN_FILEID = 0x6fc5,
 @@ -53,6 +55,106 @@ enum sim_file_access {
   SIM_FILE_ACCESS_NEVER = 15,
  };
  
 +/* 131.102 Section 4.2.8 */
 +enum sim_ust_service {
 + SIM_UST_SERVICE_LOCAL_PHONE_BOOK= 0,
 + SIM_UST_SERVICE_FDN = 1,
 + SIM_UST_SERVICE_EXT_2   = 2,
 + SIM_UST_SERVICE_SDN = 3,
 + SIM_UST_SERVICE_EXT_3   = 4,
 + SIM_UST_SERVICE_BDN = 5,
 + SIM_UST_SERVICE_EXT_4   = 6,
 + 

Re: [PATCH 0/10] Unregister AT notifiers when removing drivers

2010-08-12 Thread Marcel Holtmann
Hi Zhenhua,

 This series unregister AT notifiers in removing various AT modem drivers when 
 modem goes to offline mode. Please review them.

just a heads up here that Denis and I talked about this. And we are
going to fix this inside GAtChat in the background for you. Trying to
fix this single handed in every atom driver is wrong.

This approach creates too much of a maintenance burden. And additional
code complexity and memory footprint doing it this way. So we have to
tackle it in a central place. And that is GAtChat.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 04/16] emulator: Add APIs to send GAtServer result

2010-08-12 Thread Marcel Holtmann
Hi Zhenhua,

 Defines APIs to send final/intermediate/unsolicited result to DUN
 client.
 ---
  include/emulator.h |2 ++
  src/emulator.c |   28 
  src/ofono.h|   11 +++
  3 files changed, 41 insertions(+), 0 deletions(-)
 
 diff --git a/include/emulator.h b/include/emulator.h
 index 29e87b9..2a45c65 100644
 --- a/include/emulator.h
 +++ b/include/emulator.h
 @@ -36,6 +36,8 @@ enum ofono_emulator_status {
   OFONO_EMULATOR_STATUS_DESTROY,
  };
  
 +enum _GAtServerResult;
 +

this is not good. It should not be needed outside of GAtChat. Why not
just include gatserver.h?

And in addition the _GAt declaration should never be used. Why not use
the typedef one?

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Removal of modemconf plugin

2010-08-12 Thread Marcel Holtmann
Hello,

so the modemconf plugin seemed to be a good idea at the time I wrote it,
but lately is causes more issues and confusion than it solves. So we
will be removing this plugin shortly.

For this to happen the following dependencies need to be solved first:

1) Move phonesim plugin over to use /etc/ofono/phonesim.conf

2) Create configure option to build without phonesim support

3) Create udev rule for Calypso device detection

4) Modify Calypso plugin to work with udev rule

Using special udev rules for special devices (in conjunction with the
oFono auto-detection rule) make the process of setting up oFono for the
system integrator a lot simpler. No manual configuration file patching
anymore. Just installation of an additional udev rule.

The open here is the ISI modem detection, but this should be clearly
done via a Phonet plugin. And also the N900 specific case inside
modemconf is just wrong. This needs to be fixed and just auto-detected
via Phonet or maybe just via RTNL directly.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: Startup sequence for Online / offline mode

2010-08-12 Thread Marcel Holtmann
Hi Waldo,

  Modem drivers that support Online / offline mode default to offline when
  oFono loads them. Which component is responsible for calling oFono and
  switch the modem to online mode? Will that component be part of MeeGo?
 
 This feature is still highly experimental and not all of the details
 have been figured out (only ISI supports it today).
 
 Our current thinking is to have ConnMan manage the Online state of the
 GSM modems (e.g. replace Powered handling with Online handling.)
 However, we're still pretty far from that; we'd need to migrate all
 existing modem drivers to manage Online/Offline state properly first.

I agree with Denis here. The Online state needs to be controlled by
ConnMan, but we haven't done that change yet. It would break current
users since we haven't migrated all modems to handle Online state
properly or emulate it if needed.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


RE: Startup sequence for Online / offline mode

2010-08-12 Thread Marcel Holtmann
Hi Waldo,

  Modem drivers that support Online / offline mode default to offline when
  oFono loads them. Which component is responsible for calling oFono and
  switch the modem to online mode? Will that component be part of MeeGo?
  
 This feature is still highly experimental and not all of the details
 have been figured out (only ISI supports it today).
  
 Our current thinking is to have ConnMan manage the Online state of the
 GSM modems (e.g. replace Powered handling with Online handling.)
 However, we're still pretty far from that; we'd need to migrate all
 existing modem drivers to manage Online/Offline state properly first.
 
 I agree with Denis here. The Online state needs to be controlled by
 ConnMan, but we haven't done that change yet. It would break current
 users since we haven't migrated all modems to handle Online state
 properly or emulate it if needed.
 
 My understanding was that the Online state would be initialized by a 
 component like Telepathy-ring or some sort of system management daemon to 
 ensure that the dialer is up and running and able to accept calls before the 
 Online state is entered. If ConnMan initializes the Online state, how is it 
 ensured that all required clients (Dialer, SMS, etc.) have registered already?

ConnMan has control over the flight mode and thus is has control over
the online state. If it happens that ConnMan switches the modem online
and call gets received before the dialer is running, then it might time
out in the end. And that is just fine with me.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: Startup sequence for Online / offline mode

2010-08-13 Thread Marcel Holtmann
Hi Pekka,

  My understanding was that the Online state would be initialized by a 
  component like Telepathy-ring or some sort of system management daemon to 
  ensure that the dialer is up and running and able to accept calls before 
  the Online state is entered. If ConnMan initializes the Online state, how 
  is it ensured that all required clients (Dialer, SMS, etc.) have registered 
  already?
 
 I'd propose the connman would delay the transition of ofono modems to
 the online until the tp-ring/dialer has registered with ofono modem.
 The VoiceCallManager would need some kind of
 RegisterCallHandlermethod and add the list of the registered call
 handlers as a property. I suppose this would concern only the
 voice-centric devices.

this sounds rather complicated and I don't wanna have a dependency for
tp-ring here in the mix. Also I think this is more a theoretical problem
than a real one. tp-ring needs to check for the current ringing calls
anyway and can just pick where it left off. So lets talk about this once
we cross this bridge.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: Removal of modemconf plugin

2010-08-13 Thread Marcel Holtmann
Hi Kalle,

  Using special udev rules for special devices (in conjunction with the
  oFono auto-detection rule) make the process of setting up oFono for the
  system integrator a lot simpler. No manual configuration file patching
  anymore. Just installation of an additional udev rule.
 
  The open here is the ISI modem detection, but this should be clearly
  done via a Phonet plugin. And also the N900 specific case inside
  modemconf is just wrong. This needs to be fixed and just auto-detected
  via Phonet or maybe just via RTNL directly.
 
 What about old style serial port modems? How can we configure them if
 you remove modemconf?

with a proper udev rule. All old style modems show up in udev as well.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: Removal of modemconf plugin

2010-08-13 Thread Marcel Holtmann
Hi Pekka,

  The open here is the ISI modem detection, but this should be clearly
  done via a Phonet plugin. And also the N900 specific case inside
  modemconf is just wrong. This needs to be fixed and just auto-detected
  via Phonet or maybe just via RTNL directly.
 
 Once we get proper n900modem driver tested and integrated, the
 isimodem stuff can be removed, too.

so how close are you with that. I am going to remove modemconf sooner
than later actually.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: Enabling/disabling the GPS part of a Huawei EM770W

2010-08-13 Thread Marcel Holtmann
Hi Florian,

 is it possible to enable / disable the GPS part of a Huawei EM770W 3G 
 modem through a DBUS command? It is done with the command AT^WPDGP and 
 AT^WPEND on the modem (tested it on PCUI port). Or is it possible to 
 send generic AT commands through DBUS?

we know that we need some sort of power control atom for GPS devices.
The Ericsson MBM and Option HSO have also GPS chips on board. So far
nobody has proposed a GPS atom for power control of the GPS device yet.
So you could be the first one. Go ahead and send a patch.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: Query on Ofono Architecture for multiple RATs

2010-08-13 Thread Marcel Holtmann
Hi Naresh,

 Lets suppose that I have a modem which implements various radio access
 technologies like CDMA, WCDMA,  GSM, LTE, etc. The questions I have
 are:
 
 - How should this modem look like inside Ofono and be exposed outside of 
 Ofono?
 - Do you expect to have separate modem drivers for CDMA, WCDMA/GSM and
 LTE or a single driver covering these varied RATs would be accepted?
 - Can there be multiple atoms to support the same functionality in
 various RATs (like SMS or emergency calls) or we can modify one atom
 to support all the RATs?

so what you have is CDMA/EVDO on one side and UMTS/GSM/LTE on the other.
I haven't seen any modem supporting both types at the same time. So
CDMA/EVDO is a separate modem and so is UMTS/GSM/LTE. For these modems
you can RAT selection. For example ISI and also HSO have support for RAT
selection on UMTS/GSM. No LTE since I haven't seen any hardware for it
so far.

 - I guess connman needs to know of the different connections such a
 modem supports and trigger them accordingly? How might this
 functionality look like?

Nope. ConnMan plays pretty much stupid here. Only oFono cares about the
differences.

 - Can the Ofono DBus API differentiate between various RATs based on
 parameters or some new messages needs to be defined if we use a single
 modem driver for multiple RATs?

See my comment above and look at the description of the RAT interface we
already have.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


RE: [PATCH 10/16] gprs: Add ofono_gprs_create_context method

2010-08-13 Thread Marcel Holtmann
Hi Zhenhua,

  DUN server may create one primary context if none of contexts
  existing on the GPRS atom.
  
  so Denis and I had a chat about this. And we agreed that oFono core
  should just create one Internet context if none exists. If the plugin
  driver registers a GPRS atom, we should just create one context
  anyway. ConnMan requires this anyway and it makes sense. We can also
  go one step ahead and fail to remove this default Internet context.
  It should be present all the time.
 
 Thanks for update. I will update my patch to keep this context alive after
 DUN client disconnect from us.

actually can you just work on a separate patch that just create a
default context when the GPRS atom is supported. We should add this
support first and don't tie it to DUN support.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


RE: [PATCH 04/16] emulator: Add APIs to send GAtServer result

2010-08-13 Thread Marcel Holtmann
Hi Zhenhua,

  Defines APIs to send final/intermediate/unsolicited result to DUN
  client. ---
   include/emulator.h |2 ++
   src/emulator.c |   28 
   src/ofono.h|   11 +++
   3 files changed, 41 insertions(+), 0 deletions(-)
  
  diff --git a/include/emulator.h b/include/emulator.h
  index 29e87b9..2a45c65 100644
  --- a/include/emulator.h
  +++ b/include/emulator.h
  @@ -36,6 +36,8 @@ enum ofono_emulator_status {
 OFONO_EMULATOR_STATUS_DESTROY,
   };
  
  +enum _GAtServerResult;
  +
  
  this is not good. It should not be needed outside of GAtChat. Why not
  just include gatserver.h?
 
 My original purpose is to avoid including another gatserver header in .h file.
 So far, we don't include gatchat.h/gatserver.h in oFono core header file.
 
  And in addition the _GAt declaration should never be used. Why not use
  the typedef one?
 
 If we allow to include gatserver.h, this problem is gone. We do have typedef
 in gatserver.h.

good point here, but your way of avoiding this is not really clean
either. Lets discuss this with Denis and see what he thinks.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: Add 3GPP2 relative specifications

2010-08-16 Thread Marcel Holtmann
Hi Caiwen,

 Add 3GPP2 relative specifications.
 
 ---
 
 diff --git a/doc/standards.txt b/doc/standards.txt
 
 index a7eaa5e..1b8c23f 100644
 
 --- a/doc/standards.txt
 
 +++ b/doc/standards.txt
 
 @@ -81,3 +81,88 @@ technology specific features (e.g. UMTS/CDMA).
 
  - 102.384: Card Application Toolkit (CAT) conformance specification
 
  
 
  Describes test methodology and test cases for 102.223.
 
 +
 
 +
 
 +

using Microsoft Exchange to send patches is not gonna work out. The
Exchange servers don't honor simple RFCs. You do need to send proper
patch emails. Use git format-patch and git send-email. However avoid
Exchange servers since they will re-write your email :(

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCHv2 1/7] stemodem: Add support for STK by including MBM implementation.

2010-08-16 Thread Marcel Holtmann
Hi Sjur,

 Changes:
  o Use MBM stk driver implementation (avoid copying code).
Note: stemodm.h has to declare the mbm_stk_init/exit functions because
importing mbmdriver causes compile errors due to duplicated
declarations.
 
 
  drivers/stemodem/stemodem.c |2 ++
  drivers/stemodem/stemodem.h |2 ++
  plugins/ste.c   |2 ++
  3 files changed, 6 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/stemodem/stemodem.c b/drivers/stemodem/stemodem.c
 index c18a8b5..0ba7878 100644
 --- a/drivers/stemodem/stemodem.c
 +++ b/drivers/stemodem/stemodem.c
 @@ -38,6 +38,7 @@ static int stemodem_init(void)
  {
   ste_voicecall_init();
   ste_gprs_context_init();
 + mbm_stk_init();
  
   return 0;
  }
 @@ -46,6 +47,7 @@ static void stemodem_exit(void)
  {
   ste_voicecall_exit();
   ste_gprs_context_exit();
 + mbm_stk_exit();
  }
  
  OFONO_PLUGIN_DEFINE(stemodem, STE modem driver, VERSION,
 diff --git a/drivers/stemodem/stemodem.h b/drivers/stemodem/stemodem.h
 index 267e001..e10abd3 100644
 --- a/drivers/stemodem/stemodem.h
 +++ b/drivers/stemodem/stemodem.h
 @@ -28,3 +28,5 @@ extern void ste_gprs_context_exit();
  extern void ste_voicecall_init();
  extern void ste_voicecall_exit();
  
 +extern void mbm_stk_init();
 +extern void mbm_stk_exit();

all the magic above is not needed. The build system builds the plugins
properly and you can cross reference modem drivers from your modem
plugin without any problems.

 diff --git a/plugins/ste.c b/plugins/ste.c
 index f3ae0b2..9cb49d3 100644
 --- a/plugins/ste.c
 +++ b/plugins/ste.c
 @@ -54,6 +54,7 @@
  #include ofono/voicecall.h
  #include ofono/gprs.h
  #include ofono/gprs-context.h
 +#include ofono/stk.h
  #include drivers/atmodem/vendor.h
  
  #include drivers/stemodem/caif_socket.h
 @@ -241,6 +242,7 @@ static void ste_post_sim(struct ofono_modem *modem)
  
   gprs = ofono_gprs_create(modem,
   OFONO_VENDOR_STE, atmodem, data-chat);
 + ofono_stk_create(modem, 0, mbmmodem, data-chat);
   gc = ofono_gprs_context_create(modem, 0, stemodem, data-chat);
  
   if (gprs  gc)

Only these two changes are needed.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCHv2 2/7] atmodem: Enable STE usage of AT*EPEV and AT*EPEE for PIN handling.

2010-08-16 Thread Marcel Holtmann
Hi Sjur,

  drivers/atmodem/sim.c |9 ++---
  1 files changed, 6 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
 index eb40ad7..8e7c403 100644
 --- a/drivers/atmodem/sim.c
 +++ b/drivers/atmodem/sim.c
 @@ -567,10 +567,12 @@ static void at_pin_send_cb(gboolean ok, GAtResult 
 *result,
   decode_at_error(error, g_at_result_final_response(result));
  
   /*
 -  * On the MBM modem, AT+CPIN? keeps returning SIM PIN for a moment
 -  * after successful AT+CPIN=.., but sends *EPEV when that changes.
 +  * On the MBM and STE modem, AT+CPIN? keeps returning SIM PIN for a
 +  * moment after successful AT+CPIN=.., but sends *EPEV when that
 +  * changes.
*/
 - if (ok  sd-vendor == OFONO_VENDOR_MBM) {
 + if (ok  (sd-vendor == OFONO_VENDOR_MBM ||
 + sd-vendor == OFONO_VENDOR_STE)) {
   sd-epev_id = g_at_chat_register(sd-chat, *EPEV,
   at_epev_notify,
   FALSE, cbd, g_free);
 @@ -817,6 +819,7 @@ static int at_sim_probe(struct ofono_sim *sim, unsigned 
 int vendor,
   case OFONO_VENDOR_WAVECOM:
   g_at_chat_add_terminator(sd-chat, +CPIN:, 6, TRUE);
   break;
 + case OFONO_VENDOR_STE:
   case OFONO_VENDOR_MBM:
   g_at_chat_send(sd-chat, AT*EPEE=1, NULL, NULL, NULL, NULL);
   break;

do we really wanna create another vendor quirk here. If both modems
behave identical then just pass OFONO_VENDOR_MBM in the SIM atom
registration and be done with it.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCHv2 4/7] stemodem: Add Radio Settings to STE Modem

2010-08-16 Thread Marcel Holtmann
Hi Sjur,

 Changes since last patch set:
 o Moved updates to plugins/ste to separate patch.
 o Updated error handling.
 o Style issues.
 
 
  Makefile.am   |4 +-
  drivers/stemodem/radio-settings.c |  223 
 +
  drivers/stemodem/stemodem.c   |2 +
  drivers/stemodem/stemodem.h   |2 +
  4 files changed, 230 insertions(+), 1 deletions(-)
  create mode 100644 drivers/stemodem/radio-settings.c

is this identical to how MBM does it? If yes, then I prefer we add this
to drivers/mbmmodem/ and you just reference it. If not then I like to
see the MBM version of this and where it actually differs. Maybe a MBM
version with a STE quirk is better.

I prefer doing this in MBM since that modem driver was just the first.
There is no other preference here ;)

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCHv2 4/7] stemodem: Add Radio Settings to STE Modem

2010-08-16 Thread Marcel Holtmann
Hi Sjur,

  Changes since last patch set:
  o Moved updates to plugins/ste to separate patch.
  o Updated error handling.
  o Style issues.
 
 
   Makefile.am   |4 +-
   drivers/stemodem/radio-settings.c |  223 
  +
   drivers/stemodem/stemodem.c   |2 +
   drivers/stemodem/stemodem.h   |2 +
   4 files changed, 230 insertions(+), 1 deletions(-)
   create mode 100644 drivers/stemodem/radio-settings.c
 
  is this identical to how MBM does it? If yes, then I prefer we add this
  to drivers/mbmmodem/ and you just reference it. If not then I like to
  see the MBM version of this and where it actually differs. Maybe a MBM
  version with a STE quirk is better.
 
 Actually this is not yet implemented by MBM, so I guess STE is master
 this time ;-)

I thought that part would be similar in both cards. Then I am fine with
this.

  I prefer doing this in MBM since that modem driver was just the first.
  There is no other preference here ;)
 
 This actually raises one question. How should we handle version variations 
 from
 the same Modem vendor.
 The differences will be larger as we start adding support for LTE, and
 support different
 bearers (PC cards with USB and Modems with CAIF).
 Should we create more entries in the vendor enum, or do you have something
 else in mind?

We cross that bridge when we get to it.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH v3 1/7] plugins/ste: Include STK support from MBM driver.

2010-08-16 Thread Marcel Holtmann
Hi Sjur,

  ... The build system builds the plugins
  properly and you can cross reference modem drivers from your modem
  plugin without any problems.
 
 Simply refering to MBM driver from ste.c
 
  plugins/ste.c |2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)
 
 diff --git a/plugins/ste.c b/plugins/ste.c
 index f3ae0b2..9cb49d3 100644
 --- a/plugins/ste.c
 +++ b/plugins/ste.c
 @@ -54,6 +54,7 @@
  #include ofono/voicecall.h
  #include ofono/gprs.h
  #include ofono/gprs-context.h
 +#include ofono/stk.h
  #include drivers/atmodem/vendor.h
  
  #include drivers/stemodem/caif_socket.h
 @@ -241,6 +242,7 @@ static void ste_post_sim(struct ofono_modem *modem)
  
   gprs = ofono_gprs_create(modem,
   OFONO_VENDOR_STE, atmodem, data-chat);
 + ofono_stk_create(modem, 0, mbmmodem, data-chat);
   gc = ofono_gprs_context_create(modem, 0, stemodem, data-chat);
  
   if (gprs  gc)

one minor nitpick here. Can you please not squeeze this in between GPRS
and GPRS Context atoms. Put it at the same location the MBM plugin does.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH v3 2/7] plugins/ste: SIM - STE registers as MBM to utilize mbm quirks.

2010-08-16 Thread Marcel Holtmann
Hi Sjur,

 do we really wanna create another vendor quirk here. If both modems
 behave identical then just pass OFONO_VENDOR_MBM in the SIM atom
 registration and be done with it.
 
 Re-use the MBM quirk rather than adding STE quirks in atmodem/sim.c
 
 ---
  plugins/ste.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/plugins/ste.c b/plugins/ste.c
 index 9cb49d3..9d9afd6 100644
 --- a/plugins/ste.c
 +++ b/plugins/ste.c
 @@ -214,7 +214,7 @@ static void ste_pre_sim(struct ofono_modem *modem)
  
   ofono_devinfo_create(modem, 0, atmodem, data-chat);
   sim = ofono_sim_create(modem, 0, atmodem, data-chat);
 - ofono_voicecall_create(modem, 0, stemodem, data-chat);
 + ofono_voicecall_create(modem, OFONO_VENDOR_MBM, stemodem, data-chat);

this looks wrong. You are using a STE specific modem driver already. Why
do you need to pass a quirk here as well.

Did I confuse you with one of my other replies or did I confuse myself
here in the review?

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH v4 1/7] plugins/ste: Include STK support from MBM driver.

2010-08-16 Thread Marcel Holtmann
Hi Sjur,

  ... The build system builds the plugins
  properly and you can cross reference modem drivers from your modem
  plugin without any problems.
 
 Simply refering to MBM driver from ste.c
 
 one minor nitpick here. Can you please not squeeze this in between GPRS
 and GPRS Context atoms. Put it at the same location the MBM plugin does.
 
 Fixed.

patch has been applied. Thanks.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH v4 4/7] stemodem: Add Radio Settings to STE Modem

2010-08-16 Thread Marcel Holtmann
Hi Sjur,

  Makefile.am   |3 +-
  drivers/stemodem/radio-settings.c |  225 
 +
  drivers/stemodem/stemodem.c   |2 +
  drivers/stemodem/stemodem.h   |2 +
  4 files changed, 231 insertions(+), 1 deletions(-)
  create mode 100644 drivers/stemodem/radio-settings.c
 
 diff --git a/Makefile.am b/Makefile.am
 index 16a3a3d..9462743 100644
 --- a/Makefile.am
 +++ b/Makefile.am
 @@ -204,7 +204,8 @@ builtin_sources += drivers/atmodem/atutil.h \
   drivers/stemodem/voicecall.c \
   drivers/stemodem/gprs-context.c \
   drivers/stemodem/caif_socket.h \
 - drivers/stemodem/if_caif.h
 + drivers/stemodem/if_caif.h \
 + drivers/stemodem/radio-settings.c

please put it after or before the gprs-context.c. I wanna keep the atom
drivers together and not intermix with the CAIF includes.
 
  builtin_modules += phonesim
  builtin_sources += plugins/phonesim.c
 diff --git a/drivers/stemodem/radio-settings.c 
 b/drivers/stemodem/radio-settings.c
 new file mode 100644
 index 000..49a7906
 --- /dev/null
 +++ b/drivers/stemodem/radio-settings.c
 @@ -0,0 +1,225 @@
 +/*
 + *
 + *  oFono - Open Source Telephony
 + *
 + *  Copyright (C) 2008-2010  Intel Corporation. All rights reserved.
 + *  Copyright (C) 2010 ST-Ericsson AB.
 + *  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.
 + 

You need to put an empty line after your copyright. Please keep the
style we have for the copyright header.

 *
 + *  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 _GNU_SOURCE
 +#include string.h
 +#include stdlib.h
 +#include stdio.h
 +#include errno.h
 +
 +#include glib.h
 +
 +#include ofono/log.h
 +#include ofono/modem.h
 +#include ofono/radio-settings.h
 +
 +#include gatchat.h
 +#include gatresult.h
 +
 +#include stemodem.h
 +
 +static const char *none_prefix[] = { NULL };
 +static const char *cfun_prefix[] = { +CFUN:, NULL };
 +
 +struct radio_settings_data {
 + GAtChat *chat;
 +};
 +
 +enum ste_radio_mode {
 + STE_RADIO_OFF = 0,
 + STE_RADIO_ON = 1,
 + STE_RADIO_FLIGHT_MODE = 4,
 + STE_RADIO_GSM_ONLY = 5,
 + STE_RADIO_WCDMA_ONLY = 6
 +};

Can you please insert tabs between the constant and the = value. We
prefer having the enums nicely tabbed.

 +static gboolean ste_mode_to_ofono_mode(enum ste_radio_mode stemode,
 + enum ofono_radio_access_mode *mode)
 +{
 + switch (stemode) {
 + case STE_RADIO_ON:
 + *mode = OFONO_RADIO_ACCESS_MODE_ANY;
 + return TRUE;
 + case STE_RADIO_GSM_ONLY:
 + *mode = OFONO_RADIO_ACCESS_MODE_GSM;
 + return TRUE;
 + case STE_RADIO_WCDMA_ONLY:
 + *mode = OFONO_RADIO_ACCESS_MODE_UMTS;
 + return TRUE;
 + default:
 + return FALSE;
 + }
 +}

I prefer that you leave out the default statement and just add a return
FALSE at the end of the function. Deal with missing enum entries and the
compiler complaining by just adding a break.

This has the advantage if you accidentally add a constant the compiler
will warn you instead of just using default statement.

 +static gboolean ofono_mode_to_ste_mode(enum ofono_radio_access_mode mode,
 + enum ste_radio_mode *stemode)
 +{
 + switch (mode) {
 + case OFONO_RADIO_ACCESS_MODE_ANY:
 + *stemode = STE_RADIO_ON;
 + return TRUE;
 + case OFONO_RADIO_ACCESS_MODE_GSM:
 + *stemode = STE_RADIO_GSM_ONLY;
 + return TRUE;
 +
 + case OFONO_RADIO_ACCESS_MODE_UMTS:
 + *stemode = STE_RADIO_WCDMA_ONLY;
 + return TRUE;
 + default:
 + return FALSE;
 + }
 +}

Same here. And in addition please remove the empty line in between that
is there for no real reason.

 +static void sterat_query_cb(gboolean ok, GAtResult *result, gpointer 
 user_data)
 +{

I prefer if you call this just rat_query_cb and don't bother prefixing
it with ste.

 + struct cb_data *cbd = user_data;
 + ofono_radio_settings_rat_mode_query_cb_t cb = cbd-cb;
 + enum ofono_radio_access_mode mode;
 + struct ofono_error error;
 + GAtResultIter iter;
 + int value;
 +
 + decode_at_error(error, 

Re: [PATCH v4 2/7] plugins/ste: SIM - STE registers as MBM to utilize mbm quirks.

2010-08-16 Thread Marcel Holtmann
Hi Sjur,

 ...
 do we really wanna create another vendor quirk here. If both modems
 behave identical then just pass OFONO_VENDOR_MBM in the SIM atom
 registration and be done with it.
 
 Re-use the MBM quirk rather than adding STE quirks in atmodem/sim.c
 
 ---
  plugins/ste.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/plugins/ste.c b/plugins/ste.c
 index 9cb49d3..9d9afd6 100644
 --- a/plugins/ste.c
 +++ b/plugins/ste.c
 @@ -214,7 +214,7 @@ static void ste_pre_sim(struct ofono_modem *modem)
  
   ofono_devinfo_create(modem, 0, atmodem, data-chat);
   sim = ofono_sim_create(modem, 0, atmodem, data-chat);
 - ofono_voicecall_create(modem, 0, stemodem, data-chat);
 + ofono_voicecall_create(modem, OFONO_VENDOR_MBM, stemodem, data-chat);

I still don't get it. The stemodem/voicecall.c doesn't use any quirks
and it should never do so. It is STE specific already.

Regards

Marcel



___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH v4 3/7] stemodem: Add polling for SIM ready.

2010-08-16 Thread Marcel Holtmann
Hi Sjur,

 Interim solution until support for SIM 'ready' notification is supported.
 ---
 Changes:
  o Fixed style issues (tabs and newlines)
 
  plugins/ste.c |   55 ---
  1 files changed, 52 insertions(+), 3 deletions(-)
 
 diff --git a/plugins/ste.c b/plugins/ste.c
 index f9875fd..5fecd5e 100644
 --- a/plugins/ste.c
 +++ b/plugins/ste.c
 @@ -60,8 +60,13 @@
  #include drivers/stemodem/caif_socket.h
  #include drivers/stemodem/if_caif.h
  
 +static const char *cpin_prefix[] = { +CPIN:, NULL };
 +
  struct ste_data {
   GAtChat *chat;
 + guint cpin_poll_source;
 + guint cpin_poll_count;
 + gboolean have_sim;
  };
  
  static int ste_probe(struct ofono_modem *modem)
 @@ -88,6 +93,10 @@ static void ste_remove(struct ofono_modem *modem)
   ofono_modem_set_data(modem, NULL);
  
   g_at_chat_unref(data-chat);
 +
 + if (data-cpin_poll_source  0)
 + g_source_remove(data-cpin_poll_source);
 +
   g_free(data);
  }
  
 @@ -96,16 +105,55 @@ static void ste_debug(const char *str, void *user_data)
   ofono_info(%s, str);
  }
  
 +static gboolean init_simpin_check(gpointer user_data);
 +
 +static void simpin_check(gboolean ok, GAtResult *result, gpointer user_data)
 +{
 + struct ofono_modem *modem = user_data;
 + struct ste_data *data = ofono_modem_get_data(modem);
 +
 + /* Modem returns +CME ERROR: 10 if SIM is not ready. */
 + if (!ok  result-final_or_pdu 
 + !strcmp(result-final_or_pdu, +CME ERROR: 10) 
 + data-cpin_poll_count++  5) {
 + data-cpin_poll_source =
 + g_timeout_add_seconds(1, init_simpin_check, modem);
 + return;
 + }
 +
 + data-cpin_poll_count = 0;
 +
 + /* Modem returns ERROR if there is no SIM in slot. */
 + data-have_sim = ok;
 +
 + ofono_modem_set_powered(modem, TRUE);
 +}
 +
 +static gboolean init_simpin_check(gpointer user_data)
 +{
 + struct ofono_modem *modem = user_data;
 + struct ste_data *data = ofono_modem_get_data(modem);
 +
 + data-cpin_poll_source = 0;
 +
 + g_at_chat_send(data-chat, AT+CPIN?, cpin_prefix,
 + simpin_check, modem, NULL);
 +
 + return FALSE;
 +}
 +
  static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
  {
   struct ofono_modem *modem = user_data;
  
   DBG();
  
 - if (!ok)
 + if (!ok) {
   ofono_modem_set_powered(modem, FALSE);
 + return;
 + }
  
 - ofono_modem_set_powered(modem, TRUE);
 + init_simpin_check(modem);
  }

I am fine with this if Denis is as well.
 
  static int ste_enable(struct ofono_modem *modem)
 @@ -168,7 +216,8 @@ static int ste_enable(struct ofono_modem *modem)
   if (getenv(OFONO_AT_DEBUG))
   g_at_chat_set_debug(data-chat, ste_debug, NULL);
  
 - g_at_chat_send(data-chat, ATE0 +CMEE=1, NULL, NULL, NULL, NULL);
 + g_at_chat_send(data-chat, ATF E0 V1 X4 C1 +CMEE=1,
 + NULL, NULL, NULL, NULL);
   g_at_chat_send(data-chat, AT+CFUN=1, NULL, cfun_enable, modem, NULL);

Why this change? If so, it does not belong in this commit. Or you need
to add a comment why it does indeed.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH v4 6/7] stemodem: Use RTNL for creating CAIF interface

2010-08-16 Thread Marcel Holtmann
Hi Sjur,

 CAIF in Linux kernel 2.6.35 must use RTNL for configuring
 CAIF interfaces.
 ---
  drivers/stemodem/gprs-context.c |   51 +--
  drivers/stemodem/rtnl.c |  318 
 +++
  drivers/stemodem/rtnl.h |   24 +++
  3 files changed, 380 insertions(+), 13 deletions(-)
  create mode 100644 drivers/stemodem/rtnl.c
  create mode 100644 drivers/stemodem/rtnl.h

I can't really let you do make blocking reads to the netlink socket.
That would be pretty much a bad idea.

We need a better solution for this. Maybe one that also helps the ISI
modem drivers.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH v4 7/7] plugins/ste: Use SOCK_STREAM for CAIF and enable interface specification.

2010-08-16 Thread Marcel Holtmann
Hi Sjur,

  plugins/modem.conf  |5 +
  plugins/modemconf.c |1 +
  plugins/ste.c   |   20 +++-
  3 files changed, 25 insertions(+), 1 deletions(-)

please don't intermix modemconf and STE plugin changes. I need two
separate patches for this.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


RE: Removal of modemconf plugin

2010-08-17 Thread Marcel Holtmann
Hi Yang,

  What about dual mode modems that with two different serial ports?
  e.g. a modem support both GSM and CDMA, there is a serial port
  For GSM, and there is a serial another for CDMA.
 
 
  Why not keep both udev and modemconf plug-in. It remains for the user
  To decide which way to use. I think maybe we can change the compile
  Scrips, let user to select which plug-in to use.
 
 I don't even see a problem with this at all. Every serial port is
 present in udev, so you can write proper udev rules for it. A modem with
 a serial port for GSM and another one CDMA will be just presented as two
 modems inside oFono. So no problem at all.
 
 So modemconf plugin will be removed since it is not needed at all. The
 only component that needs it is the phonesim plugin. And that will get
 its own configuration file.
 
 Can we just add some code in phonesim to make it look like a pseudo serial 
 device which can be managed by udev? That is, if we start the phonesim, it 
 will simulate some tty ports under /dev and udevd will inform the oFono there 
 is a new device (We need some rule for phonesim). And the device disappears 
 if phonesim meets an end.
 Maybe this is possible if we can send message to udevd the same way as kernel 
 via netlink.

in theory yes, but phonesim is the one piece that also works via TCP. So
just having a /etc/ofono/phonesim.conf is a lot simpler.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


RE: Removal of modemconf plugin

2010-08-17 Thread Marcel Holtmann
Hi Pengzhuo,

  a gentle reminder that this mailing list doesn't allow top 
  posting, please follow the netiquette for open source mailing lists.
  
   What about dual mode modems that with two different serial ports? 
   e.g. a modem support both GSM and CDMA, there is a serial port For 
   GSM, and there is a serial another for CDMA.
   
   
   Why not keep both udev and modemconf plug-in. It remains 
  for the user 
   To decide which way to use. I think maybe we can change the compile 
   Scrips, let user to select which plug-in to use.
  
  I don't even see a problem with this at all. Every serial 
  port is present in udev, so you can write proper udev rules 
  for it. A modem with a serial port for GSM and another one 
  CDMA will be just presented as two modems inside oFono. So no 
  problem at all.
  
 Udev really can work, but in many embedded devices,udev service is being
 forbidden because
 of boot-up speed issue. In addtion, on these devices, device nodes are
 often explictly defined.

this is just an urban myth. udev works as nicely on embedded system as
it works on servers. If you think it is not working fine, then lets have
Kay fix it. Also with the integration of devtmpfs all the last issues
with initial device nodes are solved and udev execution time on a really
static environment is almost non existent. If you have a problem with
your boot time, then it is somewhere else. I highly doubt it is really a
problem of udev.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH v5 1/8] plugins/ste: SIM - STE registers as MBM to utilize mbm quirks.

2010-08-17 Thread Marcel Holtmann
Hi Sjur,

 ---
  Change: This time the right line is hopefully being edited.
 
  plugins/ste.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

patch has been applied. Thanks.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH v5 5/8] plugins/ste: Add Radio-Settings

2010-08-17 Thread Marcel Holtmann
Hi Sjur,

 ---
  drivers/stemodem/radio-settings.c |5 +
  plugins/ste.c |2 ++
  2 files changed, 7 insertions(+), 0 deletions(-)

your diffstat is wrong here. Please check the tools you use to create
them.

Patch has been applied.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH v5 4/7] stemodem: Add Radio Settings to STE Modem

2010-08-17 Thread Marcel Holtmann
Hi Sjur,

 ---
 Changes:
 o Changed order of files in Makefile
 o Fixed missing empty line in copyright
 o Formatted enum better
 o Removed default is switches
 o Removed ste in function names
 
  Makefile.am   |2 +
  drivers/stemodem/radio-settings.c |  230 
 +
  drivers/stemodem/stemodem.c   |2 +
  drivers/stemodem/stemodem.h   |2 +
  4 files changed, 236 insertions(+), 0 deletions(-)
  create mode 100644 drivers/stemodem/radio-settings.c

patch has been applied.

 --- a/Makefile.am
 +++ b/Makefile.am
 @@ -202,10 +202,12 @@ builtin_sources += drivers/atmodem/atutil.h \
   drivers/stemodem/stemodem.h \
   drivers/stemodem/stemodem.c \
   drivers/stemodem/voicecall.c \
 + drivers/stemodem/radio-settings.c \
   drivers/stemodem/gprs-context.c \
   drivers/stemodem/caif_socket.h \
   drivers/stemodem/if_caif.h
  
 +
  builtin_modules += phonesim
  builtin_sources += plugins/phonesim.c

I fixed this for you, but in the future, please don't add extra unneeded
empty lines.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


<    1   2   3   4   5   6   7   8   9   10   >