RE: Using ofono HFP
Hi Silva, Moises Silva wrote: On Mon, Sep 27, 2010 at 9:26 PM, Zhang, Zhenhua zhenhua.zh...@intel.com wrote: Right. See bluez/audio/unix.c for BlueZ part SCO code that cooperate with pulseaudio. BlueZ sends SCO fd to pulseaudio via UNIX type socks. This put me in the right direction and I have a clearer idea of how to do it. I found odd though that it seems I need to copy the ipc.c and ipc.h files into my own app. Any reason they are not part of libbluetooth API? Marcel may know the answer for your question. Another thing that puzzles me is that supposedly DBUS = 1.3 is needed to be able to pass file descriptors. However as you pointed out and per ipc.c the file descriptor is sent using a unix socket. Why the need for DBUS file descriptor passing? DBUS file descriptor passing is used in BlueZ/oFono communication, to pass RFCOMM port file handler to another process. I guess dbus 1.3 has some wrapper for such file descriptor passing magic. Marcel, any comments on Silva's question? I'm gonna start testing right away and most likely will be back with more questions (like what's the audio format, sampling rate and such) :-) Thanks, -- Moises Silva ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: Using ofono HFP
Hi Moises, Denis Kenzior wrote: Hi Moises, I was advised by padovan on IRC that I should try dbus = 1.3 ... I am going to try that today, I was putting that off cuz this seemed like a lower level issue and upgrading dbus seemed like non-trivial since many things depend on dbus and fedora 13 does not seem to include rpm for dbus = 1.3 Neither oFono nor BlueZ will enable the plugins for HFP support unless DBus-1.3 is installed (or alternatively a hacked dbus-1.2 with file descriptor passing support) Denis is right. For example, my ubuntu 10.04 has dbus-1.3.0 so HFP works smoothly on my laptop. I remember there's someone has a hacked dbus-1.2 with fd passing support. It does work but I would recommend you use dbus-1.3. Can I somehow to try ofono connect to my cell and not the other way around? The connection initiator is actually irrelevant. You just need to make sure that BlueZ has paired with your device and the HandsfreeGateway interface is created for your device. The final goal here is NOT use this with pulseaudio, but rather have access to the SCO socket so I can manipulate the audio. If you want this, then you will need to eventually write your own plugin that will manage the RFCOMM and SCO sockets. Today, BlueZ does this part... Right. See bluez/audio/unix.c for BlueZ part SCO code that cooperate with pulseaudio. BlueZ sends SCO fd to pulseaudio via UNIX type socks. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: Using ofono HFP
Hi Silva, From: ofono-boun...@ofono.org [mailto:ofono-boun...@ofono.org] On Behalf Of Moises Silva Sent: Tuesday, September 28, 2010 10:45 AM To: ofono@ofono.org Subject: Re: Using ofono HFP On Mon, Sep 27, 2010 at 9:26 PM, Zhang, Zhenhua zhenhua.zh...@intel.com wrote: Right. See bluez/audio/unix.c for BlueZ part SCO code that cooperate with pulseaudio. BlueZ sends SCO fd to pulseaudio via UNIX type socks. Thanks a lot for the additional help. I just setup a quick wiki page to start documenting this: http://ofono.org/wiki/hands-free-profile It's small and crappy, but it's a start. I will keep improving it as I go. [Zhenhua] I have some experience on HFP as I am main contributor of HFP in the oFono. Would you mind me to update your wiki page to add detailed information related with HFP? Btw, Could you send plain text format mail to the mailing list in the future as it's an formal upstream project. I tried but I cann't reply your mail propertly. Thanks, Zhenhua Regards, - Moises Silva ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 0/1] Patch Description
Hi, Yang Gu wrote: Phonesim is most of time a convenient way to test oFono. For example, it can be used to test MO call, as well as MT call. And talking about test area, test automation is quite important regarding to both efficiency and effectiveness. In the test of MO call, scripts can be used to interact with oFono via D-Bus to make test automation possible. However, in the test of MT call, currently we have to enter the caller number and click some button in Phonesim GUI to simulate an incoming call, which makes test automation unrealistic. This patch is to enable Qt script (JavaScript following ECMAScript spec) in Phonesim, so that we can have some script control its GUI conveniently, and satisfy the test automation. With this patch, Phonesim can work in the following way: 1. It observes some specific directory (/tmp/scripts) to see if there is some test scripts added. 2. Once Phonesim finds a new test script is added, it will parse and execute the script. 3. You may add more and more test scripts to the specific directory with your test goes on. Below are two examples: # call.js (Simulate a MT call) tabRegistration.gbIncomingCall.leCaller.text = 12345; tabRegistration.gbIncomingCall.pbIncomingCall.click(); This script will help you enter the caller number as 12345, and click Call button in Phonesim GUI. Once this script is copied to the observed directory, oFono will get an incoming call. # sms.js (Simulate a MT sms) tabSMS.gbMessage1.leMessageSender.text = Yang; tabSMS.gbMessage1.leSMSClass.text = 1; tabSMS.gbMessage1.teSMSText.setPlainText(This message is sent automatically from Phonesim); tabSMS.gbMessage1.pbSendSMSMessage.click(); This script will help you fill the sms sender, class, text, and then click the Send Message button for you. Once it's added into the specific directory, oFono will get an incoming message. Don't know if this is the best way to make Phonesim support test automation. So comments are welcome:) It's awesome! I tried the patch and simply copy call.js to /tmp/scripts and it makes a call to oFono like a magic. ;-) - Yang Gu (1): Enable Qt Script configure.ac|2 +- src/control.cpp | 60 +++ 2 files changed, 61 insertions(+), 1 deletions(-) ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 1/1] Add support to enable/disable specific plugin
Hi Holtmann, Marcel Holtmann wrote: Hi Zhenhua, Support to load or not load specific plugin when ofono is started. E.g., use 'ofonod -P hfp' to disable hfp plugin. --- src/main.c | 11 ++- 1 files changed, 10 insertions(+), 1 deletions(-) patch has been applied. I did all the hard work - just forgot to hook it up properly ;) Thanks. -__ofono_plugin_init(NULL, NULL); +__ofono_plugin_init(option_plugin, option_noplugin); + +g_free(option_plugin); +g_free(option_noplugin); Please also send a ConnMan patch to fix the memory leak that ConnMan has here actually. ConnMan does free the string propertly. So we don't need more fixes. :-) Regards, Zhenhua Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 1/1] hfp: Add faked sim driver to support SIM ready
Hi, Denis, Denis Kenzior wrote: Hi Zhenhua, On 08/30/2010 04:50 AM, Zhenhua Zhang wrote: HFP modem doesn't have IMSI at all. In order to notify SIM ready to the core, we create a special sim driver with a faked IMSI number. Can you try after commit 88024972df46d4ababbaf39354615e8a8d603cfc? I added magic which will skip sim_ready stuff completely for modems that do not add a sim atom. Yes. Your patch works well and fixes the problem in a generic way. Regards, -Denis Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: About ppp_receive()
Hi Steven, Steven wrote: Hi, In function ppp_receive, we first check the protocol type of this frame like: guint16 protocol = ppp_proto(buf); and here we assumed the length of the protocol field is 16 bits, but in RFC 1661, the protocol field should be one or two octets. The Protocol field is one or two octets, and its value identifies the datagram encapsulated in the Information field of the packet. why we given the assumption that protocol field is 16 bit length? First I am not ppp expert. :-). If you take look at pppd source code, main.c, get_input() also always fetch two bytes 'protocol' for struct protent as well. Can you give a case we failed in our ppp stack? Thanks. Zhenhua In CDMA 2000 environment, just as the Sprint Network, PPP should support a compressed protocol field. Is there anything difference between GSM and CDMA? B.R Steven --- Confidentiality Notice: The information contained in this e-mail and any accompanying attachment(s) is intended only for the use of the intended recipient and may be confidential and/or privileged of Neusoft Corporation, its subsidiaries and/or its affiliates. If any reader of this communication is not the intended recipient, unauthorized use, forwarding, printing, storing, disclosure or copying is strictly prohibited, and may be unlawful.If you have received this communication in error,please immediately notify the sender by return e-mail, and delete the original message and all copies from your system. Thank you. --- ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: About ppp_receive()
Hi Steven, Steven wrote: Hi Zhenhua Zhang, Zhenhua wrote: Hi Steven, Steven wrote: Hi, In function ppp_receive, we first check the protocol type of this frame like: guint16 protocol = ppp_proto(buf); and here we assumed the length of the protocol field is 16 bits, but in RFC 1661, the protocol field should be one or two octets. The Protocol field is one or two octets, and its value identifies the datagram encapsulated in the Information field of the packet. why we given the assumption that protocol field is 16 bit length? First I am not ppp expert. :-). If you take look at pppd source code, main.c, get_input() also always fetch two bytes 'protocol' for struct protent as well. Can you give a case we failed in our ppp stack? If you interested in this topic, you can reference RFC 1661 Section 6.5, which said -- This Configuration Option provides a method to negotiate the compression of the PPP Protocol field. By default, all implementations MUST transmit packets with two octet PPP Protocol fields. PPP Protocol field numbers are chosen such that some values may be compressed into a single octet form which is clearly distinguishable from the two octet form. This Configuration Option is sent to inform the peer that the implementation can receive such single octet Protocol fields. - In our current source code, because we only negotiate two configuration options - REQ_OPTION_MRU and REQ_OPTION_ACCM. so it's okey for our PPP stack. Yes. It's handled in the LCP layer. The code is majorly implemented by Kristen and Denis. Maybe they could have more comments on that. But some carriers, like China Telecom or Sprint Network etc, will support the full configuration options(Magic-Number,Protocol-Field-Compression,Address-and-Control-Field-Compression), So if PFC option is used ,our code will got wrong with ppp_receive(). Agree, we don't have pcompress, accompression like pppd yet. So patches are welcome to improve that part. Btw, I do see some code related with MAGIC_NUMBER in ppp_lcp.c. We should first check if PPP protocol field is compressed or not, and then get the right protocol value to form a 16 bits protocol field, and pass this value to the rest functions. Because of my company's security policy ,I can't provide a patch for this issue. But i can provide a method for doing this. Here it is. I don't understand why having such policy at all. Your code defintely won't leak any IP since we all follow with the standard spec. First byte of PPP protocol field may be compressed, if the LS bit is 1 then this indicates that the upper protocol us compressed, because the upper byte should be even, the lower byte should be odd. In CDMA 2000 environment, just as the Sprint Network, PPP should support a compressed protocol field. Is there anything difference between GSM and CDMA? B.R Steven B.R Steven --- Confidentiality Notice: The information contained in this e-mail and any accompanying attachment(s) is intended only for the use of the intended recipient and may be confidential and/or privileged of Neusoft Corporation, its subsidiaries and/or its affiliates. If any reader of this communication is not the intended recipient, unauthorized use, forwarding, printing, storing, disclosure or copying is strictly prohibited, and may be unlawful.If you have received this communication in error,please immediately notify the sender by return e-mail, and delete the original message and all copies from your system. Thank you. --- ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: About Connection between PPP and linux Sockets
Hi Marcel, Marcel Holtmann wrote: Hi Steven, I have a little question just as the title said? In Ofono how to connect PPP to Linux socket, when we receive packet from network, how the packet go through the kernel to application? modem --PPP--? -- linux kernel(network part)-- socket -- application? it is more like this: modem - TTY - PPP - TUN/TAP - Kernel Net-Stack - socket - application. With the TTY being in kernel, the PPP being in userspace, and TUN/TAP etc. begin in the kernel again. A future enhancement is to use the kernel PPP layer, but we haven't gotten there yet. I am interested to know how could we use kernel PPP layer instead of gatppp. Shall we add this item into our TODO? Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: About Connection between PPP and linux Sockets
Hi Steven, Steven wrote: Hi Zhang Zhang, Zhenhua wrote: Hi Marcel, Marcel Holtmann wrote: Hi Steven, I have a little question just as the title said? In Ofono how to connect PPP to Linux socket, when we receive packet from network, how the packet go through the kernel to application? modem --PPP--? -- linux kernel(network part)-- socket -- application? it is more like this: modem - TTY - PPP - TUN/TAP - Kernel Net-Stack - socket - application. With the TTY being in kernel, the PPP being in userspace, and TUN/TAP etc. begin in the kernel again. A future enhancement is to use the kernel PPP layer, but we haven't gotten there yet. I am interested to know how could we use kernel PPP layer instead of gatppp. Shall we add this item into our TODO? Maybe you can reference to RILD in Android, it used kernel PPP. Thanks. Will take a look then. B.R Steven --- Confidentiality Notice: The information contained in this e-mail and any accompanying attachment(s) is intended only for the use of the intended recipient and may be confidential and/or privileged of Neusoft Corporation, its subsidiaries and/or its affiliates. If any reader of this communication is not the intended recipient, unauthorized use, forwarding, printing, storing, disclosure or copying is strictly prohibited, and may be unlawful.If you have received this communication in error,please immediately notify the sender by return e-mail, and delete the original message and all copies from your system. Thank you. --- ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: About Connection between PPP and linux Sockets
Hi Steven, Steven wrote: Hi Zhang, Zhang, Zhenhua wrote: Hi Steven, Steven wrote: Hi Zhang Zhang, Zhenhua wrote: Hi Marcel, Marcel Holtmann wrote: Hi Steven, I have a little question just as the title said? In Ofono how to connect PPP to Linux socket, when we receive packet from network, how the packet go through the kernel to application? modem --PPP--? -- linux kernel(network part)-- socket -- application? it is more like this: modem - TTY - PPP - TUN/TAP - Kernel Net-Stack - socket - application. With the TTY being in kernel, the PPP being in userspace, and TUN/TAP etc. begin in the kernel again. A future enhancement is to use the kernel PPP layer, but we haven't gotten there yet. I am interested to know how could we use kernel PPP layer instead of gatppp. Shall we add this item into our TODO? Maybe you can reference to RILD in Android, it used kernel PPP. Thanks. Will take a look then. This information is a good start point. http://www.devdiv.net/viewthread-26543 But only for Chinese people:( Thanks. I have read this before. ;-). The original article is from maxleng's blog: http://blog.csdn.net/maxleng/archive/2010/05/10/5576509.aspx However, it's just a big picture about phone stack in Android. It said nothing about how Android works with kernel PPP layer. B.R Steven --- Confidentiality Notice: The information contained in this e-mail and any accompanying attachment(s) is intended only for the use of the intended recipient and may be confidential and/or privileged of Neusoft Corporation, its subsidiaries and/or its affiliates. If any reader of this communication is not the intended recipient, unauthorized use, forwarding, printing, storing, disclosure or copying is strictly prohibited, and may be unlawful.If you have received this communication in error,please immediately notify the sender by return e-mail, and delete the original message and all copies from your system. Thank you. --- ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 0/10] Unregister AT notifiers when removing drivers
Hi Marcel, Marcel Holtmann wrote: 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. Thanks for update. Yes. The fix by Denis looks smart and make sense to me. By using fa?ade pattern, we don't require user to track register/unregister of AT notifers any more. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 10/16] gprs: Add ofono_gprs_create_context method
Hi Marcel, Marcel Holtmann wrote: 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. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: Terminating emergency calls
Hi Sjur, Sjur Br?ndeland wrote: Denis Kenzior denk...@gmail.com wrote: I thought of one more issue with voice calls. I don't think it is safe to to terminate emergency calls using release_specific, AT+CHLD=1X. At least this don't work for STE modems. I suggest calls in state active should be terminated using hangup_all or hangup_active. What do you think? So in the case of a single call, the emergency call will be terminated using hangup_all / hangup_active anyway. I have relaxed the single call restriction for active calls when hangup_active is provided by the driver. Refer to c7b13ec2fe664b122216a312f2442c9ca26f5f43 Yes, it seems to be ok for voicecall_hangup, but in manager_hangup_all the active call is still terminated with release_specific in voiceall_release_next. This implies that if you have an emergency call and terminate it with manager_hangup_all AT+CHLD=1X still will be used, right? I suggest we change voicecall_release_next like this: if (vc-driver-hangup_active != NULL (call-call-status == CALL_STATUS_ALERTING || call-call-status == CALL_STATUS_DIALING || + call-call-status == CALL_STATUS_ACTIVE || call-call-status == CALL_STATUS_INCOMING)) vc-driver-hangup_active(vc, multirelease_callback, vc); else vc-driver-release_specific(vc, call-call-id, multirelease_callback, vc); For mpty calls this gets tricky. I'd like some answers to these questions: - Can Emergency calls participate in mpty? I have to verify this with some of my colleagues, but I am pretty sure emergency calls cannot be applied to the AT+CHLD command. i.e. they cannot be part of mpty. If emergency calls cannot be part of mpty call, we can use either hangup_all or hangup_active as Denis said. However, your suggested fix will break multiparty call scenario since multiparty_hangup calls voicecall_release_next as well. Maybe we should use call-type to indicate whether it's an emergy call. It looks to me that the type flag in struct ofono_call hasn't been used yet. Correct me if I am wrong. Regards Sjur ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: Crash in network-registration.c/creg_notify() when receiving unsolicited +CREG: 0
Hi Emmanuel, Berthier, Emmanuel wrote: Hi, I use ofono 0.25 and encounter a segmentation fault when trying to put the modem OffLine (flight mode). The AT command sent to modem is: AT+CFUN=4 The modem replies with: OK +CREG=0 Then, in creg_notify(), the user_data points to uninitialized (deallocated?) memory and access to “nd-vendor” causes the crash. I might know where is the problem. We should unregister all notifies from gatchat in at_netreg_remove. Same for gprs and other drivers as well. Currently, we don't do that well. I will send patches soon. Effectivelly, user_data pointer is the one set by at_creg_set_cb(), and is freed by g_at_chat_finish_command()/at_command_destroy(). So, it seems that ofono does not manage unsolicited +CREG command. What’s your feeing? Thanks. Emmanuel. - Intel Corporation SAS (French simplified joint stock company) Registered headquarters: Les Montalets- 2, rue de Paris, 92196 Meudon Cedex, France Registration Number: 302 456 199 R.C.S. NANTERRE Capital: 4,572,000 Euros This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 2/3] Add dun_enable() function
Hi Padovan, Gustavo F. Padovan wrote: dun_enable() is called by setting the Powered property to true. It creates a rfcomm link throught the BlueZ Serial API. --- drivers/dunmodem/dunmodem.h |1 + plugins/bluetooth.h |1 + plugins/dun.c | 61 +- 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/drivers/dunmodem/dunmodem.h b/drivers/dunmodem/dunmodem.h index 6bbf7b9..16eb9e7 100644 --- a/drivers/dunmodem/dunmodem.h +++ b/drivers/dunmodem/dunmodem.h @@ -23,6 +23,7 @@ struct dun_data { char *dun_path; + const char *rfcomm; }; #endif diff --git a/plugins/bluetooth.h b/plugins/bluetooth.h index 09e6efa..c20b36d 100644 --- a/plugins/bluetooth.h +++ b/plugins/bluetooth.h @@ -22,6 +22,7 @@ #define BLUEZ_MANAGER_INTERFACE BLUEZ_SERVICE .Manager #define BLUEZ_ADAPTER_INTERFACE BLUEZ_SERVICE .Adapter #define BLUEZ_DEVICE_INTERFACE BLUEZ_SERVICE .Device +#define BLUEZ_SERIAL_INTERFACE BLUEZ_SERVICE .Serial #define HFP_AG_UUID 111F--1000-8000-00805F9B34FB #define DUN_GW_UUID 1103--1000-8000-00805F9B34FB diff --git a/plugins/dun.c b/plugins/dun.c index 9b4288e..7dc8422 100644 --- a/plugins/dun.c +++ b/plugins/dun.c @@ -25,6 +25,7 @@ #include stdio.h #include string.h #include errno.h +#include gdbus.h #include glib.h #include ofono.h @@ -138,10 +139,66 @@ static void dun_remove(struct ofono_modem *modem) ofono_modem_set_data(modem, NULL); } +static void dun_connect_reply(DBusPendingCall *call, gpointer user_data) +{ + struct ofono_modem *modem = user_data; + struct dun_data *data = ofono_modem_get_data(modem); + const char *dev; + DBusError derr; + DBusMessage *reply, *msg; + + reply = dbus_pending_call_steal_reply(call); + + if (ofono_modem_get_powered(modem)) + goto done; + + if (!dbus_message_get_args(reply, NULL, DBUS_TYPE_STRING, dev, + DBUS_TYPE_INVALID)) + goto done; + + data-rfcomm = dev; One more comment here, I'd suggest to use g_strdup(dev). Because the device String is from dbus reply and later we will unref the reply. What do you think? + dbus_error_init(derr); + if (!dbus_set_error_from_message(derr, reply)) + goto done; + + DBG(Connect reply: %s, derr.message); + + if (dbus_error_has_name(derr, DBUS_ERROR_NO_REPLY)) { + msg = dbus_message_new_method_call(BLUEZ_SERVICE, + data-dun_path, + BLUEZ_SERIAL_INTERFACE, Disconnect); + if (!msg) + ofono_error(Disconnect failed); + else + g_dbus_send_message(connection, msg); + } + + ofono_modem_set_powered(modem, FALSE); + + dbus_error_free(derr); + +done: + ofono_modem_set_powered(modem, TRUE); + dbus_message_unref(reply); +} + static int dun_enable(struct ofono_modem *modem) { - DBG(%p, modem); - return 0; + struct dun_data *data = ofono_modem_get_data(modem); + int status; + const char *uuid = DUN_GW_UUID; + + status = bluetooth_send_with_reply(data-dun_path, + BLUEZ_SERIAL_INTERFACE, Connect, + dun_connect_reply, modem, NULL, + 15, DBUS_TYPE_STRING, uuid, + DBUS_TYPE_INVALID); + + if (status 0) + return -EINVAL; + + return -EINPROGRESS; } static int dun_disable(struct ofono_modem *modem) Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 1/3] Bluetooth DUN modem prototype
Hi Padovan, Gustavo F. Padovan wrote: Add a still dummy DUN code, now it can only creates and removes modems. The DUN plugin follows the HFP one a lot, the is basics a copy of some HFP plugin's parts. --- Makefile.am |6 ++ drivers/dunmodem/dunmodem.c | 51 +++ drivers/dunmodem/dunmodem.h | 29 ++ plugins/bluetooth.c | 11 +++ plugins/bluetooth.h |2 + plugins/dun.c | 202 +++ 6 files changed, 301 insertions(+), 0 deletions(-) create mode 100644 drivers/dunmodem/dunmodem.c create mode 100644 drivers/dunmodem/dunmodem.h create mode 100644 plugins/dun.c diff --git a/Makefile.am b/Makefile.am index e256841..a6f2bff 100644 --- a/Makefile.am +++ b/Makefile.am @@ -179,6 +179,9 @@ builtin_sources += drivers/atmodem/atutil.h \ drivers/hfpmodem/network-registration.c \ drivers/hfpmodem/call-volume.c +builtin_modules += dunmodem +builtin_sources += drivers/dunmodem/dunmodem.h drivers/dunmodem/dunmodem.c + builtin_modules += mbmmodem builtin_sources += drivers/atmodem/atutil.h \ drivers/mbmmodem/mbmmodem.h \ @@ -242,6 +245,9 @@ builtin_sources += plugins/bluetooth.c plugins/bluetooth.h builtin_modules += hfp builtin_sources += plugins/hfp.c plugins/bluetooth.h +builtin_modules += dun +builtin_sources += plugins/dun.c + builtin_modules += palmpre builtin_sources += plugins/palmpre.c diff --git a/drivers/dunmodem/dunmodem.c b/drivers/dunmodem/dunmodem.c new file mode 100644 index 000..8658bee --- /dev/null +++ b/drivers/dunmodem/dunmodem.c @@ -0,0 +1,51 @@ +/* + * + * oFono - Open Source Telephony + * + * Copyright (C) 2008-2010 Intel Corporation. All rights reserved. + * Copyright (C) 2010 Gustavo F. Padovan gust...@padovan.org + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + */ + +#ifdef HAVE_CONFIG_H +#include config.h +#endif + +#define _GNU_SOURCE +#include stdio.h +#include string.h +#include errno.h +#include glib.h + +#define OFONO_API_SUBJECT_TO_CHANGE +#include ofono/plugin.h +#include ofono/log.h +#include ofono/modem.h + +#include dunmodem.h + +static int dunmodem_init(void) +{ + return 0; +} + +static void dunmodem_exit(void) +{ +} + +OFONO_PLUGIN_DEFINE(dunmodem, Dial-up Networking Driver, VERSION, + OFONO_PLUGIN_PRIORITY_DEFAULT, dunmodem_init, dunmodem_exit) + diff --git a/drivers/dunmodem/dunmodem.h b/drivers/dunmodem/dunmodem.h new file mode 100644 index 000..6bbf7b9 --- /dev/null +++ b/drivers/dunmodem/dunmodem.h @@ -0,0 +1,29 @@ +/* + * + * oFono - Open Source Telephony + * + * Copyright (C) 2010 Gustavo F. Padovan gust...@padovan.org + * + * 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 + * + */ +#ifndef __DUN_MODEM_H__ +#define __DUN_MODEM_H__ + +struct dun_data { + char *dun_path; +}; + +#endif + diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c index 5a85eaa..b7ec0d3 100644 --- a/plugins/bluetooth.c +++ b/plugins/bluetooth.c @@ -218,6 +218,9 @@ static void has_uuid(DBusMessageIter *array, gpointer user_data) if (!strcasecmp(uuid, HFP_AG_UUID)) *profiles |= HFP_AG; + if (!strcasecmp(uuid, DUN_GW_UUID)) + *profiles |= DUN_GW; + dbus_message_iter_next(value); } } @@ -276,6 +279,14 @@ static void device_properties_cb(DBusPendingCall *call, gpointer user_data) profile-create(path, device_addr, adapter_addr, alias); } + if ((have_uuid DUN_GW) device_addr adapter_addr) { +
RE: [PATCH] Fix busylooped in ppp_disconnect for huawei modem
Hi Kalle, Zhenhua Zhang wrote: Huawei modem closes the modem port after PPP disconnect. So the channel of gatchat is NULL in ppp_disconnect. In such case, we should not resume the chat and call disconnect function when removing the context. Secondly, before removing the gprs context, we should reply the pending DBus message to the client. --- drivers/atmodem/gprs-context.c | 13 - 1 files changed, 12 insertions(+), 1 deletions(-) diff --git a/drivers/atmodem/gprs-context.c b/drivers/atmodem/gprs-context.c index fea80b0..e2f291a 100644 --- a/drivers/atmodem/gprs-context.c +++ b/drivers/atmodem/gprs-context.c @@ -88,11 +88,22 @@ 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); + GAtIO *io = g_at_chat_get_io(gcd-chat); DBG(); g_at_ppp_unref(gcd-ppp); gcd-ppp = NULL; + + if (g_at_io_get_channel(io) == NULL) { + CALLBACK_WITH_FAILURE(gcd-up_cb, NULL, FALSE, NULL, + NULL, NULL, NULL, gcd-cb_data); + gcd-active_context = 0; + gcd-state = STATE_IDLE; + g_at_chat_resume(gcd-chat); + return; + } + g_at_chat_resume(gcd-chat); switch (gcd-state) { @@ -257,7 +268,7 @@ static void at_gprs_context_remove(struct ofono_gprs_context *gc) DBG(); - if (gcd-state != STATE_IDLE) { + if (gcd-state != STATE_IDLE gcd-ppp) { g_at_ppp_unref(gcd-ppp); g_at_chat_resume(gcd-chat); } Could you try my fix for this busy loop issue? It is not perfect but at least work for me. The problem is: in ppp_disconnect, after g_at_chat_resume(), the gprs context has been removed and recreated, so the rest gc and gcd are totally wrong. Btw, I still meet the kernel warning or even Ubuntu crash after activate/deactivate gprs context multiple times on E1552. Do you see such problem? the kernel dmesg is attached for reference. Regards, Zhenhua Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301093] [ cut here ] Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301106] WARNING: at /build/buildd/linux-2.6.32/drivers/usb/serial/usb-serial.c:406 serial_write_room+0x75/0x80 [usbserial]() Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301108] Hardware name: 7661BL4 Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301110] Modules linked in: nls_utf8 isofs option usbserial usb_storage aes_i586 aes_generic hidp rfcomm ppdev sco bnep l2cap binfmt_misc snd_hda_codec_analog fbcon tileblit font bitblit softcursor vga16fb vgastate joydev snd_hda_intel snd_hda_codec snd_hwdep snd_pcm_oss snd_mixer_oss thinkpad_acpi snd_pcm snd_seq_dummy snd_seq_oss snd_seq_midi snd_rawmidi snd_seq_midi_event arc4 snd_seq pcmcia iwlagn snd_timer iwlcore snd_seq_device i915 drm_kms_helper snd mac80211 drm uvcvideo videodev v4l1_compat btusb bluetooth nvram yenta_socket rsrc_nonstatic sdhci_pci sdhci led_class ricoh_mmc pcmcia_core psmouse serio_raw cfg80211 intel_agp agpgart i2c_algo_bit video output soundcore snd_page_alloc lp parport usbhid hid ohci1394 ieee1394 e1000e Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301187] Pid: 2513, comm: ofonod Tainted: GW 2.6.32-21-generic #32-Ubuntu Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301189] Call Trace: Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301196] [c014c3d2] warn_slowpath_common+0x72/0xa0 Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301200] [fdaaa735] ? serial_write_room+0x75/0x80 [usbserial] Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301204] [fdaaa735] ? serial_write_room+0x75/0x80 [usbserial] Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301208] [c014c41a] warn_slowpath_null+0x1a/0x20 Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301211] [fdaaa735] serial_write_room+0x75/0x80 [usbserial] Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301216] [c03b831d] tty_write_room+0x1d/0x20 Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301219] [c03b6127] n_tty_poll+0x177/0x190 Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301222] [c03b21f9] tty_poll+0x69/0x80 Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301226] [c058b5e8] ? _spin_lock_irq+0x18/0x20 Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301229] [c03b5fb0] ? n_tty_poll+0x0/0x190 Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301232] [c021727b] do_poll+0xdb/0x230 Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301236] [c0217d56] do_sys_poll+0x136/0x1f0 Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301239] [c0217ae0] ? __pollwait+0x0/0xe0 Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301242] [c0217bc0] ? pollwake+0x0/0x60 Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301244] [c0217bc0] ? pollwake+0x0/0x60 Jul 27 10:13:17 zzhan17-mobile kernel: [ 599.301247] [c0217bc0] ? pollwake+0x0/0x60 Jul 27 10:13:17
RE: [PATCH] Fix busylooped in ppp_disconnect for huawei modem
Hi Denis, Denis Kenzior wrote: Hi Zhenhua, On 07/26/2010 09:39 PM, Zhenhua Zhang wrote: Huawei modem closes the modem port after PPP disconnect. So the channel of gatchat is NULL in ppp_disconnect. In such case, we should not resume the chat and call disconnect function when removing the context. Please reword the last sentence, we should resume the chat, but the question is of timing... It should be something like 'not resume the chat twice'. One is in ppp_disconnect and the second is in at_gprs_context_remove. Secondly, before removing the gprs context, we should reply the pending DBus message to the client. --- drivers/atmodem/gprs-context.c | 13 - 1 files changed, 12 insertions(+), 1 deletions(-) diff --git a/drivers/atmodem/gprs-context.c b/drivers/atmodem/gprs-context.c index fea80b0..e2f291a 100644 --- a/drivers/atmodem/gprs-context.c +++ b/drivers/atmodem/gprs-context.c @@ -88,11 +88,22 @@ 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); + GAtIO *io = g_at_chat_get_io(gcd-chat); DBG(); g_at_ppp_unref(gcd-ppp); gcd-ppp = NULL; + +if (g_at_io_get_channel(io) == NULL) { +CALLBACK_WITH_FAILURE(gcd-up_cb, NULL, FALSE, NULL, +NULL, NULL, NULL, gcd-cb_data); +gcd-active_context = 0; +gcd-state = STATE_IDLE; +g_at_chat_resume(gcd-chat); +return; +} + Instead of doing that, can't we simply move g_at_chat_resume to the bottom of the function and put in a comment this might cause gprs_context_remove to get called? It's better than my current way. The updated patch will send out soon. Please review it. g_at_chat_resume(gcd-chat); switch (gcd-state) { @@ -257,7 +268,7 @@ static void at_gprs_context_remove(struct ofono_gprs_context *gc) DBG(); -if (gcd-state != STATE_IDLE) { +if (gcd-state != STATE_IDLE gcd-ppp) { This along with the funny g_at_chat_resume logic is what seems to be causing the infinite loop. g_at_ppp_unref(gcd-ppp); g_at_chat_resume(gcd-chat); } Thanks, -Denis Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: Howto about Qt QML and Ofono
Hi Matthias, Matthias Günther wrote: Hi Yang, 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. Thanks for your comment. As mentioned in the thread before, there are two scripts. The problem with bootstrap-configure is, that it enables the --enable-capng switch which may cause problems. It'll probably end up in missing files. Therefore I chose bootstrap to just create the configure script and then use it without any configuration arguments - just the defaults. For the purpose of the howto, the debug switches are not really necessary. You could use 'sudo apt-get install libcap-ng-dev' or 'yum install -y libcap-ng-devel' to install this library. One comment is that we normally call our project as 'oFono' instead 'Ofono'. :-) Best regards, Matthias ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: Ofono in Maemo scratchbox--compilation failed.
Hi Arun, Marcel Holtmann wrote: Hi Arun, I am trying to compile Ofono in Maemo Scratchbox SDK enviornment. Here is the bootstrap-configure output: --- --- - [sbox-FREMANTLE_X86: ~/ofono] ./bootstrap-configure configure.ac: installing `./install-sh' configure.ac: installing `./missing' Makefile.am: installing `./compile' Makefile.am: installing `./depcomp' checking for a BSD-compatible install... /scratchbox/tools/bin/install -c checking whether build environment is sane... yes checking for gawk... gawk checking whether make sets $(MAKE)... yes checking whether to enable maintainer-specific portions of Makefiles... yes checking for pkg-config... /scratchbox/tools/bin/pkg-config checking pkg-config is at least version 0.9.0... yes checking for style of include used by make... GNU checking for gcc... gcc checking for C compiler default output file name... a.out checking whether the C compiler works... yes checking whether we are cross compiling... no checking for suffix of executables... checking for suffix of object files... o checking whether we are using the GNU C compiler... yes checking whether gcc accepts -g... yes checking for gcc option to accept ISO C89... none needed checking dependency style of gcc... gcc3 checking for C/C++ restrict keyword... no checking for gcc... (cached) gcc checking whether we are using the GNU C compiler... (cached) yes checking whether gcc accepts -g... (cached) yes checking for gcc option to accept ISO C89... (cached) none needed checking dependency style of gcc... (cached) gcc3 checking whether gcc and cc understand -c and -o together... yes checking whether gcc accepts -fPIE... yes checking for a BSD-compatible install... /scratchbox/tools/bin/install -c checking for a sed that does not truncate output... /scratchbox/tools/bin/sed checking for gawk... (cached) gawk checking build system type... i486-pc-linux-gnu checking host system type... i486-pc-linux-gnu checking for a sed that does not truncate output... /scratchbox/tools/bin/sed checking for grep that handles long lines and -e... /scratchbox/tools/bin/grep checking for egrep... /scratchbox/tools/bin/grep -E checking for ld used by gcc... /scratchbox/compilers/cs2007q3-glibc2.5-i486/i486-pc-linux-gnu/bin/ld checking if the linker (/scratchbox/compilers/cs2007q3-glibc2.5-i486/i486-pc-linux-gnu /bin/ld) is GNU ld... yes checking for /scratchbox/compilers/cs2007q3-glibc2.5-i486/i486-pc-linux-gnu/ bin/ld option to reload object files... -r checking for BSD-compatible nm... /scratchbox/compilers/bin/nm -B checking whether ln -s works... yes checking how to recognize dependent libraries... pass_all checking how to run the C preprocessor... gcc -E checking for ANSI C header files... yes checking for sys/types.h... yes checking for sys/stat.h... yes checking for stdlib.h... yes checking for string.h... yes checking for memory.h... yes checking for strings.h... yes checking for inttypes.h... yes checking for stdint.h... yes checking for unistd.h... yes checking dlfcn.h usability... yes checking dlfcn.h presence... yes checking for dlfcn.h... yes checking the maximum length of command line arguments... 98304 checking command to parse /scratchbox/compilers/bin/nm -B output from gcc object... failed checking for objdir... .libs checking for ar... ar checking for ranlib... ranlib checking for strip... strip checking if gcc supports -fno-rtti -fno-exceptions... no checking for gcc option to produce PIC... -fPIC checking if gcc PIC flag -fPIC works... yes checking if gcc static flag -static works... yes checking if gcc supports -c -o file.o... yes checking whether the gcc linker (/scratchbox/compilers/cs2007q3-glibc2.5-i486/i486-pc-linux-gnu /bin/ld) supports shared libraries... yes checking whether -lc should be explicitly linked in... no checking dynamic linker characteristics... GNU/Linux ld.so checking how to hardcode library paths into programs... immediate checking whether stripping libraries is possible... yes checking if libtool supports shared libraries... yes checking whether to build shared libraries... yes checking whether to build static libraries... no configure: creating libtool checking for BSD-compatible nm... (cached) /scratchbox/compilers/bin/nm -B checking for signalfd in -lc... yes checking for dlopen in -ldl... yes checking for GLIB... yes checking for DBUS... yes checking for dbus_watch_get_unix_fd in -ldbus-1... yes checking for dbus_connection_can_send_type in -ldbus-1... no checking for UDEV... yes checking for CAPNG... yes configure: creating ./config.status config.status: creating Makefile config.status: creating include/version.h config.status: creating config.h config.status: executing depfiles commands
RE: Modem emulator and DUN server side for oFono and BlueZ
Hi Suraj, suraj wrote: Hi Zhenhua, On Wed, 2010-05-05 at 06:27 +0530, Zhang, Zhenhua wrote: Hi Padovan, Gustavo F. Padovan wrote: Hi Zhenhua, * Zhang, Zhenhua zhenhua.zh...@intel.com [2010-04-27 15:53:54 +0800]: Hi, I am now working on modem emulator and one usage is for DUN server role. Since Padovan is working on client role, it's good to share my rough thinking for server side implementation. Here are the simple steps I have: 1. Create an oFono emulator atom in oFono. It's the emulator manager that could create DUN, HFP AG or SPP type emulators. It exposes dbus methods like CreateEmulator, DestroyEmulator, GetProperty, etc. 2. DUN agent server in BlueZ watch oFono and call CreateEmulator and pass the file descriptor to oFono. This server could further implement HFP AG and SPP connection. Shouldn't the emulator register itself on the BlueZ agent server? It's fine to register the emulator like HFP client. In this way, we might able to share the agent with DUN server as well. Thanks. -- Gustavo F. Padovan http://padovan.org ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua -- To unsubscribe from this list: send the line unsubscribe linux-bluetooth in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Do you have DUNP server emulator implemented? I was thinking about implementing the same. But would like to use your implementation if you have already done it. Regards Suraj Yes, I am working DUN server part now. (I assume 'DUNP' you mentioned is 'DUN', dialing up networking). I am trying to submit patches to upstream now. Hope it could be ready asap so you can try it if interested. Thanks. Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH] test-server: Use cfmakeraw to set TTY raw mode
Hi Marcel, Marcel Holtmann wrote: 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. The cleanup patch has been sent. Please review it. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: Crash in at_gprs_context_remove()
Hi, Zhang, Zhenhua wrote: Hi Kalle, Kalle Valo wrote: Hi, (gdb) bt #0 0x7790b642 in IA__g_atomic_int_exchange_and_add (atomic=0x0, val=-1) at /build/buildd/glib2.0-2.25.8/glib/gatomic-gcc.c:30 #1 0x004325a3 in g_at_ppp_unref (ppp=0x0) at gatchat/gatppp.c:448 #2 0x00448e12 in at_gprs_context_remove (gc=0x6e2f50) at drivers/atmodem/gprs-context.c:260 #3 0x004923c9 in As Denis has pointed out, we can add a check for gcd-ppp in at_gprs_context_remove() to avoid this crash. When I tried to activate deactivate context on my Huawei EM770W modem, I see kernel module usb_serial warning. [ cut here ] WARNING: at /build/buildd/linux-2.6.31/drivers/usb/serial/usb-serial.c:460 serial_ioctl+0x99/0xa0 [usbserial]() Hardware name: 7661BL4 Modules linked in: tun option usbserial usb_storage hidp aes_i586 aes_generic ppdev binfmt_misc bridge stp bnep btusb joydev snd_hda_codec_analog pcmcia snd_hda_intel snd_hda_codec snd_hwdep snd_pcm_oss snd_mixer_oss arc4 snd_pcm ecb snd_seq_dummy uvcvideo videodev v4l1_compat snd_seq_oss yenta_socket rsrc_nonstatic ricoh_mmc pcmcia_core iwlagn iwlcore mac80211 sdhci_pci sdhci psmouse serio_raw snd_seq_midi snd_rawmidi snd_seq_midi_event snd_seq snd_timer snd_seq_device cfg80211 thinkpad_acpi led_class nvram snd soundcore snd_page_alloc heci(C) lp parport usbhid dm_raid45 xor fbcon tileblit font bitblit softcursor ohci1394 e1000e ieee1394 i915 drm i2c_algo_bit video output intel_agp agpgart Pid: 5180, comm: ofonod Tainted: G C 2.6.31-16-generic #53-Ubuntu Call Trace: [c014518d] warn_slowpath_common+0x6d/0xa0 [fa3ab689] ? serial_ioctl+0x99/0xa0 [usbserial] [fa3ab689] ? serial_ioctl+0x99/0xa0 [usbserial] [c01451d5] warn_slowpath_null+0x15/0x20 [fa3ab689] serial_ioctl+0x99/0xa0 [usbserial] [c0386414] ? tty_buffer_flush+0x54/0xe0 [fa3ab5f0] ? serial_ioctl+0x0/0xa0 [usbserial] [c03801b7] tty_ioctl+0x77/0x620 [c0380140] ? tty_ioctl+0x0/0x620 [c01f518c] vfs_ioctl+0x1c/0x90 [c01f0726] ? putname+0x26/0x40 [c01f54b1] do_vfs_ioctl+0x71/0x310 [c01f57af] sys_ioctl+0x5f/0x80 [c01e5939] ? sys_open+0x29/0x40 [c010336c] syscall_call+0x7/0xb ---[ end trace ac231b55ebb1fdca ]--- When I shutdown the oFono, kernel reports: tty_port_close_start: tty-count = 1 port count = 0. Any ideas? I observed the warning shows up when /dev/ttyUSB2 send us any intermediate result. If I remove gprs atom and recreate it in huawei_disconnect, the kernel warning is gone. Does anyone see the same problem in other Huawei modem? If not, I assume it's a problem only for EM770 modem. (I don't have other Huawei modem at hand) Thanks, Zhenhua -- Kalle Valo ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 2/3] huawei: Add Huawei EM770 modem support
Hi Marcel, Marcel Holtmann wrote: Hi Zhenhua Zhang, Huawei EM770W is a 3G WCDMA modem that supports HSPA/UMTS/EDGE/GPRS/GSM data service and WCDMA/GSM short message services. It also has voice call capability that supports both 2G and 3G network. --- plugins/huawei.c| 31 ++- plugins/ofono.rules |3 +++ plugins/udev.c |3 +++ 3 files changed, 36 insertions(+), 1 deletions(-) diff --git a/plugins/udev.c b/plugins/udev.c index 09ee93e..7d34b0b 100644 --- a/plugins/udev.c +++ b/plugins/udev.c @@ -227,6 +227,9 @@ static void add_huawei(struct ofono_modem *modem, const char *name = udev_list_entry_get_name(entry); type = udev_list_entry_get_value(entry); +if (g_str_equal(name, ID_MODEL_ID) == TRUE) +ofono_modem_set_string(modem, Model, type); + so this is the part that I don't like. I prefer we have some udev rules that create OFONO_HUAWEI_VOICE=1 and we use that as indicator to enable voice capabilities or not. Okay. Will resend out patches and rebase it to latest git tree. Thanks. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 3/5] test-server: Add PPP server support
Hi Denis, Zhenhua Zhang wrote: So that gsmdial and wvdial could talk to test-server and establish PPP connection. --- gatchat/test-server.c | 131 ++--- 1 files changed, 123 insertions(+), 8 deletions(-) diff --git a/gatchat/test-server.c b/gatchat/test-server.c index 5c1cfa4..96faab4 100644 --- a/gatchat/test-server.c +++ b/gatchat/test-server.c --snip-- static void server_destroy(gpointer user) @@ -706,15 +825,11 @@ static void server_destroy(gpointer user) static void set_raw_mode(int fd) { - struct termios options; - - 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); + struct termios ti; - tcsetattr(fd, TCSANOW, options); + tcflush(fd, TCIOFLUSH); + cfmakeraw(ti); + tcsetattr(fd, TCSANOW, ti); } static gboolean create_tty(const char *modem_path) I found above changes does not contain latest git tree. The part of change is necessary when I tried to use bluetooth serial proxy between two machines. Without cfmakeraw, the server responses: '\r\nOK\r\n' would change to: '\n\nOK\n\n' And this issue doesn't exist if both server and client on the same machine. -- 1.6.3.3 ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 2/6] gatppp: Add PPP server extension
Hi Denis, Denis Kenzior wrote: Hi Zhenhua, 1. Add interface to set PPP server info by g_at_ppp_set_server_info. 2. Pass local and peer address through IPCP handshaking. Ok getting pretty close now :) +static void ipcp_reset_server_config_options(struct ipcp_data *ipcp) +{ + ipcp-req_options = REQ_OPTION_IPADDR; Might want to only request IP Addr if local_addr is not zero. Just like in set_server_info. Ok. Local updated. + +ipcp_generate_config_options(ipcp); +} + snip @@ -167,7 +211,7 @@ static void ipcp_rca(struct pppcp_data *pppcp, switch (ppp_option_iter_get_type(iter)) { case IP_ADDRESS: -memcpy(ipcp-ipaddr, data, 4); +memcpy(ipcp-local_addr, data, 4); break; case PRIMARY_DNS_SERVER: memcpy(ipcp-dns1, data, 4); You might not want to do anything here in the case of a server role. Agree. So in case of a server role, we simply do nothing in ipcp_rca and ipcp_rcn_nak. Good catch. @@ -204,7 +248,7 @@ static void ipcp_rcn_nak(struct pppcp_data *pppcp, case IP_ADDRESS: g_print(Setting suggested ip addr\n); ipcp-req_options |= REQ_OPTION_IPADDR; -memcpy(ipcp-ipaddr, data, 4); +memcpy(ipcp-local_addr, data, 4); break; case PRIMARY_DNS_SERVER: g_print(Setting suggested dns1\n); Again, probably don't want to set the local_addr in the case of a server role. @@ -269,17 +313,102 @@ static void ipcp_rcn_rej(struct pppcp_data *pppcp, pppcp_set_local_options(pppcp, ipcp-options, ipcp-options_len); } +static guint8 *ipcp_generate_peer_config_options(struct ipcp_data *ipcp, + guint16 *new_len) +{ +guint8 *options; +guint16 len = 0; + +options = g_try_new0(guint8, MAX_CONFIG_OPTION_SIZE); + if (!options) + return NULL; + +FILL_IP(options, TRUE, IP_ADDRESS, ipcp-peer_addr); +FILL_IP(options, TRUE, PRIMARY_DNS_SERVER, ipcp-dns1); +FILL_IP(options, TRUE, SECONDARY_DNS_SERVER, ipcp-dns2); +FILL_IP(options, TRUE, PRIMARY_NBNS_SERVER, ipcp-nbns1); +FILL_IP(options, TRUE, SECONDARY_NBNS_SERVER, ipcp-nbns2); + +*new_len = MAX_CONFIG_OPTION_SIZE; Don't use MAX_CONFIG_OPTION_SIZE, instead use len (which is filled properly by the FILL_IP macro) Also, we shouldn't bother supporting NBNS, lets never suggest those options as a server or set them in set_server_info. Fixed, so I have removed NBNS parameters for set_server_info() at all. + +return options; +} + static enum rcr_result ipcp_rcr(struct pppcp_data *pppcp, const struct pppcp_packet *packet, guint8 **new_options, guint16 *new_len) { struct ppp_option_iter iter; +struct ipcp_data *ipcp = pppcp_get_data(pppcp); +guint32 peer_addr = 0; +guint32 dns1 = 0; +guint32 dns2 = 0; +guint32 nbns1 = 0; +guint32 nbns2 = 0; ppp_option_iter_init(iter, packet); -if (ppp_option_iter_next(iter) == FALSE) +while (ppp_option_iter_next(iter) == TRUE) { +const guint8 *data = ppp_option_iter_get_data(iter); + +switch (ppp_option_iter_get_type(iter)) { +case IP_ADDRESS: +memcpy(peer_addr, data, 4); +break; +case PRIMARY_DNS_SERVER: +memcpy(dns1, data, 4); +break; +case SECONDARY_DNS_SERVER: +memcpy(dns2, data, 4); +break; +case PRIMARY_NBNS_SERVER: +memcpy(nbns1, data, 4); +break; +case SECONDARY_NBNS_SERVER: +memcpy(nbns2, data, 4); +break; +default: +break; +} +} + As mentioned above, if Primary / Secondary NBNS server is sent by the client, we need to Conf-Rej just those options to the client. Any other unrecognized options should also be Conf-Rejected. The order is important here, read the spec for details. The IP Address, DNS1/DNS2 should not be Conf-Rejected. Looks like we need more Conf-Nak than Conf-Reject in ipcp_rcr. :-) As PPP client, actually I think we don't need to send NBNS request at all. It's clear that pppd client only requests IP/DNS in Conf-Req. Secondly, if server sent us empty Conf-Req, the client should request server IP address in Conf-Nak response, instead of Conf-Rej all options. Once we get local_addr from server, we return Conf-Ack in ipcp_rcr and don't request server IP any more. See attached pppd.log for details. +if (ipcp-is_server) { +guint8 *options; +guint16
RE: [PATCH 2/6] gatppp: Add PPP server extension
Hi Denis, Zhang, Zhenhua wrote: Hi Denis, As mentioned above, if Primary / Secondary NBNS server is sent by the client, we need to Conf-Rej just those options to the client. Any other unrecognized options should also be Conf-Rejected. The order is important here, read the spec for details. The IP Address, DNS1/DNS2 should not be Conf-Rejected. Looks like we need more Conf-Nak than Conf-Reject in ipcp_rcr. :-) As PPP client, actually I think we don't need to send NBNS request at all. It's clear that pppd client only requests IP/DNS in Conf-Req. Secondly, if server sent us empty Conf-Req, the client should request server IP address in Conf-Nak response, instead of Conf-Rej all options. Once we get local_addr from server, we return Conf-Ack in ipcp_rcr and don't request server IP any more. See attached pppd.log for details. Ops, forgot the attachment. Attached again. + if (ipcp-is_server) { + guint8 *options; + guint16 len; + + /* Reject if we have not assign client address yet */ + if (ipcp-peer_addr == 0 ipcp-dns1 == 0 ipcp-dns2 == 0) + goto reject; Actually you should be NAKing here, not Rejecting. Reject means you don't support this option at all. Okay. + + /* Acknowledge client options if it matches with server options +*/ + if (ipcp-peer_addr == peer_addr + ipcp-dns1 == dns1 ipcp-dns2 == dns2 + ipcp-nbns1 == nbns1 ipcp-nbns2 == nbns2) + return RCR_ACCEPT; + + /* Send client IP/DNS/NBNS information in the config options */ + options = ipcp_generate_peer_config_options(ipcp, len); + if (!options) +goto reject; + + *new_len = len; + *new_options = options; + + return RCR_NAK; + } + + /* Client */ + if (peer_addr ipcp-peer_addr == 0) { + /* RFC 1332 section 3.3 +* As client, accept the server IP as peer's address + */ + ipcp-peer_addr = peer_addr; + As client, can we just accept peer_addr as long as it's no zero, and return Conf-Ack? No matter what ipcp-peer_addr is. return RCR_ACCEPT; + } +reject: /* Reject all options */ *new_len = ntohs(packet-length) - sizeof(*packet); *new_options = g_memdup(packet-data, *new_len); @@ -317,7 +446,7 @@ struct pppcp_data *ipcp_new(GAtPPP *ppp)} pppcp_set_data(pppcp, ipcp); - ipcp_reset_config_options(ipcp); + ipcp_reset_client_config_options(ipcp); pppcp_set_local_options(pppcp, ipcp-options, ipcp-options_len); return pppcp; Regards, -Denis Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua pppd.log Description: pppd.log ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 1/5] gatppp: Add PPP server extension
Hi Denis, Denis Kenzior wrote: Hi Zhenhua, static enum rcr_result ipcp_rcr(struct pppcp_data *pppcp, const struct pppcp_packet *packet, guint8 **new_options, guint16 *new_len) { struct ppp_option_iter iter; +struct ipcp_data *ipcp = pppcp_get_data(pppcp); +guint32 peer_addr = 0; +guint32 dns1 = 0; +guint32 dns2 = 0; +guint32 nbns1 = 0; +guint32 nbns2 = 0; ppp_option_iter_init(iter, packet); -if (ppp_option_iter_next(iter) == FALSE) -return RCR_ACCEPT; +while (ppp_option_iter_next(iter) == TRUE) { +const guint8 *data = ppp_option_iter_get_data(iter); + +switch (ppp_option_iter_get_type(iter)) { +case IP_ADDRESS: +memcpy(peer_addr, data, 4); +break; +case PRIMARY_DNS_SERVER: +memcpy(dns1, data, 4); +break; +case SECONDARY_DNS_SERVER: +memcpy(dns2, data, 4); +break; +case PRIMARY_NBNS_SERVER: +memcpy(nbns1, data, 4); +break; +case SECONDARY_NBNS_SERVER: +memcpy(nbns2, data, 4); +break; +default: +break; +} +} + +if (peer_addr) { +if (ipcp-peer_addr == 0) { +/* RFC 1332 section 3.3 + * As client, accept the server IP as peer's address + */ +ipcp-peer_addr = peer_addr; + +return RCR_ACCEPT; I'm still confused about this part. As client we should accept server's IP address as long as its non-zero. Otherwise Conf-Rej the option. Right, initially, client know nothing about server, so client-peer_addr is zero. Once client get a non-zero IP from configure option, we fill this IP as peer's IP address. Looks like this IP is exposed as gateway to Connman. Maybe I could change it like: if (ipcp-is_server == FALSE ipcp-peer_addr == 0) +} else if (ipcp-peer_addr == peer_addr ipcp-dns1 == dns1 + ipcp-nbns1 == nbns1 ipcp-nbns2 == nbns2) +/* As server, verify the client's info and then send + * acknowledgement back + */ +return RCR_ACCEPT; +} else { +/* Client requests server to send IP/DNS/NBNS information in the + * config options + */ +if (ipcp-peer_addr) { +guint8 *options; +guint16 len; + +options = ipcp_generate_peer_config_options(ipcp, len); +if (!options) +goto reject; + +*new_len = len; +*new_options = options; +return RCR_NAK; +} Here we want to ensure that we nak the client's options for as long as the options don't match. Even ipcp-peer_addr is 0 (e.g. we're allocating client's address via DHCP or something) In theory, it's true. But I don't see a PPP modem without client's address here. Even in drivers/atmodem/gprs-context.c, we assume the PPP interface has a static IP instead of dynamic one. Anyway, I will modify this part. Overall I think this code path is a little too complicated. Can you see whether you can clean it up a bit? Okay, I will try to elimiate some conditional branchs here. Regards, -Denis Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 03/11] gatppp: Add PPP server extension
Hi Denis, Denis Kenzior wrote: Hi Zhenhua, +void ipcp_set_server_info(struct pppcp_data *pppcp, guint32 local_addr, + guint32 peer_addr, + guint32 dns1, guint32 dns2, + guint32 nbns1, guint32 nbns2) +{ + struct ipcp_data *ipcp = pppcp_get_data(pppcp); + + ipcp-req_options = REQ_OPTION_IPADDR | REQ_OPTION_DNS1 | + REQ_OPTION_DNS2 | REQ_OPTION_NBNS1 | + REQ_OPTION_NBNS2; + + ipcp-local_addr = local_addr; + ipcp-peer_addr = peer_addr; + ipcp-dns1 = dns1; + ipcp-dns2 = dns2; + ipcp-nbns1 = nbns1; + ipcp-nbns2 = nbns2; + + ipcp_generate_config_options(ipcp); Please note that this function is currently having no effect, you're missing a call to pppcp_set_local_options. However, in the case of a server, we actually do not want to request DNS or NBNS, and we should only send our local address to the peer if it is non-zero. Please note that all PPP modems we have encountered so far, do not send their local IP address in a Configure- Request. Thus calling ipcp_set_server_info should end up generating an empty Configure-Request or with only the local IP address present. It's confused me a little. Please correct me if I am wrong. I think PPP modem should send their local IP address to peer in a Configure-Request. So client could accept the peer address in ipcp_rcr. This is correct if the PPP server wants to do so, it is perfectly OK for the server not to send its IP address (e.g. if it doesn't have one.) This is what we observe on most PPP modems. Okay. That's fine. At least, PPP server need to send peer_addr in Conf-Nak reply. Client 'thinks' it's server address. In the case of a server, ipcp_set_server_info() should set local, peer, DNS and NBNS info and call pppcp_set_local_options(). And server only send our local address to peer in a Configure-Request. I'd modify this slightly. If ipcp_set_server_info is called with server's local address as 0, then don't even send IP-Address in a configure Request. This wouldn't make sense, in effect the server is asking the client to provide its IP address. Otherwise, go ahead and send it (as the client will simply ack). No problems. If server local address is zero, we don't send IP in a configure request. +} + static void ipcp_reset_config_options(struct ipcp_data *ipcp) { ipcp-req_options = REQ_OPTION_IPADDR | REQ_OPTION_DNS1 | REQ_OPTION_DNS2 | REQ_OPTION_NBNS1 | REQ_OPTION_NBNS2; - ipcp-ipaddr = 0; + ipcp-local_addr = 0; In the case of a server, the local_address should not be reset. It seems we need to add a flag in ipcp_data to indicate whether it's server or not. Does it make sense? Might be a good idea. Regards, -Denis Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 03/11] gatppp: Add PPP server extension
Hi Denis, Denis Kenzior wrote: Hi Zhenhua, 1. Add interface to set PPP server info by g_at_ppp_set_server_info. 2. Pass local and peer address through IPCP handshaking. --- Its great that you're working on the PPP Server extensions. Can I get you to submit a patch taking ownership of this task? See ofono/TODO 'PPP Server support'. +static void ipcp_generate_peer_config_options(struct ipcp_data *ipcp) +{ + guint16 len = 0; + +FILL_IP(ipcp-req_options REQ_OPTION_IPADDR, +IP_ADDRESS, ipcp-peer_addr); +FILL_IP(ipcp-req_options REQ_OPTION_DNS1, +PRIMARY_DNS_SERVER, ipcp-dns1); +FILL_IP(ipcp-req_options REQ_OPTION_DNS2, +SECONDARY_DNS_SERVER, ipcp-dns2); +FILL_IP(ipcp-req_options REQ_OPTION_NBNS1, +PRIMARY_NBNS_SERVER, ipcp-nbns1); +FILL_IP(ipcp-req_options REQ_OPTION_NBNS2, +SECONDARY_NBNS_SERVER, ipcp-nbns2); + Using ipcp-req_options is the wrong thing here. That is used for options sent in a Configure-Req. Here you're filling in options to be sent in a Configure-Nak, Configure-Reject or a Configure-Ack. You should simply be filling these in ipcp_rcr. Feel free to modify the FILL_IP macro to take an additional destination parameter. Okay. I will do that. +ipcp-options_len = len; +} + +void ipcp_set_server_info(struct pppcp_data *pppcp, guint32 local_addr, +guint32 peer_addr, + guint32 dns1, guint32 dns2, +guint32 nbns1, guint32 nbns2) +{ +struct ipcp_data *ipcp = pppcp_get_data(pppcp); + +ipcp-req_options = REQ_OPTION_IPADDR | REQ_OPTION_DNS1 | +REQ_OPTION_DNS2 | REQ_OPTION_NBNS1 | +REQ_OPTION_NBNS2; + +ipcp-local_addr = local_addr; +ipcp-peer_addr = peer_addr; +ipcp-dns1 = dns1; +ipcp-dns2 = dns2; +ipcp-nbns1 = nbns1; +ipcp-nbns2 = nbns2; + +ipcp_generate_config_options(ipcp); Please note that this function is currently having no effect, you're missing a call to pppcp_set_local_options. However, in the case of a server, we actually do not want to request DNS or NBNS, and we should only send our local address to the peer if it is non-zero. Please note that all PPP modems we have encountered so far, do not send their local IP address in a Configure- Request. Thus calling ipcp_set_server_info should end up generating an empty Configure-Request or with only the local IP address present. It's confused me a little. Please correct me if I am wrong. I think PPP modem should send their local IP address to peer in a Configure-Request. So client could accept the peer address in ipcp_rcr. In the case of a server, ipcp_set_server_info() should set local, peer, DNS and NBNS info and call pppcp_set_local_options(). And server only send our local address to peer in a Configure-Request. +} + static void ipcp_reset_config_options(struct ipcp_data *ipcp) { ipcp-req_options = REQ_OPTION_IPADDR | REQ_OPTION_DNS1 | REQ_OPTION_DNS2 | REQ_OPTION_NBNS1 | REQ_OPTION_NBNS2; -ipcp-ipaddr = 0; +ipcp-local_addr = 0; In the case of a server, the local_address should not be reset. It seems we need to add a flag in ipcp_data to indicate whether it's server or not. Does it make sense? +ipcp-peer_addr = 0; ipcp-dns1 = 0; ipcp-dns2 = 0; ipcp-nbns1 = 0; @@ -274,11 +321,54 @@ static enum rcr_result ipcp_rcr(struct pppcp_data *pppcp, guint8 **new_options, guint16 *new_len) { struct ppp_option_iter iter; +struct ipcp_data *ipcp = pppcp_get_data(pppcp); +guint32 peer_addr; +guint32 dns1; +guint32 dns2; ppp_option_iter_init(iter, packet); -if (ppp_option_iter_next(iter) == FALSE) -return RCR_ACCEPT; Again, be careful here. In the case of a client we should only accept the Server's IP-Address, nothing else. +while (ppp_option_iter_next(iter) == TRUE) { +const guint8 *data = ppp_option_iter_get_data(iter); + +switch (ppp_option_iter_get_type(iter)) { +case IP_ADDRESS: +memcpy(peer_addr, data, 4); +break; +case PRIMARY_DNS_SERVER: +memcpy(dns1, data, 4); +break; +case SECONDARY_DNS_SERVER: +memcpy(dns2, data, 4); +break; +default: +break; +} +} + +if (peer_addr dns1 dns2) { +if (ipcp-peer_addr == 0) { +/* RFC 1332 Section 3.3 + * Accept the value of IP address as peer IP address +
RE: [PATCH v2] atmodem: fix crash during context deactivation
Hi Valo, Kalle Valo wrote: --- 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); } - 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; You should use g_at_chat_resume(gcd-chat) to resume the chat before you returns FALSE here. @@ -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, ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 5/9] Move bluetooth utils from hfp.c to bluetooth.c
Hi Padovan, Gustavo F. Padovan wrote: --- plugins/bluetooth.c | 482 +++ plugins/hfp.c | 458 2 files changed, 518 insertions(+), 422 deletions(-) diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c index b4fe676..fc89579 100644 --- a/plugins/bluetooth.c +++ b/plugins/bluetooth.c + int bluetooth_register_uuid(const char *uuid, struct bluetooth_profile *profile) { + int err; + if (uuid_hash) goto done; connection = ofono_dbus_get_connection(); + bluetooth_watch = g_dbus_add_service_watch(connection, BLUEZ_SERVICE, + NULL, bluetooth_disconnect, NULL, NULL); + + adapter_added_watch = g_dbus_add_signal_watch(connection, NULL, NULL, + BLUEZ_MANAGER_INTERFACE, + AdapterAdded, + adapter_added, NULL, NULL); + + adapter_removed_watch = g_dbus_add_signal_watch(connection, NULL, NULL, + BLUEZ_MANAGER_INTERFACE, + AdapterRemoved, + adapter_removed, NULL, NULL); + + property_watch = g_dbus_add_signal_watch(connection, NULL, NULL, + BLUEZ_DEVICE_INTERFACE, + PropertyChanged, + property_changed, NULL, NULL); + + if (bluetooth_watch == 0 || adapter_added_watch == 0 || + adapter_removed_watch == 0 || property_watch == 0) { + err = -EIO; + goto remove; + } I may introduce API to register service like bluetooth_register_service() later. So I wonder if we could put above the initialization code into bluetooth_init(). I'd like to introduce a hash table similar with uuid_hash so uuid_hash is NULL in my case. uuid_hash = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, NULL); @@ -56,7 +521,19 @@ int bluetooth_register_uuid(const char *uuid, struct bluetooth_profile *profile) done: g_hash_table_insert(uuid_hash, g_strdup(uuid), profile); + send_method_call_with_reply(BLUEZ_SERVICE, /, + BLUEZ_MANAGER_INTERFACE, GetProperties, + manager_properties_cb, NULL, NULL, -1, + DBUS_TYPE_INVALID); + return 0; + +remove: + g_dbus_remove_watch(connection, bluetooth_watch); + g_dbus_remove_watch(connection, adapter_added_watch); + g_dbus_remove_watch(connection, adapter_removed_watch); + g_dbus_remove_watch(connection, property_watch); + return err; } void bluetooth_unregister_uuid(const char *uuid) @@ -66,6 +543,11 @@ void bluetooth_unregister_uuid(const char *uuid) if (g_hash_table_size(uuid_hash)) return; + g_dbus_remove_watch(connection, bluetooth_watch); + g_dbus_remove_watch(connection, adapter_added_watch); + g_dbus_remove_watch(connection, adapter_removed_watch); + g_dbus_remove_watch(connection, property_watch); + g_hash_table_destroy(uuid_hash); g_hash_table_destroy(adapter_address_hash); uuid_hash = NULL; Can we move g_dbus_remove_watch() to bluetooth_exit() as well? diff --git a/plugins/hfp.c b/plugins/hfp.c index aca6b4e..12e7daf 100644 --- a/plugins/hfp.c +++ b/plugins/hfp.c @@ -62,8 +62,7 @@ static const char *cmer_prefix[] = { +CMER:, NULL }; static const char *chld_prefix[] = { +CHLD:, NULL }; static DBusConnection *connection; -static GHashTable *uuid_hash = NULL; -static GHashTable *adapter_address_hash; +static GHashTable *modem_hash = NULL; static void hfp_debug(const char *str, void *user_data) { @@ -259,101 +258,6 @@ fail: return err; } -typedef void (*PropertyHandler)(DBusMessageIter *iter, gpointer user_data); - -struct property_handler { - const char *property; - PropertyHandler callback; - gpointer user_data; -}; - -static gint property_handler_compare(gconstpointer a, gconstpointer b) -{ - const struct property_handler *handler = a; - const char *property = b; - - return strcmp(handler-property, property); -} - -static void parse_properties_reply(DBusMessage *reply, - const char *property, ...) -{ - va_list args; - GSList *prop_handlers = NULL; - DBusMessageIter array, dict; - - va_start(args, property); - - while (property != NULL) { - struct property_handler *handler = - g_new0(struct property_handler, 1); - - handler-property = property; - handler-callback = va_arg(args, PropertyHandler); - handler-user_data = va_arg(args, gpointer); - - property = va_arg(args, const
RE: commit Fix Use hashtable to record udev path breaks huawei
Hi Kalle, Kalle Valo wrote: Kalle Valo kalle.v...@canonical.com writes: while working on third version of my huawei gprs patches I noticed that this commit breaks huawei: commit af976f7e524746b1b55645967e11ab8250f593a8 Author: Zhenhua Zhang zhenhua.zh...@intel.com Date: Tue May 11 09:04:28 2010 +0800 Fix Use hashtable to record udev path Sometimes, Udev device 'remove' event could not report correct parent node of current udev_device. Current code replies on the devpath attached on the parent node to find modem and then remove it. This fix is to change the way to store the devpath info into a hashtable. So that we search hashtable to get devpath and remove the modem. I reverted the commit and huawei works again for me. More details later today. Sorry, I was too hasty. The patch is fine, I was just testing inproperly. After a bit more testing I found out that problem always happens when I plugin the usb modem: May 19 17:20:09 tukki connmand[30925]: plugins/ofono.c:modem_changed() path /huawei1 May 19 17:20:10 tukki ofonod[672]: ATE0\r\r\nOK\r\n May 19 17:20:10 tukki ofonod[672]: AT+CFUN=1\r May 19 17:20:10 tukki ofonod[672]: \r\nOK\r\n May 19 17:20:10 tukki ofonod[672]: plugins/huawei.c:cfun_enable() May 19 17:20:10 tukki ofonod[672]: AT+CRC=1\r May 19 17:20:10 tukki ofonod[672]: \r\n+CME ERROR: SIM busy\r\n May 19 17:20:10 tukki ofonod[672]: AT+CLIP=1\r May 19 17:20:10 tukki ofonod[672]: \r\n+CME ERROR: SIM busy\r\n May 19 17:20:10 tukki ofonod[672]: AT+COLP=1\r May 19 17:20:10 tukki ofonod[672]: \r\n+CME ERROR: SIM busy\r\n May 19 17:20:10 tukki ofonod[672]: AT+CCWA=1\r May 19 17:20:10 tukki ofonod[672]: \r\n+CME ERROR: SIM busy\r\n May 19 17:20:10 tukki ofonod[672]: drivers/atmodem/voicecall.c:at_voicecall_initialized() voicecall_init: registering to notifications May 19 17:20:10 tukki ofonod[672]: src/sim.c:ofono_sim_add_state_watch() 0x132bcd0 May 19 17:20:10 tukki ofonod[672]: * \r\n^STIN:0,0,0\r\n May 19 17:20:10 tukki ofonod[672]: AT+CGMI\r May 19 17:20:10 tukki connmand[30925]: plugins/ofono.c:modem_changed() path /huawei1 May 19 17:20:10 tukki ofonod[672]: \r\nhuawei\r\n\r\nOK\r\n May 19 17:20:10 tukki ofonod[672]: AT+CRSM=176,28590,0,0,1\r May 19 17:20:10 tukki connmand[30925]: plugins/ofono.c:modem_changed() path /huawei1 May 19 17:20:10 tukki ofonod[672]: \r\n+CME ERROR: SIM busy\r\n May 19 17:20:10 tukki ofonod[672]: AT+CLCC\r May 19 17:20:10 tukki ofonod[672]: \r\n+CME ERROR: SIM busy\r\n May 19 17:20:10 tukki ofonod[672]: AT+CRSM=192,28599\r May 19 17:20:10 tukki ofonod[672]: \r\n+CME ERROR: SIM busy\r\n May 19 17:20:10 tukki ofonod[672]: src/voicecall.c:ecc_g2_read_cb() 0 May 19 17:20:10 tukki ofonod[672]: AT+CGMM\r May 19 17:20:10 tukki ofonod[672]: \r\nE1552\r\n\r\nOK\r\n But if I start ofono while the usb modem is plugged in it works fine. Also modem disable and then enable workarounds it. Oh well. My commit af976f7e524 should not impact modem detection and power up stage. If you found huawei modem still exists in oFono after you unplug the dungle, I guess something maybe wrong in my patch. :-) Maybe you need to wait until SIM is settled to power on it. As Denis said, listen '^SIMST' notification should be a good idea. -- Kalle Valo ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: How to start ofono-phonesim in ubuntu-10.04 -reg
Hi Krishna, From: ofono-boun...@ofono.org [mailto:ofono-boun...@ofono.org] On Behalf Of krishna k Sent: Monday, May 10, 2010 2:43 PM To: ofono@ofono.org Subject: How to start ofono-phonesim in ubuntu-10.04 -reg Hi Zhenhua, Thanks for your reply.. I added the following code in /usr/share/alsa/pcm/modem.conf file... [Zhenhua] Please put modem.conf under /etc/ofono. You should see phonesim0 from list_modems output like your E71 modem. 1. # #Sample for using phone simulator # [phonesim] Driver=phonesim Address=127.0.0.1 port=12345 - I tried to ./list_modems. I got nothing on command prompt. According to my knowledge i should get the available modems (atleast phonesim0) I tried to enable the modem using the following command.. ./enable-modem /phonesim0 I got output as follows... -- Connecting modem /phonesim0... Traceback (most recent call last): File ./enable-modem, line 20, in module modem.SetProperty(Powered, dbus.Boolean(1)) File /usr/lib/pymodules/python2.6/dbus/proxies.py, line 68, in __call__ return self._proxy_method(*args, **keywords) File /usr/lib/pymodules/python2.6/dbus/proxies.py, line 140, in __call__ **keywords) File /usr/lib/pymodules/python2.6/dbus/connection.py, line 620, in call_blocking message, timeout) dbus.exceptions.DBusException: org.freedesktop.DBus.Error.UnknownMethod: Method SetProperty with signature sb on interface org.ofono.Modem doesn't exist -- Have a nice time. Regards, Krishna K Kandula The battle for the FIH Hockey World Cup Drag n' drop http://specials.msn.co.in/sp10/hockey/index.aspx ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: How to start ofono-phonesim in ubuntu-10.04 -reg
Hi Krishna, From: ofono-boun...@ofono.org [mailto:ofono-boun...@ofono.org] On Behalf Of krishna k Sent: Monday, May 10, 2010 4:13 PM To: ofono@ofono.org Subject: How to start ofono-phonesim in ubuntu-10.04 -reg Hi Yoon, I placed modem.conf file under /etc/ofono, and i tried with ./test/list-modems script. Still it is not showing any thing on command line. I tried to enable the phonesim0 modem with following command and i am getting the same o/p [Zhenhua] Do you build oFono with bootscrap-configure? If does, you will have '--prefix=/usr --sysconfdir=/etc ...' configuration. So oFono reads configuration from /etc/ofono. Without that, you might put modem.conf under /usr/local/etc/ofono. Btw, you could raise your question on our IRC room #ofono at irc.freenode.net as well. == ./enable-modem /phonesim0 **etc. dbus.exceptions.DBusException: org.freedesktop.DBus.Error.UnknownMethod: Method SetProperty with signature sb on interface org.ofono.Modem doesn't exist. == The same error as below. It is not able to detect the phonesim0? regards, Krishna K Kandula. Hi Krishna, You should put your modem.conf file under /etc/ofono/. Thanks, Yoon From: ofono-bounces at ofono.org http://lists.ofono.org/listinfo/ofono [mailto:ofono-bounces at ofono.org http://lists.ofono.org/listinfo/ofono ] On Behalf Of krishna k Sent: Monday, May 10, 2010 3:43 PM To: ofono at ofono.org http://lists.ofono.org/listinfo/ofono Subject: How to start ofono-phonesim in ubuntu-10.04 -reg Hi Zhenhua, Thanks for your reply.. I added the following code in /usr/share/alsa/pcm/modem.conf file... 1. # #Sample for using phone simulator # [phonesim] Driver=phonesim Address=127.0.0.1 port=12345 - I tried to ./list_modems. I got nothing on command prompt. According to my knowledge i should get the available modems (atleast phonesim0) I tried to enable the modem using the following command.. ./enable-modem /phonesim0 I got output as follows... -- Connecting modem /phonesim0... Traceback (most recent call last): File ./enable-modem, line 20, in module modem.SetProperty(Powered, dbus.Boolean(1)) File /usr/lib/pymodules/python2.6/dbus/proxies.py, line 68, in __call__ return self._proxy_method(*args, **keywords) File /usr/lib/pymodules/python2.6/dbus/proxies.py, line 140, in __call__ **keywords) File /usr/lib/pymodules/python2.6/dbus/connection.py, line 620, in call_blocking message, timeout) dbus.exceptions.DBusException: org.freedesktop.DBus.Error.UnknownMethod: Method SetProperty with signature sb on interface org.ofono.Modem doesn't exist -- Have a nice time. Regards, Krishna K Kandula The battle for the FIH Hockey World Cup Drag n' drop http://specials.msn.co.in/sp10/hockey/index.aspx ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: How to start ofono-phonesim in ubuntu-10.04 -reg
Hi Krishna, From: ofono-boun...@ofono.org [mailto:ofono-boun...@ofono.org] On Behalf Of krishna k Sent: Monday, May 10, 2010 1:24 PM To: ofono@ofono.org Subject: How to start ofono-phonesim in ubuntu-10.04 -reg Hi Yoon, Thank very much for your guidance. I followed the same procedure ... You already did following procedure. 1. ofono-phonesim -p 12345 -gui ~/data/moblin.xml 2. sudo ofonod -nd And then, you need to type following command to power up modem(phonesim). 3. dbus-send --system --print-reply --dest=org.ofono / phonesim0 org.ofono.Modem.SetProperty string:Powered variant:boolean:true [Zhenhua] I guess there's no space '/' and 'phonesim0'. Alternatively, You can use 'test/list-modem' to get your modem name and then use 'test/enable-modem /phonesim0' to power on it. I got below response Must use org.mydomain.Interface.Method notation, no dot in phonesim0 Please see the ps ax | grep ofono* - 1609 pts/0S+ 0:00 ofono-phonesim -p 12345 -gui /home/kandukis/data/moblin.xml 1654 pts/2S+ 0:00 ofonod -nd Can you please suggest how to get-rid from this problem. regards, Krishna K Kandula Catch the changing security environment Get it now. http://news.in.msn.com/internalsecurity/ ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 1/2] Fix Use hashtable to record udev path
Hi, Zhang, Zhenhua wrote: Sometimes, Udev device 'remove' event could not report correct parent node of current udev_device. Current code replies on the devpath attached on the parent node to find modem and then remove it. This fix is to change the way to store the devpath info into a hashtable. So that we search hashtable to get devpath and remove the modem. --- It's a proposed patch to fix Meego bug #395 [1]. oFono relies on udev to report device remove event. The devpath info of the modem is stored in the parent node of tty port. In below case, we try to find this parent node of /dev/ttyACM0. The expected path of node is .../usb1/1-4, which is modem device itself. On Netbook, however, it returns .../usb1 sometimes. In such case, we could not find the correct devpath and modem so that modem was never removed. My idea is to use a hashtable to store the mapping between tty path and modem path. So that we could find the modem path without depending on udev_device_get_parent() in remove_modem. Wrong log is listed below. The expected path is '/devices/pci:00/:00:1d.7/usb1/1-4/', but it returns '.../:00:1d.7/usb1', which is Linux USB host controller. Wrong log 1: remove_modem() device 0x9223820 path #0 /devices/pci:00/:00:1d.7/usb1/1-4/1-4:1.1/tty/ttyACM0 path #1 /devices/pci:00/:00:1d.7/usb1 path #2 /devices/pci:00/:00:1d.7/usb1 path #3 /devices/pci:00/:00:1d.7 path #4 /devices/pci:00 remove_modem() device 0x9222628 path #0 /devices/pci:00/:00:1d.7/usb1/1-4/1-4:1.3/tty/ttyACM1 path #1 /devices/pci:00/:00:1d.7/usb1 path #2 /devices/pci:00/:00:1d.7/usb1 path #3 /devices/pci:00/:00:1d.7 path #4 /devices/pci:00 remove_modem() device 0x92233a8 path #0 /devices/pci:00/:00:1d.7/usb1/1-4/1-4:1.7/net/wwan0 path #1 /devices/pci:00/:00:1d.7/usb1 path #2 /devices/pci:00/:00:1d.7/usb1 path #3 /devices/pci:00/:00:1d.7 path #4 /devices/pci:00 remove_modem() device 0x9224520 path #0 /devices/pci:00/:00:1d.7/usb1/1-4/1-4:1.9/tty/ttyACM2 path #1 /devices/pci:00/:00:1d.7/usb1 path #2 /devices/pci:00/:00:1d.7/usb1 path #3 /devices/pci:00/:00:1d.7 path #4 /devices/pci:00 Wrong log 2: remove_modem() device 0x9226398 path #0 /1-4:1.1/tty/ttyACM0 remove_modem() device 0x9226398 path #0 /1-4:1.3/tty/ttyACM1 remove_modem() device 0x9226398 path #0 /devices/pci:00/:00:1d.7/usb1/1-4/1-4:1.9/tty/ttyACM2 path #1 /devices/pci:00/:00:1d.7/usb1 path #2 /devices/pci:00/:00:1d.7/usb1 path #3 /devices/pci:00/:00:1d.7 path #4 /devices/pci:00 remove_modem() device 0x9224880 path #0 /devices/pci:00/:00:1d.7/usb1/1-4/1-4:1.7/net/wwan0 path #1 /devices/pci:00/:00:1d.7/usb1 path #2 /devices/pci:00/:00:1d.7/usb1 path #3 /devices/pci:00/:00:1d.7 path #4 /devices/pci:00 [1] http://bugs.meego.com/show_bug.cgi?id=395 Regards, Zhenhua - Plug a USB dangle into netbook ofonod[1829]: plugins/udev.c:add_modem() device 0x9222e78 - udev 0x9220ec8 ofonod[1829]: plugins/udev.c:add_modem() add path /devices/pci:00/:00:1d.7/usb1/1-4 ofonod[1829]: src/modem.c:ofono_modem_create() name: 3558620217367190, type: mbm ofonod[1829]: src/modem.c:set_modem_property() modem 0x92236b8 property Path ofonod[1829]: src/modem.c:set_modem_property() modem 0x92236b8 property Registered ofonod[1829]: plugins/udev.c:add_mbm() desc: Dell Wireless 5530 HSPA Mobile Broadband Minicard Modem ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property Registered ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property ModemDevice ofonod[1829]: src/modem.c:set_modem_property() modem 0x92236b8 property ModemDevice ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property ModemDevice ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property NetworkInterface ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property Path ofonod[1829]: plugins/udev.c:add_mbm() desc: Dell Wireless 5530 HSPA Mobile Broadband Minicard NetworkAdapter ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property Registered ofonod[1829]: src/modem.c:set_modem_property() modem 0x92236b8 property NetworkInterface ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property ModemDevice ofonod[1829]: src/modem.c:get_modem_property() modem 0x92236b8 property NetworkInterface ofonod[1829]: src/modem.c:set_modem_property() modem 0x92236b8 property Registered ofonod[1829]: src/modem.c:unregister_property() property 0x9223d10 ofonod[1829
RE: [PATCH 2/2] Fix check data device before register the modem
Hi, Zhang, Zhenhua wrote: To avoid the race condition that modem is registered before we retrieve the data device property. --- The second patch is for MBM modem only. We should retrieve data device property before register the modem. Please review it. Thanks. Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: Modem emulator and DUN server side for oFono and BlueZ
Hi Padovan, Gustavo F. Padovan wrote: Hi Zhenhua, * Zhang, Zhenhua zhenhua.zh...@intel.com [2010-04-27 15:53:54 +0800]: Hi, I am now working on modem emulator and one usage is for DUN server role. Since Padovan is working on client role, it's good to share my rough thinking for server side implementation. Here are the simple steps I have: 1. Create an oFono emulator atom in oFono. It's the emulator manager that could create DUN, HFP AG or SPP type emulators. It exposes dbus methods like CreateEmulator, DestroyEmulator, GetProperty, etc. 2. DUN agent server in BlueZ watch oFono and call CreateEmulator and pass the file descriptor to oFono. This server could further implement HFP AG and SPP connection. Shouldn't the emulator register itself on the BlueZ agent server? It's fine to register the emulator like HFP client. In this way, we might able to share the agent with DUN server as well. Thanks. -- Gustavo F. Padovan http://padovan.org ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Modem emulator and DUN server side for oFono and BlueZ
Hi, I am now working on modem emulator and one usage is for DUN server role. Since Padovan is working on client role, it's good to share my rough thinking for server side implementation. Here are the simple steps I have: 1. Create an oFono emulator atom in oFono. It's the emulator manager that could create DUN, HFP AG or SPP type emulators. It exposes dbus methods like CreateEmulator, DestroyEmulator, GetProperty, etc. 2. DUN agent server in BlueZ watch oFono and call CreateEmulator and pass the file descriptor to oFono. This server could further implement HFP AG and SPP connection. 3. Once an emulator is created, other atom like voicecall, grps, sms register their interested AT command handlers to it. The goal is that we could handle all mandatory AT commands defined in DUN profile spec. 4. Once a DUN emulator received ATD*99#, DUN client performs ppp connection so we forward ppp command to ppp stack. It is done by ppp server side extension. It should be the simple command forwarding. 5. Once the PPP link over DUN is established, DUN client performs ConnMan integration and setup IP address, DNS server, etc. 6. Once the Bluetooth link is disconnected, we destroy the PPP and DUN emulator. If emulator atom itself is destroyed, we destroy the PPP and the Bluetooth connection. If the PPP link is disconnected but Bluetooth link is alive, we destroy the PPP and stay emulator alive. Comments are welcome. :) Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: DUN client for oFono and BlueZ
Hi Padovan, Gustavo F. Padovan wrote: Hi all, 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. Is this bluetooth.c like a utility to watch BlueZ status, watch adapter change and signals? If I understand correctly, every plugin like HFP, DUN, SAP will add status callback for different event. 3. plugin/dun.c prototype. A simple prototype for the DUN client. Not quite sure whether we need this. Denis suggests to create an Emulator atom in the ofono core. I am now making a prototype to create this atom in oFono. The structure is similar to voicecall.c. An emulator manager could create HFP AG, DUN or SPP type emulators. When one is created, other atom get notified and register their interested AT command handers to GAtServer. So in this way, agent server on BlueZ may call oFono CreateEmulator method and pass file descriptor directly to oFono. 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. Actually, my original prototype is quite simliar with yours. DUN, HFP and SPP are implemented as plugins. See my previous patches in the mailing list for details. 6. AT command parser and PPP stack integration with DUN. The biggest task, where the core of the project is. GAtServer and PPP are already there. We still need to add DUN specific command and callbacks. 7. ConnMan integration. Setup of the NAT and Internet Connections. Comments are welcome. :) -- Gustavo F. Padovan http://padovan.org ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: DUN client for oFono and BlueZ
Hi Padovan, Gustavo F. Padovan wrote: Hi Zhenhua, * Zhang, Zhenhua zhenhua.zh...@intel.com [2010-04-27 10:14:38 +0800]: Hi Padovan, Gustavo F. Padovan wrote: Hi all, 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. Is this bluetooth.c like a utility to watch BlueZ status, watch adapter change and signals? If I understand correctly, every plugin like HFP, DUN, SAP will add status callback for different event. Exactly, that's the idea. 3. plugin/dun.c prototype. A simple prototype for the DUN client. Not quite sure whether we need this. Denis suggests to create an Emulator atom in the ofono core. I am now making a prototype to create this atom in oFono. The structure is similar to voicecall.c. An emulator manager could create HFP AG, DUN or SPP type emulators. When one is created, other atom get notified and register their interested AT command handers to GAtServer. So in this way, agent server on BlueZ may call oFono CreateEmulator method and pass file descriptor directly to oFono. If I understood correctly the Emulator will be useful for the DUN Gateway side. Right? Yes. This is DUN gateway side. So I think we need to define a PPP command forwarding so that PPP command could be sent and received throught DUN properly. Right now I'm going to work on 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. Actually, my original prototype is quite simliar with yours. DUN, HFP and SPP are implemented as plugins. See my previous patches in the mailing list for details. In my mind that will be similar to the HFP agent implementation I did inside oFono. 6. AT command parser and PPP stack integration with DUN. The biggest task, where the core of the project is. GAtServer and PPP are already there. We still need to add DUN specific command and callbacks. 7. ConnMan integration. Setup of the NAT and Internet Connections. Regards, -- Gustavo F. Padovan http://padovan.org Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 1/5] Refactor the command parsing framework
Hi Denis, Denis Kenzior wrote: Hi Zhenhua, --- gatchat/gatserver.c | 182 +++--- gatchat/gatserver.h | 7 ++- 2 files changed, 133 insertions(+), 56 deletions(-) -at_command_notify(server, buf, prefix, type); +notify = at_node_new(server, buf, prefix, type); +if (!notify) +goto error; /* Also consume the terminating null */ -return i + 1; +server-read_pos += i + 1; +server-notify_source = g_idle_add_full(G_PRIORITY_DEFAULT, +at_command_notify, + notify, g_free); + +return; + +error: +g_free(notify); + +g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR); } Err, OK stop right there. This is really way too complicated. How about we simply set a flag before calling at_command_notify. If after executing it send_final or send_ext_final response has been sent, then we continue processing, otherwise we restart when send_ext_final or send_final will be called again. You really don't need to touch this function at all. OK. So the problem is if the at_command_notify is blocked by user callback, the mainloop won't have chance to read data in from non-blocking I/O I guess. That's the reason I added this. If we don't need to handle such case, then the logic could be much simplier. We could discuss it on IRC. +typedef void (*GAtServerNotifyCallback)(gpointer user_data); + Get rid of this, not necessary. typedef void (*GAtServerNotifyFunc)(GAtServerRequestType type, -GAtResult *result, gpointer user_data); +GAtResult *result, +gpointer user_data, +GAtServerNotifyCallback cb, +gpointer cb_data); Keep the NotifyFunc the way it is, your changes are really not required. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 1/4] Add gatutil.c to share common APIs with GAtServer
Hi Denis, Denis Kenzior wrote: Hi Zhenhua, +gboolean g_at_util_set_io(GIOChannel *io) Rename this g_at_util_setup_io +typedef void (*GAtDisconnectFunc)(gpointer user_data); typedef void +(*GAtDebugFunc)(const char *str, gpointer user_data); These don't belong here, create a new file called gat.h and put them there. Regards, -Denis Thanks for your comments. Updated patch is attached. Regards, Zhenhua 0001-Add-gatutil.c-to-share-common-APIs-with-GAtServer.patch Description: 0001-Add-gatutil.c-to-share-common-APIs-with-GAtServer.patch ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: Merry Christmas to everyone
Xu, Martin wrote: -Original Message- From: ofono-boun...@ofono.org [mailto:ofono-boun...@ofono.org] On Behalf Of Marcel Holtmann Sent: Friday, December 25, 2009 12:22 AM To: ofono@ofono.org Subject: Merry Christmas to everyone Hello everyone, and Santa is coming to town and bringing the 0.15 release with him. Many Marcel: You should give each contributor a small gift like Santa Claus. ;-) My address is: No. 4 room 202, lane 1459, RD Luoxiu, Shanghai PRC 200237 Marcel IS a young and kindly Santa Claus. He will give the gift to the contributor sending him the address. ;-) So mine is: No. 3 room 401, lane 3105, Lao Huming Road, Shanghai PRC 201108 _ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 1/2] Fix remove all atoms in ofono_modem_set_powered
Hi, Zhang, Zhenhua wrote: We should remove all atoms when we disable the modem. In function set_powered, we remove all atoms if modem is powered down by dbus call. --- src/modem.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) Let me explain more about this fix. In atmodem world, we use dbus call to power on/off modem and then it calls set_powered() to remove all atoms. However, in hfpmodem, the plugin may receive signal from BlueZ that remote has closed the service level connection with us. So we must power off the modem through ofono_modem_set_powered(modem, FALSE). I also tried phonesim and don't find problem. Let me know if any questions or issues. Thanks. Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 1/4] Add hfp_release_specific to release specific call
Hi, Zhang, Zhenhua wrote: Use AT+CHLD=1x to release a specific call if AG supports that. --- drivers/hfpmodem/voicecall.c | 51 +- 1 files changed, 50 insertions(+), 1 deletions(-) Update the patch to initlize 'struct release_id_req *req' as NULL. Regards, Zhenhua 0001-Add-hfp_release_specific-to-release-specific-call.patch Description: 0001-Add-hfp_release_specific-to-release-specific-call.patch ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 4/4] Fix schedule clcc poll in the ring notify
Hi, Zhang, Zhenhua wrote: Similar to atmodem, We need schedule CLCC poll in the ring notify in case AG doesn't send CLIP notification to us. If CLIP comes right after RING, we cancel the CLCC poll. --- drivers/hfpmodem/voicecall.c | 15 +++ 1 files changed, 15 insertions(+), 0 deletions(-) Changed to add clip_timeout to notify the call directly if CLIP notification doesn't arrive. Please review it. Regards, Zhenhua 0004-Add-clip_timeout-for-notify-incoming-call.patch Description: 0004-Add-clip_timeout-for-notify-incoming-call.patch ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 1/3] Add hfp_list_calls to get current call list
Hi Denis, Denis Kenzior wrote: Hi Zhenhua, Use AT+CLCC to get current call list. So I did things 'slightly' differently. The new setup should be much simpler for both the plugin and the core. I've tested it on phonesim, but not on HFP. Can you let me know if there are any issues? It works fine on HFP and new code looks better than older interface. I got 3 new PASS from PTS results. ;-) Regards, -Denis Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH] Fix: Poll CLCC to replace the code in call_held=1
Hi Marcel, Marcel Holtmann wrote: Hi Zhenhua, When using CHLD=2x in multiparty call, call_held is 1 and we should not swap all active-held. So we cannot save this pull in private chat. --- drivers/hfpmodem/voicecall.c | 50 + 1 files changed, 6 insertions(+), 44 deletions(-) patch is corrupted. Please send again or better even use git send-email for sending it. Thanks Marcel. Patch has been attached. The evolution wrapped the line that over 80 characters so the patch is corrupted. I will try to use git send-email next time. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua 0001-Fix-Poll-CLCC-to-replace-the-code-in-call_held-1.patch Description: 0001-Fix-Poll-CLCC-to-replace-the-code-in-call_held-1.patch ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 4/4] Fix: Fill in the phone number info for outgoing call
Hi Denis, Denis Kenzior wrote: Hi Zhenhua, +ofono_voicecall_notify(vc, call); + As I mentioned above, you do not need to do this. oFono will synthesize the outgoing call properly. Worst case we will send a PropertyChanged signal after CLCC is polled and the new number is obtained. I recreate the patch to use the way you mentioned. We notify the voicecall to core after CLCC is polled. Instead of notify it in atd_cb. The comments in patch is also updated. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua 0001-Fix-Fill-in-the-phone-number-info-for-outgoing-call.patch Description: 0001-Fix-Fill-in-the-phone-number-info-for-outgoing-call.patch ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 4/4] Fix: Fill in the phone number info for outgoing call
Hi Denis, Denis Kenzior wrote: Hi Zhenhua, There're two cases of outgoing call: dial from HF or dial from phone. In the first case, some phones may not support enhanced call status (AT+CLCC) to query outgoing number. So we need to pass struct atd_cb_data to fill in the number and type. Later, we invoke AT+CLCC to query phone number and overwrite ours if we are wrong. This is actually not necessary. The core already tracks the outgoing number and fills it in if the dial returns before the new call is notified. This should be always the case in HFP, since the ATD returns before CIEV is signaled, correct? The reason to record outgoing number is for later clcc_poll_cb in multicall hanlding, because HFP doesn't have COLP so it don't know the outgoing number unless we do +CLCC poll. So we need to sync it. +static GSList *parse_clcc(GAtResult *result) { +GAtResultIter iter; +GSList *l = NULL; +int id, dir, status, type; +struct ofono_call *call; + +g_at_result_iter_init(iter, result); + +while (g_at_result_iter_next(iter, +CLCC:)) { +const char *str = ; +int number_type = 129; + +if (!g_at_result_iter_next_number(iter, id)) +continue; + +if (!g_at_result_iter_next_number(iter, dir)) +continue; + +if (!g_at_result_iter_next_number(iter, status)) + continue; + +if (!g_at_result_iter_next_number(iter, type)) + continue; + +if (!g_at_result_iter_skip_next(iter)) +continue; + +if (g_at_result_iter_next_string(iter, str)) +g_at_result_iter_next_number(iter, number_type); + +call = g_try_new0(struct ofono_call, 1); + +if (!call) +break; + +call-id = id; +call-direction = dir; +call-status = status; +call-type = type; +strncpy(call-phone_number.number, str, +OFONO_MAX_PHONE_NUMBER_LENGTH); +call-phone_number.type = number_type; + +if (strlen(call-phone_number.number) 0) +call-clip_validity = 0; +else +call-clip_validity = 2; + +l = g_slist_insert_sorted(l, call, at_util_call_compare); + } + +return l; +} If this function is a copy from atmodem, please put it into atutils instead of duplicating it here. I will send a separate patch for that. +ofono_voicecall_notify(vc, call); + As I mentioned above, you do not need to do this. oFono will synthesize the outgoing call properly. Worst case we will send a PropertyChanged signal after CLCC is polled and the new number is obtained. Also, there's actually two separate cases here. Ideally I'd like them separated into two patches. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 1/3] Add multi calls support for HFP voicecall driver
Hi Denis, Denis Kenzior wrote: Hi Zhenhua, +static void reset_to_single_call(struct voicecall_data *vd) { + struct ofono_call *oc = vd-calls-data; + + vd-call = oc; + vd-cind_val[HFP_INDICATOR_CALL] = 1; + + if (oc-direction == 1) + vd-cind_val[HFP_INDICATOR_CALLSETUP] = 1; + else + vd-cind_val[HFP_INDICATOR_CALLSETUP] = 3; } + I don't see how this is supposed to work. What if the last call is a held call? Don't see any problem if it is a held call. Still can hang up correctly. What's the problem you have see. +if (c) { +nc = c-data; + +if (memcmp(nc, oc, sizeof(struct ofono_call))) { + oc-status = nc-status; + +/* If phone does not support multiparty call, + * there will have only one active call at one + * time. +CHUP only hang up current active call. + */ +if (oc-status == CALL_STATUS_ACTIVE) +vd-call = oc; Lets not set this silly variable anymore. It has way too many meanings and uses throughout the code (most of which can be easily simplified) for the logic to be understandable. As discussed on IRC, I will rename vd-call to vd-active. But the src/voicecall.c has the similar variable in struct ofono_voicecall too. +static void hfp_hold_all_active(struct ofono_voicecall *vc, +ofono_voicecall_cb_t cb, void *data) { +struct voicecall_data *vd = ofono_voicecall_get_data(vc); + +if (check_mpty_feature(vc, AG_CHLD_2, cb, data)) { +hfp_template(AT+CHLD=2, vc, generic_cb, 0, cb, data); + +/* Some phones fail to send response back after CHLD=2. + * But if we have another dialing out call, we should hold + * +CLCC until AG creates the new call. + */ +vd-clcc_source = g_timeout_add(POLL_CLCC_INTERVAL, +poll_clcc, vc); This comment makes no sense. If the phone doesn't respond after CHLD=2 the parser is stuck and we're not sending CLCC out anyway. I will update the comments. It means phone should send update notification Like callheld=1. But some phones doesn't send CIEV update. +static void hfp_set_udub(struct ofono_voicecall *vc, +ofono_voicecall_cb_t cb, void *data) { +unsigned int incoming_or_waiting = (1 4) | (1 5); +struct voicecall_data *vd = ofono_voicecall_get_data(vc); +struct ofono_call *call = vd-call; +struct release_id_req *req; + +if (vd-ag_mpty_features AG_CHLD_0) +hfp_template(AT+CHLD=0, vc, generic_cb, incoming_or_waiting, +cb, data); +else { +req = g_try_new0(struct release_id_req, 1); + +if (!req || !call) { +CALLBACK_WITH_FAILURE(cb, data); +return; +} + +req-vc = vc; +req-cb = cb; +req-data = data; +req-id = call-id; + +g_at_chat_send(vd-chat, AT+CHUP, none_prefix, +release_id_cb, req, g_free); +} The core is quite specifically asking to set UDUB, not to hangup the call. If this is not supported, then we should say so. Please get rid of this. Ok. Remove it and report errors. +} + +static void hfp_release_all_active(struct ofono_voicecall *vc, +ofono_voicecall_cb_t cb, void *data) { +struct voicecall_data *vd = ofono_voicecall_get_data(vc); + +if (check_mpty_feature(vc, AG_CHLD_1, cb, data)) { +hfp_template(AT+CHLD=1, vc, generic_cb, 0x1, cb, data); + +vd-clcc_source = g_timeout_add(POLL_CLCC_INTERVAL, poll_clcc, +vc); Why are we scheduling the CLCC before the CHLD returned? We should wait until CHLD fails or succeeds. It doesn't matter if we have 1 extra CLCC even if CHLD=1 is failed. Anyway, I will move it into generic_cb. +static void hfp_release_specific(struct ofono_voicecall *vc, int id, +ofono_voicecall_cb_t cb, void *data) { +if (vd-ag_features AG_FEATURE_ENHANCED_CALL_CONTROL) { +sprintf(buf, AT+CHLD=1%d, id); Should we be checking CHLD_1X feature? You mean checking CHLD_1X instead of enhanced_call_control? In theoriy, They are the same. I don't think the phone has CHLD_1X without CHLD=2X. ;-) + +g_at_chat_send(vd-chat, buf, none_prefix, +release_id_cb, req, g_free); +} else if (id == (int) vd-call-id) +g_at_chat_send(vd-chat, AT+CHUP, none_prefix, +release_id_cb, req,
[PATCH] Add voicecall driver for Handsfree profile
Hi, The first patch is to handle right bracket in at command parser. It's identical with the previous patch I sent in the list. The second patch depends on the first one so I resend it again. It implements voicecall driver for handsfree profile and use AT+CIEV indicator to notify call status change. A field 'struct ofono_call *call' that point to the current call is used to avoid searching call list each time. A boolean 'mpty_call' is to indicate whether it is multi call or single call. The first version only supports single call. The multicall patch will be based on this patch. Please review it. Regards, Zhenhua 0002-Add-voicecall-driver-for-Handsfree-profile.patch Description: 0002-Add-voicecall-driver-for-Handsfree-profile.patch 0001-Handle-right-bracket-in-g_at_result_iter_next_unquot.patch Description: 0001-Handle-right-bracket-in-g_at_result_iter_next_unquot.patch ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
[PATCH] Replace Glib type with standard C type
Hi, Below patch is to replace Glib type with standard C type in hfpmodem.h, like guint, guint8, etc. Please review it. Regards, Zhenhua --- drivers/hfpmodem/hfpmodem.h |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/hfpmodem/hfpmodem.h b/drivers/hfpmodem/hfpmodem.h index 6c6ab6f..c4e2a34 100644 --- a/drivers/hfpmodem/hfpmodem.h +++ b/drivers/hfpmodem/hfpmodem.h @@ -55,10 +55,10 @@ enum hfp_indicator { struct hfp_data { GAtChat *chat; - guint ag_features; - guint hf_features; - guint8 cind_pos[HFP_INDICATOR_LAST]; - gint cind_val[HFP_INDICATOR_LAST]; + unsigned int ag_features; + unsigned int hf_features; + unsigned char cind_pos[HFP_INDICATOR_LAST]; + unsigned int cind_val[HFP_INDICATOR_LAST]; }; extern void hfp_voicecall_init(); -- 1.6.1.3 ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
[PATCH] Handle right bracket in g_at_result_iter_next_unquoted_string
Allow g_at_result_iter_next_unquoted_string to handle the bracket when parsing string like chld (1, 2). Original, it returns '1' and '2)'. Now it returns '1' and '2'. g_at_result_iter_close_list handles the right bracket later. --- gatchat/gatresult.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gatchat/gatresult.c b/gatchat/gatresult.c index 1436ae3..3d07e13 100644 --- a/gatchat/gatresult.c +++ b/gatchat/gatresult.c @@ -131,12 +131,12 @@ gboolean g_at_result_iter_next_unquoted_string(GAtResultIter *iter, goto out; } - if (line[pos] == '') + if (line[pos] == '' || line[pos] == ')') return FALSE; end = pos; - while (end len line[end] != ',') + while (end len line[end] != ',' line[end] != ')') end += 1; iter-buf[end] = '\0'; Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
[PATCH] Handsfree profile plugins update
Hi, Attached is update patch for handsfree profile plugin. I move most stuff in drivers/hfpmodem/hfp.c into plugins/hfp.c and simplify them. GSList *indies is replaced as two indexable arrays (cind_names and cind_values), to improve efficiency when we need access name and value by index. The whole logic is the same comparing with previous patch. Thanks for Denis and Gustavo for comments. Please review it. Regards, Zhenhua 0001-handsfree-profile-plugin-update.patch Description: 0001-handsfree-profile-plugin-update.patch ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH] Add call-volume interface to allow user adjust speaker and mic volume
Hi Marcel, Marcel Holtmann wrote: Hi Zhenhua, +#define CALL_VOLUME_INTERFACE org.ofono.CallVolume we have to stop doing this. Start using this: #define FOO_INTERFACE OFONO_SERVICE .FooInterface +static void call_volume_unregister(struct ofono_atom *atom); That forward declaration seems pointless unless I actually missed something. No forward declaration unless they are really needed. Just shuffle the code around. Coding style. Actually twice. No space behind if and single if statements need no { }. + + if (!strcmp(property, MicrophoneVolume)) { + cv-microphone_volume = percent; + } No { } needed. What is this variable non-sense for. You can use cv-speaker_volume directly from D-Bus message append functions. Only for certain strings you need this. + + if (cv-pending) + return __ofono_error_busy(msg); + + cv-pending = dbus_message_ref(msg); + + reply = dbus_message_new_method_return(cv-pending); + + if (!reply) + return NULL; What is this empty line for? Just don't do that. The error check logically belongs to the function creating reply. Also who unrefs cv-pending in this case? + dbus_message_iter_init_append(reply, iter); + + dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, + OFONO_PROPERTIES_ARRAY_SIGNATURE, + dict); + + ofono_dbus_dict_append(dict, SpeakerVolume, DBUS_TYPE_UINT32, + speaker_volume); + + ofono_dbus_dict_append(dict, MicrophoneVolume, + DBUS_TYPE_UINT32, microphone_volume); Why do we use UINT32 for percentage values? A BYTE would be clearly enough. +static void mv_set_callback(const struct ofono_error *error, void *data) +{ + struct ofono_call_volume *cv = data; + + if (error-type != OFONO_ERROR_TYPE_NO_ERROR) { + ofono_debug(Error occurred during Microphone Volume set: %s, + telephony_error_to_str(error)); + } else { + cv-microphone_volume = cv-temp_volume; + } + + generic_callback(error, data); +} So personally I think the ofono_debug() call makes these two function hard to read and more complicated than needed. If we really want to display this error then either use ofono_error() or just don't bother at all with any kind of output. +static DBusMessage *cv_set_call_volume(DBusMessage *msg, struct ofono_call_volume *cv, + const char *property, unsigned int percent) +{ + if (percent 100) + return __ofono_error_invalid_format(msg); + + DBG(set %s volume to %d percent, property, percent); + + cv-flags |= CALL_VOLUME_FLAG_PENDING; + cv-temp_volume = percent; + + if (msg) + cv-pending = dbus_message_ref(msg); + + if(!strcmp(property, SpeakerVolume)) { + if (!cv-driver-speaker_volume) { + return (msg != NULL)?__ofono_error_not_implemented(msg):NULL; + } + cv-driver-speaker_volume(cv, percent, sv_set_callback, cv); + } So the space after if is missing again. And this return statement is not readable at all. It violates coding style and makes my brain hurt. Redo this it becomes readable. Cramping everything in one line is just no helpful. +static GDBusMethodTable cv_methods[] = { + { GetProperties, , a{sv}, cv_get_properties, + G_DBUS_METHOD_FLAG_ASYNC}, + { SetProperty,sv, , cv_set_property, + G_DBUS_METHOD_FLAG_ASYNC }, + { } +}; I am missing the point why GetProperties has to be done async. That looks like too much overhead for something just returning stored values. + + if (driver == NULL) + return NULL; + + cv = g_try_new0(struct ofono_call_volume, 1); + cv-speaker_volume = 100; + cv-microphone_volume = 100; + + if (cv == NULL) + return NULL; How do you expect this to work actually? If cv == NULL, then your assignment are NULL pointer dereferences and the daemon will crash. Your error handling is not protecting it. +int ofono_call_volume_driver_register(const struct ofono_call_volume_driver *d) +{ + DBG(driver: %p, name: %s, d, d-name); + + if (d-probe == NULL) + return -EINVAL; + + g_drivers = g_slist_prepend(g_drivers, (void *)d); I really hate these casts only because of const driver declaration. Personally I am against it, but I see some value behind having this const. However there has to be a space between the cast and the variable. + + return 0; +} + +void
[PATCH] Add call-volume interface to allow user adjust speaker and mic volume
Hi, Attached is the patch to add call-volume interface for Ofono. It exposes two properties: 'SpeakerVolume' and 'MicrophoneVolume' to allow user could control phone volume through lower level driver. The ofono_call_volume_notify() is used to report call volume property change notification to the upper application. The default volume could be stored in the modem.conf or query from remote. Each driver could report value in percentage to call-volume through ofono_call_volume_notify(). And then the call-volume will sync volume with remote side right after probe(). Please review it. Regards, Zhenhua 0001-Add-call-volume-interface-to-allow-user-to-adjust-sp.patch Description: 0001-Add-call-volume-interface-to-allow-user-to-adjust-sp.patch ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono