Re: [PATCH 2/3] hdlc: handle wrapped buffers
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
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
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
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
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
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
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
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
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.
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
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
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.
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.
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
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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.
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
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
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.
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.
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.
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
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.
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.
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
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.
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
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
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.
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
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
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